Re: [PATCH v4 11/16] fanotify: disable readahead if we have pre-content watches

Jan Kara
2024-08-29T10:48:09Z

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@kernel.org>
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

...

> @@ -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?

Honza

--
Jan Kara <jack@suse.com>
SUSE Labs, CR