Re: Oops in nfs_scan_commit running xfstest generic/005 with NFSv4.2 and hammerspace flexfiles server

[Trond Myklebust <trondmy@hammerspace.com>]

On Sun, 2021-10-10 at 10:35 -0400, David Wysochanski wrote:
> Now I get the below WARN_ON pop though indicating
> nfs_have_writebacks() is true when inside nfs_clear_inode() and I
> think I saw this once before.
> I think we need some simple fixup to nfs_have_writebacks() due to the
> union-ization in your patch:
> commit b712e11b99eadba5b4003dd815adb368835fb5d5
> Author: Trond Myklebust <trond.myklebust@hammerspace.com>
> Date:   Tue Sep 28 17:41:41 2021 -0400
> 
>     NFS: Save some space in the inode
> 
>     Save some space in the nfs_inode by setting up an anonymous union
> with
>     the fields that are peculiar to a specific type of filesystem
> object.
> 
>     Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> 
> 
> You may want to fold something like this into the above which fixes
> the WARN for me:
> diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
> index a5aef2cbe4ee..5a110ecf2d85 100644
> --- a/include/linux/nfs_fs.h
> +++ b/include/linux/nfs_fs.h
> @@ -579,7 +579,9 @@ extern int nfs_access_get_cached(struct inode
> *inode, const struct cred *cred, s
>  static inline int
>  nfs_have_writebacks(struct inode *inode)
>  {
> -       return atomic_long_read(&NFS_I(inode)->nrequests) != 0;
> +       if (S_ISREG(inode->i_mode))
> +               return atomic_long_read(&NFS_I(inode)->nrequests) !=
> 0;
> +       return 0;
>  }
> 


Thanks again for testing and for the bug reports, Dave! Can you please
resend the above patch with a signed-off-by line? I'll be happy to
apply it.

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com