| Message ID | 20230519111723.20612-1-jlayton@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | nfsd: don't provide pre/post-op attrs if fh_getattr fails | expand |
On Fri, 2023-05-19 at 07:17 -0400, Jeff Layton wrote: > nfsd calls fh_getattr to get the latest inode attrs for pre/post-op > info. In the event that fh_getattr fails, it resorts to scraping > cached > values out of the inode directly. > > Since these attributes are optional, we can just skip providing them > altogether when this happens. > > Signed-off-by: Jeff Layton <jlayton@kernel.org> > --- > fs/nfsd/nfsfh.c | 26 +++++++------------------- > 1 file changed, 7 insertions(+), 19 deletions(-) > > diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c > index ccd8485fee04..e8e13ae72e3c 100644 > --- a/fs/nfsd/nfsfh.c > +++ b/fs/nfsd/nfsfh.c > @@ -623,16 +623,9 @@ void fh_fill_pre_attrs(struct svc_fh *fhp) > > inode = d_inode(fhp->fh_dentry); > err = fh_getattr(fhp, &stat); > - if (err) { > - /* Grab the times from inode anyway */ > - stat.mtime = inode->i_mtime; > - stat.ctime = inode->i_ctime; > - stat.size = inode->i_size; > - if (v4 && IS_I_VERSION(inode)) { > - stat.change_cookie = > inode_query_iversion(inode); > - stat.result_mask |= STATX_CHANGE_COOKIE; > - } > - } > + if (err) > + return; > + > if (v4) > fhp->fh_pre_change = nfsd4_change_attribute(&stat, > inode); > > @@ -660,15 +653,10 @@ void fh_fill_post_attrs(struct svc_fh *fhp) > printk("nfsd: inode locked twice during > operation.\n"); > > err = fh_getattr(fhp, &fhp->fh_post_attr); > - if (err) { > - fhp->fh_post_saved = false; > - fhp->fh_post_attr.ctime = inode->i_ctime; > - if (v4 && IS_I_VERSION(inode)) { > - fhp->fh_post_attr.change_cookie = > inode_query_iversion(inode); > - fhp->fh_post_attr.result_mask |= > STATX_CHANGE_COOKIE; > - } > - } else > - fhp->fh_post_saved = true; > + if (err) > + return; > + > + fhp->fh_post_saved = true; > if (v4) > fhp->fh_post_change = > nfsd4_change_attribute(&fhp->fh_post_attr, > inode); Unfortunately, I did recently find a corner case where this behaviour will break the Linux NFSv3 client. In the case where READ sometimes returns post-op attributes, and sometimes not, we can end up triggering the 'out_overflow' in xdr_get_next_encode_buffer(), resulting in an EIO error. The problem is ultimately due to the attempt by the client to align the pages to where it expects the READ reply to occur. When the behaviour is unpredictable, then it sometimes ends up allocating too little memory for the attributes, and ends up getting confused. This bug does need to be fixed in the client, but just a warning that the above server patch would be capable of triggering it.
On Fri, 2023-05-19 at 13:08 +0000, Trond Myklebust wrote: > On Fri, 2023-05-19 at 07:17 -0400, Jeff Layton wrote: > > nfsd calls fh_getattr to get the latest inode attrs for pre/post-op > > info. In the event that fh_getattr fails, it resorts to scraping > > cached > > values out of the inode directly. > > > > Since these attributes are optional, we can just skip providing them > > altogether when this happens. > > > > Signed-off-by: Jeff Layton <jlayton@kernel.org> > > --- > > fs/nfsd/nfsfh.c | 26 +++++++------------------- > > 1 file changed, 7 insertions(+), 19 deletions(-) > > > > diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c > > index ccd8485fee04..e8e13ae72e3c 100644 > > --- a/fs/nfsd/nfsfh.c > > +++ b/fs/nfsd/nfsfh.c > > @@ -623,16 +623,9 @@ void fh_fill_pre_attrs(struct svc_fh *fhp) > > > > inode = d_inode(fhp->fh_dentry); > > err = fh_getattr(fhp, &stat); > > - if (err) { > > - /* Grab the times from inode anyway */ > > - stat.mtime = inode->i_mtime; > > - stat.ctime = inode->i_ctime; > > - stat.size = inode->i_size; > > - if (v4 && IS_I_VERSION(inode)) { > > - stat.change_cookie = > > inode_query_iversion(inode); > > - stat.result_mask |= STATX_CHANGE_COOKIE; > > - } > > - } > > + if (err) > > + return; > > + > > if (v4) > > fhp->fh_pre_change = nfsd4_change_attribute(&stat, > > inode); > > > > @@ -660,15 +653,10 @@ void fh_fill_post_attrs(struct svc_fh *fhp) > > printk("nfsd: inode locked twice during > > operation.\n"); > > > > err = fh_getattr(fhp, &fhp->fh_post_attr); > > - if (err) { > > - fhp->fh_post_saved = false; > > - fhp->fh_post_attr.ctime = inode->i_ctime; > > - if (v4 && IS_I_VERSION(inode)) { > > - fhp->fh_post_attr.change_cookie = > > inode_query_iversion(inode); > > - fhp->fh_post_attr.result_mask |= > > STATX_CHANGE_COOKIE; > > - } > > - } else > > - fhp->fh_post_saved = true; > > + if (err) > > + return; > > + > > + fhp->fh_post_saved = true; > > if (v4) > > fhp->fh_post_change = > > nfsd4_change_attribute(&fhp->fh_post_attr, > > inode); > > Unfortunately, I did recently find a corner case where this behaviour > will break the Linux NFSv3 client. In the case where READ sometimes > returns post-op attributes, and sometimes not, we can end up triggering > the 'out_overflow' in xdr_get_next_encode_buffer(), resulting in an EIO > error. > > The problem is ultimately due to the attempt by the client to align the > pages to where it expects the READ reply to occur. When the behaviour > is unpredictable, then it sometimes ends up allocating too little > memory for the attributes, and ends up getting confused. > > This bug does need to be fixed in the client, but just a warning that > the above server patch would be capable of triggering it. > Thanks for the heads up. This is not a critical issue, so I'm OK with delaying this change if it's going to cause undue pain in the field. We could also consider providing a module option or something in the meantime, to give people a workaround if they hit it. OTOH, this should only rarely happen. getattr doesn't often fail unless you're exporting something like NFS or Ceph and someone does something nefarious on the backend server/cluster.
> On May 19, 2023, at 9:26 AM, Jeff Layton <jlayton@kernel.org> wrote: > > On Fri, 2023-05-19 at 13:08 +0000, Trond Myklebust wrote: >> On Fri, 2023-05-19 at 07:17 -0400, Jeff Layton wrote: >>> nfsd calls fh_getattr to get the latest inode attrs for pre/post-op >>> info. In the event that fh_getattr fails, it resorts to scraping >>> cached >>> values out of the inode directly. >>> >>> Since these attributes are optional, we can just skip providing them >>> altogether when this happens. >>> >>> Signed-off-by: Jeff Layton <jlayton@kernel.org> >>> --- >>> fs/nfsd/nfsfh.c | 26 +++++++------------------- >>> 1 file changed, 7 insertions(+), 19 deletions(-) >>> >>> diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c >>> index ccd8485fee04..e8e13ae72e3c 100644 >>> --- a/fs/nfsd/nfsfh.c >>> +++ b/fs/nfsd/nfsfh.c >>> @@ -623,16 +623,9 @@ void fh_fill_pre_attrs(struct svc_fh *fhp) >>> >>> inode = d_inode(fhp->fh_dentry); >>> err = fh_getattr(fhp, &stat); >>> - if (err) { >>> - /* Grab the times from inode anyway */ >>> - stat.mtime = inode->i_mtime; >>> - stat.ctime = inode->i_ctime; >>> - stat.size = inode->i_size; >>> - if (v4 && IS_I_VERSION(inode)) { >>> - stat.change_cookie = >>> inode_query_iversion(inode); >>> - stat.result_mask |= STATX_CHANGE_COOKIE; >>> - } >>> - } >>> + if (err) >>> + return; >>> + >>> if (v4) >>> fhp->fh_pre_change = nfsd4_change_attribute(&stat, >>> inode); >>> >>> @@ -660,15 +653,10 @@ void fh_fill_post_attrs(struct svc_fh *fhp) >>> printk("nfsd: inode locked twice during >>> operation.\n"); >>> >>> err = fh_getattr(fhp, &fhp->fh_post_attr); >>> - if (err) { >>> - fhp->fh_post_saved = false; >>> - fhp->fh_post_attr.ctime = inode->i_ctime; >>> - if (v4 && IS_I_VERSION(inode)) { >>> - fhp->fh_post_attr.change_cookie = >>> inode_query_iversion(inode); >>> - fhp->fh_post_attr.result_mask |= >>> STATX_CHANGE_COOKIE; >>> - } >>> - } else >>> - fhp->fh_post_saved = true; >>> + if (err) >>> + return; >>> + >>> + fhp->fh_post_saved = true; >>> if (v4) >>> fhp->fh_post_change = >>> nfsd4_change_attribute(&fhp->fh_post_attr, >>> inode); >> >> Unfortunately, I did recently find a corner case where this behaviour >> will break the Linux NFSv3 client. In the case where READ sometimes >> returns post-op attributes, and sometimes not, we can end up triggering >> the 'out_overflow' in xdr_get_next_encode_buffer(), resulting in an EIO >> error. >> >> The problem is ultimately due to the attempt by the client to align the >> pages to where it expects the READ reply to occur. When the behaviour >> is unpredictable, then it sometimes ends up allocating too little >> memory for the attributes, and ends up getting confused. >> >> This bug does need to be fixed in the client, but just a warning that >> the above server patch would be capable of triggering it. >> > > Thanks for the heads up. This is not a critical issue, so I'm OK with > delaying this change if it's going to cause undue pain in the field. We > could also consider providing a module option or something in the > meantime, to give people a workaround if they hit it. > > OTOH, this should only rarely happen. getattr doesn't often fail unless > you're exporting something like NFS or Ceph and someone does something > nefarious on the backend server/cluster. Right, we expect that the fh_getattr() operation will fail only very rarely, usually due to a file getting removed or renamed over at the wrong instant. If you can provide a fix for the client for v6.5, IMO it would be a low risk to apply both this patch and your fix at the same time. -- Chuck Lever
On Fri, 19 May 2023, Jeff Layton wrote: > nfsd calls fh_getattr to get the latest inode attrs for pre/post-op > info. In the event that fh_getattr fails, it resorts to scraping cached > values out of the inode directly. > > Since these attributes are optional, we can just skip providing them > altogether when this happens. > > Signed-off-by: Jeff Layton <jlayton@kernel.org> > --- > fs/nfsd/nfsfh.c | 26 +++++++------------------- > 1 file changed, 7 insertions(+), 19 deletions(-) > > diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c > index ccd8485fee04..e8e13ae72e3c 100644 > --- a/fs/nfsd/nfsfh.c > +++ b/fs/nfsd/nfsfh.c > @@ -623,16 +623,9 @@ void fh_fill_pre_attrs(struct svc_fh *fhp) > > inode = d_inode(fhp->fh_dentry); > err = fh_getattr(fhp, &stat); > - if (err) { > - /* Grab the times from inode anyway */ > - stat.mtime = inode->i_mtime; > - stat.ctime = inode->i_ctime; > - stat.size = inode->i_size; > - if (v4 && IS_I_VERSION(inode)) { > - stat.change_cookie = inode_query_iversion(inode); > - stat.result_mask |= STATX_CHANGE_COOKIE; > - } > - } > + if (err) > + return; > + I wondered if this might exercise error paths which had not previously been tested. Before this change fh_pre_saved is always set, now it is not. The code looks OK, but I was amused by xdr_stream_encode_item_absent(). Various places in the code test for "< 0" or "> 0" which seems to suggest that "0" is not being handled consistently. But of course xdr_stream_encode_item_absent() can never return 0. It returns either XDR_UNIT or -EMSGSIZE. I wonder if we should be consistent in how we test for an error .... or if it it really matters. Patch itself looks good. Thanks, NeilBrown > if (v4) > fhp->fh_pre_change = nfsd4_change_attribute(&stat, inode); > > @@ -660,15 +653,10 @@ void fh_fill_post_attrs(struct svc_fh *fhp) > printk("nfsd: inode locked twice during operation.\n"); > > err = fh_getattr(fhp, &fhp->fh_post_attr); > - if (err) { > - fhp->fh_post_saved = false; > - fhp->fh_post_attr.ctime = inode->i_ctime; > - if (v4 && IS_I_VERSION(inode)) { > - fhp->fh_post_attr.change_cookie = inode_query_iversion(inode); > - fhp->fh_post_attr.result_mask |= STATX_CHANGE_COOKIE; > - } > - } else > - fhp->fh_post_saved = true; > + if (err) > + return; > + > + fhp->fh_post_saved = true; > if (v4) > fhp->fh_post_change = > nfsd4_change_attribute(&fhp->fh_post_attr, inode); > -- > 2.40.1 > >
> On May 21, 2023, at 9:24 PM, NeilBrown <neilb@suse.de> wrote: > > On Fri, 19 May 2023, Jeff Layton wrote: >> nfsd calls fh_getattr to get the latest inode attrs for pre/post-op >> info. In the event that fh_getattr fails, it resorts to scraping cached >> values out of the inode directly. >> >> Since these attributes are optional, we can just skip providing them >> altogether when this happens. >> >> Signed-off-by: Jeff Layton <jlayton@kernel.org> >> --- >> fs/nfsd/nfsfh.c | 26 +++++++------------------- >> 1 file changed, 7 insertions(+), 19 deletions(-) >> >> diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c >> index ccd8485fee04..e8e13ae72e3c 100644 >> --- a/fs/nfsd/nfsfh.c >> +++ b/fs/nfsd/nfsfh.c >> @@ -623,16 +623,9 @@ void fh_fill_pre_attrs(struct svc_fh *fhp) >> >> inode = d_inode(fhp->fh_dentry); >> err = fh_getattr(fhp, &stat); >> - if (err) { >> - /* Grab the times from inode anyway */ >> - stat.mtime = inode->i_mtime; >> - stat.ctime = inode->i_ctime; >> - stat.size = inode->i_size; >> - if (v4 && IS_I_VERSION(inode)) { >> - stat.change_cookie = inode_query_iversion(inode); >> - stat.result_mask |= STATX_CHANGE_COOKIE; >> - } >> - } >> + if (err) >> + return; >> + > > I wondered if this might exercise error paths which had not previously > been tested. Before this change fh_pre_saved is always set, now it is > not. > > The code looks OK, but I was amused by xdr_stream_encode_item_absent(). > Various places in the code test for "< 0" or "> 0" which seems to > suggest that "0" is not being handled consistently. You can read those as "returns positive" and "returns negative" tests. > But of course xdr_stream_encode_item_absent() can never return 0. It > returns either XDR_UNIT or -EMSGSIZE. I don't see any tests for it returning exactly zero. > I wonder if we should be consistent in how we test for an error .... or > if it it really matters. The xdr_stream_encode_* functions conventionally return a negative errno or a positive number of bytes encoded. The "< 0" and "> 0" tests convert that return value into a boolean. I reviewed the call sites just now and do not see an evident problem. > Patch itself looks good. May I add "Reviewed-by: Neil Brown <neilb@suse.de <mailto:neilb@suse.de>>" ? > Thanks, > NeilBrown > > >> if (v4) >> fhp->fh_pre_change = nfsd4_change_attribute(&stat, inode); >> >> @@ -660,15 +653,10 @@ void fh_fill_post_attrs(struct svc_fh *fhp) >> printk("nfsd: inode locked twice during operation.\n"); >> >> err = fh_getattr(fhp, &fhp->fh_post_attr); >> - if (err) { >> - fhp->fh_post_saved = false; >> - fhp->fh_post_attr.ctime = inode->i_ctime; >> - if (v4 && IS_I_VERSION(inode)) { >> - fhp->fh_post_attr.change_cookie = inode_query_iversion(inode); >> - fhp->fh_post_attr.result_mask |= STATX_CHANGE_COOKIE; >> - } >> - } else >> - fhp->fh_post_saved = true; >> + if (err) >> + return; >> + >> + fhp->fh_post_saved = true; >> if (v4) >> fhp->fh_post_change = >> nfsd4_change_attribute(&fhp->fh_post_attr, inode); >> -- >> 2.40.1 -- Chuck Lever
On Tue, 23 May 2023, Chuck Lever III wrote: > > > On May 21, 2023, at 9:24 PM, NeilBrown <neilb@suse.de> wrote: > > > > On Fri, 19 May 2023, Jeff Layton wrote: > >> nfsd calls fh_getattr to get the latest inode attrs for pre/post-op > >> info. In the event that fh_getattr fails, it resorts to scraping cached > >> values out of the inode directly. > >> > >> Since these attributes are optional, we can just skip providing them > >> altogether when this happens. > >> > >> Signed-off-by: Jeff Layton <jlayton@kernel.org> > >> --- > >> fs/nfsd/nfsfh.c | 26 +++++++------------------- > >> 1 file changed, 7 insertions(+), 19 deletions(-) > >> > >> diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c > >> index ccd8485fee04..e8e13ae72e3c 100644 > >> --- a/fs/nfsd/nfsfh.c > >> +++ b/fs/nfsd/nfsfh.c > >> @@ -623,16 +623,9 @@ void fh_fill_pre_attrs(struct svc_fh *fhp) > >> > >> inode = d_inode(fhp->fh_dentry); > >> err = fh_getattr(fhp, &stat); > >> - if (err) { > >> - /* Grab the times from inode anyway */ > >> - stat.mtime = inode->i_mtime; > >> - stat.ctime = inode->i_ctime; > >> - stat.size = inode->i_size; > >> - if (v4 && IS_I_VERSION(inode)) { > >> - stat.change_cookie = inode_query_iversion(inode); > >> - stat.result_mask |= STATX_CHANGE_COOKIE; > >> - } > >> - } > >> + if (err) > >> + return; > >> + > > > > I wondered if this might exercise error paths which had not previously > > been tested. Before this change fh_pre_saved is always set, now it is > > not. > > > > The code looks OK, but I was amused by xdr_stream_encode_item_absent(). > > Various places in the code test for "< 0" or "> 0" which seems to > > suggest that "0" is not being handled consistently. > > You can read those as "returns positive" and "returns negative" tests. That leaves the curious reader, who isn't completely familiar with the code, wondering what "0" would mean. It's not a big deal, but it looked odd so I thought I would mention it. > > > > But of course xdr_stream_encode_item_absent() can never return 0. It > > returns either XDR_UNIT or -EMSGSIZE. > > I don't see any tests for it returning exactly zero. > > > > I wonder if we should be consistent in how we test for an error .... or > > if it it really matters. > > The xdr_stream_encode_* functions conventionally return a negative errno > or a positive number of bytes encoded. The "< 0" and "> 0" tests convert > that return value into a boolean. > > I reviewed the call sites just now and do not see an evident problem. > > > > Patch itself looks good. > > May I add "Reviewed-by: Neil Brown <neilb@suse.de <mailto:neilb@suse.de>>" ? Yes please. (though maybe without the "mailto:" :-) NeilBrown
> On May 23, 2023, at 6:03 PM, NeilBrown <neilb@suse.de> wrote: > > On Tue, 23 May 2023, Chuck Lever III wrote: >> >>> On May 21, 2023, at 9:24 PM, NeilBrown <neilb@suse.de> wrote: >>> >>> On Fri, 19 May 2023, Jeff Layton wrote: >>>> nfsd calls fh_getattr to get the latest inode attrs for pre/post-op >>>> info. In the event that fh_getattr fails, it resorts to scraping cached >>>> values out of the inode directly. >>>> >>>> Since these attributes are optional, we can just skip providing them >>>> altogether when this happens. >>>> >>>> Signed-off-by: Jeff Layton <jlayton@kernel.org> >>>> --- >>>> fs/nfsd/nfsfh.c | 26 +++++++------------------- >>>> 1 file changed, 7 insertions(+), 19 deletions(-) >>>> >>>> diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c >>>> index ccd8485fee04..e8e13ae72e3c 100644 >>>> --- a/fs/nfsd/nfsfh.c >>>> +++ b/fs/nfsd/nfsfh.c >>>> @@ -623,16 +623,9 @@ void fh_fill_pre_attrs(struct svc_fh *fhp) >>>> >>>> inode = d_inode(fhp->fh_dentry); >>>> err = fh_getattr(fhp, &stat); >>>> - if (err) { >>>> - /* Grab the times from inode anyway */ >>>> - stat.mtime = inode->i_mtime; >>>> - stat.ctime = inode->i_ctime; >>>> - stat.size = inode->i_size; >>>> - if (v4 && IS_I_VERSION(inode)) { >>>> - stat.change_cookie = inode_query_iversion(inode); >>>> - stat.result_mask |= STATX_CHANGE_COOKIE; >>>> - } >>>> - } >>>> + if (err) >>>> + return; >>>> + >>> >>> I wondered if this might exercise error paths which had not previously >>> been tested. Before this change fh_pre_saved is always set, now it is >>> not. >>> >>> The code looks OK, but I was amused by xdr_stream_encode_item_absent(). >>> Various places in the code test for "< 0" or "> 0" which seems to >>> suggest that "0" is not being handled consistently. >> >> You can read those as "returns positive" and "returns negative" tests. > > That leaves the curious reader, who isn't completely familiar with the > code, wondering what "0" would mean. > It's not a big deal, but it looked odd so I thought I would mention it. Code readability is always on point. The return values are a feature of all the xdr_stream_encode_* helpers. For _item_absent() in particular, we could go with "!= XDR_UNIT" but that's more verbose and still not especially more clear. Probably the one spot that is confusing is the "_item_absent() > 0" call site. That's meant to mean "true if it worked, false if not". There was again no real better alternative. >>> But of course xdr_stream_encode_item_absent() can never return 0. It >>> returns either XDR_UNIT or -EMSGSIZE. >> >> I don't see any tests for it returning exactly zero. >> >> >>> I wonder if we should be consistent in how we test for an error .... or >>> if it it really matters. >> >> The xdr_stream_encode_* functions conventionally return a negative errno >> or a positive number of bytes encoded. The "< 0" and "> 0" tests convert >> that return value into a boolean. >> >> I reviewed the call sites just now and do not see an evident problem. >> >> >>> Patch itself looks good. >> >> May I add "Reviewed-by: Neil Brown <neilb@suse.de <mailto:neilb@suse.de>>" ? > > Yes please. (though maybe without the "mailto:" :-) Thanks. I typed the address correctly, but oddly, Mail.app helpfully added the mailto: tag when it sent the mail. Ho hum. -- Chuck Lever
diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c index ccd8485fee04..e8e13ae72e3c 100644 --- a/fs/nfsd/nfsfh.c +++ b/fs/nfsd/nfsfh.c @@ -623,16 +623,9 @@ void fh_fill_pre_attrs(struct svc_fh *fhp) inode = d_inode(fhp->fh_dentry); err = fh_getattr(fhp, &stat); - if (err) { - /* Grab the times from inode anyway */ - stat.mtime = inode->i_mtime; - stat.ctime = inode->i_ctime; - stat.size = inode->i_size; - if (v4 && IS_I_VERSION(inode)) { - stat.change_cookie = inode_query_iversion(inode); - stat.result_mask |= STATX_CHANGE_COOKIE; - } - } + if (err) + return; + if (v4) fhp->fh_pre_change = nfsd4_change_attribute(&stat, inode); @@ -660,15 +653,10 @@ void fh_fill_post_attrs(struct svc_fh *fhp) printk("nfsd: inode locked twice during operation.\n"); err = fh_getattr(fhp, &fhp->fh_post_attr); - if (err) { - fhp->fh_post_saved = false; - fhp->fh_post_attr.ctime = inode->i_ctime; - if (v4 && IS_I_VERSION(inode)) { - fhp->fh_post_attr.change_cookie = inode_query_iversion(inode); - fhp->fh_post_attr.result_mask |= STATX_CHANGE_COOKIE; - } - } else - fhp->fh_post_saved = true; + if (err) + return; + + fhp->fh_post_saved = true; if (v4) fhp->fh_post_change = nfsd4_change_attribute(&fhp->fh_post_attr, inode);
nfsd calls fh_getattr to get the latest inode attrs for pre/post-op info. In the event that fh_getattr fails, it resorts to scraping cached values out of the inode directly. Since these attributes are optional, we can just skip providing them altogether when this happens. Signed-off-by: Jeff Layton <jlayton@kernel.org> --- fs/nfsd/nfsfh.c | 26 +++++++------------------- 1 file changed, 7 insertions(+), 19 deletions(-)