[PATCH 0/2] Better fanotify support for tmpfs

[PATCH 0/2] Better fanotify support for tmpfs

[Amir Goldstein]

Jan,

I needed the tmpfs patch for the userns filesystem mark POC, but it
looks useful for its own right to be able to set filesystem mount or
inode marks on tmpfs with FAN_REPORT_FID.

I can break the cleanup patch into helper and individual patches
for the fs maintainers, but since its so dummy, I thought it might
be best to get an ACK from fs maintainers and carry this as a single
patch.

Thanks,
Amir.

Amir Goldstein (2):
  fs: introduce a wrapper uuid_to_fsid()
  shmem: allow reporting fanotify events with file handles on tmpfs

 fs/ext2/super.c        | 5 +----
 fs/ext4/super.c        | 5 +----
 fs/zonefs/super.c      | 5 +----
 include/linux/statfs.h | 7 +++++++
 mm/shmem.c             | 3 +++
 5 files changed, 13 insertions(+), 12 deletions(-)

-- 
2.25.1

[PATCH 1/2] fs: introduce a wrapper uuid_to_fsid()

[Amir Goldstein]

Some filesystem's use a digest of their uuid for f_fsid.
Create a simple wrapper for this open coded folding.

Filesystems that have a non null uuid but use the block device
number for f_fsid may also consider using this helper.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/ext2/super.c        | 5 +----
 fs/ext4/super.c        | 5 +----
 fs/zonefs/super.c      | 5 +----
 include/linux/statfs.h | 7 +++++++
 4 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/fs/ext2/super.c b/fs/ext2/super.c
index 6c4753277916..0d679451657c 100644
--- a/fs/ext2/super.c
+++ b/fs/ext2/super.c
@@ -1399,7 +1399,6 @@ static int ext2_statfs (struct dentry * dentry, struct kstatfs * buf)
 	struct super_block *sb = dentry->d_sb;
 	struct ext2_sb_info *sbi = EXT2_SB(sb);
 	struct ext2_super_block *es = sbi->s_es;
-	u64 fsid;
 
 	spin_lock(&sbi->s_lock);
 
@@ -1453,9 +1452,7 @@ static int ext2_statfs (struct dentry * dentry, struct kstatfs * buf)
 	buf->f_ffree = ext2_count_free_inodes(sb);
 	es->s_free_inodes_count = cpu_to_le32(buf->f_ffree);
 	buf->f_namelen = EXT2_NAME_LEN;
-	fsid = le64_to_cpup((void *)es->s_uuid) ^
-	       le64_to_cpup((void *)es->s_uuid + sizeof(u64));
-	buf->f_fsid = u64_to_fsid(fsid);
+	buf->f_fsid = uuid_to_fsid(es->s_uuid);
 	spin_unlock(&sbi->s_lock);
 	return 0;
 }
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index ad34a37278cd..3581c1cdc19e 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -6148,7 +6148,6 @@ static int ext4_statfs(struct dentry *dentry, struct kstatfs *buf)
 	struct ext4_sb_info *sbi = EXT4_SB(sb);
 	struct ext4_super_block *es = sbi->s_es;
 	ext4_fsblk_t overhead = 0, resv_blocks;
-	u64 fsid;
 	s64 bfree;
 	resv_blocks = EXT4_C2B(sbi, atomic64_read(&sbi->s_resv_clusters));
 
@@ -6169,9 +6168,7 @@ static int ext4_statfs(struct dentry *dentry, struct kstatfs *buf)
 	buf->f_files = le32_to_cpu(es->s_inodes_count);
 	buf->f_ffree = percpu_counter_sum_positive(&sbi->s_freeinodes_counter);
 	buf->f_namelen = EXT4_NAME_LEN;
