> > > 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. > > Ah I see this makes sense. And as long as we are still able to open files via open_handle as the tests you shared below show, then at least for our case I don't see issue with switching to the new FAN_CLASS_PRE_CONTENT | FAN_REPORT_FID mode. > > 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? > > Regarding Meta usages of pre-content events as far as I am aware we are the only ones, and we don't set other events in the mask, only pre-content. I can confirm there are no other use cases relying on this and follow up here. > > *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. > > > Sounds great. If we do go that route, being able to overload event-fd for now will definitely make this patch much simpler. > 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.
