threads / discuss / 38523

read() MAX_IO_SIZE bytes, more than SSIZE_MAX?

Subject: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?

## tl;dr

20 messages between Feb 7, 2015 and Feb 11, 2015.

replies: 19people: 5as markdown or json

Joachim Schmitz· Feb 7, 2015, 16:45 UTC · lore
Hi there

While investigating the problem with hung git-upload-pack we think to have found a bug in wrapper.c:

#define MAX_IO_SIZE (8*1024*1024)

This is then used in xread() to split read()s into suitable chunks. So far so good, but read() is only guaranteed to read as much as SSIZE_MAX bytes at a time. And on our platform that is way lower than those 8MB (only 52kB, POSIX allows it to be as small as 32k), and as a (rather strange) consequence mmap() (from compat/mmap.c) fails with EACCESS (why EACCESS?), because xpread() returns something > 0.

How large is SSIZE_MAX on other platforms? What happens there if you try to read() more? Should't we rather use SSIZE_MAX on all platforms? If I'm reading the header files right, on Linux it is LONG_MAX (2TB?), so I guess we should really go for MIN(8*1024*1024,SSIZE_MAX)?

bye, Jojo
Torsten Bögershausen· Feb 7, 2015, 17:19 UTC · re: Joachim Schmitz · lore

Re: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?

On 2015-02-07 17.45, Joachim Schmitz wrote:
Show 18 quoted lines
> Hi there
> 
> While investigating the problem with hung git-upload-pack we think to have 
> found a bug in wrapper.c:
> 
> #define MAX_IO_SIZE (8*1024*1024)
> 
> This is then used in xread() to split read()s into suitable chunks.
> So far so good, but read() is only guaranteed to read as much as SSIZE_MAX 
> bytes at a time. And on our platform that is way lower than those 8MB (only 
> 52kB, POSIX allows it to be as small as 32k), and as a (rather strange) 
> consequence mmap() (from compat/mmap.c) fails with EACCESS (why EACCESS?), 
> because xpread() returns something > 0.
> 
> How large is SSIZE_MAX on other platforms? What happens there if you try to 
> read() more? Should't we rather use SSIZE_MAX on all platforms? If I'm 
> reading the header files right, on Linux it is LONG_MAX (2TB?), so I guess 
> we should really go for MIN(8*1024*1024,SSIZE_MAX)?
How about changing wrapper.c like this:
#ifndef MAX_IO_SIZE
 #define MAX_IO_SIZE (8*1024*1024)
#endif
---------------------
and to change config.mak.uname like this:
ifeq ($(uname_S),NONSTOP_KERNEL)
	BASIC_CFLAGS += -DMAX_IO_SIZE=(32*1024)
Does this work for you ?
Joachim Schmitz· Feb 7, 2015, 17:29 UTC · re: Torsten Bögershausen · lore

Re: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?

Torsten Bögershausen <tboegi <at> web.de> writes:
> 
> On 2015-02-07 17.45, Joachim Schmitz wrote:
<snip>
Show 13 quoted lines
> 
> How about changing wrapper.c like this:
> 
> #ifndef MAX_IO_SIZE
>  #define MAX_IO_SIZE (8*1024*1024)
> #endif
> ---------------------
> and to change config.mak.uname like this:
> 
> ifeq ($(uname_S),NONSTOP_KERNEL)
> 
> 	BASIC_CFLAGS += -DMAX_IO_SIZE=(32*1024)
> Does this work for you ?

Of course it would, but, a) 32k is smaller than we can go (and yes, we could make it 52k) b) never ever should read() be asked to read more than SSIZE_MAX, this should be true for every platform on the planet? You may want to have is smaller than SSIZE_MAX (like the current 8MB vs. the possible 2TB on Linux), but surely never larger?

Bye, Jojo
Joachim Schmitz· Feb 7, 2015, 18:03 UTC · re: Joachim Schmitz · lore

Re: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?