-	fsid = le64_to_cpup((void *)es->s_uuid) ^
-	       le64_to_cpup((void *)es->s_uuid + sizeof(u64));
-	buf->f_fsid = u64_to_fsid(fsid);
+	buf->f_fsid = uuid_to_fsid(es->s_uuid);
 
 #ifdef CONFIG_QUOTA
 	if (ext4_test_inode_flag(dentry->d_inode, EXT4_INODE_PROJINHERIT) &&
diff --git a/fs/zonefs/super.c b/fs/zonefs/super.c
index 0fe76f376dee..e09810311162 100644
--- a/fs/zonefs/super.c
+++ b/fs/zonefs/super.c
@@ -1104,7 +1104,6 @@ static int zonefs_statfs(struct dentry *dentry, struct kstatfs *buf)
 	struct super_block *sb = dentry->d_sb;
 	struct zonefs_sb_info *sbi = ZONEFS_SB(sb);
 	enum zonefs_ztype t;
-	u64 fsid;
 
 	buf->f_type = ZONEFS_MAGIC;
 	buf->f_bsize = sb->s_blocksize;
@@ -1127,9 +1126,7 @@ static int zonefs_statfs(struct dentry *dentry, struct kstatfs *buf)
 
 	spin_unlock(&sbi->s_lock);
 
-	fsid = le64_to_cpup((void *)sbi->s_uuid.b) ^
-		le64_to_cpup((void *)sbi->s_uuid.b + sizeof(u64));
-	buf->f_fsid = u64_to_fsid(fsid);
+	buf->f_fsid = uuid_to_fsid(sbi->s_uuid.b);
 
 	return 0;
 }
diff --git a/include/linux/statfs.h b/include/linux/statfs.h
index 20f695b90aab..ed86ac090a1b 100644
--- a/include/linux/statfs.h
+++ b/include/linux/statfs.h
@@ -50,4 +50,11 @@ static inline __kernel_fsid_t u64_to_fsid(u64 v)
 	return (__kernel_fsid_t){.val = {(u32)v, (u32)(v>>32)}};
 }
 
+/* Fold 16 bytes uuid to 64 bit fsid */
+static inline __kernel_fsid_t uuid_to_fsid(__u8 *uuid)
+{
+	return u64_to_fsid(le64_to_cpup((void *)uuid) ^
+		le64_to_cpup((void *)(uuid + sizeof(u64))));
+}
+
 #endif
-- 
2.25.1

Re: [PATCH 1/2] fs: introduce a wrapper uuid_to_fsid()

[Damien Le Moal]

