On Fri, Oct 15, 2021 at 12:37 AM Gabriel Krisman Bertazi <krisman@xxxxxxxxxxxxx> 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.
