On Tue, Jul 15, 2025 at 2:11 PM Jan Kara <jack@xxxxxxx> wrote: > > On Mon 14-07-25 21:59:22, Amir Goldstein wrote: > > On Mon, Jul 14, 2025 at 7:25 PM Jan Kara <jack@xxxxxxx> wrote: > > > > I don't think there is much to lose from this retry behavior. > > > > The only reason we want to opt-in for it is to avoid surprises of > > > > behavior change in existing deployments. > > > > > > > > While we could have FAN_RETRY_UNANSWERED as an > > > > independent feature without a handover ioctl, > > > > In order to avoid test matrix bloat, at least for a start (we can relax later), > > > > I don't think that we should allow it as an independent feature > > > > and especially not for legacy modes (i.e. for Anti-Virus) unless there > > > > is a concrete user requesting/testing these use cases. > > > > > > With queue-fd design I agree there's no reason not to have the "resend > > > pending events" behavior from the start. > > > > > > > Going on about feature dependency. > > > > > > > > Practically, a handover ioctl is useless without > > > > FAN_REPORT_RESPONSE_ID, so for sure we need to require > > > > FAN_REPORT_RESPONSE_ID for the handover ioctl feature. > > > > > > > > Because I do not see an immediate use case for > > > > FAN_REPORT_RESPONSE_ID without handover, > > > > I would start by only allowing them together and consider relaxing > > > > later if such a use case is found. > > > > > > We can tie them together but I think queue-fd design doesn't require > > > FAN_REPORT_RESPONSE_ID. Since we resend events anyway, we'd generate new > > > fds for events as well and things would just work AFAICT. > > > > > > > Right. hmm. > > I'd still like to consider the opportunity of the new-ish API for > > deprecating some old legacy API baggage. > > > > For example: do you consider the fact that async events are mixed > > together with permission/pre-content events in the same queue > > an historic mistake or a feature? > > So far I do consider it a feature although not a particularly useful one. > Given fanotify doesn't guarantee ordering of events and async events can > arbitrarily skip over the permission events there's no substantial > difference to having two notification groups. I am not sure I understand your claim. It sounds like you are arguing for my case, because mixing mergeable async events and non-mergeable permission events in the same queue can be very confusing. Therefore, I consider it an anti-feature. Users can get the same functionality from having two groups, with much more sane semantics. > If you can make a good case > of what would be simpler if we disallowed that I'm open to consider > disallowing that. > Clearly, handling permission events should have had higher priority, but we do not have a priority queue, nor do we provide any means for userspace to handle permission events before async events. Nudging users to separate the events of different urgency to different queues/groups can allow userspace to read from the higher priority fd first as they should. > > I'm tempted, as we split the control fd from the queue fd to limit > > a queue to events of the same "type". > > Maybe an O_RDONLY async queue fd > > or an O_RDWR permission events queue fd > > or only allow the latter with the new API? > > There's value in restricting unneeded functionality and there's also value > in having features not influencing one another so the balance has to be > found :). <nod> > So far I don't find that disallowing async events with queue fd > or restricting fd mode for queue fd would simplify our life in a > significant way but maybe I'm missing something. > It only simplifies our life if we are going to be honest about test coverage. When we return a pending permission events to the head of the queue when queue fd is closed, does it matter if the queue has async events or other permission events or a mix of them? Logically, it shouldn't matter, but assuming that for tests is not the best practice. Thus, reducing the test matrix for a new feature by removing a configuration that I consider misguided feels like a good idea, but I am also feeling that my opinion on this matter is very biased, so I'd be happy if you can provide a more objective decision about the restrictions. > > Please note that FAN_CLOEXEC and FAN_NONBLOCK > > currently apply to the control fd. > > I guess they can also apply to the queue fd, > > Anyway the control fd should not be O_RDWR > > probably O_RDONLY even though it's not for reading. > > I guess the ioctl to create queue fd can take desired flags for the queue > fd so we don't have to second guess what the application wants. > Correct, but note that FAN_NONBLOCK is meaningless to the io-less control fd. Thanks, Amir.