On 2021/03/23 2:39, Amir Goldstein wrote:
> Some filesystem's use a digest of their uuid for f_fsid.
> Create a simple wrapper for this open coded folding.
> 
> Filesystems that have a non null uuid but use the block device
> number for f_fsid may also consider using this helper.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  fs/ext2/super.c        | 5 +----
>  fs/ext4/super.c        | 5 +----
>  fs/zonefs/super.c      | 5 +----
>  include/linux/statfs.h | 7 +++++++
>  4 files changed, 10 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/ext2/super.c b/fs/ext2/super.c
> index 6c4753277916..0d679451657c 100644
> --- a/fs/ext2/super.c
> +++ b/fs/ext2/super.c
> @@ -1399,7 +1399,6 @@ static int ext2_statfs (struct dentry * dentry, struct kstatfs * buf)
>  	struct super_block *sb = dentry->d_sb;
>  	struct ext2_sb_info *sbi = EXT2_SB(sb);
>  	struct ext2_super_block *es = sbi->s_es;
> -	u64 fsid;
>  
>  	spin_lock(&sbi->s_lock);
>  
> @@ -1453,9 +1452,7 @@ static int ext2_statfs (struct dentry * dentry, struct kstatfs * buf)
>  	buf->f_ffree = ext2_count_free_inodes(sb);
>  	es->s_free_inodes_count = cpu_to_le32(buf->f_ffree);
>  	buf->f_namelen = EXT2_NAME_LEN;
> -	fsid = le64_to_cpup((void *)es->s_uuid) ^
> -	       le64_to_cpup((void *)es->s_uuid + sizeof(u64));
> -	buf->f_fsid = u64_to_fsid(fsid);
> +	buf->f_fsid = uuid_to_fsid(es->s_uuid);
>  	spin_unlock(&sbi->s_lock);
>  	return 0;
>  }
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index ad34a37278cd..3581c1cdc19e 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -6148,7 +6148,6 @@ static int ext4_statfs(struct dentry *dentry, struct kstatfs *buf)
>  	struct ext4_sb_info *sbi = EXT4_SB(sb);
>  	struct ext4_super_block *es = sbi->s_es;
>  	ext4_fsblk_t overhead = 0, resv_blocks;
> -	u64 fsid;
>  	s64 bfree;
>  	resv_blocks = EXT4_C2B(sbi, atomic64_read(&sbi->s_resv_clusters));
>  
> @@ -6169,9 +6168,7 @@ static int ext4_statfs(struct dentry *dentry, struct kstatfs *buf)
>  	buf->f_files = le32_to_cpu(es->s_inodes_count);
>  	buf->f_ffree = percpu_counter_sum_positive(&sbi->s_freeinodes_counter);
>  	buf->f_namelen = EXT4_NAME_LEN;
> -	fsid = le64_to_cpup((void *)es->s_uuid) ^
> -	       le64_to_cpup((void *)es->s_uuid + sizeof(u64));
> -	buf->f_fsid = u64_to_fsid(fsid);
> +	buf->f_fsid = uuid_to_fsid(es->s_uuid);
>  
>  #ifdef CONFIG_QUOTA
>  	if (ext4_test_inode_flag(dentry->d_inode, EXT4_INODE_PROJINHERIT) &&
> diff --git a/fs/zonefs/super.c b/fs/zonefs/super.c
> index 0fe76f376dee..e09810311162 100644
> --- a/fs/zonefs/super.c
> +++ b/fs/zonefs/super.c
> @@ -1104,7 +1104,6 @@ static int zonefs_statfs(struct dentry *dentry, struct kstatfs *buf)
>  	struct super_block *sb = dentry->d_sb;
>  	struct zonefs_sb_info *sbi = ZONEFS_SB(sb);
>  	enum zonefs_ztype t;
> -	u64 fsid;
>  
>  	buf->f_type = ZONEFS_MAGIC;
>  	buf->f_bsize = sb->s_blocksize;
> @@ -1127,9 +1126,7 @@ static int zonefs_statfs(struct dentry *dentry, struct kstatfs *buf)
>  
>  	spin_unlock(&sbi->s_lock);
>  
> -	fsid = le64_to_cpup((void *)sbi->s_uuid.b) ^
> -		le64_to_cpup((void *)sbi->s_uuid.b + sizeof(u64));
> -	buf->f_fsid = u64_to_fsid(fsid);
> +	buf->f_fsid = uuid_to_fsid(sbi->s_uuid.b);
>  
>  	return 0;
>  }
> diff --git a/include/linux/statfs.h b/include/linux/statfs.h
> index 20f695b90aab..ed86ac090a1b 100644
> --- a/include/linux/statfs.h
> +++ b/include/linux/statfs.h
> @@ -50,4 +50,11 @@ static inline __kernel_fsid_t u64_to_fsid(u64 v)
>  	return (__kernel_fsid_t){.val = {(u32)v, (u32)(v>>32)}};
>  }
>  
> +/* Fold 16 bytes uuid to 64 bit fsid */
> +static inline __kernel_fsid_t uuid_to_fsid(__u8 *uuid)
> +{
> +	return u64_to_fsid(le64_to_cpup((void *)uuid) ^
> +		le64_to_cpup((void *)(uuid + sizeof(u64))));
> +}
> +
>  #endif
> 

For the zonefs bits, looks good to me.

