Subject: Re: [PATCH net-next] net/tls: Use RCU API to access tls_ctx->netdev

From: Jakub Kicinski <[email-masked]>
Date: Thu, 4 Aug 2022 09:18:04 -0700

On Thu, 4 Aug 2022 08:08:37 +0000 Maxim Mikityanskiy wrote:
> 2. ctx->refcount goes down to 0, no one can access ctx->netdev anymore,
> we tear down ctx and need to check whether ctx->netdev is NULL.
> 
>  if (!refcount_dec_and_test(&ctx->refcount))
>          return;
>  netdev = rcu_dereference_protected(ctx->netdev,
>                                     !refcount_read(&ctx->refcount));
>  if (netdev)
>          queue_work(...);
> 
> It's somewhat similar to the "structure is beyond being updated" case,
> but it's ensured by refcount, not by RCU (i.e. you example assigned
> my_rcu_pointer = NULL and called synchronize_rcu() to ensure no one
> touches it, and I ensure that we are the only user of ctx by dropping
> refcount to zero).
> 
> So, hoping that my understanding of your explanation is correct, both
> cases can use any of rcu_access_pointer or rcu_dereference_protected.
> Is there some rule of thumb which one to pick in such case?

IMHO, rcu_dereference_protected() documents why it's safe to
dereference the pointer outside of the rcu read section. 

We are only documenting the writer side locking. The fact that there
is a RCU pointer involved is coincidental - I think we could 
as well be checking the TLS_RX_DEV_DEGRADED bit here.

We can add asserts or comments to document the writer side locking.
Piggy backing on RCU seems coincidental. But again, I'm fine either 
way. I'm just saying this because I really doubt there is a rule of
thumb for this level of detail. It's most likely your call.