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 <weiyongjun1@xxxxxxxxxx>
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
