On 7/10/25 23:53, Peter Zijlstra wrote:
On Thu, Jul 10, 2025 at 03:54:32PM +0200, Peter Zijlstra wrote:
@@ -132,8 +136,15 @@ struct iommu_sva *iommu_sva_bind_device(struct device *dev, struct mm_struct *mm if (ret) goto out_free_domain; domain->users = 1;
- list_add(&domain->next, &mm->iommu_mm->sva_domains);
- if (list_empty(&iommu_mm->sva_domains)) {
scoped_guard(spinlock_irqsave, &iommu_mms_lock) {if (list_empty(&iommu_sva_mms))static_branch_enable(&iommu_sva_present);list_add(&iommu_mm->mm_list_elm, &iommu_sva_mms);}- }
- list_add(&domain->next, &iommu_mm->sva_domains); out: refcount_set(&handle->users, 1); mutex_unlock(&iommu_sva_lock);
@@ -175,6 +186,15 @@ void iommu_sva_unbind_device(struct iommu_sva *handle) list_del(&domain->next); iommu_domain_free(domain); }
- 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(); }
Well, for one, you can't do static_branch_{en,dis}able() from atomic context...
Was this ever tested?
I conducted unit tests for vmalloc()/vfree() scenarios, and Yi performed fuzzing tests. We have not observed any warning messages. Perhaps static_branch_disable() is not triggered in the test cases?
Thanks, baolu