Acked-by: Damien Le Moal <damien.lemoal@wdc.com>


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH 1/2] fs: introduce a wrapper uuid_to_fsid()

[Christian Brauner]

On Mon, Mar 22, 2021 at 07:39:43PM +0200, Amir Goldstein wrote:
> Some filesystem's use a digest of their uuid for f_fsid.
> Create a simple wrapper for this open coded folding.
> 
> Filesystems that have a non null uuid but use the block device
> number for f_fsid may also consider using this helper.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---

Seen that as part of your other series yesterday. Looks like a good
improvement and removes a bunch of open-coding.
Reviewed-by: Christian Brauner <christian.brauner@ubuntu.com>

[PATCH 2/2] shmem: allow reporting fanotify events with file handles on tmpfs

[Amir Goldstein]

Since kernel v5.1, fanotify_init(2) supports the flag FAN_REPORT_FID
for identifying objects using file handle and fsid in events.

fanotify_mark(2) fails with -ENODEV when trying to set a mark on
filesystems that report null f_fsid in stasfs(2).

Use the digest of uuid as f_fsid for tmpfs to uniquely identify tmpfs
objects as best as possible and allow setting an fanotify mark that
reports events with file handles on tmpfs.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 mm/shmem.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/mm/shmem.c b/mm/shmem.c
index b2db4ed0fbc7..162d8f8993bb 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -2846,6 +2846,9 @@ static int shmem_statfs(struct dentry *dentry, struct kstatfs *buf)
 		buf->f_ffree = sbinfo->free_inodes;
 	}
 	/* else leave those fields 0 like simple_statfs */
+
+	buf->f_fsid = uuid_to_fsid(dentry->d_sb->s_uuid.b);
+
 	return 0;
 }
 
-- 
2.25.1

Re: [PATCH 2/2] shmem: allow reporting fanotify events with file handles on tmpfs

[Jan Kara]

On Mon 22-03-21 19:39:44, Amir Goldstein wrote:
> Since kernel v5.1, fanotify_init(2) supports the flag FAN_REPORT_FID
> for identifying objects using file handle and fsid in events.
> 
> fanotify_mark(2) fails with -ENODEV when trying to set a mark on
> filesystems that report null f_fsid in stasfs(2).
> 
> Use the digest of uuid as f_fsid for tmpfs to uniquely identify tmpfs
> objects as best as possible and allow setting an fanotify mark that
> reports events with file handles on tmpfs.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>

Hugh, any opinion on this patch?

								Honza

> ---
>  mm/shmem.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/mm/shmem.c b/mm/shmem.c
> index b2db4ed0fbc7..162d8f8993bb 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -2846,6 +2846,9 @@ static int shmem_statfs(struct dentry *dentry, struct kstatfs *buf)
>  		buf->f_ffree = sbinfo->free_inodes;
>  	}
>  	/* else leave those fields 0 like simple_statfs */
> +
> +	buf->f_fsid = uuid_to_fsid(dentry->d_sb->s_uuid.b);
> +
>  	return 0;
>  }
>  
> -- 
> 2.25.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 2/2] shmem: allow reporting fanotify events with file handles on tmpfs

[Amir Goldstein]

