Re: [PATCH 3/5] iommu/s390: Use RCU to allow concurrent domain_list iteration

Jason Gunthorpe
2022-10-28T11:28:19Z

On Fri, Oct 28, 2022 at 11:29:00AM +0200, Niklas Schnelle wrote:

> > rcu_head = kzalloc(rcu_head, GFP_NOWAIT, GFP_NOWARN)
> > if (!rcu_head)
> > synchronize_rcu()
> > else
> > call_rcu(rcu_head)
> >
> > And then call kmem_cache_free() from the rcu callback
>
> Hmm, maybe a stupid question but why can't I just put the rcu_head in
> struct s390_domain and then do a call_rcu() on that with a callback
> that does:
>
> dma_cleanup_tables(s390_domain->dma_table);
> kfree(s390_domain);
>
> I.e. the rest of the current s390_domain_free().
> Then I don't have to worry about failing to allocate the rcu_head and
> it's simple enough. Basically just do the actual freeing of the
> s390_domain via call_rcu().

Oh, if you never reallocate the dma_table then yes that is a good idea

> Or do you mean to use a kref that is taken by RCU readers together with
> rcu_read_lock() and dropped at rcu_read_unlock() such that during the
> RCU read critical sections the refcount can't fall below 1 and the
> domain is actually freed once we have a) put the initial reference
> during s390_domain_free() and b) put all temporary references on
> exiting the RCU read critical sections?

Yes, this is a common pattern. Usually you want to optimize away the
global lock that protects, say, a linked list and then accept a local
lock/refcount inside the object

Jason