On 2/17/23 20:45, Matthew Wilcox wrote:
On Fri, Feb 17, 2023 at 02:44:09PM +0100, Danilo Krummrich wrote:\#define SAMPLE_ITER(name, __mgr) \ struct sample_iter name = { \ .mas = __MA_STATE(&(__mgr)->mt, 0, 0),This is usually called MA_STATE_INIT()
Yep, that's better.
#define sample_iter_for_each_range(it__, start__, end__) \ for ((it__).mas.index = start__, (it__).entry = mas_find(&(it__).mas, end__ - 1); \ (it__).entry; (it__).entry = mas_find(&(it__).mas, end__ - 1))This is a bad iterator design. It's usually best to do this: struct sample *sample; SAMPLE_ITERATOR(si, min); sample_iter_for_each(&si, sample, max) { frob(mgr, sample); }
The reason why I don't set index (and max) within SAMPLE_ITER() is that the range to iterate might not yet be known at that time, so I thought it could just be set in sample_iter_for_each_range().
However, I see that this might prevail users to assume that it's safe to iterate a range based on the same iterator instance multiple times though. Instead users should maybe move the tree walk to another function once the range is known.
The reason for the payload structure to be part of the iterator is that I have two maple trees in the GPUVA manager and hence two different payload types. Within the iterator structure they're just within a union allowing me to implement the tree walk macro just once rather than twice.
Anyway, I feel like your approach looks cleaner, hence I'll change it.
I don't mind splitting apart MA_STATE_INIT from MA_STATE, and if you do that, we can also use it in VMA_ITERATOR.
