Messages in this thread | | | Date | Tue, 15 Feb 2022 13:46:18 +0800 | Subject | Re: [PATCH v2 10/11] iommu/vt-d: Use xarray for global device_domain_info | From | Lu Baolu <> |
| |
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
| |