Re: [PATCH for-next] RDMA/rxe: Fix "Replace red-black trees by xarrays"

[Jason Gunthorpe <jgg@nvidia.com>]

On Tue, Apr 12, 2022 at 09:32:25PM +0800, Yanjun Zhu wrote:
> 在 2022/4/12 0:02, Jason Gunthorpe 写道:
> > On Mon, Apr 11, 2022 at 03:52:23PM +0000, Pearson, Robert B wrote:
> > > 
> > > > Yes, you cannot use irq_save variants here. You have to know your calling context is non-atomic already and use the irq wrapper.
> > > 
> > > Not sure why. If you call irqsave in a non-atomic context doesn't it
> > > behave the same as irq? I.e. flags = 0.  xarray provides
> > > xa_lock_xxx() for people to use. Are you saying I have to call
> > > xa_alloc_cyclic_xxx() instead which is the same thing?
> > 
> > The xa_lock is a magic thing, when you call a __xa_*(.., GFP_KERNEL)
> > type function then it will unlock and relock the xa_lock internally.
> > 
> > Thus you cannot wrapper an irqsave lock across the GFP_KERNEL call
> > sites because XA doesn't know the 'flags' to be able to properly
> > unlock it.
> > 
> > > The problem is I've gotten gun shy about people calling into the
> > > verbs APIs in strange contexts. The rules don't seem to be written
> > > down. If I could be certain that no one ever is allowed to call a
> > > verbs API in an interrupt then this is correct but how do I know
> > > that?
> > 
> > rxe used GFP_KERNEL so it already has assumed it is a process context.
> Got it.
> 
> __xa_alloc_cyclic(..., GFP_NOWAIT) will unlock xa lock internally. But it
> does not handle flags. So the irq is still disabled. Because GFP_NOWAIT is
> used in __xa_alloc_cyclic, it will not sleep.
> 
> __xa_alloc_cyclic will lock xa lock and return to xa_unlock_irqrestore.
> 
> So the following code should be OK?
> 
>         xa_lock_irqsave(&pool->xa, flags);
>         err = __xa_alloc_cyclic(&pool->xa, &elem->index, elem, pool->limit,
>                                 &pool->next, GFP_NOWAIT);
>         xa_unlock_irqrestore(&pool->xa, flags);

No, use GFP_KERNEL an use the proper irq or bh version of
xa_alloc_cyclic.

save is only required if you don't know the contex you are in, and
here we know we are in a process context.

Jason