linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Amir Goldstein <amir73il@gmail.com>
To: Jan Kara <jack@suse.cz>
Cc: Josef Bacik <josef@toxicpanda.com>,
	kernel-team@fb.com, linux-fsdevel@vger.kernel.org,
	 brauner@kernel.org, torvalds@linux-foundation.org,
	viro@zeniv.linux.org.uk,  linux-xfs@vger.kernel.org,
	linux-btrfs@vger.kernel.org, linux-mm@kvack.org,
	 linux-ext4@vger.kernel.org
Subject: Re: [PATCH v8 09/19] fsnotify: generate pre-content permission event on truncate
Date: Wed, 20 Nov 2024 16:57:30 +0100	[thread overview]
Message-ID: <CAOQ4uxiyAU7n4w-BMZx9gzL_DTeKMPkBOy9OZzZYEsqkMHWAGw@mail.gmail.com> (raw)
In-Reply-To: <20241120152340.gu7edmtm2j3lmxoy@quack3>

On Wed, Nov 20, 2024 at 4:23 PM Jan Kara <jack@suse.cz> wrote:
>
> On Fri 15-11-24 10:30:22, Josef Bacik wrote:
> > From: Amir Goldstein <amir73il@gmail.com>
> >
> > Generate FS_PRE_ACCESS event before truncate, without sb_writers held.
> >
> > Move the security hooks also before sb_start_write() to conform with
> > other security hooks (e.g. in write, fallocate).
> >
> > The event will have a range info of the page surrounding the new size
> > to provide an opportunity to fill the conetnt at the end of file before
> > truncating to non-page aligned size.
> >
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
>
> I was thinking about this. One small issue is that similarly as the
> filesystems may do RMW of tail page during truncate, they will do RMW of
> head & tail pages on hole punch or zero range so we should have some
> strategically sprinkled fsnotify_truncate_perm() calls there as well.
> That's easy enough to fix.

fallocate already has fsnotify_file_area_perm() hook.
What is missing?

>
> But there's another problem which I'm more worried about: If we have
> a file 64k large, user punches 12k..20k and then does read for 0..64k, then
> how does HSM daemon in userspace know what data to fill in? When we'll have
> modify pre-content event, daemon can watch it and since punch will send modify
> for 12k-20k, the daemon knows the local (empty) page cache is the source of
> truth. But without modify event this is just a recipe for data corruption
> AFAICT.
>
> So it seems the current setting with access pre-content event has only chance
> to work reliably in read-only mode? So we should probably refuse writeable
> open if file is being watched for pre-content events and similarly refuse
> truncate?

I am confused. not sure I understand the problem.

In the events that you specific, punch hole WILL generate a FS_PRE_ACCESS
event for 12k-20k.

When HSM gets a FS_PRE_ACCESS event for 12k-20k it MUST fill the content
and keep to itself that 12k-20k is the source of truth from now on.

The extra FS_PRE_ACCESS event on 0..64k absolutely does not change that.
IOW, a FS_PRE_ACCESS event on 0..64k definitely does NOT mean that
HSM NEEDS to fill content in 0..64k, it just means that it MAY needs
to fill content
if it hasn't done that for a range before the event.

To reiterate this important point, it is HSM responsibility to maintain the
 "content filled map" per file is its own way, under no circumstances it is
assumed that fiemap or page cache state has anything to do with the
"content filled map".

The *only* thing that HSM can assume if that if its "content filled map"
is empty for some range, then page cache is NOT yet populated in that
range and that also relies on how HSM and mount are being initialized
and exposed to users.

Did I misunderstand your concern?

Thanks,
Amir.


  reply	other threads:[~2024-11-20 15:57 UTC|newest]

