Re: [PATCH v4 00/16] fanotify: add pre-content hooks

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Aug 30, 2024 at 10:55:10AM +0200, Amir Goldstein wrote:
> On Thu, Aug 29, 2024 at 11:41 PM Darrick J. Wong <djwong@xxxxxxxxxx> wrote:
> >
> > On Wed, Aug 14, 2024 at 05:25:18PM -0400, Josef Bacik wrote:
> > > v3: https://lore.kernel.org/linux-fsdevel/cover.1723228772.git.josef@xxxxxxxxxxxxxx/
> > > v2: https://lore.kernel.org/linux-fsdevel/cover.1723144881.git.josef@xxxxxxxxxxxxxx/
> > > v1: https://lore.kernel.org/linux-fsdevel/cover.1721931241.git.josef@xxxxxxxxxxxxxx/
> > >
> > > v3->v4:
> > > - Trying to send a final verson Friday at 5pm before you go on vacation is a
> > >   recipe for silly mistakes, fixed the xfs handling yet again, per Christoph's
> > >   review.
> > > - Reworked the file system helper so it's handling of fpin was a little less
> > >   silly, per Chinner's suggestion.
> > > - Updated the return values to not or in VM_FAULT_RETRY, as we have a comment
> > >   in filemap_fault that says if VM_FAULT_ERROR is set we won't have
> > >   VM_FAULT_RETRY set.
> > >
> > > v2->v3:
> > > - Fix the pagefault path to do MAY_ACCESS instead, updated the perm handler to
> > >   emit PRE_ACCESS in this case, so we can avoid the extraneous perm event as per
> > >   Amir's suggestion.
> > > - Reworked the exported helper so the per-filesystem changes are much smaller,
> > >   per Amir's suggestion.
> > > - Fixed the screwup for DAX writes per Chinner's suggestion.
> > > - Added Christian's reviewed-by's where appropriate.
> > >
> > > v1->v2:
> > > - reworked the page fault logic based on Jan's suggestion and turned it into a
> > >   helper.
> > > - Added 3 patches per-fs where we need to call the fsnotify helper from their
> > >   ->fault handlers.
> > > - Disabled readahead in the case that there's a pre-content watch in place.
> > > - Disabled huge faults when there's a pre-content watch in place (entirely
> > >   because it's untested, theoretically it should be straightforward to do).
> > > - Updated the command numbers.
> > > - Addressed the random spelling/grammer mistakes that Jan pointed out.
> > > - Addressed the other random nits from Jan.
> > >
> > > --- Original email ---
> > >
> > > Hello,
> > >
> > > These are the patches for the bare bones pre-content fanotify support.  The
> > > majority of this work is Amir's, my contribution to this has solely been around
> > > adding the page fault hooks, testing and validating everything.  I'm sending it
> > > because Amir is traveling a bunch, and I touched it last so I'm going to take
> > > all the hate and he can take all the credit.
> > >
> > > There is a PoC that I've been using to validate this work, you can find the git
> > > repo here
> > >
> > > https://github.com/josefbacik/remote-fetch
> > >
> > > This consists of 3 different tools.
> > >
> > > 1. populate.  This just creates all the stub files in the directory from the
> > >    source directory.  Just run ./populate ~/linux ~/hsm-linux and it'll
> > >    recursively create all of the stub files and directories.
> > > 2. remote-fetch.  This is the actual PoC, you just point it at the source and
> > >    destination directory and then you can do whatever.  ./remote-fetch ~/linux
> > >    ~/hsm-linux.
> > > 3. mmap-validate.  This was to validate the pagefault thing, this is likely what
> > >    will be turned into the selftest with remote-fetch.  It creates a file and
> > >    then you can validate the file matches the right pattern with both normal
> > >    reads and mmap.  Normally I do something like
> > >
> > >    ./mmap-validate create ~/src/foo
> > >    ./populate ~/src ~/dst
> > >    ./rmeote-fetch ~/src ~/dst
> > >    ./mmap-validate validate ~/dst/foo
> > >
> > > I did a bunch of testing, I also got some performance numbers.  I copied a
> > > kernel tree, and then did remote-fetch, and then make -j4
> > >
> > > Normal
> > > real    9m49.709s
> > > user    28m11.372s
> > > sys     4m57.304s
> > >
> > > HSM
> > > real    10m6.454s
> > > user    29m10.517s
> > > sys     5m2.617s
> > >
> > > So ~17 seconds more to build with HSM.  I then did a make mrproper on both trees
> > > to see the size
> > >
> > > [root@fedora ~]# du -hs /src/linux
> > > 1.6G    /src/linux
> > > [root@fedora ~]# du -hs dst
> > > 125M    dst
> > >
> > > This mirrors the sort of savings we've seen in production.
> > >
> > > Meta has had these patches (minus the page fault patch) deployed in production
> > > for almost a year with our own utility for doing on-demand package fetching.
> > > The savings from this has been pretty significant.
> > >
> > > The page-fault hooks are necessary for the last thing we need, which is
> > > on-demand range fetching of executables.  Some of our binaries are several gigs
> > > large, having the ability to remote fetch them on demand is a huge win for us
> > > not only with space savings, but with startup time of containers.
> >
> > So... does this pre-content fetcher already work for regular reads and
> > writes, and now you're looking to wire up page faults?  Or does it only
> > handle page faults?  Judging from Amir's patches I'm guessing the
> > FAN_PRE_{ACCESS,MODIFY} events are new, so this only sends notifications
> > prior to read and write page faults?  The XFS change looks reasonable to
> > me, but I'm left wondering "what does this shiny /do/?"
> >
> 
> I *think* I understand the confusion.
> 
> Let me try to sort it out.
> 
> This patch set collaboration aims to add the functionality of HSM
> service by adding events FS_PRE_{ACCESS,MODIFY} prior to
> read/write and page faults.
> 
> Maybe you are puzzled by not seeing any new read/write hooks?
> This is because the HSM events utilize the existing fsnotify_file_*perm()
> hooks that are in place for the legacy FS_ACCESS_PERM event.
> 
> So why is a new FS_PRE_ACCESS needed?
> Let me first quote commit message of patch 2/16 [1]:
> ---
> The new FS_PRE_ACCESS permission event is similar to FS_ACCESS_PERM,
> but it meant for a different use case of filling file content before
> access to a file range, so it has slightly different semantics.
> 
> Generate FS_PRE_ACCESS/FS_ACCESS_PERM as two seperate events, same as
> we did for FS_OPEN_PERM/FS_OPEN_EXEC_PERM.
> 
> FS_PRE_MODIFY is a new permission event, with similar semantics as
> FS_PRE_ACCESS, which is called before a file is modified.
> 
> FS_ACCESS_PERM is reported also on blockdev and pipes, but the new
> pre-content events are only reported for regular files and dirs.
> 
> The pre-content events are meant to be used by hierarchical storage
> managers that want to fill the content of files on first access.
> ---
> 
> And from my man page draft [2]:
> ---
>        FAN_PRE_ACCESS (since Linux 6.x)
>               Create  an  event before a read from a directory or a file range,
>               that provides an opportunity for the event listener to
> modify the content of the object
>               before the reader is going to access the content in the
> specified range.
>               An additional information record of type
> FAN_EVENT_INFO_TYPE_RANGE is returned
>              for each event in the read buffer.
> ...
> ---
> 
> So the semantics of the two events is slightly different, but also the
> meaning of "an opportunity for the event listener to modify the content".
> 
> FS_ACCESS_PERM already provided this opportunity on old kernels,
> but prior to "Tidy up file permission hooks" series [3] in v6.8, writing
> file content in FS_ACCESS_PERM event context was prone to deadlocks.
> 
> Therefore, an application using FS_ACCESS_PERM may be prone
> for deadlocks, while an application using FAN_PRE_ACCESS should
> be safe in that regard.

Ah, ok, that's where the missing pieces are. :)

--D

> Thanks,
> Amir.
> 
> [1] https://lore.kernel.org/all/a96217d84dfebb15582a04524dc9821ba3ea1406.1723670362.git.josef@xxxxxxxxxxxxxx/
> [2] https://github.com/amir73il/man-pages/commits/fan_pre_path
> [3] https://lore.kernel.org/all/20231122122715.2561213-1-amir73il@xxxxxxxxx/
> 




[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux