Re: [PATCH v5 12/18] fanotify: disable readahead if we have pre-content watches

Amir Goldstein
2024-09-05T08:12:44Z

On Wed, Sep 4, 2024 at 10:29 PM Josef Bacik <josef@toxicpanda.com> 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>

Reviewed-by: Amir Goldstein <amir73il@gmail.com>

Thanks,
Amir.

> ---
> mm/filemap.c | 12 ++++++++++++
> mm/readahead.c | 13 +++++++++++++
> 2 files changed, 25 insertions(+)
>
> diff --git a/mm/filemap.c b/mm/filemap.c
> index ca8c8d889eef..8b1684b62177 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -3122,6 +3122,14 @@ static struct file *do_sync_mmap_readahead(struct vm_fault *vmf)
> unsigned long vm_flags = vmf->vma->vm_flags;
> unsigned int mmap_miss;
>
> + /*
> + * If we have pre-content watches we need to disable readahead to make
> + * sure that we don't populate our mapping with 0 filled pages that we
> + * never emitted an event for.
> + */
> + if (fsnotify_file_has_pre_content_watches(file))
> + return fpin;
> +
> #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> /* Use the readahead code, even if readahead is disabled */
> if ((vm_flags & VM_HUGEPAGE) && HPAGE_PMD_ORDER <= MAX_PAGECACHE_ORDER) {
> @@ -3190,6 +3198,10 @@ static struct file *do_async_mmap_readahead(struct vm_fault *vmf,
> struct file *fpin = NULL;
> unsigned int mmap_miss;
>
> + /* See comment in do_sync_mmap_readahead. */
> + if (fsnotify_file_has_pre_content_watches(file))
> + return fpin;
> +
> /* If we don't want any read-ahead, don't bother */
> if (vmf->vma->vm_flags & VM_RAND_READ || !ra->ra_pages)
> return fpin;
> diff --git a/mm/readahead.c b/mm/readahead.c
> index 817b2a352d78..bc068d9218e3 100644
> --- a/mm/readahead.c
> +++ b/mm/readahead.c
> @@ -128,6 +128,7 @@
> #include <linux/blk-cgroup.h>
> #include <linux/fadvise.h>
> #include <linux/sched/mm.h>
> +#include <linux/fsnotify.h>
>
> #include "internal.h"
>
> @@ -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;
> +
> /*
> * Even if readahead is disabled, issue this request as readahead
> * as we'll need it to satisfy the requested range. The forced
> @@ -704,6 +713,10 @@ void page_cache_async_ra(struct readahead_control *ractl,
> if (!ractl->ra->ra_pages)
> return;
>
> + /* See the comment in page_cache_sync_ra. */
> + if (ractl->file && fsnotify_file_has_pre_content_watches(ractl->file))
> + return;
> +
> /*
> * Same bit is used for PG_readahead and PG_reclaim.
> */
> --
> 2.43.0
>