On Thu, Mar 25, 2021 at 5:00 PM Jan Kara <jack@suse.cz> wrote:
>
> On Mon 22-03-21 19:39:44, Amir Goldstein wrote:
> > Since kernel v5.1, fanotify_init(2) supports the flag FAN_REPORT_FID
> > for identifying objects using file handle and fsid in events.
> >
> > fanotify_mark(2) fails with -ENODEV when trying to set a mark on
> > filesystems that report null f_fsid in stasfs(2).
> >
> > Use the digest of uuid as f_fsid for tmpfs to uniquely identify tmpfs
> > objects as best as possible and allow setting an fanotify mark that
> > reports events with file handles on tmpfs.
> >
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
>
> Hugh, any opinion on this patch?
>
>                                                                 Honza
>
> > ---
> >  mm/shmem.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/mm/shmem.c b/mm/shmem.c
> > index b2db4ed0fbc7..162d8f8993bb 100644
> > --- a/mm/shmem.c
> > +++ b/mm/shmem.c
> > @@ -2846,6 +2846,9 @@ static int shmem_statfs(struct dentry *dentry, struct kstatfs *buf)
> >               buf->f_ffree = sbinfo->free_inodes;
> >       }
> >       /* else leave those fields 0 like simple_statfs */
> > +
> > +     buf->f_fsid = uuid_to_fsid(dentry->d_sb->s_uuid.b);
> > +
> >       return 0;
> >  }
> >


Ping.

Hugh, are you ok with this change?

Thanks,
Amir.

Re: [PATCH 2/2] shmem: allow reporting fanotify events with file handles on tmpfs

[Hugh Dickins]

On Fri, 16 Apr 2021, Amir Goldstein wrote:
> On Thu, Mar 25, 2021 at 5:00 PM Jan Kara <jack@suse.cz> wrote:
> >
> > On Mon 22-03-21 19:39:44, Amir Goldstein wrote:
> > > Since kernel v5.1, fanotify_init(2) supports the flag FAN_REPORT_FID
> > > for identifying objects using file handle and fsid in events.
> > >
> > > fanotify_mark(2) fails with -ENODEV when trying to set a mark on
> > > filesystems that report null f_fsid in stasfs(2).
> > >
> > > Use the digest of uuid as f_fsid for tmpfs to uniquely identify tmpfs
> > > objects as best as possible and allow setting an fanotify mark that
> > > reports events with file handles on tmpfs.
> > >
> > > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> >
> > Hugh, any opinion on this patch?
> >
> >                                                                 Honza
> >
> > > ---
> > >  mm/shmem.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/mm/shmem.c b/mm/shmem.c
> > > index b2db4ed0fbc7..162d8f8993bb 100644
> > > --- a/mm/shmem.c
> > > +++ b/mm/shmem.c
> > > @@ -2846,6 +2846,9 @@ static int shmem_statfs(struct dentry *dentry, struct kstatfs *buf)
> > >               buf->f_ffree = sbinfo->free_inodes;
> > >       }
> > >       /* else leave those fields 0 like simple_statfs */
> > > +
> > > +     buf->f_fsid = uuid_to_fsid(dentry->d_sb->s_uuid.b);
> > > +
> > >       return 0;
> > >  }
> > >
> 
> 
> Ping.
> 
> Hugh, are you ok with this change?
> 
> Thanks,
> Amir.

Yes, apologies for my delay to you, Amir and Jan:
sure I'm ok with this change, and thank you for taking care of tmpfs.

Acked-by: Hugh Dickins <hughd@google.com>

But you have more valuable acks on this little series already,
so don't bother rebasing some tree to add mine in now.  I don't yet
see the uuid_to_fsid() 1/2 which this depends on in linux-next, and
fear that's my fault for holding you back: sorry, please go ahead now.

Hugh

Re: [PATCH 2/2] shmem: allow reporting fanotify events with file handles on tmpfs

[Jan Kara]