Joachim Schmitz <jojo <at> schmitz-digital.de> writes:
Show 5 quoted lines
> 
> Torsten Bögershausen <tboegi <at> web.de> writes:
> 
> > 
> > On 2015-02-07 17.45, Joachim Schmitz wrote:
<snip>
> b) never ever should read() be asked to read more than SSIZE_MAX, this  
> should be true for every platform on the planet? You may want to have is 
> smaller than SSIZE_MAX (like the current 8MB vs. the possible 2TB on 
> Linux), but surely never larger?
Se also $gmane/232469, where that issue cropped up for MacOS X 64bit?
Bye, Jojo
Torsten Bögershausen· Feb 7, 2015, 20:32 UTC · re: Joachim Schmitz · lore

Re: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?

On 2015-02-07 18.29, Joachim Schmitz wrote:
Show 21 quoted lines
> Torsten Bögershausen <tboegi <at> web.de> writes:
> 
>>
>> On 2015-02-07 17.45, Joachim Schmitz wrote:
> <snip>
>>
>> How about changing wrapper.c like this:
>>
>> #ifndef MAX_IO_SIZE
>>  #define MAX_IO_SIZE (8*1024*1024)
>> #endif
>> ---------------------
>> and to change config.mak.uname like this:
>>
>> ifeq ($(uname_S),NONSTOP_KERNEL)
>>
>> 	BASIC_CFLAGS += -DMAX_IO_SIZE=(32*1024)
>> Does this work for you ?
> 
> Of course it would, but, 
> a) 32k is smaller than we can go (and yes, we could make it 52k)
Sorry, I missed that:  (52*1024)
Show 5 quoted lines
> b) never ever should read() be asked to read more than SSIZE_MAX, this  
> should be true for every platform on the planet? You may want to have is 
> smaller than SSIZE_MAX (like the current 8MB vs. the possible 2TB on 
> Linux), but surely never larger?
>

Good question. I don't know every platform of the planet well enough to be helpful here, especially the ones which don't follow all the specifications.

In other words: As long as we can not guarantee that SSIZE_MAX is defined, (and is defined to somethong useful for xread()/xwrite() ) we should be more defensive here:

tweak only on platform where we know it is needed and we know that it works.

And leave the other ones alone, until someone finds another platform which needs the same or another tweak and sends a tested patch.

Thanks for the report, do you want to send a patch to the list ?
Junio C Hamano· Feb 7, 2015, 22:13 UTC · re: Torsten Bögershausen · lore

Re: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?

On Sat, Feb 7, 2015 at 12:32 PM, Torsten Bögershausen <tboegi@web.de> wrote:
Show 8 quoted lines
> I don't know every platform of the planet well enough to be helpful here,
> especially the ones which don't follow all the specifications.
>
> In other words: As long as we can not guarantee that SSIZE_MAX is defined,
> (and is defined to somethong useful for xread()/xwrite() )
> we should be more defensive here:
>
> tweak only on platform where we know it is needed and we know that it works.
Yup, I agree that is a sensible way to go.
 (1) if Makefile overrides the size, use it; otherwise
 (2) if SSIZE_MAX is defined, and it is smaller than our internal
default, use it; otherwise
 (3) use our internal default.
And leave our internal default to 8MB.

That way, nobody needs to do anything differently from his current build set-up, and I suspect that it would make step (1) optional.

Joachim Schmitz· Feb 7, 2015, 22:31 UTC · re: Junio C Hamano · lore

Re: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?

