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. Thanks for review! Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
