From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-yb0-f179.google.com ([209.85.213.179]:36379 "EHLO mail-yb0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753252AbeBZNRn (ORCPT ); Mon, 26 Feb 2018 08:17:43 -0500 Received: by mail-yb0-f179.google.com with SMTP id e135-v6so5247121ybb.3 for ; Mon, 26 Feb 2018 05:17:43 -0800 (PST) MIME-Version: 1.0 In-Reply-To: <20180226130605.v7o3ki53j3aqgdne@quack2.suse.cz> References: <20180221154430.19195-1-jack@suse.cz> <20180221154430.19195-2-jack@suse.cz> <20180222162249.5zqyieudl7cdxecv@quack2.suse.cz> <20180226130605.v7o3ki53j3aqgdne@quack2.suse.cz> From: Amir Goldstein Date: Mon, 26 Feb 2018 15:17:42 +0200 Message-ID: Subject: Re: [PATCH 1/2] fanotify: Avoid lost events due to ENOMEM for unlimited queues To: Jan Kara Cc: linux-fsdevel , Shakeel Butt Content-Type: text/plain; charset="UTF-8" Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Mon, Feb 26, 2018 at 3:06 PM, Jan Kara wrote: > On Thu 22-02-18 20:34:48, Amir Goldstein wrote: >> On Thu, Feb 22, 2018 at 6:22 PM, Jan Kara wrote: >> > On Thu 22-02-18 18:04:36, Amir Goldstein wrote: >> >> On Wed, Feb 21, 2018 at 5:44 PM, Jan Kara 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. > > Can I add your reviewed-by to the patch then? > Yes, of course. Thanks, Amir.