From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-yw1-f66.google.com ([209.85.161.66]:41488 "EHLO mail-yw1-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726488AbeJHWxU (ORCPT ); Mon, 8 Oct 2018 18:53:20 -0400 MIME-Version: 1.0 References: <20181008101229.7923-1-amir73il@gmail.com> <20181008115440.GB21682@quack2.suse.cz> In-Reply-To: <20181008115440.GB21682@quack2.suse.cz> From: Amir Goldstein Date: Mon, 8 Oct 2018 18:40:48 +0300 Message-ID: Subject: Re: [PATCH] fanotify: self describing event metadata To: Jan Kara Cc: Nixiaoming , linux-fsdevel , linux-api@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Mon, Oct 8, 2018 at 2:54 PM Jan Kara wrote: > > Hi Amir, > > On Mon 08-10-18 13:12:29, Amir Goldstein wrote: > > Use the 'reserved' u8 field in event metadata to describe event > > optional information. > > > > When event->pid is thread id, set the flag FAN_EVENT_INFO_TID in > > event->flags. > > > > Rename the init flag that is used to request reporting thread id > > in event to FAN_REPORT_TID. > > > > This change is semantic, because in the only case that user requests > > FAN_REPORT_TID and fanotify is not able to meet that request, > > event->pid will be zero anyway. > > > > However, for future event info requests, it is better to be explicit > > about whether fanotify was able to meet the request, similar to how > > statx() API behaves. > > > > Cc: > > Signed-off-by: Amir Goldstein > > --- > > > > Jan, > > > > While working on reporting file handles, I realized that the API would > > be more solid if the event info flags are bi-directional similar to > > statx(). > > That makes some sense but I'd really like to see how you apply this to > other things because e.g. with PID vs TGID I don't really see the need for > any flags. It might be interesting to have PID vs TGID flag there for > consistency once we really start to send them for other things but I don't > see a need to rush it now. Plus we are at rc7 already so we are out of > time for changes going to the coming merge window. > I agree, I posted the event flags as a concept. There is no need to rush it now and frankly, just the single use case of FAN_REPORT_FID, I don't think the flags will be needed either. > > Even if you disapprove of the way this patch modifies the event foramt, > > or if you would like to take more time to consider that, I would still > > like to rename the fanotify_init flag to FAN_REPORT_TID, becasue I think > > the name is better describing. See example man page with new name [1]. > > I agree the name is somewhat better. I did the renaming. > Great! > > I realize that the reserved/flags union is a bit ugly, but I think it > > will be less painful than introducing event metadata format v4 at this > > time. > > > > What do you thing? > > Why would be a new metadata format problem? You will quickly run out of > bits in that u8 anyways... > Supporting and documenting two format versions is a burden. v3 was designed with several flexible design concept we still never used, so we can still squeeze some extra usability from it without maintaining and documenting a new format version. FAN_REPORT_FID is going to use meta->event_len > FAN_EVENT_METADATA_LEN. I thought that making use of the reserved bits for self descriptive info is harmless, but anyway its not a requirement for my work (yet). Thanks, Amir.