[PATCH] xfs_io: actually issue 0 size writes

[PATCH] xfs_io: actually issue 0 size writes

[Eric Sandeen]

While testing some stuff in generic_write_checks() in the
kernel I realized that you can't actually use xfs_io to send
a 0-byte write in.  This is actually a condition worth testing:

       If  count  is zero and fd refers to a regular file,
       then write() may return a failure status if one  of
       the  errors  below  is  detected.  If no errors are
       detected, 0 will be returned  without  causing  any
       other  effect.

So fix that up.

Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
---

iff --git a/io/pwrite.c b/io/pwrite.c
index 54c3f78..26a7850 100644
--- a/io/pwrite.c
+++ b/io/pwrite.c
@@ -163,7 +163,7 @@ write_buffer(
 	int		ops = 0;
 
 	*total = 0;
-	while (count > 0) {
+	while (count >= 0) {
 		if (fd > 0) {	/* input file given, read buffer first */
 			if (read_buffer(fd, skip + *total, bs, &bar, 0, 1) < 0)
 				break;
@@ -182,6 +182,8 @@ write_buffer(
 			break;
 		offset += bytes;
 		count -= bytes;
+		if (count == 0)
+			break;
 	}
 	return ops;
 }

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs_io: actually issue 0 size writes

[Felix Blyakher]

On Aug 13, 2009, at 5:15 PM, Eric Sandeen wrote:

> While testing some stuff in generic_write_checks() in the
> kernel I realized that you can't actually use xfs_io to send
> a 0-byte write in.  This is actually a condition worth testing:
>
>       If  count  is zero and fd refers to a regular file,
>       then write() may return a failure status if one  of
>       the  errors  below  is  detected.  If no errors are
>       detected, 0 will be returned  without  causing  any
>       other  effect.

As I understand the desire to be able to issue 0 size writes
from xfs_io is to test the possibility of writing to a given fd.
What kind of errors would you expect to test for?

Otherwise looks good.

Felix

>
>
> So fix that up.
>
> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
> ---
>
> iff --git a/io/pwrite.c b/io/pwrite.c
> index 54c3f78..26a7850 100644
> --- a/io/pwrite.c
> +++ b/io/pwrite.c
> @@ -163,7 +163,7 @@ write_buffer(
> 	int		ops = 0;
>
> 	*total = 0;
> -	while (count > 0) {
> +	while (count >= 0) {
> 		if (fd > 0) {	/* input file given, read buffer first */
> 			if (read_buffer(fd, skip + *total, bs, &bar, 0, 1) < 0)
> 				break;
> @@ -182,6 +182,8 @@ write_buffer(
> 			break;
> 		offset += bytes;
> 		count -= bytes;
> +		if (count == 0)
> +			break;
> 	}
> 	return ops;
> }
>
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs_io: actually issue 0 size writes

[Eric Sandeen]

Felix Blyakher wrote:
> On Aug 13, 2009, at 5:15 PM, Eric Sandeen wrote:
> 
>> While testing some stuff in generic_write_checks() in the
>> kernel I realized that you can't actually use xfs_io to send
>> a 0-byte write in.  This is actually a condition worth testing:
>>
>>       If  count  is zero and fd refers to a regular file,
>>       then write() may return a failure status if one  of
>>       the  errors  below  is  detected.  If no errors are
>>       detected, 0 will be returned  without  causing  any
>>       other  effect.
> 
> As I understand the desire to be able to issue 0 size writes
> from xfs_io is to test the possibility of writing to a given fd.
> What kind of errors would you expect to test for?

In general EFBIG or ENOSPC.

This sort of thing in generic_write_checks():

        if (unlikely(*pos >= inode->i_sb->s_maxbytes)) {
                if (*count || *pos > inode->i_sb->s_maxbytes) {
                     return -EFBIG;
                }
                /* zero-length writes at ->s_maxbytes are OK */
        }

Although I'm a little confused about why "*pos == s_maxbytes" is ok; I
thought s_maxbytes was a count/size whereas pos is an offset, so it
seems to me that pos == s_maxbytes is one past the max.  But anyway,
that's mostly unrelated to the patch in this thread.  :)

-Eric

> Otherwise looks good.
> 
> Felix


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs_io: actually issue 0 size writes

[Lachlan McIlroy]

----- "Eric Sandeen" <sandeen@sandeen.net> wrote:

> Felix Blyakher wrote:
> > On Aug 13, 2009, at 5:15 PM, Eric Sandeen wrote:
> > 
> >> While testing some stuff in generic_write_checks() in the
> >> kernel I realized that you can't actually use xfs_io to send
> >> a 0-byte write in.  This is actually a condition worth testing:
> >>
> >>       If  count  is zero and fd refers to a regular file,
> >>       then write() may return a failure status if one  of
> >>       the  errors  below  is  detected.  If no errors are
> >>       detected, 0 will be returned  without  causing  any
> >>       other  effect.
> > 
> > As I understand the desire to be able to issue 0 size writes
> > from xfs_io is to test the possibility of writing to a given fd.
> > What kind of errors would you expect to test for?
> 
> In general EFBIG or ENOSPC.
> 
> This sort of thing in generic_write_checks():
> 
>         if (unlikely(*pos >= inode->i_sb->s_maxbytes)) {
>                 if (*count || *pos > inode->i_sb->s_maxbytes) {
>                      return -EFBIG;
>                 }
>                 /* zero-length writes at ->s_maxbytes are OK */
>         }
> 
> Although I'm a little confused about why "*pos == s_maxbytes" is ok;
> I
> thought s_maxbytes was a count/size whereas pos is an offset, so it
> seems to me that pos == s_maxbytes is one past the max.  But anyway,
> that's mostly unrelated to the patch in this thread.  :)
pos == s_maxbytes is only okay if count == 0 also.  So even though we
are writing at the limit we are not actually going to write anything.
At s_maxbytes-1 we are allowed to write one byte and at s_maxbytes we
are allowed to write nothing - literally.

> 
> -Eric
> 
> > Otherwise looks good.
> > 
> > Felix
> 
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs_io: actually issue 0 size writes

[Eric Sandeen]

Lachlan McIlroy wrote:
> ----- "Eric Sandeen" <sandeen@sandeen.net> wrote:
> 
>> Felix Blyakher wrote:
>>> On Aug 13, 2009, at 5:15 PM, Eric Sandeen wrote:
>>>
>>>> While testing some stuff in generic_write_checks() in the
>>>> kernel I realized that you can't actually use xfs_io to send
>>>> a 0-byte write in.  This is actually a condition worth testing:
>>>>
>>>>       If  count  is zero and fd refers to a regular file,
>>>>       then write() may return a failure status if one  of
>>>>       the  errors  below  is  detected.  If no errors are
>>>>       detected, 0 will be returned  without  causing  any
>>>>       other  effect.
>>> As I understand the desire to be able to issue 0 size writes
>>> from xfs_io is to test the possibility of writing to a given fd.
>>> What kind of errors would you expect to test for?
>> In general EFBIG or ENOSPC.
>>
>> This sort of thing in generic_write_checks():
>>
>>         if (unlikely(*pos >= inode->i_sb->s_maxbytes)) {
>>                 if (*count || *pos > inode->i_sb->s_maxbytes) {
>>                      return -EFBIG;
>>                 }
>>                 /* zero-length writes at ->s_maxbytes are OK */
>>         }
>>
>> Although I'm a little confused about why "*pos == s_maxbytes" is ok;
>> I
>> thought s_maxbytes was a count/size whereas pos is an offset, so it
>> seems to me that pos == s_maxbytes is one past the max.  But anyway,
>> that's mostly unrelated to the patch in this thread.  :)

> pos == s_maxbytes is only okay if count == 0 also.  So even though we
> are writing at the limit we are not actually going to write anything.
> At s_maxbytes-1 we are allowed to write one byte and at s_maxbytes we
> are allowed to write nothing - literally.

I think my confusion over maxbytes is whether it's a size or an offset.

The comment says ... max size.

Also in the above function it does i_size_read on the block device -
again a size.

If it's a max offset you're right; if it's a max -size- then pos ==
s_maxbytes is already off the end, one past the limit.

-eric

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs_io: actually issue 0 size writes

[Lachlan McIlroy]

----- "Eric Sandeen" <sandeen@sandeen.net> wrote:

> Lachlan McIlroy wrote:
> > ----- "Eric Sandeen" <sandeen@sandeen.net> wrote:
> > 
> >> Felix Blyakher wrote:
> >>> On Aug 13, 2009, at 5:15 PM, Eric Sandeen wrote:
> >>>
> >>>> While testing some stuff in generic_write_checks() in the
> >>>> kernel I realized that you can't actually use xfs_io to send
> >>>> a 0-byte write in.  This is actually a condition worth testing:
> >>>>
> >>>>       If  count  is zero and fd refers to a regular file,
> >>>>       then write() may return a failure status if one  of
> >>>>       the  errors  below  is  detected.  If no errors are
> >>>>       detected, 0 will be returned  without  causing  any
> >>>>       other  effect.
> >>> As I understand the desire to be able to issue 0 size writes
> >>> from xfs_io is to test the possibility of writing to a given fd.
> >>> What kind of errors would you expect to test for?
> >> In general EFBIG or ENOSPC.
> >>
> >> This sort of thing in generic_write_checks():
> >>
> >>         if (unlikely(*pos >= inode->i_sb->s_maxbytes)) {
> >>                 if (*count || *pos > inode->i_sb->s_maxbytes) {
> >>                      return -EFBIG;
> >>                 }
> >>                 /* zero-length writes at ->s_maxbytes are OK */
> >>         }
> >>
> >> Although I'm a little confused about why "*pos == s_maxbytes" is
> ok;
> >> I
> >> thought s_maxbytes was a count/size whereas pos is an offset, so
> it
> >> seems to me that pos == s_maxbytes is one past the max.  But
> anyway,
> >> that's mostly unrelated to the patch in this thread.  :)
> 
> > pos == s_maxbytes is only okay if count == 0 also.  So even though
> we
> > are writing at the limit we are not actually going to write
> anything.
> > At s_maxbytes-1 we are allowed to write one byte and at s_maxbytes
> we
> > are allowed to write nothing - literally.
> 
> I think my confusion over maxbytes is whether it's a size or an
> offset.
> 
> The comment says ... max size.
Yes it's a size.

> 
> Also in the above function it does i_size_read on the block device -
> again a size.
Yes.

> 
> If it's a max offset you're right; if it's a max -size- then pos ==
> s_maxbytes is already off the end, one past the limit.
Technically it's only off the end if you try to read something.

> 
> -eric
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs_io: actually issue 0 size writes

[Christoph Hellwig]

On Thu, Aug 13, 2009 at 05:15:50PM -0500, Eric Sandeen wrote:
> While testing some stuff in generic_write_checks() in the
> kernel I realized that you can't actually use xfs_io to send
> a 0-byte write in.  This is actually a condition worth testing:
> 
>        If  count  is zero and fd refers to a regular file,
>        then write() may return a failure status if one  of
>        the  errors  below  is  detected.  If no errors are
>        detected, 0 will be returned  without  causing  any
>        other  effect.
> 
> So fix that up.
> 
> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>

Any reason you didn't put this in despite two positive reviews?

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs_io: actually issue 0 size writes

[Eric Sandeen]

On Aug 26, 2009, at 3:11 PM, Christoph Hellwig <hch@infradead.org>  
wrote:

> On Thu, Aug 13, 2009 at 05:15:50PM -0500, Eric Sandeen wrote:
>> While testing some stuff in generic_write_checks() in the
>> kernel I realized that you can't actually use xfs_io to send
>> a 0-byte write in.  This is actually a condition worth testing:
>>
>>       If  count  is zero and fd refers to a regular file,
>>       then write() may return a failure status if one  of
>>       the  errors  below  is  detected.  If no errors are
>>       detected, 0 will be returned  without  causing  any
>>       other  effect.
>>
>> So fix that up.
>>
>> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
>
> Any reason you didn't put this in despite two positive reviews?

Just general busyness and a vacation :)

Will do it soon.

-Eric

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs