Re: [PATCH] NFS: Fix "BUG at fs/aio.c:554!"

6 messages, 5 authors, 2011-01-19 · open the first message on its own page

Re: [PATCH] NFS: Fix "BUG at fs/aio.c:554!"

From: Nick Piggin <hidden>
Date: 2011-01-19 23:37:35

On Thu, Jan 20, 2011 at 10:31 AM, Trond Myklebust
[off-list ref] wrote:
On Thu, 2011-01-20 at 10:26 +1100, Nick Piggin wrote:
quoted
On Thu, Jan 20, 2011 at 10:25 AM, Trond Myklebust
quoted
quoted
Also, why is EIO the correct reply when no bytes were read/written? Why
shouldn't the VFS aio code be able to cope with a zero byte reply?
What would it do?
Just return that zero byte reply to userland.

zero bytes is a valid reply for ordinary read() and write(), so why
should we have to do anything different for aio_read()/aio_write()?
It doesn't give userspace much to do. zero reply from read means
EOF. Zero reply from write is pretty useless, I don't think we do it
in the buffered write path -- we either ensure we write at least
something or have a meaningful error to return.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] NFS: Fix "BUG at fs/aio.c:554!"

From: Chuck Lever <hidden>
Date: 2011-01-19 23:39:16

On Jan 19, 2011, at 6:37 PM, Nick Piggin wrote:
On Thu, Jan 20, 2011 at 10:31 AM, Trond Myklebust
[off-list ref] wrote:
quoted
On Thu, 2011-01-20 at 10:26 +1100, Nick Piggin wrote:
quoted
On Thu, Jan 20, 2011 at 10:25 AM, Trond Myklebust
quoted
quoted
quoted
Also, why is EIO the correct reply when no bytes were read/written? Why
shouldn't the VFS aio code be able to cope with a zero byte reply?
What would it do?
Just return that zero byte reply to userland.

zero bytes is a valid reply for ordinary read() and write(), so why
should we have to do anything different for aio_read()/aio_write()?
It doesn't give userspace much to do. zero reply from read means
EOF. Zero reply from write is pretty useless, I don't think we do it
in the buffered write path -- we either ensure we write at least
something or have a meaningful error to return.
I think in this case, the zero-length requests are already shunted off.  No zero-length requests make it down here, IIRC.  So we expect that either some bytes are started, or an error occurs.  If zero bytes were started and no error occurred, that's just... wrong.

-- 
Chuck Lever
chuck[dot]lever[at]oracle[dot]com




--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] NFS: Fix "BUG at fs/aio.c:554!"

From: Nick Piggin <npiggin@gmail.com>
Date: 2011-01-19 23:42:46

On Thu, Jan 20, 2011 at 10:39 AM, Chuck Lever [off-list ref] wrote:
On Jan 19, 2011, at 6:37 PM, Nick Piggin wrote:
quoted
On Thu, Jan 20, 2011 at 10:31 AM, Trond Myklebust
[off-list ref] wrote:
quoted
On Thu, 2011-01-20 at 10:26 +1100, Nick Piggin wrote:
quoted
On Thu, Jan 20, 2011 at 10:25 AM, Trond Myklebust
quoted
quoted
quoted
Also, why is EIO the correct reply when no bytes were read/written? Why
shouldn't the VFS aio code be able to cope with a zero byte reply?
What would it do?
Just return that zero byte reply to userland.

zero bytes is a valid reply for ordinary read() and write(), so why
should we have to do anything different for aio_read()/aio_write()?
It doesn't give userspace much to do. zero reply from read means
EOF. Zero reply from write is pretty useless, I don't think we do it
in the buffered write path -- we either ensure we write at least
something or have a meaningful error to return.
I think in this case, the zero-length requests are already shunted off.  No zero-length requests make it down here, IIRC.  So we expect that either some bytes are started, or an error occurs.  If zero bytes were started and no error occurred, that's just... wrong.
Right. So in this case you should have an error to return.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] NFS: Fix "BUG at fs/aio.c:554!"

From: Trond Myklebust <hidden>
Date: 2011-01-19 23:49:36

