On Fri, Aug 11, 2023 at 4:21 PM Miklos Szeredi <miklos@xxxxxxxxxx> wrote: > > On Fri, 11 Aug 2023 at 11:44, Amir Goldstein <amir73il@xxxxxxxxx> wrote: > > > > On Thu, Jul 13, 2023 at 3:03 PM Amir Goldstein <amir73il@xxxxxxxxx> wrote: > > > > > > Miklos, > > > > > > This is the second part of the work to support fanotify reporting of > > > events with file ids on overlayfs. The fanotify_event_info_fid struct > > > reported with fanotify events has an object file handle and an fsid, > > > so fanotify requires that filesystems can encode file handles and have > > > a non-zero f_fsid. > > > > > > The first part [1] that was merged to v6.5-rc1, relaxed the fanotify > > > requirements for filesystems to support reporting events with fid to > > > require only the ->encode_fh() export operation. > > > > > > Patch 1 changes overlayfs export_operations to meet the new requirements > > > with the default overlay configurations (i.e. no need for nfs_export=on), > > > thus, allowing an fanotify watch with FAN_REPORT_FID on overlayfs. > > > There are LTS tests [2] for fanotify(FAN_REPORT_FID) + overlayfs. > > > > > > Patches 2-4 are not strcitly needed to support reporting fanotify events > > > with fid, because overlayfs already reports a non-zero f_fsid, it's just > > > not a unique fsid. So before allowing to report events with overlayfs > > > fids, I wanted to fix overlayfs fsid to be more unique. > > > > > > I wanted to implement a persistent and unique fsid for overlayfs. > > > I wanted it to be the default behavior, but needed to avoid breaking > > > applications that rely on an existing overlayfs fsid to persist. > > > I came up with a solution that is described in patch 4 (uuid=auto) that > > > meets all the requirement above. > > > There is an xfstest to test the persistent-unique fsid feature [3]. > > > > > > This patch set has been in overlayfs-next for soaking since v6.5-rc1. > > > If you have any reservations that we will not be able to resolve in time > > > for 6.6, especially regarding the on-disk format and backward compat, > > > we could also merge only patch 1 and leave the fsid patches for later. > > > > > > > Hi Miklos, > > > > Any comments on this series (which is currently on overlayfs-next)? > > No issues from me on the design. Will review the patches. > > > Please let me know if you want me to take care of the 6.6 PR or > > if you intend to prepare it yourself. > > I'd be very thankful If you could take care of the 6.6 PR. No problem. > > > > Other candidates for 6.6 include: > > - ovl lock re-ordering > > https://lore.kernel.org/linux-unionfs/20230720153731.420290-1-amir73il@xxxxxxxxx/ > > - CONFIG_OVERLAY_FS_DEBUG > > https://lore.kernel.org/linux-unionfs/20230521082813.17025-1-andrea.righi@xxxxxxxxxxxxx/ > > Yep, those look good, with the exception of the lockdep silencing. > If it's a false positive, then we should teach lockdep about why. If > it's a real deadlock, then let's try to fix it properly (only holding > sb_writers over operations that need it). > Although it's a corner case, I think the deadlock is real. All it takes is two different ovl mount with the same upper fs, one is the lower of the other doing - upper sb_writers - lower ovl_inode_lock - upper sb_writers during copy up. Technically, to avoid the hack, we can remove ovl_want_write() from ovl_copy_up_start() and do it individually for every low level helper that needs to modify upper fs. I can try to see how noisy this will be. Thanks, Amir.
