On Wed 11-05-22 12:29:13, Amir Goldstein wrote:
> fsnotify_foreach_iter_mark_type() is used to reduce boilerplate code
> of iteratating all marks of a specific group interested in an event
> by consulting the iterator report_mask.
>
> Use an open coded version of that iterator in fsnotify_iter_next()
> that collects all marks of the current iteration group without
> consulting the iterator report_mask.
>
> At the moment, the two iterator variants are the same, but this
> decoupling will allow us to exclude some of the group's marks from
> reporting the event, for example for event on child and inode marks
> on parent did not request to watch events on children.
>
> Fixes: 2f02fd3fa13e ("fanotify: fix ignore mask logic for events on child and on dir")
> Reported-by: Jan Kara <jack@xxxxxxxx>
> Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
Mostly looks good. Two nits below.
> /*
> - * Pop from iter_info multi head queue, the marks that were iterated in the
> + * Pop from iter_info multi head queue, the marks that belong to the group of
> * current iteration step.
> */
> static void fsnotify_iter_next(struct fsnotify_iter_info *iter_info)
> {
> + struct fsnotify_mark *mark;
> int type;
>
> fsnotify_foreach_iter_type(type) {
> - if (fsnotify_iter_should_report_type(iter_info, type))
> + mark = iter_info->marks[type];
> + if (mark && mark->group == iter_info->current_group)
> iter_info->marks[type] =
> fsnotify_next_mark(iter_info->marks[type]);
Wouldn't it be more natural here to use the new helper
fsnotify_foreach_iter_mark_type()? In principle we want to advance mark
types which were already reported...
> @@ -438,6 +438,9 @@ FSNOTIFY_ITER_FUNCS(sb, SB)
>
> #define fsnotify_foreach_iter_type(type) \
> for (type = 0; type < FSNOTIFY_ITER_TYPE_COUNT; type++)
> +#define fsnotify_foreach_iter_mark_type(iter, mark, type) \
> + for (type = 0; type < FSNOTIFY_ITER_TYPE_COUNT; type++) \
> + if (!(mark = fsnotify_iter_mark(iter, type))) {} else
Hum, you're really inventive here ;) I'd rather go for something a bit more
conservative and readable like:
static inline int fsnotify_iter_step(struct fsnotify_iter_info *iter, int type,
struct fsnotify_mark **markp)
{
while (type < FSNOTIFY_ITER_TYPE_COUNT) {
*markp = fsnotify_iter_mark(iter, type);
if (*markp)
break;
type++;
}
return type;
}
#define fsnotify_foreach_iter_mark_type(iter, mark, type) \
for (type = 0; \
(type = fsnotify_iter_step(iter, type, &mark)) < FSNOTIFY_ITER_TYPE_COUNT; \
type++)
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
