On Thu 03-04-25 19:24:57, Amir Goldstein wrote: > On Thu, Apr 3, 2025 at 7:10 PM Jan Kara <jack@xxxxxxx> wrote: > > > > On Wed 02-04-25 08:27:07, Amir Goldstein wrote: > > > Like files, a FAN_PRE_ACCESS event will be generated before every > > > read access to directory, that is on readdir(3). > > > > > > Unlike files, there will be no range info record following a > > > FAN_PRE_ACCESS event, because the range of access on a directory > > > is not well defined. > > > > > > FAN_PRE_ACCESS events on readdir are only generated when user opts-in > > > with FAN_ONDIR request in event mask and the FAN_PRE_ACCESS events on > > > readdir report the FAN_ONDIR flag, so user can differentiate them from > > > event on read. > > > > > > An HSM service is expected to use those events to populate directories > > > from slower tier on first readdir access. Having to range info means > > > that the entire directory will need to be populated on the first > > > readdir() call. > > > > > > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx> > > > --- > > > > > > Jan, > > > > > > IIRC, the reason we did not allow FAN_ONDIR with FAN_PRE_ACCESS event > > > in initial API version was due to uncertainty around reporting range info. > > > > > > Circling back to this, I do not see any better options other than not > > > reporting range info and reporting the FAN_ONDIR flag. > > > > > > HSM only option is to populate the entire directory on first access. > > > Doing a partial range populate for directories does not seem practical > > > with exising POSIX semantics. > > > > I agree that range info for directory events doesn't make sense (or better > > there's no way to have a generic implementation since everything is pretty > > fs specific). If I remember our past discussion, filling in directory > > content on open has unnecessarily high overhead because the user may then > > just do e.g. lookup in the opened directory and not full readdir. That's > > why you want to generate it on readdir. Correct? > > > > Right. > > > > If you accept this claim, please consider fast tracking this change into > > > 6.14.y. > > > > Hum, why the rush? It is just additional feature to allow more efficient > > filling in of directory entries... > > > > Well, no rush really. > > My incentive is not having to confuse users with documentation that > version X supports FAN_PRE_ACCESS but only version Y supports > it with FAN_ONDIR. > > It's not a big deal, but if we have no reason to delay this, I'd just > treat it as a fix to the new api (removing unneeded limitations). The patch is easy enough so I guess we may push it for rc2. When testing it, I've noticed a lot of LTP test cases fail (e.g. fanotify02) because they get unexpected event. So likely the patch actually breaks something in reporting of other events. I don't have time to analyze it right now so I'm just reporting it in case you have time to have a look... > I would point out that FAN_ACCESS_PERM already works > for directories and in effect provides (almost) the exact same > functionality as FAN_PRE_ACCESS without range info. > > But in order to get the FAN_ACCESS_PERM events on directories > listener would also be forced to get FAN_ACCESS_PERM on > special files and regular files > and assuming that this user is an HSM, it cannot request > FAN_ACCESS_PERM|FAN_ONDIR in the same mask as > FAN_PRE_ACCESS (which it needs for files) so it will need to > open another group for populating directories. > > So that's why I would maybe consider this a last minute fix to the new API. Yeah, this would be really a desperate way to get the functionality :) Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR