On Thu, Feb 22, 2018 at 6:22 PM, Jan Kara <jack@xxxxxxx> wrote: > On Thu 22-02-18 18:04:36, Amir Goldstein wrote: >> On Wed, Feb 21, 2018 at 5:44 PM, Jan Kara <jack@xxxxxxx> wrote: >> > Fanotify queues of unlimited length do not expect events can be lost. >> > Since these queues are used for system auditing and other security >> >> Change looks good to me, but the reasoning is going backwards. >> IMO, you should mention that we are going to change -ENOMEM >> behavior to result in Q_OVERFLOW and user does not expect >> Q_OVERFLOW from an UNLIMITED queue. > > See below. > >> > related tasks, loosing events can even have security implications. So >> > avoid loosing events due to failure to allocate memory by making event >> > allocation use __GFP_NOFAIL. Since the allocation is small (32-bytes), >> > currently there is no practical difference of this change but still it >> > is good to have the expectation explicitely documented. >> > >> >> But if currently allocations cannot fail, then why do we need patch >> 2/2 (queue overflow event). It is because small allocations can fail when >> accounted to non-root memcg? > > Yes. So how about changelog like: > > Fanotify queues of unlimited length do not expect events can be lost. > Since these queues are used for system auditing and other security > related tasks, loosing events can even have security implications. > Currently, since the allocation is small (32-bytes), it cannot fail > however when we start accounting events in memcgs, allocation can start > failing. So avoid loosing events due to failure to allocate memory by > making event allocation use __GFP_NOFAIL. > Very good. Thanks, Amir.