On Wed, 01 Dec 2021, paulmck@xxxxxxxxxx wrote: > On Mon, Nov 29, 2021 at 01:46:07PM -0500, Chuck Lever wrote: > > To address this error: > > > > CC [M] fs/nfsd/filecache.o > > CHECK /home/cel/src/linux/linux/fs/nfsd/filecache.c > > /home/cel/src/linux/linux/fs/nfsd/filecache.c:772:9: error: incompatible types in comparison expression (different address spaces): > > /home/cel/src/linux/linux/fs/nfsd/filecache.c:772:9: struct net [noderef] __rcu * > > /home/cel/src/linux/linux/fs/nfsd/filecache.c:772:9: struct net * > > > > The "net" field in struct nfsd_fcache_disposal is not annotated as > > requiring an RCU assignment, so replace the macro that includes an > > invocation of rcu_check_sparse() with an equivalent that does not. > > > > Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx> > > From an RCU perspective: > > Acked-by: Paul E. McKenney <paulmck@xxxxxxxxxx> > > But it would be good to get someone more familiar with the code to > look at this. There is a global list of 'struct nfsd_fcache_disposal', potentially one for each network namespace. Each contains a '*net' which is effectively an opaque lookup key. It is never dereferenced - only used to find the nfsd_fcache_disposal which matches a given 'struct net *'. The lookup happens under rcu_read_lock(). A 'struct net' is freed after a grace period (see cleanup_net() in net_namespace.c) so to ensure the lookup doesn't find a false positive - caused by a 'struct net' being deallocated and reallocated, it is sufficient to clear the ->net pointer while the the net is known to still be active. At most, a write-barrier might be justified. i.e. smp_store_release(l->net, NULL); However ... the list of nfsd_fcache_disposal seems unnecessary. nfsd has a 'struct nfsd_net' which parallels any interesting 'struct net' using the net_generic() framework. If the nfs_fcache_disposal were linked from the nfsd_net, there would be no need for the list, no need to record the ->net, and not need for the code in question here. The nfsd_fcache_disposal is destroyed (nfsd_file_cache_shutdown_new()) before the nfsd_net is destroyed, so there are no lifetime issues with keeping the separate list. So I think this patch is not the best fix for the problem highlighted by the warning... it's not wrong though. NeilBrown
