Hi Jason, On 2/14/22 10:00 PM, Jason Gunthorpe wrote:
+ +/* Convert device source ID into the index of device_domain_array. */ +static inline unsigned long devi_idx(unsigned long seg, u8 bus, u8 devfn) +{ + return (seg << 16) | PCI_DEVID(bus, devfn); +}/*- * Iterate over elements in device_domain_list and call the specified + * Iterate over elements in device_domain_array and call the specified * callback @fn against each element. */ int for_each_device_domain(int (*fn)(struct device_domain_info *info, void *data), void *data) { - int ret = 0; - unsigned long flags; struct device_domain_info *info; + unsigned long index; + int ret = 0;- spin_lock_irqsave(&device_domain_lock, flags);- list_for_each_entry(info, &device_domain_list, global) { + rcu_read_lock(); + xa_for_each(&device_domain_array, index, info) { ret = fn(info, data); - if (ret) { - spin_unlock_irqrestore(&device_domain_lock, flags); - return ret; - } + if (ret) + break;And you probably shouldn't try to use RCU. It is really unclear how this function can be useful while racing against intel_iommu_release_device(), eg today the only user of this function does:static int search_pasid_table(struct device_domain_info *info, void *opaque) { struct pasid_table_opaque *data = opaque; if (info->iommu->segment == data->segment && info->bus == data->bus && info->devfn == data->devfn && And even if you kfree_rcu(info) then 'info->iommu->' is still racy unlocked. RCU is complicated to use, it is not just a drop in replacement for a spinlock.
Thanks for your comments. I am going to stop this patch (and the next 11/11) and spend more time figuring them out. Best regards, baolu _______________________________________________ iommu mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/iommu
