Messages in this thread | | | From | Amir Goldstein <> | Date | Tue, 13 Feb 2018 08:30:27 +0200 | Subject | Re: [PATCH v2] fs: fsnotify: account fsnotify metadata to kmemcg |
| |
On Thu, Jan 25, 2018 at 10:36 PM, Amir Goldstein <amir73il@gmail.com> wrote: > On Thu, Jan 25, 2018 at 10:20 PM, Shakeel Butt <shakeelb@google.com> wrote: >> On Wed, Jan 24, 2018 at 11:51 PM, Amir Goldstein <amir73il@gmail.com> wrote: >>> >>> There is a nicer alternative, instead of failing the file access, >>> an overflow event can be queued. I sent a patch for that and Jan >>> agreed to the concept, but thought we should let user opt-in for this >>> change: >>> https://marc.info/?l=linux-fsdevel&m=150944704716447&w=2 >>> >>> So IMO, if user opts-in for OVERFLOW instead of ENOMEM, >>> charging the listener memcg would be non controversial. >>> Otherwise, I cannot say that starting to charge the listener memgc >>> for events won't break any application. >>> >>
Shakeel, Jan,
Reviving this thread and adding linux-api, because I think it is important to agree on the API before patches.
The last message on the thread you referenced suggest an API change for opting in for Q_OVERFLOW on ENOMEM: https://marc.info/?l=linux-api&m=150946878623441&w=2
However, the suggested API change in in fanotify_mark() syscall and this is not the time when fsnotify_group is initialized. I believe for opting-in to accounting events for listener, you will need to add an opt-in flag for the fanotify_init() syscall.
Something like FAN_GROUP_QUEUE (better name is welcome) which is mutually exclusive (?) with FAN_UNLIMITED_QUEUE.
The question is, do we need the user to also explicitly opt-in for Q_OVERFLOW on ENOMEM with FAN_Q_ERR mark mask? Should these 2 new APIs be coupled or independent?
Another question is whether FAN_GROUP_QUEUE may require less than CAP_SYS_ADMIN? Of course for now, this is only a semantic change, because fanotify_init() requires CAP_SYS_ADMIN but as the documentation suggests, this may be relaxed in the future.
Thought?
Amir.
| |