On Thu 27-09-18 22:32:23, Amir Goldstein wrote: > On Thu, Sep 27, 2018 at 4:39 PM Jan Kara <jack@xxxxxxx> wrote: > > > > On Fri 21-09-18 21:20:29, Amir Goldstein wrote: > > > Jan, > > > > > > Reposting my slightly modified cleanup patch along with the patch > > > from nixiaoming that uses it to add a new fanotify_init() flag. > > > > > > After a few review cycles with nixiaoming, per his request, I took > > > his FAN_EVENT_INFO_TID patch to my tree, fixes a couple of issues > > > including commit message wording and tested it. > > > > > > For me, the new API seems very intuitive, not sure why thread id was > > > not reported to begin with, but if you like more concrete use cases, > > > you will need to ask them from nixiaoming. > > > > > > Thanks, > > > Amir. > > > > > > Amir Goldstein (1): > > > fanotify: store fanotify_init() flags in group's fanotify_data > > > > > > nixiaoming (1): > > > fanotify: support reporting thread id instead of process id > > > > Thanks. Both patches look good to me and I've queued them up to my tree. > > > > Thanks! > However... I have found some issue that may require v3: > > - I do not like the idea of changing uapi bit group constants > (FAN_ALL_INIT_FLAGS) and adding new bit group constants > (FAN_EVENT_INFO_FLAGS) which I don't think should be in uapi. I agree on the "should not be in uapi" part. For FAN_ALL_INIT_FLAGS changing the constant is IMO no real issue as I just cannot imagine how anybody could use that. For FAN_EVENT_INFO_FLAGS the concern is real, I agree. > - uapi flag FAN_MARK_FILESYSTEM collides with kernel internal flag > FAN_MARK_ONDIR Good point, that needs to be fixed. Probably we miss some bit in the BUILD_BUG_ON tests... Looking at FAN_MARK_ONDIR I just don't see why that is needed at all but let's leave that for a future cleanup. > Sigh! > I will send a suggestion patch of how I think those issues should be sorted > out as a basis for making the API change for_next safer. OK, I'll have a look at your patch and decide how to go about it. Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
