From: Baolu Lu baolu.lu@linux.intel.com Sent: Friday, July 11, 2025 11:00 AM
On 7/10/25 21:54, Peter Zijlstra wrote:
On Wed, Jul 09, 2025 at 02:28:00PM +0800, Lu Baolu wrote:
- if (list_empty(&iommu_mm->sva_domains)) {
scoped_guard(spinlock_irqsave, &iommu_mms_lock) {list_del(&iommu_mm->mm_list_elm);if (list_empty(&iommu_sva_mms))static_branch_disable(&iommu_sva_present);}- }
- mutex_unlock(&iommu_sva_lock); kfree(handle); }
This seems an odd coding style choice; why the extra unneeded indentation? That is, what's wrong with:
if (list_empty()) { guard(spinlock_irqsave)(&iommu_mms_lock); list_del(); if (list_empty() static_branch_disable(); }
Perhaps I overlooked or misunderstood something, but my understanding is,
The lock order in this function is:
mutex_lock(&iommu_sva_lock); spin_lock(&iommu_mms_lock); spin_unlock(&iommu_mms_lock); mutex_unlock(&iommu_sva_lock);
With above change, it is changed to:
mutex_lock(&iommu_sva_lock); spin_lock(&iommu_mms_lock); mutex_unlock(&iommu_sva_lock); spin_unlock(&iommu_mms_lock);
guard() follows the scope of variable declaration. Actually above is a perfect match to the example in cleanup.h:
* The lifetime of the lock obtained by the guard() helper follows the * scope of automatic variable declaration. Take the following example:: * * func(...) * { * if (...) { * ... * guard(pci_dev)(dev); // pci_dev_lock() invoked here * ... * } // <- implied pci_dev_unlock() triggered here * } * * Observe the lock is held for the remainder of the "if ()" block not * the remainder of "func()".