> > Do we prefer to scope this change to adding (s32) response id and not add new > > event id field yet. > > > > > Thinking out loud, if we use idr to allocate an event id, as Jan suggested, > > > and we do want to allow it along side event->fd, > > > then we could also overload event->pid, i.e. the meaning of > > > FAN_ERPORT_EVENT_ID would be "event->pid is an event id", > > > Similarly to the way that we overloaded event->pid with FAN_REPORT_TID. > > > Users that need both event id and pid can use FAN_REPORT_PIDFD. > > > > > > > At least for our usecase, having event->fd along with response id available > > would be helpful as we do not use fid mode mentioned above. > > You cannot use the fid mode mentioned above because it is not yet > supported with pre-content events :) > > My argument goes like this: > 1. We are planning to add fid support for pre-content events for other > reasons anyway (pre-dir-content events) > 2. For this mode, event->fd will (probably) not be reported anyway, > so for this mode, we will have to use a different response id > 3. Since event->fd will not be used, it would make a lot of sense and > very natural to reuse the field for a response id > > So if we accept the limitation that writing an advanced hsm service > that supports non-interrupted restart requires that service to use the > new fid mode, we hit two birds with one event field ;) > > If we take into account that (the way I see it) an advanced hsm service > will need to also support pre-dir-content events, then the argument makes > even more sense. > > The fact that for your current use cases, you are ok with populating the > entire directory tree in a non-lazy way, does not mean that the use case > will not change in the future to require a lazy population of directory trees. > > I have another "hidden motive" with the nudge trying to push you over > towards pre-content events in fid mode: > > Allowing pre-content events together with legacy events in the same > mark/group brings up some ugly semantic issues that we did not > see when we added the API. > > The flag combination FAN_CLASS_PRE_CONTENT | FAN_REPORT_FID > was never supported, so when we support it, we can start fresh with new rules > like "only pre-content events are allowed in this group" and that simplifies > some of the API questions. > > While I have your attention I wanted to ask, as possibly the only > current user of pre-content events, is any of the Meta use cases > setting any other events in the mask along with pre-content events? > > *if* we agree to this course I can post a patch to add support for > FAN_CLASS_PRE_CONTENT | FAN_REPORT_FID, temporarily > leaving event->fd in use, so that you can later replace it with > a response id. > FWIW, here is that patch: https://github.com/amir73il/linux/commits/fan_pre_content_fid/ And here is an LTP test which demonstrates how to use this API: https://github.com/amir73il/ltp/commits/fan_pre_content_fid/ This kernel patch does not yet eliminate event->fd, but it makes it optional because that file can be opened by handle as the test demonstrates. Thanks, Amir.