Thread overview: 69+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-11-15 15:30 [PATCH v8 00/19] fanotify: add pre-content hooks Josef Bacik
2024-11-15 15:30 ` [PATCH v8 01/19] fs: get rid of __FMODE_NONOTIFY kludge Josef Bacik
2024-11-18 18:14   ` Jan Kara
2024-11-15 15:30 ` [PATCH v8 02/19] fsnotify: opt-in for permission events at file open time Josef Bacik
2024-11-20 15:53   ` Jan Kara
2024-11-20 16:12     ` Amir Goldstein
2024-11-21  9:39       ` Jan Kara
2024-11-21 10:09         ` Christian Brauner
2024-11-21 11:04           ` Amir Goldstein
2024-11-21 11:16             ` Jan Kara
2024-11-21 11:32               ` Amir Goldstein
2024-11-21  9:45     ` Christian Brauner
2024-11-21 11:39       ` Amir Goldstein
2024-11-15 15:30 ` [PATCH v8 03/19] fsnotify: add helper to check if file is actually being watched Josef Bacik
2024-11-20 16:02   ` Jan Kara
2024-11-20 16:42     ` Amir Goldstein
2024-11-21  8:54       ` Jan Kara
2024-11-15 15:30 ` [PATCH v8 04/19] fanotify: don't skip extra event info if no info_mode is set Josef Bacik
2024-11-15 15:30 ` [PATCH v8 05/19] fanotify: rename a misnamed constant Josef Bacik
2024-11-15 15:30 ` [PATCH v8 06/19] fanotify: reserve event bit of deprecated FAN_DIR_MODIFY Josef Bacik
2024-11-15 15:30 ` [PATCH v8 07/19] fsnotify: introduce pre-content permission events Josef Bacik
2024-11-15 15:30 ` [PATCH v8 08/19] fsnotify: pass optional file access range in pre-content event Josef Bacik
2024-11-15 15:30 ` [PATCH v8 09/19] fsnotify: generate pre-content permission event on truncate Josef Bacik
2024-11-20 15:23   ` Jan Kara
2024-11-20 15:57     ` Amir Goldstein [this message]
2024-11-20 16:16       ` Jan Kara
2024-11-15 15:30 ` [PATCH v8 10/19] fanotify: introduce FAN_PRE_ACCESS permission event Josef Bacik
2024-11-15 15:59   ` Amir Goldstein
2024-11-21 10:44   ` Jan Kara
2024-11-21 14:18     ` Amir Goldstein
2024-11-21 16:36       ` Jan Kara
2024-11-21 18:31         ` Amir Goldstein
2024-11-21 18:37           ` Amir Goldstein
2024-11-22 12:42             ` Jan Kara
2024-11-22 13:51               ` Amir Goldstein
2024-11-27 12:18                 ` Jan Kara
2024-11-27 12:20                   ` Amir Goldstein
2024-11-15 15:30 ` [PATCH v8 11/19] fanotify: report file range info with pre-content events Josef Bacik
2024-11-15 15:30 ` [PATCH v8 12/19] fanotify: allow to set errno in FAN_DENY permission response Josef Bacik
2024-11-15 15:30 ` [PATCH v8 13/19] fanotify: add a helper to check for pre content events Josef Bacik
2024-11-20 15:44   ` Jan Kara
2024-11-20 16:43     ` Amir Goldstein
2024-11-15 15:30 ` [PATCH v8 14/19] fanotify: disable readahead if we have pre-content watches Josef Bacik
2024-11-15 15:30 ` [PATCH v8 15/19] mm: don't allow huge faults for files with pre content watches Josef Bacik
2025-01-31 19:17   ` [REGRESSION] " Alex Williamson
2025-01-31 19:59     ` Linus Torvalds
2025-02-01  1:19       ` Peter Xu
2025-02-01 14:38         ` Christian Brauner
2025-02-02  0:58           ` Linus Torvalds
2025-02-02  7:46             ` Amir Goldstein
2025-02-02 10:04               ` Christian Brauner
2025-02-03 12:41                 ` Jan Kara
2025-02-03 20:39                   ` Amir Goldstein
2025-02-03 21:41                     ` Alex Williamson
2025-02-03 22:04                       ` Amir Goldstein
2024-11-15 15:30 ` [PATCH v8 16/19] fsnotify: generate pre-content permission event on page fault Josef Bacik
2024-12-08 16:58   ` Klara Modin
2024-12-09 10:45     ` Aithal, Srikanth
2024-12-09 12:34       ` Jan Kara
2024-12-09 12:31     ` Jan Kara
2024-12-09 12:56       ` Klara Modin
2024-12-09 14:16         ` Jan Kara
2024-12-10 21:12     ` Randy Dunlap
2024-12-11 16:30       ` Jan Kara
2024-11-15 15:30 ` [PATCH v8 17/19] xfs: add pre-content fsnotify hook for write faults Josef Bacik
2024-11-21 10:22   ` Jan Kara
2024-11-15 15:30 ` [PATCH v8 18/19] btrfs: disable defrag on pre-content watched files Josef Bacik
2024-11-15 15:30 ` [PATCH v8 19/19] fs: enable pre-content events on supported file systems Josef Bacik
2024-11-21 11:29 ` [PATCH v8 00/19] fanotify: add pre-content hooks Jan Kara

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAOQ4uxiyAU7n4w-BMZx9gzL_DTeKMPkBOy9OZzZYEsqkMHWAGw@mail.gmail.com \
    --to=amir73il@gmail.com \
    --cc=brauner@kernel.org \
    --cc=jack@suse.cz \
    --cc=josef@toxicpanda.com \
    --cc=kernel-team@fb.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox