Re: [PATCH v4 1/1] PCI/IOV: Add reentrant locking in sriov_add_vfs/sriov_del_vfs for complete serialization

[Manivannan Sadhasivam]

On Mon, Mar 02, 2026 at 11:11:05AM +0100, Benjamin Block wrote:
> On Mon, Mar 02, 2026 at 11:43:04AM +0530, Manivannan Sadhasivam wrote:
> > On Sat, Feb 28, 2026 at 05:39:55PM +0100, Benjamin Block wrote:
> > > On Sat, Feb 28, 2026 at 08:43:33PM +0530, Manivannan Sadhasivam wrote:
> > > > On Sat, Feb 28, 2026 at 02:01:40PM +0200, Ionut Nechita (Wind River) wrote:
> > > > > From: Ionut Nechita <ionut.nechita@xxxxxxxxxxxxx>
> > > > > Instead, introduce owner tracking for pci_rescan_remove_lock via a new
> > > > > pci_lock_rescan_remove_reentrant() helper. This function checks if the
> > > > > current task already holds the lock:
> > > > >  - If the lock is not held: acquires it and returns true, providing
> > > > >    full serialization against concurrent hotplug events (including
> > > > >    platform-generated events on s390).
> > > > >  - If the lock is already held by the current task (reentrant call from
> > > > >    remove_store or sriov_numvfs_store paths): returns false without
> > > > >    re-acquiring, avoiding deadlock while the caller already provides
> > > > >    the necessary serialization.
> > > > >  - If the lock is held by another task (concurrent hotplug): blocks
> > > > >    until the lock is released, then acquires it, providing complete
> > > > >    serialization. This is the key improvement over a trylock approach.
> > > > 
> > > > Just curious. Why can't you use mutex_trylock() here?
> > > 
> > > One problem with mutex_trylock() is we don't know whether we ourself or
> > > someone else is holding the lock when it fails, we just know someone holds it;
> > > and we can't wait for someone else to release it when there is a chance we
> > > hold it ourself already. That was the problem with
> > > 05703271c3cd ("PCI/IOV: Add PCI rescan-remove locking when enabling/disabling SR-IOV")
> > > before it was reverted.
> > 
> > Okay, thanks for the info. I also failed to notice the mention of 'trylock' in
> > the cover letter.
> > 
> > But I think, instead of caching the owner task struct locally, you can make use
> > of mutex_get_owner() to extact the embedded owner task struct.
> 
> True. Didn't know/see that one, yet. We'd have to treat the return value as
> `struct task_struct *` to compare it, but I see debug_show_blocker() already
> does that effectively (when I saw the function returns ulong, I thought it was
> meant to be treated as transparent value).

Yeah, I don't know the reasoning behind it. But atleast it avoids caching the
task struct pointer locally.

- Mani

-- 
மணிவண்ணன் சதாசிவம்