Junio C Hamano <gitster <at> pobox.com> writes:
> 
> On Sat, Feb 7, 2015 at 12:32 PM, Torsten Bögershausen <tboegi <at> 
web.de> wrote:
> > I don't know every platform of the planet well enough to be helpful 
here,
> > especially the ones which don't follow all the specifications.
> >
> > In other words: As long as we can not guarantee that SSIZE_MAX is 
defined,
> > (and is defined to somethong useful for xread()/xwrite() )
> > we should be more defensive here:
> >
> > tweak only on platform where we know it is needed and we know that it 
works.
Show 11 quoted lines
> 
> Yup, I agree that is a sensible way to go.
> 
>  (1) if Makefile overrides the size, use it; otherwise
>  (2) if SSIZE_MAX is defined, and it is smaller than our internal
> default, use it; otherwise
>  (3) use our internal default.
> 
> And leave our internal default to 8MB.
> 
> That way, nobody needs to do anything differently from his current build 
set-up,
> and I suspect that it would make step (1) optional.
> 
something like this:

/* allow overwriting from e.g. Makefile */ #if !defined(MAX_IO_SIZE) # define MAX_IO_SIZE (8*1024*1024) #endif /* for plattforms that have SSIZE and have it smaller */ #if defined(SSIZE_MAX && (SSIZE_MAX < MAX_IO_SIZE) # undef MAX_IO_SIZE /* avoid warning */ # define MAX_IO_SIZE SSIZE_MAX #endif

Steps 2 and 3 only , indeed step 1 not needed...
Bye, Jojo
Junio C Hamano· Feb 8, 2015, 02:13 UTC · re: Joachim Schmitz · lore

Re: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?

On Sat, Feb 7, 2015 at 2:31 PM, Joachim Schmitz <jojo@schmitz-digital.de> wrote:
Show 26 quoted lines
> Junio C Hamano <gitster <at> pobox.com> writes:
>>
>> Yup, I agree that is a sensible way to go.
>>
>>  (1) if Makefile overrides the size, use it; otherwise
>>  (2) if SSIZE_MAX is defined, and it is smaller than our internal
>> default, use it; otherwise
>>  (3) use our internal default.
>>
>> And leave our internal default to 8MB.
>>
>> That way, nobody needs to do anything differently from his current build
> set-up,
>> and I suspect that it would make step (1) optional.
>
> something like this:
>
> /* allow overwriting from e.g. Makefile */
> #if !defined(MAX_IO_SIZE)
> # define MAX_IO_SIZE (8*1024*1024)
> #endif
> /* for plattforms that have SSIZE and have it smaller */
> #if defined(SSIZE_MAX && (SSIZE_MAX < MAX_IO_SIZE)
> # undef MAX_IO_SIZE /* avoid warning */
> # define MAX_IO_SIZE SSIZE_MAX
> #endif

No, not like that. If you do (1), that is only so that the Makefile can override a broken definition a platform may give to SSIZE_MAX. So

 (1) if Makefile gives one, use it without second-guessing with SSIZE_MAX.
 (2) if SSIZE_MAX is defined, and if it is smaller than our internal
default, use it.
 (3) all other cases, us our internal default.
Randall S. Becker· Feb 8, 2015, 02:32 UTC · re: Junio C Hamano · lore

RE: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?

On Feb 7 2015 at 9:14 PM Junio C Hamano wrote:
Show 28 quoted lines
>On Sat, Feb 7, 2015 at 2:31 PM, Joachim Schmitz <jojo@schmitz-digital.de> wrote:
>> Junio C Hamano <gitster <at> pobox.com> writes:
>>>
>>> Yup, I agree that is a sensible way to go.
>>>
>>>  (1) if Makefile overrides the size, use it; otherwise
>>>  (2) if SSIZE_MAX is defined, and it is smaller than our internal 
>>> default, use it; otherwise
>>>  (3) use our internal default.
>>>
>>> And leave our internal default to 8MB.
>>>
>>> That way, nobody needs to do anything differently from his current 
>>> build
>> set-up,
>>> and I suspect that it would make step (1) optional.
>>
>> something like this:
>>
>> /* allow overwriting from e.g. Makefile */ #if !defined(MAX_IO_SIZE) # 
>> define MAX_IO_SIZE (8*1024*1024) #endif
>> /* for plattforms that have SSIZE and have it smaller */ #if 
>> defined(SSIZE_MAX && (SSIZE_MAX < MAX_IO_SIZE) # undef MAX_IO_SIZE /* 
>> avoid warning */ # define MAX_IO_SIZE SSIZE_MAX #endif
>No, not like that. If you do (1), that is only so that the Makefile can override a broken definition a platform may give to SSIZE_MAX.  So
> (1) if Makefile gives one, use it without second-guessing with SSIZE_MAX.
> (2) if SSIZE_MAX is defined, and if it is smaller than our internal default, use it.
> (3) all other cases, us our internal default.
That is reasonable. I am more concerned about our git-upload-pak (separate thread) anyway :)
Cheers, Randall
Joachim Schmitz· Feb 8, 2015, 12:05 UTC · re: Junio C Hamano · lore

