Re: [PATCH v2 1/5] fsnotify: pass dentry instead of inode when available

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

 



> > w.r.t btrfs - btrfs has a single sb for multiple subvolumes so by
> > definition the FAN_MARK_FILESYSTEM feature is only capable of watching
> > ALL subvolumes.
>
> Agreed but then if some event happens on inode A in subvolume S, you have
> to be sure the handle+fsid you return will allow you to open the file with
> that content and not inode A on subvolume T which has a different
> contents...

So as written below, the btrfs file handle is unique across all subvolumes.
fsid is therefore not needed to find the subvolume. It is only needed to find
the btrfs super block and listening on many block devices and/or many
filesystem types.

> > Basically, the purpose, of fid->fsid is:
> > 1. A key for finding a mount point to use as mount_fd argument to
> > open_by_handle_at()
> > 2. Make fid unique across the system
>
> Understood, that's what I thought.
>
> > Since btrfs file handles encode the subvolume root object id in the file
> > handle, fid->fsid is only needed to find a btrfs mount of the same sb
> > (blockdev).
>
> Ah good. I didn't realize btrfs will record subvolume root in the file
> handle. On a second thought how else could NFS mount work, right, but it
> didn't occur to me yesterday.
>
> > Regardless, IIUC, btrfs_statfs() returns an fsid which is associated with
> > the single super block struct, so all dentries in all subvolumes will
> > return the same fsid: btrfs_sb(dentry->d_sb)->fsid.
>
> This is not true AFAICT. Looking at btrfs_statfs() I can see:
>
>         buf->f_fsid.val[0] = be32_to_cpu(fsid[0]) ^ be32_to_cpu(fsid[2]);
>         buf->f_fsid.val[1] = be32_to_cpu(fsid[1]) ^ be32_to_cpu(fsid[3]);
>         /* Mask in the root object ID too, to disambiguate subvols */
>         buf->f_fsid.val[0] ^=
>                 BTRFS_I(d_inode(dentry))->root->root_key.objectid >> 32;
>         buf->f_fsid.val[1] ^=
>                 BTRFS_I(d_inode(dentry))->root->root_key.objectid;
>
> So subvolume root is xored into the FSID. Thus dentries from different
> subvolumes are going to return different fsids...
>

Right... how could I have missed that :-/

I do not want to bring back FSNOTIFY_EVENT_DENTRY just for that
and I saw how many flaws you pointed to in $SUBJECT patch.
Instead, I will use:
statfs_by_dentry(d_find_any_alias(inode) ?: inode->i_sb->sb_root,...

So we are only left with the corner case of fsnotify_inoderemove()
on btrfs and the like. I checked that for all the rest of fsnotify hooks
an alias is guarantied to exist at the time of the hook.

I could go with either of two options:
1. No support for FAN_DELETE_SELF as my v2 patch set already does
2. Best effort support for FAN_DELETE_SELF - it works for non-btrfs
    and if application listens on a single filesystem and ignores fsid
    (document this culprit).

What do you think?

Thanks,
Amir.



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux