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

[Josef Bacik <josef@toxicpanda.com>]

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

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