Re: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?

Junio C Hamano <gitster <at> pobox.com> writes:
<snip>
Show 14 quoted lines
> >
> > something like this:
> >
> > /* allow overwriting from e.g. Makefile */
> > #if !defined(MAX_IO_SIZE)
> > # define MAX_IO_SIZE (8*1024*1024)
> > #endif
> > /* for plattforms that have SSIZE and have it smaller */
> > #if defined(SSIZE_MAX && (SSIZE_MAX < MAX_IO_SIZE)
> > # undef MAX_IO_SIZE /* avoid warning */
> > # define MAX_IO_SIZE SSIZE_MAX
> > #endif
> 
> No, not like that. If you do (1), that is only so that the Makefile can 
override
Show 6 quoted lines
> a broken definition a platform may give to SSIZE_MAX.  So
> 
>  (1) if Makefile gives one, use it without second-guessing with SSIZE_MAX.
>  (2) if SSIZE_MAX is defined, and if it is smaller than our internal
> default, use it.
>  (3) all other cases, us our internal default.
oops, yes, of course
/* allow overwriting from e.g. Makefile */
#ifndef(MAX_IO_SIZE)
# define MAX_IO_SIZE (8*1024*1024)
  /* for plattforms that have SSIZE and have it smaller */
# if defined(SSIZE_MAX) && (SSIZE_MAX < MAX_IO_SIZE)
#  undef MAX_IO_SIZE /* avoid warning */
#  define MAX_IO_SIZE SSIZE_MAX
# endif
#endif
Eric Sunshine· Feb 8, 2015, 17:09 UTC · re: Joachim Schmitz · lore

Re: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?

On Sun, Feb 8, 2015 at 7:05 AM, Joachim Schmitz <jojo@schmitz-digital.de> wrote:
Show 17 quoted lines
> Junio C Hamano <gitster <at> pobox.com> writes:
>>  (1) if Makefile gives one, use it without second-guessing with SSIZE_MAX.
>>  (2) if SSIZE_MAX is defined, and if it is smaller than our internal
>> default, use it.
>>  (3) all other cases, us our internal default.
>
> oops, yes, of course
>
> /* allow overwriting from e.g. Makefile */
> #ifndef(MAX_IO_SIZE)
> # define MAX_IO_SIZE (8*1024*1024)
>   /* for plattforms that have SSIZE and have it smaller */
> # if defined(SSIZE_MAX) && (SSIZE_MAX < MAX_IO_SIZE)
> #  undef MAX_IO_SIZE /* avoid warning */
> #  define MAX_IO_SIZE SSIZE_MAX
> # endif
> #endif
A bit cleaner:

#ifndef(MAX_IO_SIZE) # define MAX_IO_SIZE_DEFAULT (8*1024*1024) # if defined(SSIZE_MAX) && (SSIZE_MAX < MAX_IO_SIZE_DEFAULT) # define MAX_IO_SIZE SSIZE_MAX # else # define MAX_IO_SIZE MAX_IO_SIZE_DEFAULT # endif #endif

Junio C Hamano· Feb 11, 2015, 21:13 UTC · re: Eric Sunshine · lore

Re: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?

