| Message ID | 1413209295-13509-1-git-send-email-andros@netapp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Oct 13, 2014, at 10:08 AM, andros@netapp.com wrote: > From: Andy Adamson <andros@netapp.com> > > If a pgio error is caught before the nfs_pgio_header args->context is set > with get_nfs_open_context, the context can be NULL. It looks like all three calls to nfs_pgio_error would hit this! The fix looks good to me. -dros > > Signed-off-by: Andy Adamson <andros@netapp.com> > --- > fs/nfs/inode.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c > index 141c9f4..a2b148e 100644 > --- a/fs/nfs/inode.c > +++ b/fs/nfs/inode.c > @@ -798,6 +798,9 @@ static void __put_nfs_open_context(struct nfs_open_context *ctx, int is_sync) > struct inode *inode = ctx->dentry->d_inode; > struct super_block *sb = ctx->dentry->d_sb; > > + if (ctx == NULL) > + return; > + > if (!list_empty(&ctx->list)) { > if (!atomic_dec_and_lock(&ctx->lock_context.count, &inode->i_lock)) > return; > -- > 1.8.3.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Resending to list... Apologies for the duplicate... On Mon, Oct 13, 2014 at 11:24 AM, Trond Myklebust <trond.myklebust@primarydata.com> wrote: > Hi Andy, > > > On Mon, Oct 13, 2014 at 10:08 AM, <andros@netapp.com> wrote: >> >> From: Andy Adamson <andros@netapp.com> >> >> If a pgio error is caught before the nfs_pgio_header args->context is set >> with get_nfs_open_context, the context can be NULL. >> >> Signed-off-by: Andy Adamson <andros@netapp.com> >> --- >> fs/nfs/inode.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c >> index 141c9f4..a2b148e 100644 >> --- a/fs/nfs/inode.c >> +++ b/fs/nfs/inode.c >> @@ -798,6 +798,9 @@ static void __put_nfs_open_context(struct >> nfs_open_context *ctx, int is_sync) >> struct inode *inode = ctx->dentry->d_inode; >> struct super_block *sb = ctx->dentry->d_sb; >> >> + if (ctx == NULL) >> + return; >> + >> if (!list_empty(&ctx->list)) { >> if (!atomic_dec_and_lock(&ctx->lock_context.count, >> &inode->i_lock)) >> return; >> -- >> 1.8.3.1 >> > > AFAICS, there is only one callsite where we need to check the context, so we > can avoid the check on all calls to put_nfs_open_context. Please see PATCH > 1/2 that I just posted in response to an off-list bug report from Steve > Dickson. > > Cheers > Trond > -- > > Trond Myklebust > > Linux NFS client maintainer, PrimaryData > > trond.myklebust@primarydata.com
On Oct 13, 2014, at 11:25 AM, Trond Myklebust <trond.myklebust@primarydata.com> wrote: > Resending to list... Apologies for the duplicate... > > On Mon, Oct 13, 2014 at 11:24 AM, Trond Myklebust > <trond.myklebust@primarydata.com> wrote: >> Hi Andy, >> >> >> On Mon, Oct 13, 2014 at 10:08 AM, <andros@netapp.com> wrote: >>> >>> From: Andy Adamson <andros@netapp.com> >>> >>> If a pgio error is caught before the nfs_pgio_header args->context is set >>> with get_nfs_open_context, the context can be NULL. >>> >>> Signed-off-by: Andy Adamson <andros@netapp.com> >>> --- >>> fs/nfs/inode.c | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c >>> index 141c9f4..a2b148e 100644 >>> --- a/fs/nfs/inode.c >>> +++ b/fs/nfs/inode.c >>> @@ -798,6 +798,9 @@ static void __put_nfs_open_context(struct >>> nfs_open_context *ctx, int is_sync) >>> struct inode *inode = ctx->dentry->d_inode; >>> struct super_block *sb = ctx->dentry->d_sb; >>> >>> + if (ctx == NULL) >>> + return; >>> + >>> if (!list_empty(&ctx->list)) { >>> if (!atomic_dec_and_lock(&ctx->lock_context.count, >>> &inode->i_lock)) >>> return; >>> -- >>> 1.8.3.1 >>> >> >> AFAICS, there is only one callsite where we need to check the context, so we >> can avoid the check on all calls to put_nfs_open_context. Please see PATCH >> 1/2 that I just posted in response to an off-list bug report from Steve >> Dickson. Thanks Trond, Both patches should fix the OOPS, but I think your reasoning to avoid the check in all calls to put_nfs_open_context makes sense. -dros >> >> Cheers >> Trond >> -- >> >> Trond Myklebust >> >> Linux NFS client maintainer, PrimaryData >> >> trond.myklebust@primarydata.com > > > > -- > Trond Myklebust > > Linux NFS client maintainer, PrimaryData > > trond.myklebust@primarydata.com > -- > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c index 141c9f4..a2b148e 100644 --- a/fs/nfs/inode.c +++ b/fs/nfs/inode.c @@ -798,6 +798,9 @@ static void __put_nfs_open_context(struct nfs_open_context *ctx, int is_sync) struct inode *inode = ctx->dentry->d_inode; struct super_block *sb = ctx->dentry->d_sb; + if (ctx == NULL) + return; + if (!list_empty(&ctx->list)) { if (!atomic_dec_and_lock(&ctx->lock_context.count, &inode->i_lock)) return;