On Thu, Aug 29, 2024 at 12:48:05PM +0200, Jan Kara wrote:
> On Wed 14-08-24 17:25:29, Josef Bacik wrote:
> > With page faults we can trigger readahead on the file, and then
> > subsequent faults can find these pages and insert them into the file
> > without emitting an fanotify event. To avoid this case, disable
> > readahead if we have pre-content watches on the file. This way we are
> > guaranteed to get an event for every range we attempt to access on a
> > pre-content watched file.
> >
> > Reviewed-by: Christian Brauner <brauner@xxxxxxxxxx>
> > Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
>
> ...
>
> > @@ -674,6 +675,14 @@ void page_cache_sync_ra(struct readahead_control *ractl,
> > {
> > bool do_forced_ra = ractl->file && (ractl->file->f_mode & FMODE_RANDOM);
> >
> > + /*
> > + * If we have pre-content watches we need to disable readahead to make
> > + * sure that we don't find 0 filled pages in cache that we never emitted
> > + * events for.
> > + */
> > + if (ractl->file && fsnotify_file_has_pre_content_watches(ractl->file))
> > + return;
> > +
>
> There are callers which don't pass struct file to readahead (either to
> page_cache_sync_ra() or page_cache_async_ra()). Luckily these are very few
> - cramfs for a block device (we don't care) and btrfs from code paths like
> send-receive or defrag. Now if you tell me you're fine breaking these
> corner cases for btrfs, I'll take your word for it but it looks like a
> nasty trap to me. Now doing things like defrag or send-receive on offline
> files on HSM managed filesystem doesn't look like a terribly good idea
> anyway so perhaps we just want btrfs to check and refuse such things?
>
We can't have HSM on a send subvolume because they have to be read only. I
hadn't thought of defrag, I'll respin and add a patch to disallow defrag on a
file that has content watches. Thanks,
Josef
