On Thu, 28 Oct 2010 08:34:35 -0400 Jeff Layton <jlayton@xxxxxxxxxx> wrote: > On Thu, 28 Oct 2010 08:17:54 -0400 > Steve Dickson <steved@xxxxxxxxxx> wrote: > > > A typo, introduced by commit f11ac8db, in the nfs_direct_write() > > routine causes writes with O_DIRECT set to fail with a ENOMEM error. > > > > Found-by: Jeff Layton <jlayton@xxxxxxxxxx> > > Signed-off-by: Steve Dickson <steved@xxxxxxxxxx> > > --- > > fs/nfs/direct.c | 2 +- > > 1 files changed, 1 insertions(+), 1 deletions(-) > > > > diff --git a/fs/nfs/direct.c b/fs/nfs/direct.c > > index 064a809..84d3c8b 100644 > > --- a/fs/nfs/direct.c > > +++ b/fs/nfs/direct.c > > @@ -873,7 +873,7 @@ static ssize_t nfs_direct_write(struct kiocb *iocb, const struct iovec *iov, > > dreq->inode = inode; > > dreq->ctx = get_nfs_open_context(nfs_file_open_context(iocb->ki_filp)); > > dreq->l_ctx = nfs_get_lock_context(dreq->ctx); > > - if (dreq->l_ctx != NULL) > > + if (dreq->l_ctx == NULL) > > goto out_release; > > if (!is_sync_kiocb(iocb)) > > dreq->iocb = iocb; > > Also, since get_lock_context holds references, this prevents the fs > from being unmounted. It looks like this bug is in 2.6.36 too, so this > may be suitable for stable series as well. > Oh...and another thing I noticed too... nfs_create_request doesn't check for a NULL return from nfs_get_lock_context. If it ever does, it looks like that will likely trickle down to an oops in encode_stateid. It might be good to fix that as well. Maybe something like this compile-tested-only patch? --------------------[snip]--------------------- nfs: handle lock context allocation failures in nfs_create_request nfs_get_lock_context can return NULL on an allocation failure. Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx> --- fs/nfs/pagelist.c | 8 +++++++- 1 files changed, 7 insertions(+), 1 deletions(-) diff --git a/fs/nfs/pagelist.c b/fs/nfs/pagelist.c index 9194902..137b549 100644 --- a/fs/nfs/pagelist.c +++ b/fs/nfs/pagelist.c @@ -65,6 +65,13 @@ nfs_create_request(struct nfs_open_context *ctx, struct inode *inode, if (req == NULL) return ERR_PTR(-ENOMEM); + /* get lock context early so we can deal with alloc failures */ + req->wb_lock_context = nfs_get_lock_context(ctx); + if (req->wb_lock_context == NULL) { + nfs_page_free(req); + return ERR_PTR(-ENOMEM); + } + /* Initialize the request struct. Initially, we assume a * long write-back delay. This will be adjusted in * update_nfs_request below if the region is not locked. */ @@ -79,7 +86,6 @@ nfs_create_request(struct nfs_open_context *ctx, struct inode *inode, req->wb_pgbase = offset; req->wb_bytes = count; req->wb_context = get_nfs_open_context(ctx); - req->wb_lock_context = nfs_get_lock_context(ctx); kref_init(&req->wb_kref); return req; } -- 1.7.2.3 -- 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