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

[Benjamin Block]

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).

-- 
Best Regards, Benjamin Block        /        Linux on IBM Z Kernel Development
IBM Deutschland Research & Development GmbH    /   https://www.ibm.com/privacy
Vors. Aufs.-R.: Wolfgang Wendt         /        Geschäftsführung: David Faller
Sitz der Ges.: Ehningen     /     Registergericht: AmtsG Stuttgart, HRB 243294