Eric Sunshine <sunshine@sunshineco.com> writes:
Show 10 quoted lines
> A bit cleaner:
>
> #ifndef(MAX_IO_SIZE)
> # define MAX_IO_SIZE_DEFAULT (8*1024*1024)
> # if defined(SSIZE_MAX) && (SSIZE_MAX < MAX_IO_SIZE_DEFAULT)
> #  define MAX_IO_SIZE SSIZE_MAX
> # else
> #  define MAX_IO_SIZE MAX_IO_SIZE_DEFAULT
> # endif
> #endif
OK, then let's do this.
-- >8 --
Subject: xread/xwrite: clip MAX_IO_SIZE to SSIZE_MAX

Since 0b6806b9 (xread, xwrite: limit size of IO to 8MB, 2013-08-20), we chomp our calls to read(2) and write(2) into chunks of MAX_IO_SIZE bytes (8 MiB), because a large IO results in a bad latency when the program needs to be killed. This also brought our IO below SSIZE_MAX, which is a limit POSIX allows read(2) and write(2) to fail when the IO size exceeds it, for OS X, where a problem was originally reported.

However, there are other systems that define SSIZE_MAX smaller than our default X-<. Make sure we clip our calls to this as well.

Reported-by: Joachim Schmitz <jojo@schmitz-digital.de>
Helped-by: Torsten Bögershausen <tboegi@web.de>
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 wrapper.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/wrapper.c b/wrapper.c
index 007ec0d..50e6697 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -172,8 +172,21 @@ void *xcalloc(size_t nmemb, size_t size)
  * 64-bit is buggy, returning EINVAL if len >= INT_MAX; and even in
  * the absence of bugs, large chunks can result in bad latencies when
  * you decide to kill the process.
+ *
+ * We pick 8 MiB as our default, but if the platform defines SSIZE_MAX
+ * that is smaller than that, clip it to SSIZE_MAX, as a call to
+ * read(2) or write(2) larger than taht is allowed to fail.  As the last
+ * resort, we allow a port to pass via CFLAGS e.g. "-DMAX_IO_SIZE=value"
+ * to override this, if the definition of SSIZE_MAX platform is broken.
  */
-#define MAX_IO_SIZE (8*1024*1024)
+#ifndef(MAX_IO_SIZE)
+# define MAX_IO_SIZE_DEFAULT (8*1024*1024)
+# if defined(SSIZE_MAX) && (SSIZE_MAX < MAX_IO_SIZE_DEFAULT)
+#  define MAX_IO_SIZE SSIZE_MAX
+# else
+#  define MAX_IO_SIZE MAX_IO_SIZE_DEFAULT
+# endif
+#endif
 
 /*
  * xread() is the same a read(), but it automatically restarts read()
Joachim Schmitz· Feb 11, 2015, 22:05 UTC · re: Joachim Schmitz · lore

Re: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?

Joachim Schmitz <jojo <at> schmitz-digital.de> writes:
Show 6 quoted lines
> 
> Junio C Hamano <gitster <at> pobox.com> writes:
> 
> <snip> 
> > OK, then let's do this.
> > 
Except for the type "taht"
Junio C Hamano· Feb 11, 2015, 23:15 UTC · re: Joachim Schmitz · lore

Re: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?

Joachim Schmitz <jojo@schmitz-digital.de> writes:
Show 11 quoted lines
> Joachim Schmitz <jojo <at> schmitz-digital.de> writes:
>
>> 
>> Junio C Hamano <gitster <at> pobox.com> writes:
>> 
>> <snip> 
>> > OK, then let's do this.
>> > 
>
>
> Except for the type "taht"
Also #ifndef part X-<
Here is what I queued for the day.
-- >8 --
Subject: xread/xwrite: clip MAX_IO_SIZE to SSIZE_MAX

Since 0b6806b9 (xread, xwrite: limit size of IO to 8MB, 2013-08-20), we chomp our calls to read(2) and write(2) into chunks of MAX_IO_SIZE bytes (8 MiB), because a large IO results in a bad latency when the program needs to be killed. This also brought our IO below SSIZE_MAX, which is a limit POSIX allows read(2) and write(2) to fail when the IO size exceeds it, for OS X, where a problem was originally reported.

However, there are other systems that define SSIZE_MAX smaller than our default, and feeding 8 MiB to underlying read(2)/write(2) would fail. Make sure we clip our calls to the lower limit as well.

Reported-by: Joachim Schmitz <jojo@schmitz-digital.de>
Helped-by: Torsten Bögershausen <tboegi@web.de>
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 wrapper.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/wrapper.c b/wrapper.c
index f92b147..c77c2eb 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -135,8 +135,21 @@ void *xcalloc(size_t nmemb, size_t size)
  * 64-bit is buggy, returning EINVAL if len >= INT_MAX; and even in
  * the absense of bugs, large chunks can result in bad latencies when
  * you decide to kill the process.
+ *
+ * We pick 8 MiB as our default, but if the platform defines SSIZE_MAX
+ * that is smaller than that, clip it to SSIZE_MAX, as a call to
+ * read(2) or write(2) larger than that is allowed to fail.  As the last
+ * resort, we allow a port to pass via CFLAGS e.g. "-DMAX_IO_SIZE=value"
+ * to override this, if the definition of SSIZE_MAX platform is broken.
  */
