Re: [PATCH v2 1/4] KVM: PPC: Book3S HV: Ensure that calls to idr_alloc are synchronized

[Gautam Menghani]

On Sun, Oct 11, 2026 at 08:47:35AM +0530, Ritesh Harjani wrote:
> Gautam Menghani <gautam@xxxxxxxxxxxxx> 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
> <...>
> 

Yes this is a miss from my end, will fix in v3.

> > [1]: lib/idr.c
> >
> > Fixes: c0f00a18e2a8 ("KVM: PPC: Book3S HV Nested: Change nested guest lookup to use idr")
> > Cc: stable@xxxxxxxxxxxxxxx # 5.19+
> > Signed-off-by: Gautam Menghani <gautam@xxxxxxxxxxxxx>
> > Co-developed-by: Ritesh Harjani (IBM) <ritesh.list@xxxxxxxxx>
> > Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@xxxxxxxxx>
> 
> Otherwise LGTM. Thanks for taking it ahead and validating this.
> 
> -ritesh

Thanks,
Gautam