All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] NFSD: Fix misuse of rcu_assign_pointer
@ 2021-11-21 19:57 Chuck Lever
  2021-11-22  1:59 ` Paul E. McKenney
  0 siblings, 1 reply; 4+ messages in thread
From: Chuck Lever @ 2021-11-21 19:57 UTC (permalink / raw)
  To: linux-nfs; +Cc: paulmck

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.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 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);
+	l->net = NULL;
 	cancel_work_sync(&l->work);
 	nfsd_file_dispose_list(&l->freeme);
 	kfree_rcu(l, rcu);


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] NFSD: Fix misuse of rcu_assign_pointer
  2021-11-21 19:57 [PATCH] NFSD: Fix misuse of rcu_assign_pointer Chuck Lever
@ 2021-11-22  1:59 ` Paul E. McKenney
  2021-11-22  3:01   ` Chuck Lever III
  0 siblings, 1 reply; 4+ messages in thread
From: Paul E. McKenney @ 2021-11-22  1:59 UTC (permalink / raw)
  To: Chuck Lever; +Cc: linux-nfs

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

> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  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);
> +	l->net = NULL;
>  	cancel_work_sync(&l->work);
>  	nfsd_file_dispose_list(&l->freeme);
>  	kfree_rcu(l, rcu);
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] NFSD: Fix misuse of rcu_assign_pointer
  2021-11-22  1:59 ` Paul E. McKenney
@ 2021-11-22  3:01   ` Chuck Lever III
  2021-11-22 17:54     ` Chuck Lever III
  0 siblings, 1 reply; 4+ messages in thread
From: Chuck Lever III @ 2021-11-22  3:01 UTC (permalink / raw)
  To: paulmck; +Cc: linux-nfs


> On Nov 21, 2021, at 8:59 PM, Paul E. McKenney <paulmck@kernel.org> 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@oracle.com>
>> ---
>> 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);
>> +    l->net = NULL;
>>    cancel_work_sync(&l->work);
>>    nfsd_file_dispose_list(&l->freeme);
>>    kfree_rcu(l, rcu);
>> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] NFSD: Fix misuse of rcu_assign_pointer
  2021-11-22  3:01   ` Chuck Lever III
@ 2021-11-22 17:54     ` Chuck Lever III
  0 siblings, 0 replies; 4+ messages in thread
From: Chuck Lever III @ 2021-11-22 17:54 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: Linux NFS Mailing List, paulmck



> On Nov 21, 2021, at 10:01 PM, Chuck Lever III <chuck.lever@oracle.com> wrote:
> 
>> 
>> On Nov 21, 2021, at 8:59 PM, Paul E. McKenney <paulmck@kernel.org> 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@oracle.com>
>>> ---
>>> 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




^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-11-22 17:54 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-21 19:57 [PATCH] NFSD: Fix misuse of rcu_assign_pointer Chuck Lever
2021-11-22  1:59 ` Paul E. McKenney
2021-11-22  3:01   ` Chuck Lever III
2021-11-22 17:54     ` Chuck Lever III

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.