On Thu, 2011-01-20 at 10:37 +1100, Nick Piggin wrote: 
On Thu, Jan 20, 2011 at 10:31 AM, Trond Myklebust
[off-list ref] wrote:
quoted
On Thu, 2011-01-20 at 10:26 +1100, Nick Piggin wrote:
quoted
On Thu, Jan 20, 2011 at 10:25 AM, Trond Myklebust
quoted
quoted
quoted
Also, why is EIO the correct reply when no bytes were read/written? Why
shouldn't the VFS aio code be able to cope with a zero byte reply?
What would it do?
Just return that zero byte reply to userland.

zero bytes is a valid reply for ordinary read() and write(), so why
should we have to do anything different for aio_read()/aio_write()?
It doesn't give userspace much to do. zero reply from read means
EOF. Zero reply from write is pretty useless, I don't think we do it
in the buffered write path -- we either ensure we write at least
something or have a meaningful error to return.
zero reply from read means EOF _or_ user supplied a zero length buffer.

zero reply from write may also be useless, but it is a valid value. It
can simply mean the user supplied a zero length buffer.

-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org
www.netapp.com

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] NFS: Fix "BUG at fs/aio.c:554!"

From: Nick Piggin <npiggin@gmail.com>
Date: 2011-01-19 23:50:49

On Thu, Jan 20, 2011 at 10:49 AM, Trond Myklebust
[off-list ref] wrote:
On Thu, 2011-01-20 at 10:37 +1100, Nick Piggin wrote:
quoted
On Thu, Jan 20, 2011 at 10:31 AM, Trond Myklebust
[off-list ref] wrote:
quoted
On Thu, 2011-01-20 at 10:26 +1100, Nick Piggin wrote:
quoted
On Thu, Jan 20, 2011 at 10:25 AM, Trond Myklebust
quoted
quoted
quoted
Also, why is EIO the correct reply when no bytes were read/written? Why
shouldn't the VFS aio code be able to cope with a zero byte reply?
What would it do?
Just return that zero byte reply to userland.

zero bytes is a valid reply for ordinary read() and write(), so why
should we have to do anything different for aio_read()/aio_write()?
It doesn't give userspace much to do. zero reply from read means
EOF. Zero reply from write is pretty useless, I don't think we do it
in the buffered write path -- we either ensure we write at least
something or have a meaningful error to return.
zero reply from read means EOF _or_ user supplied a zero length buffer.

zero reply from write may also be useless, but it is a valid value. It
can simply mean the user supplied a zero length buffer.
OK, yes. I'm ignoring zero length request.

Re: [PATCH] NFS: Fix "BUG at fs/aio.c:554!"

From: Chuck Lever <hidden>
Date: 2011-01-19 23:58:29

On Jan 19, 2011, at 6:49 PM, Trond Myklebust wrote:
On Thu, 2011-01-20 at 10:37 +1100, Nick Piggin wrote: 
quoted
On Thu, Jan 20, 2011 at 10:31 AM, Trond Myklebust
[off-list ref] wrote:
quoted
On Thu, 2011-01-20 at 10:26 +1100, Nick Piggin wrote:
quoted
On Thu, Jan 20, 2011 at 10:25 AM, Trond Myklebust
quoted
quoted
quoted
Also, why is EIO the correct reply when no bytes were read/written? Why
shouldn't the VFS aio code be able to cope with a zero byte reply?
What would it do?
Just return that zero byte reply to userland.

zero bytes is a valid reply for ordinary read() and write(), so why
should we have to do anything different for aio_read()/aio_write()?
It doesn't give userspace much to do. zero reply from read means
EOF. Zero reply from write is pretty useless, I don't think we do it
in the buffered write path -- we either ensure we write at least
something or have a meaningful error to return.
zero reply from read means EOF _or_ user supplied a zero length buffer.

zero reply from write may also be useless, but it is a valid value. It
can simply mean the user supplied a zero length buffer.
This code is in the forward path.  So, here we are just dealing with starting the I/O.  If the server replies with a short read or write, that's handled in the callbacks, not here.

So, -EIO is appropriate if we couldn't even start the I/O, right?

Anyway, this is copied from the existing code, not something new with this patch.  If we want to change this part of the logic, maybe it should be part of a different patch?

-- 
Chuck Lever
chuck[dot]lever[at]oracle[dot]com



Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help