-#define MAX_IO_SIZE (8*1024*1024)
+#ifndef MAX_IO_SIZE
+# define MAX_IO_SIZE_DEFAULT (8*1024*1024)
+# if defined(SSIZE_MAX) && (SSIZE_MAX < MAX_IO_SIZE_DEFAULT)
+#  define MAX_IO_SIZE SSIZE_MAX
+# else
+#  define MAX_IO_SIZE MAX_IO_SIZE_DEFAULT
+# endif
+#endif
 
 /*
  * xread() is the same a read(), but it automatically restarts read()

-- 
2.3.0-186-g9f73ee1
Randall S. Becker· Feb 7, 2015, 18:06 UTC · re: Torsten Bögershausen · lore

RE: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?

On 2015-02-07 12:30PM Torsten Bögershausen wrote:
Show 28 quoted lines
>On 2015-02-07 17.45, Joachim Schmitz wrote:
>> Hi there
>> 
>> While investigating the problem with hung git-upload-pack we think to 
>> have found a bug in wrapper.c:
>> 
>> #define MAX_IO_SIZE (8*1024*1024)
>> 
>> This is then used in xread() to split read()s into suitable chunks.
>> So far so good, but read() is only guaranteed to read as much as 
>> SSIZE_MAX bytes at a time. And on our platform that is way lower than 
>> those 8MB (only 52kB, POSIX allows it to be as small as 32k), and as a 
>> (rather strange) consequence mmap() (from compat/mmap.c) fails with 
>> EACCESS (why EACCESS?), because xpread() returns something > 0.
>> 
>> How large is SSIZE_MAX on other platforms? What happens there if you 
>> try to
>> read() more? Should't we rather use SSIZE_MAX on all platforms? If I'm 
>> reading the header files right, on Linux it is LONG_MAX (2TB?), so I 
>> guess we should really go for MIN(8*1024*1024,SSIZE_MAX)?
>How about changing wrapper.c like this: 
>#ifndef MAX_IO_SIZE
> #define MAX_IO_SIZE (8*1024*1024)
>#endif
>---------------------
>and to change config.mak.uname like this:
>ifeq ($(uname_S),NONSTOP_KERNEL)
>	BASIC_CFLAGS += -DMAX_IO_SIZE=(32*1024) Does this work for you ?

Yes, thank you Torsten. I have made this change in our branch (on behalf of Jojo). I think we can accept it. The (32*1024) does need to be properly quoted, however.

Randall S. Becker· Feb 7, 2015, 18:20 UTC · re: Randall S. Becker · lore

RE: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?

On 2015-02-07 13:07PM Randall S. Becker wrote:
Show 26 quoted lines
>On 2015-02-07 12:30PM Torsten Bögershausen wrote:
>>On 2015-02-07 17.45, Joachim Schmitz wrote:
>>> Hi there
>>> 
>>> While investigating the problem with hung git-upload-pack we think to 
>>> have found a bug in wrapper.c:
>>> 
>>> #define MAX_IO_SIZE (8*1024*1024)
>>> 
>>> This is then used in xread() to split read()s into suitable chunks.
>>> So far so good, but read() is only guaranteed to read as much as 
>>> SSIZE_MAX bytes at a time. And on our platform that is way lower than 
>>> those 8MB (only 52kB, POSIX allows it to be as small as 32k), and as a 
>>> (rather strange) consequence mmap() (from compat/mmap.c) fails with 
>>> EACCESS (why EACCESS?), because xpread() returns something > 0.
>>> 
>>> How large is SSIZE_MAX on other platforms? What happens there if you 
>>> try to
>>> read() more? Should't we rather use SSIZE_MAX on all platforms? If I'm 
>>> reading the header files right, on Linux it is LONG_MAX (2TB?), so I 
>>> guess we should really go for MIN(8*1024*1024,SSIZE_MAX)?
>>How about changing wrapper.c like this: 
>>#ifndef MAX_IO_SIZE
>> #define MAX_IO_SIZE (8*1024*1024)
>>#endif
>>---------------------

Although I do agree with Jojo, that MAX_IO_SIZE seems to be a platform constant and should be defined in terms of SSIZE_MAX. So something like:

#ifndef MAX_IO_SIZE # ifdef SSIZE_MAX # define MAX_IO_SIZE (SSIZE_MAX) # else # define MAX_IO_SIZE (8*1024*1024) # endif #endif

would be desirable.
Cheers, Randall
Joachim Schmitz· Feb 7, 2015, 18:36 UTC · re: Randall S. Becker · lore

Re: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?

Randall S. Becker <rsbecker <at> nexbridge.com> writes:
> 
> On 2015-02-07 13:07PM Randall S. Becker wrote:
> >On 2015-02-07 12:30PM Torsten Bögershausen wrote:
> >>On 2015-02-07 17.45, Joachim Schmitz wrote:
<spip>
Show 12 quoted lines
> Although I do agree with Jojo, that MAX_IO_SIZE seems to be a platform
> constant and should be defined in terms of SSIZE_MAX. So something like:
> 
> #ifndef MAX_IO_SIZE
> # ifdef SSIZE_MAX
> #  define MAX_IO_SIZE (SSIZE_MAX)
> # else
> #  define MAX_IO_SIZE (8*1024*1024)
> # endif
> #endif
> 
> would be desirable.

It would be way too large on some platforms. those 8MB had been chosen for a good reason, I assume:

/*
 * Limit size of IO chunks, because huge chunks only cause pain.  OS X
 * 64-bit is buggy, returning EINVAL if len >= INT_MAX; and even in
 * the absence of bugs, large chunks can result in bad latencies when
 * you decide to kill the process.
 */
However it should never be larger than SSIZE_MAX
Joachim Schmitz· Feb 7, 2015, 19:14 UTC · re: Joachim Schmitz · lore

Re: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?

Joachim Schmitz <jojo <at> schmitz-digital.de> writes:
<snip>
> and as a (rather strange) 
> consequence mmap() (from compat/mmap.c) fails with EACCESS (why 
EACCESS?), 
> because xpread() returns something > 0.

Seems mmap() should either set errno to EINVAL or not set it at all an just 'forward' whatever xpread() has set.

As per http://man7.org/linux/man-pages/man2/mmap.2.html mmap() sets EINVAL if (amongst other things) it doesn't like the value of len, exactly the case here.

bye, Jojo

← back to recent threads