On Thu, Jan 20, 2011 at 10:25 AM, Trond Myklebust <Trond.Myklebust@xxxxxxxxxx> wrote: > On Wed, 2011-01-19 at 17:36 -0500, Chuck Lever wrote: >> Nick Piggin reports: >> >> > I'm getting use after frees in aio code in NFS >> > >> > [ 2703.396766] Call Trace: >> > [ 2703.396858] [<ffffffff8100b057>] ? native_sched_clock+0x27/0x80 >> > [ 2703.396959] [<ffffffff8108509e>] ? put_lock_stats+0xe/0x40 >> > [ 2703.397058] [<ffffffff81088348>] ? lock_release_holdtime+0xa8/0x140 >> > [ 2703.397159] [<ffffffff8108a2a5>] lock_acquire+0x95/0x1b0 >> > [ 2703.397260] [<ffffffff811627db>] ? aio_put_req+0x2b/0x60 >> > [ 2703.397361] [<ffffffff81039701>] ? get_parent_ip+0x11/0x50 >> > [ 2703.397464] [<ffffffff81612a31>] _raw_spin_lock_irq+0x41/0x80 >> > [ 2703.397564] [<ffffffff811627db>] ? aio_put_req+0x2b/0x60 >> > [ 2703.397662] [<ffffffff811627db>] aio_put_req+0x2b/0x60 >> > [ 2703.397761] [<ffffffff811647fe>] do_io_submit+0x2be/0x7c0 >> > [ 2703.397895] [<ffffffff81164d0b>] sys_io_submit+0xb/0x10 >> > [ 2703.397995] [<ffffffff8100307b>] system_call_fastpath+0x16/0x1b >> > >> > Adding some tracing, it is due to nfs completing the request then >> > returning something other than -EIOCBQUEUED, so aio.c >> > also completes the request. >> >> To address this, prevent the NFS direct I/O engine from completing >> async iocbs when the forward path returns an error other than >> EIOCBQUEUED. >> >> This appears to survive ^C during both "xfstest no. 208" and "fsx -Z." >> >> Cc: Stable <stable@xxxxxxxxxx> >> Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx> >> --- >> >> Here's my take. >> >> fs/nfs/direct.c | 32 +++++++++++++++++--------------- >> 1 files changed, 17 insertions(+), 15 deletions(-) >> >> diff --git a/fs/nfs/direct.c b/fs/nfs/direct.c >> index e6ace0d..c2176f4 100644 >> --- a/fs/nfs/direct.c >> +++ b/fs/nfs/direct.c >> @@ -407,15 +407,16 @@ static ssize_t nfs_direct_read_schedule_iovec(struct nfs_direct_req *dreq, >> pos += vec->iov_len; >> } >> >> - if (put_dreq(dreq)) >> - nfs_direct_complete(dreq); >> - >> - if (requested_bytes != 0) >> - return 0; >> + /* >> + * If no bytes were started, return the error, and let the >> + * generic layer handle the completion. >> + */ >> + if (requested_bytes == 0) >> + return result < 0 ? result : -EIO; >> >> - if (result < 0) >> - return result; >> - return -EIO; >> + if (put_dreq(dreq)) >> + nfs_direct_write_complete(dreq, dreq->inode); > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > In nfs_direct_read_schedule_iovec()? Shouldn't that be > nfs_direct_complete(dreq); > > 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? -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
