On Thu, Oct 03, 2024 at 01:45:55PM +0200, Jan Kara wrote:
> /* Find next inode on the inode list eligible for processing */
> #define sb_inode_iter_next(sb, inode, old_inode, inode_eligible) \
> ({ \
> struct inode *ret = NULL; \
<snip>
> ret; \
> })
How is this going to interact with calling into the file system
to do the interaction, which is kinda the point of this series?
Sure, you could have a get_next_inode-style method, but it would need
a fair amount of entirely file system specific state that needs
to be stashed away somewhere, and all options for that looks pretty
ugly.
Also even with your pre-iget callback we'd at best halve the number
of indirect calls for something that isn't exactly performance
critical. So while it could be done that way, it feels like a
more complex and much harder to reason about version for no real
benefit.
> #define for_each_sb_inode(sb, inode, inode_eligible) \
> for (DEFINE_FREE(old_inode, struct inode *, if (_T) iput(_T)), \
> inode = NULL; \
> inode = sb_inode_iter_next((sb), inode, &old_inode, \
> inode_eligible); \
> )
And while I liked:
obj = NULL;
while ((obj = get_next_object(foo, obj))) {
}
style iterators, magic for_each macros that do magic cleanup are just
a nightmare to read. Keep it simple and optimize for someone actually
having to read and understand the code, and not for saving a few lines
of code.
