> On Nov 21, 2021, at 10:01 PM, Chuck Lever III <chuck.lever@xxxxxxxxxx> wrote: > >> >> On Nov 21, 2021, at 8:59 PM, Paul E. McKenney <paulmck@xxxxxxxxxx> wrote: >> >> On Sun, Nov 21, 2021 at 02:57:14PM -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. >> >> I am not immediately seeing this field indirected through by RCU readers, >> though maybe that is happening in some other file. >> >> However, it does look like this field is being accessed locklessly by >> read-side code. What prevents the compiler from applying unfortunate >> optimizations? >> >> See tools/memory-model/Documentation/access-marking.txt in a recent >> kernel or these LWN articles: https://lwn.net/Articles/816854 and >> https://lwn.net/Articles/793253. >> >> Thanx, Paul > > Thank you, Paul. I’ll look into it. > > >>> Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx> >>> --- >>> fs/nfsd/filecache.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/fs/nfsd/filecache.c b/fs/nfsd/filecache.c >>> index fdf89fcf1a0c..3fa172f86441 100644 >>> --- a/fs/nfsd/filecache.c >>> +++ b/fs/nfsd/filecache.c >>> @@ -772,7 +772,7 @@ nfsd_alloc_fcache_disposal(struct net *net) >>> static void >>> nfsd_free_fcache_disposal(struct nfsd_fcache_disposal *l) >>> { >>> - rcu_assign_pointer(l->net, NULL); Hi Trond, 9542e6a643fc ("nfsd: Containerise filecache laundrette") added an rcu_assign_pointer() for a field that is not annotated with __rcu. l->net is assigned and compared only to a non-RCU "struct net *" value. Can you say why you believe rcu_assign_pointer() is necessary here? >>> + l->net = NULL; >>> cancel_work_sync(&l->work); >>> nfsd_file_dispose_list(&l->freeme); >>> kfree_rcu(l, rcu); -- Chuck Lever
