Gautam Menghani <[email protected]> writes:
> Calls to idr_alloc() in __prealloc_nested() are not currently synchronized.
> According to the documentation [1], the caller should provide their own
> locking to prevent concurrent modifications to the idr to prevent bugs.
>
> Wrap the call to __prealloc_nested() in a spinlock to prevent concurrent
> modifications to idr.
>
This looks to be the leftover of previous implementation. IMO, the
commit msg needs fixing.
How about this?
Calls to idr_alloc() in __prealloc_nested() are not currently synchronized.
According to the documentation [1], the caller should provide their own
locking to prevent concurrent modifications to the idr to prevent bugs.
Now __prealloc_nested() calls idr_alloc() with GFP_KERNEL which can
sleep and mmu_lock is a spinlock. So idr_alloc() cannot be moved under
mmu_lock. In such cases the idr Documentation says:
If you need to take a lock while allocating a new ID from the IDR,
you may need to pass a restrictive set of GFP flags, which can lead
to the IDR being unable to allocate memory. To work around this,
you can call idr_preload() before taking the lock, and then
idr_preload_end() after the allocation.
Hence preload the idr memory with idr_preload() before taking mmu_lock,
which allows us to use idr_alloc(..., GFP_NOWAIT) under the spinlock.
This also allows to kill __prealloc_nested() and __add_nested(), as they
are not needed anymore with idr_preload().
[1]: lib/idr.c
<...>
> [1]: lib/idr.c
>
> Fixes: c0f00a18e2a8 ("KVM: PPC: Book3S HV Nested: Change nested guest lookup
> to use idr")
> Cc: [email protected] # 5.19+
> Signed-off-by: Gautam Menghani <[email protected]>
> Co-developed-by: Ritesh Harjani (IBM) <[email protected]>
> Signed-off-by: Ritesh Harjani (IBM) <[email protected]>
Otherwise LGTM. Thanks for taking it ahead and validating this.
-ritesh