On Fri 16-04-21 20:54:46, Hugh Dickins wrote:
> On Fri, 16 Apr 2021, Amir Goldstein wrote:
> > On Thu, Mar 25, 2021 at 5:00 PM Jan Kara <jack@suse.cz> wrote:
> > >
> > > On Mon 22-03-21 19:39:44, Amir Goldstein wrote:
> > > > Since kernel v5.1, fanotify_init(2) supports the flag FAN_REPORT_FID
> > > > for identifying objects using file handle and fsid in events.
> > > >
> > > > fanotify_mark(2) fails with -ENODEV when trying to set a mark on
> > > > filesystems that report null f_fsid in stasfs(2).
> > > >
> > > > Use the digest of uuid as f_fsid for tmpfs to uniquely identify tmpfs
> > > > objects as best as possible and allow setting an fanotify mark that
> > > > reports events with file handles on tmpfs.
> > > >
> > > > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > >
> > > Hugh, any opinion on this patch?
> > >
> > >                                                                 Honza
> > >
> > > > ---
> > > >  mm/shmem.c | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/mm/shmem.c b/mm/shmem.c
> > > > index b2db4ed0fbc7..162d8f8993bb 100644
> > > > --- a/mm/shmem.c
> > > > +++ b/mm/shmem.c
> > > > @@ -2846,6 +2846,9 @@ static int shmem_statfs(struct dentry *dentry, struct kstatfs *buf)
> > > >               buf->f_ffree = sbinfo->free_inodes;
> > > >       }
> > > >       /* else leave those fields 0 like simple_statfs */
> > > > +
> > > > +     buf->f_fsid = uuid_to_fsid(dentry->d_sb->s_uuid.b);
> > > > +
> > > >       return 0;
> > > >  }
> > > >
> > 
> > 
> > Ping.
> > 
> > Hugh, are you ok with this change?
> > 
> > Thanks,
> > Amir.
> 
> Yes, apologies for my delay to you, Amir and Jan:
> sure I'm ok with this change, and thank you for taking care of tmpfs.
> 
> Acked-by: Hugh Dickins <hughd@google.com>

Thanks for the ack Hugh!

> But you have more valuable acks on this little series already,
> so don't bother rebasing some tree to add mine in now.  I don't yet
> see the uuid_to_fsid() 1/2 which this depends on in linux-next, and
> fear that's my fault for holding you back: sorry, please go ahead now.

Yeah, I was reluctant to push this patch without your ack and honestly I
forgot about the first patch while waiting for your ack. Anyway, both
patches are now in my tree and since Linus tagged rc8 and they are
nobrainers, I plan to push them to Linus for the next merge window.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 0/2] Better fanotify support for tmpfs

[Jan Kara]

On Mon 22-03-21 19:39:42, Amir Goldstein wrote:
> Jan,
> 
> I needed the tmpfs patch for the userns filesystem mark POC, but it
> looks useful for its own right to be able to set filesystem mount or
> inode marks on tmpfs with FAN_REPORT_FID.
> 
> I can break the cleanup patch into helper and individual patches
> for the fs maintainers, but since its so dummy, I thought it might
> be best to get an ACK from fs maintainers and carry this as a single
> patch.

The series looks fine to me. I'll wait if I get some feedback from Hugh. If
Hugh doesn't object, I'll take the two patches to my tree.

								Honza

> Amir Goldstein (2):
>   fs: introduce a wrapper uuid_to_fsid()
>   shmem: allow reporting fanotify events with file handles on tmpfs
> 
>  fs/ext2/super.c        | 5 +----
>  fs/ext4/super.c        | 5 +----
>  fs/zonefs/super.c      | 5 +----
>  include/linux/statfs.h | 7 +++++++
>  mm/shmem.c             | 3 +++
>  5 files changed, 13 insertions(+), 12 deletions(-)
> 
> -- 
> 2.25.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 0/2] Better fanotify support for tmpfs

[Christian Brauner]

On Mon, Mar 22, 2021 at 07:39:42PM +0200, Amir Goldstein wrote:
> Jan,
> 
> I needed the tmpfs patch for the userns filesystem mark POC, but it
> looks useful for its own right to be able to set filesystem mount or
> inode marks on tmpfs with FAN_REPORT_FID.

Indeed.
Seems how to generate the fs_id part based on the uuid is a bit
controversial for some fses (from the xfs thread) but this series seems
useful. Being able to place marks on tmpfs is worth it.

Reviewed-by: Christian Brauner <christian.brauner@ubuntu.com>