Re: [PATCH v7 00/28] file system-wide error monitoring

On Fri, Oct 15, 2021 at 12:37 AM Gabriel Krisman Bertazi
[off-list ref] wrote:

Hi,

This attempts to get the ball rolling again for the FAN_FS_ERROR.  This
version is slightly different from the previous approaches, since it uses
mempool for memory allocation, as suggested by Jan.  It has the
advantage of simplifying a lot the enqueue/dequeue, which is now much
more similar to other event types, but it also means the guarantee that
an error event will be available is diminished.

Makes me very happy not having to worry about new enqueue/dequeue bugs :)

The way we propagate superblock errors also changed. Now we use
FILEID_ROOT internally, and mangle it prior to copy_to_user.

I am no longer sure how to guarantee that at least one mempoll slot will
be available for each filesystem.  Since we are now tying the poll to
the entire group, a stream of errors in a single file system might
prevent others from emitting an error.  The possibility of this is
reduced since we merge errors to the same filesystem, but it is still
possible that they occur during the small window where the event is
dequeued and before it is freed, in which case another filesystem might
not be able to obtain a slot.

Double buffering. Each mark/fs should have one slot reserved for equeue
and one reserved for copying the event to user.

I'm also creating a poll of 32 entries initially to avoid spending too
much memory.  This means that only 32 filesystems can be watched per
group with the FAN_FS_ERROR mark, before fanotify_mark starts returning
ENOMEM.

I don't see a problem to grow the pool dynamically up to a reasonable
size, although it is a shame that the pool is not accounted to the group's
memcg (I think?).

Overall, the series looks very good to me, modulo to above comments
about the mempool size/resize and a few minor implementation details.

Good job!

Thanks,
Amir.