Hello together,

On 4/28/20 1:14 PM, Matthew Wilcox wrote:
On Tue, Apr 28, 2020 at 03:47:36AM +0000, Wei Yongjun wrote:
The function ipc_id_alloc() is called from ipc_addid(), in which
a spin lock is held, so we should use GFP_ATOMIC instead.

Fixes: de5738d1c364 ("ipc: convert ipcs_idr to XArray")
Signed-off-by: Wei Yongjun <weiyongj...@huawei.com>
I see why you think that, but it's not true.  Yes, we hold a spinlock, but
the spinlock is in an object which is not reachable from any other CPU.

Is it really allowed that spin_lock()/spin_unlock may happen on different cpus?

CPU1: spin_lock()

CPU1: schedule() -> sleeps

CPU2: -> schedule() returns

CPU2: spin_unlock().


Converting to GFP_ATOMIC is completely wrong.

What is your solution proposal?

xa_store() also gets a gfp_ flag. Thus even splitting _alloc() and _store() won't help

    xa_alloc(,entry=NULL,)
    new->seq = ...
    spin_lock();
    xa_store(,entry=new,GFP_KERNEL);

--

    Manfred


Reply via email to