On Wed, Feb 14, 2018 at 12:20 AM, Shakeel Butt <shake...@google.com> wrote: > On Tue, Feb 13, 2018 at 1:54 PM, Amir Goldstein <amir7...@gmail.com> wrote: >> On Tue, Feb 13, 2018 at 11:10 PM, Shakeel Butt <shake...@google.com> wrote: [...] >>>> 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? >>>> >>> >>> Are there any error which are not related to queue overflows? I see >>> the mention of ENODEV and EOVERFLOW in the discussion. If there are >>> such errors and might be interesting to the listener then we should >>> have 2 independent APIs. >>> >> >> These are indeed 2 different use cases. >> A Q_OVERFLOW event is only expected one of ENOMEM or >> EOVERFLOW in event->fd, but other events (like open of special device >> file) can have ENODEV in event->fd. >> >> But I am not convinced that those require 2 independent APIs. >> Specifying FAN_Q_ERR means that the user expects to reads errors >> from event->fd. >> > > Can you please explain what you mean by 2 independent APIs? I thought > "no independent APIs" means FAN_Q_ERR can only be used with > FAN_Q_OVERFLOW and without FAN_Q_OVERFLOW, FAN_Q_ERR is ignored. Is > that right or I misunderstood? >
What I initially meant to say was, we actually consider several behavior changes: 1. Charge event allocations to memcg of listener 2. Queue a Q_OVERFLOW event on ENOMEM of event allocation 3. Report the error to user on metadata->fd (instead of FAN_NOFD) 4. Allow non Q_OVERFLOW event to have negative metadata->fd. #3 is applicable both to Q_OVERFLOW event and other events that can't provide and open file descriptor for some reason (i.e. ENODEV). #1 and #2 could be independent, but they both make sense together. When enabling #1 user increases the chance of ENOMEM and therefore #2 is desired. So if we are going to let distro/admin/programmer to opt-in for what we believe to be a "change of behavior for the best", then we could consider that opting-in for #1 will also imply opting-in for #2 and #3 (as the means to report Q_OVERFLOW due to ENOMEM). I guess we will need to allow user to opt-in to #4 and #3 by FAN_Q_ERR mask flag to cover the ENODEV case independently from opting-in to charging memcg. How was I doing in the balance of adding clarity vs. adding confusion? Thanks, Amir.
