Re: [PATCH v13 15/19] pnfs/flexfiles: enable localio support

["NeilBrown" <neilb@suse.de>]

On Tue, 27 Aug 2024, Mike Snitzer wrote:
> On Mon, Aug 26, 2024 at 11:39:31AM +1000, NeilBrown wrote:
> > On Sat, 24 Aug 2024, Mike Snitzer wrote:
> > > From: Trond Myklebust <trond.myklebust@hammerspace.com>
> > > 
> > > If the DS is local to this client use localio to write the data.
> > > 
> > > Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> > > Signed-off-by: Mike Snitzer <snitzer@kernel.org>
> > > ---
> > >  fs/nfs/flexfilelayout/flexfilelayout.c    | 136 +++++++++++++++++++++-
> > >  fs/nfs/flexfilelayout/flexfilelayout.h    |   2 +
> > >  fs/nfs/flexfilelayout/flexfilelayoutdev.c |   6 +
> > >  3 files changed, 140 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c b/fs/nfs/flexfilelayout/flexfilelayout.c
> > > index 01ee52551a63..d91b640f6c05 100644
> > > --- a/fs/nfs/flexfilelayout/flexfilelayout.c
> > > +++ b/fs/nfs/flexfilelayout/flexfilelayout.c
> > > @@ -11,6 +11,7 @@
> > >  #include <linux/nfs_mount.h>
> > >  #include <linux/nfs_page.h>
> > >  #include <linux/module.h>
> > > +#include <linux/file.h>
> > >  #include <linux/sched/mm.h>
> > >  
> > >  #include <linux/sunrpc/metrics.h>
> > > @@ -162,6 +163,72 @@ decode_name(struct xdr_stream *xdr, u32 *id)
> > >  	return 0;
> > >  }
> > >  
> > > +/*
> > > + * A dummy definition to make RCU (and non-LOCALIO compilation) happy.
> > > + * struct nfsd_file should never be dereferenced in this file.
> > > + */
> > > +struct nfsd_file {
> > > +       int undefined__;
> > > +};
> > 
> > I removed this and tried building both with and without LOCALIO enabled
> > and the compiler didn't complain.
> > Could you tell me what to do to see the unhappiness you mention?
> 
> Sorry, I can remove the dummy definition for upstream.  That was
> leftover from the backport I did to 5.15.y stable@ kernel.  Older
> kernels' RCU code dereferences what should just be an opaque pointer
> and (ab)use typeof.  So without the dummy definition compiling against
> 5.15.y fails with:
> 
>   CC [M]  fs/nfs/flexfilelayout/flexfilelayout.o
> In file included from ./include/linux/rbtree.h:24,
>                  from ./include/linux/mm_types.h:10,
>                  from ./include/linux/mmzone.h:21,
>                  from ./include/linux/gfp.h:6,
>                  from ./include/linux/mm.h:10,
>                  from ./include/linux/nfs_fs.h:23,
>                  from fs/nfs/flexfilelayout/flexfilelayout.c:10:
> fs/nfs/flexfilelayout/flexfilelayout.c: In function `ff_local_open_fh´:
> ./include/linux/rcupdate.h:441:9: error: dereferencing pointer to incomplete type `struct nfsd_file´
>   typeof(*p) *________p1 = (typeof(*p) *__force)READ_ONCE(p); \
>          ^
> ./include/linux/rcupdate.h:580:2: note: in expansion of macro `__rcu_dereference_check´
>   __rcu_dereference_check((p), (c) || rcu_read_lock_held(), __rcu)
>   ^~~~~~~~~~~~~~~~~~~~~~~
> ./include/linux/rcupdate.h:648:28: note: in expansion of macro `rcu_dereference_check´
>  #define rcu_dereference(p) rcu_dereference_check(p, 0)
>                             ^~~~~~~~~~~~~~~~~~~~~
> fs/nfs/flexfilelayout/flexfilelayout.c:193:7: note: in expansion of macro `rcu_dereference´
>   nf = rcu_dereference(*pnf);
>        ^~~~~~~~~~~~~~~

Ahhh - good to know.  Thanks!

NeilBrown