[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20211015124337.GK23102@quack2.suse.cz>
Date: Fri, 15 Oct 2021 14:43:37 +0200
From: Jan Kara <jack@...e.cz>
To: Gabriel Krisman Bertazi <krisman@...labora.com>
Cc: jack@...e.com, amir73il@...il.com, djwong@...nel.org,
tytso@....edu, dhowells@...hat.com, khazhy@...gle.com,
linux-fsdevel@...r.kernel.org, linux-ext4@...r.kernel.org,
linux-api@...r.kernel.org, repnop@...gle.com, kernel@...labora.com
Subject: Re: [PATCH v7 21/28] fanotify: Support merging of error events
On Thu 14-10-21 18:36:39, Gabriel Krisman Bertazi wrote:
> Error events (FAN_FS_ERROR) against the same file system can be merged
> by simply iterating the error count. The hash is taken from the fsid,
> without considering the FH. This means that only the first error object
> is reported.
>
> Signed-off-by: Gabriel Krisman Bertazi <krisman@...labora.com>
...
> +static void fanotify_merge_error_event(struct fanotify_error_event *dest,
> + struct fanotify_error_event *origin)
> +{
> + dest->err_count++;
> +}
> +
> +static void fanotify_merge_event(struct fanotify_event *dest,
> + struct fanotify_event *origin)
> +{
> + dest->mask |= origin->mask;
> +
> + if (origin->type == FANOTIFY_EVENT_TYPE_FS_ERROR)
> + fanotify_merge_error_event(FANOTIFY_EE(dest),
> + FANOTIFY_EE(origin));
> +}
> +
> /* Limit event merges to limit CPU overhead per event */
> #define FANOTIFY_MAX_MERGE_EVENTS 128
>
> @@ -175,7 +204,7 @@ static int fanotify_merge(struct fsnotify_group *group,
> if (++i > FANOTIFY_MAX_MERGE_EVENTS)
> break;
> if (fanotify_should_merge(old, new)) {
> - old->mask |= new->mask;
> + fanotify_merge_event(old, new);
I guess no need for two functions (fanotify_merge_event(),
fanotify_merge_error_event()) for three lines of code? I'd just put those
three lines into fanotify_merge().
> @@ -591,6 +621,9 @@ static struct fanotify_event *fanotify_alloc_error_event(
> return NULL;
>
> fee->fae.type = FANOTIFY_EVENT_TYPE_FS_ERROR;
> + fee->err_count = 1;
> +
> + *hash ^= fanotify_hash_fsid(fsid);
>
> return &fee->fae;
> }
As Amir mentioned, you should set fsid here.
Honza
--
Jan Kara <jack@...e.com>
SUSE Labs, CR
Powered by blists - more mailing lists