Messages in this thread | | | From | Thomas Gleixner <> | Subject | Re: [patch 04/32] genirq/msi: Provide a set of advanced MSI accessors and iterators | Date | Sun, 28 Nov 2021 20:22:30 +0100 |
| |
On Sat, Nov 27 2021 at 21:00, Jason Gunthorpe wrote: > On Sat, Nov 27, 2021 at 02:22:33AM +0100, Thomas Gleixner wrote: >> + * Notes: >> + * - The loop must be protected with a msi_lock_descs()/msi_unlock_descs() >> + * pair. >> + * - It is safe to remove a retrieved MSI descriptor in the loop. >> + */ >> +#define msi_for_each_desc_from(desc, dev, filter, base_index) \ >> + for ((desc) = __msi_first_desc((dev), (filter), (base_index)); (desc); \ >> + (desc) = msi_next_desc((dev))) > > Given this ends up as an xarray it feels really weird that there is a > hidden shared __next/__iter_idx instead of having the caller provide > the index storage as is normal for xa operations. > > I understand why that isn't desirable at this patch where the storage > would have to be a list_head pointer, but still, seems like an odd > place to end up at the end of the series. > > eg add index here unused and then the last patch uses it instead of > __iter_idx.
TBH, didn't think about doing just that. OTH, given the experience of looking at the creative mess people create, this was probably also a vain attempt to make it harder in the future.
> Also, I don't understand why filter was stored in the dev and not > passed into msi_next_desc() in the macro here?
No real reason. I probably just stored it along with the rest. Lemme try that index approach.
Thanks,
tglx
| |