* gold patch committed: Check IOV_MAX
@ 2011-03-09 1:55 Ian Lance Taylor
2011-03-09 9:37 ` Andreas Schwab
0 siblings, 1 reply; 6+ messages in thread
From: Ian Lance Taylor @ 2011-03-09 1:55 UTC (permalink / raw)
To: binutils
[-- Attachment #1: Type: text/plain, Size: 445 bytes --]
This gold patch limits the number of entries passed to readv to IOV_MAX.
Apparently Solaris has a limit of 16, and gold was using more.
Committed to mainline and 2.21 branch.
Ian
2011-03-08 Ian Lance Taylor <iant@google.com>
PR gold/12525
* fileread.cc: #include <climits>.
(GOLD_IOV_MAX): Define.
(File_read::read_multiple): Limit number of entries by iov_max.
* fileread.h (class File_read): Always set max_readv_entries to
128.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 2609 bytes --]
Index: fileread.h
===================================================================
RCS file: /cvs/src/src/gold/fileread.h,v
retrieving revision 1.46
diff -p -u -r1.46 fileread.h
--- fileread.h 14 Dec 2010 19:03:29 -0000 1.46
+++ fileread.h 9 Mar 2011 01:45:43 -0000
@@ -1,6 +1,6 @@
// fileread.h -- read files for gold -*- C++ -*-
-// Copyright 2006, 2007, 2008, 2009, 2010 Free Software Foundation, Inc.
+// Copyright 2006, 2007, 2008, 2009, 2010, 2011 Free Software Foundation, Inc.
// Written by Ian Lance Taylor <iant@google.com>.
// This file is part of gold.
@@ -399,13 +399,7 @@ class File_read
{ return (file_size + (page_size - 1)) & ~ (page_size - 1); }
// The maximum number of entries we will pass to ::readv.
-#ifdef HAVE_READV
static const size_t max_readv_entries = 128;
-#else
- // On targets that don't have readv set the max to 1 so readv is not
- // used.
- static const size_t max_readv_entries = 1;
-#endif
// Use readv to read data.
void
Index: fileread.cc
===================================================================
RCS file: /cvs/src/src/gold/fileread.cc,v
retrieving revision 1.69
diff -p -u -r1.69 fileread.cc
--- fileread.cc 14 Dec 2010 21:33:26 -0000 1.69
+++ fileread.cc 9 Mar 2011 01:45:43 -0000
@@ -1,6 +1,6 @@
// fileread.cc -- read files for gold
-// Copyright 2006, 2007, 2008, 2009, 2010 Free Software Foundation, Inc.
+// Copyright 2006, 2007, 2008, 2009, 2010, 2011 Free Software Foundation, Inc.
// Written by Ian Lance Taylor <iant@google.com>.
// This file is part of gold.
@@ -24,6 +24,7 @@
#include <cstring>
#include <cerrno>
+#include <climits>
#include <fcntl.h>
#include <unistd.h>
#include <sys/mman.h>
@@ -605,11 +606,22 @@ File_read::do_readv(off_t base, const Re
got, want, static_cast<long long>(base + first_offset));
}
+// Portable IOV_MAX.
+
+#if !defined(HAVE_READV)
+#define GOLD_IOV_MAX 1
+#elif defined(IOV_MAX)
+#define GOLD_IOV_MAX IOV_MAX
+#else
+#define GOLD_IOV_MAX (File_read::max_readv_entries * 2)
+#endif
+
// Read several pieces of data from the file.
void
File_read::read_multiple(off_t base, const Read_multiple& rm)
{
+ static size_t iov_max = GOLD_IOV_MAX;
size_t count = rm.size();
size_t i = 0;
while (i < count)
@@ -622,7 +634,7 @@ File_read::read_multiple(off_t base, con
size_t j;
for (j = i + 1; j < count; ++j)
{
- if (j - i >= File_read::max_readv_entries)
+ if (j - i >= File_read::max_readv_entries || j - i >= iov_max / 2)
break;
const Read_multiple_entry& j_entry(rm[j]);
off_t j_off = j_entry.file_offset;
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: gold patch committed: Check IOV_MAX
2011-03-09 1:55 gold patch committed: Check IOV_MAX Ian Lance Taylor
@ 2011-03-09 9:37 ` Andreas Schwab
2011-03-09 15:35 ` Ian Lance Taylor
0 siblings, 1 reply; 6+ messages in thread
From: Andreas Schwab @ 2011-03-09 9:37 UTC (permalink / raw)
To: Ian Lance Taylor; +Cc: binutils
Ian Lance Taylor <iant@google.com> writes:
> @@ -605,11 +606,22 @@ File_read::do_readv(off_t base, const Re
> got, want, static_cast<long long>(base + first_offset));
> }
>
> +// Portable IOV_MAX.
> +
> +#if !defined(HAVE_READV)
> +#define GOLD_IOV_MAX 1
> +#elif defined(IOV_MAX)
> +#define GOLD_IOV_MAX IOV_MAX
> +#else
> +#define GOLD_IOV_MAX (File_read::max_readv_entries * 2)
> +#endif
> +
> // Read several pieces of data from the file.
>
> void
> File_read::read_multiple(off_t base, const Read_multiple& rm)
> {
> + static size_t iov_max = GOLD_IOV_MAX;
const?
Andreas.
--
Andreas Schwab, schwab@redhat.com
GPG Key fingerprint = D4E8 DBE3 3813 BB5D FA84 5EC7 45C6 250E 6F00 984E
"And now for something completely different."
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: gold patch committed: Check IOV_MAX
2011-03-09 9:37 ` Andreas Schwab
@ 2011-03-09 15:35 ` Ian Lance Taylor
2011-03-09 16:12 ` John Reiser
2011-03-09 16:15 ` Andreas Schwab
0 siblings, 2 replies; 6+ messages in thread
From: Ian Lance Taylor @ 2011-03-09 15:35 UTC (permalink / raw)
To: Andreas Schwab; +Cc: binutils
Andreas Schwab <schwab@redhat.com> writes:
> Ian Lance Taylor <iant@google.com> writes:
>
>> @@ -605,11 +606,22 @@ File_read::do_readv(off_t base, const Re
>> got, want, static_cast<long long>(base + first_offset));
>> }
>>
>> +// Portable IOV_MAX.
>> +
>> +#if !defined(HAVE_READV)
>> +#define GOLD_IOV_MAX 1
>> +#elif defined(IOV_MAX)
>> +#define GOLD_IOV_MAX IOV_MAX
>> +#else
>> +#define GOLD_IOV_MAX (File_read::max_readv_entries * 2)
>> +#endif
>> +
>> // Read several pieces of data from the file.
>>
>> void
>> File_read::read_multiple(off_t base, const Read_multiple& rm)
>> {
>> + static size_t iov_max = GOLD_IOV_MAX;
>
> const?
I've written the code this way because IOV_MAX need not be a constant.
It may be a call to sysconf.
Ian
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: gold patch committed: Check IOV_MAX
2011-03-09 15:35 ` Ian Lance Taylor
@ 2011-03-09 16:12 ` John Reiser
2011-03-09 16:20 ` Ian Lance Taylor
2011-03-09 16:15 ` Andreas Schwab
1 sibling, 1 reply; 6+ messages in thread
From: John Reiser @ 2011-03-09 16:12 UTC (permalink / raw)
To: binutils
Ian Lance Taylor wrote:
> Andreas Schwab <schwab@redhat.com> writes:
>
>> Ian Lance Taylor <iant@google.com> writes:
>>> void
>>> File_read::read_multiple(off_t base, const Read_multiple& rm)
>>> {
>>> + static size_t iov_max = GOLD_IOV_MAX;
>>
>> const?
>
> I've written the code this way because IOV_MAX need not be a constant.
> It may be a call to sysconf.
static size_t const iov_max = GOLD_IOV_MAX;
That use of 'const' denotes a property of the local variable 'iov_max'
which can be valuable for documentation, maintenance, and optimization.
Nothing is implied regarding the initializing expression 'GOLD_IOV_MAX',
which may be simple or arbitrarily complicated.
If it is desired to emphasize that the expansion of GOLD_IOV_MAX might
involve more than literal numeric constants, then perhaps an explicit
[and superfluous] cast
static size_t const iov_max = (size_t const) GOLD_IOV_MAX;
might be appropriate.
--
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: gold patch committed: Check IOV_MAX
2011-03-09 16:12 ` John Reiser
@ 2011-03-09 16:20 ` Ian Lance Taylor
0 siblings, 0 replies; 6+ messages in thread
From: Ian Lance Taylor @ 2011-03-09 16:20 UTC (permalink / raw)
To: John Reiser; +Cc: binutils
John Reiser <jreiser@bitwagon.com> writes:
> Ian Lance Taylor wrote:
>> Andreas Schwab <schwab@redhat.com> writes:
>>
>>> Ian Lance Taylor <iant@google.com> writes:
>
>>>> void
>>>> File_read::read_multiple(off_t base, const Read_multiple& rm)
>>>> {
>>>> + static size_t iov_max = GOLD_IOV_MAX;
>>>
>>> const?
>>
>> I've written the code this way because IOV_MAX need not be a constant.
>> It may be a call to sysconf.
>
> static size_t const iov_max = GOLD_IOV_MAX;
>
> That use of 'const' denotes a property of the local variable 'iov_max'
> which can be valuable for documentation, maintenance, and optimization.
> Nothing is implied regarding the initializing expression 'GOLD_IOV_MAX',
> which may be simple or arbitrarily complicated.
>
> If it is desired to emphasize that the expansion of GOLD_IOV_MAX might
> involve more than literal numeric constants, then perhaps an explicit
> [and superfluous] cast
>
> static size_t const iov_max = (size_t const) GOLD_IOV_MAX;
>
> might be appropriate.
Everything you say is true, but I find the code to be clear as it stands
and see no reason to change it.
Ian
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: gold patch committed: Check IOV_MAX
2011-03-09 15:35 ` Ian Lance Taylor
2011-03-09 16:12 ` John Reiser
@ 2011-03-09 16:15 ` Andreas Schwab
1 sibling, 0 replies; 6+ messages in thread
From: Andreas Schwab @ 2011-03-09 16:15 UTC (permalink / raw)
To: Ian Lance Taylor; +Cc: binutils
Ian Lance Taylor <iant@google.com> writes:
> Andreas Schwab <schwab@redhat.com> writes:
>
>> Ian Lance Taylor <iant@google.com> writes:
>>
>>> @@ -605,11 +606,22 @@ File_read::do_readv(off_t base, const Re
>>> got, want, static_cast<long long>(base + first_offset));
>>> }
>>>
>>> +// Portable IOV_MAX.
>>> +
>>> +#if !defined(HAVE_READV)
>>> +#define GOLD_IOV_MAX 1
>>> +#elif defined(IOV_MAX)
>>> +#define GOLD_IOV_MAX IOV_MAX
>>> +#else
>>> +#define GOLD_IOV_MAX (File_read::max_readv_entries * 2)
>>> +#endif
>>> +
>>> // Read several pieces of data from the file.
>>>
>>> void
>>> File_read::read_multiple(off_t base, const Read_multiple& rm)
>>> {
>>> + static size_t iov_max = GOLD_IOV_MAX;
>>
>> const?
>
> I've written the code this way because IOV_MAX need not be a constant.
But that doesn't prevent iov_max from being const, doesn't it?
Andreas.
--
Andreas Schwab, schwab@redhat.com
GPG Key fingerprint = D4E8 DBE3 3813 BB5D FA84 5EC7 45C6 250E 6F00 984E
"And now for something completely different."
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-03-09 16:20 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-03-09 1:55 gold patch committed: Check IOV_MAX Ian Lance Taylor
2011-03-09 9:37 ` Andreas Schwab
2011-03-09 15:35 ` Ian Lance Taylor
2011-03-09 16:12 ` John Reiser
2011-03-09 16:20 ` Ian Lance Taylor
2011-03-09 16:15 ` Andreas Schwab
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).