diff mbox series

[RFC,4/8] ext4: Add statx and other atomic write helper routines

Message ID 9def15d6ffb88f7352713c65292513fab532112a.1709361537.git.ritesh.list@gmail.com
State New
Headers show
Series None | expand

Commit Message

Ritesh Harjani (IBM) March 2, 2024, 7:42 a.m. UTC 
This patch adds the statx (STATX_WRITE_ATOMIC) support in ext4_getattr()
to query for atomic_write_unit_min(awu_min), awu_max and other
attributes for atomic writes.
This adds a new runtime mount flag (EXT4_MF_ATOMIC_WRITE_FSAWU),
for querying whether ext4 supports atomic write using fsawu
(filesystem atomic write unit).

Co-developed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
---
 fs/ext4/ext4.h  | 53 ++++++++++++++++++++++++++++++++++++++++++++++++-
 fs/ext4/inode.c | 16 +++++++++++++++
 2 files changed, 68 insertions(+), 1 deletion(-)

Comments

John Garry March 6, 2024, 11:14 a.m. UTC | #1 
On 02/03/2024 07:42, Ritesh Harjani (IBM) wrote:
>   	}
>   
> +	if (request_mask & STATX_WRITE_ATOMIC) {
> +		unsigned int fsawu_min = 0, fsawu_max = 0;
> +
> +		/*
> +		 * Get fsawu_[min|max] value which we can advertise to userspace
> +		 * in statx call, if we support atomic writes using
> +		 * EXT4_MF_ATOMIC_WRITE_FSAWU.
> +		 */
> +		if (ext4_can_atomic_write_fsawu(inode->i_sb)) {

To me, it does not make sense to fill this in unless 
EXT4_INODE_ATOMIC_WRITE is also set for the inode.

> +			ext4_atomic_write_fsawu(inode->i_sb, &fsawu_min,
> +						&fsawu_max);
> +		}
> +
> +		generic_fill_statx_atomic_writes(stat, fsawu_min, fsawu_max);
> +	}
> +
>   	flags = ei->i_flags & EXT4_FL_USER_VISIBLE;
Ritesh Harjani (IBM) March 8, 2024, 8:10 a.m. UTC | #2 
John Garry <john.g.garry@oracle.com> writes:

> On 02/03/2024 07:42, Ritesh Harjani (IBM) wrote:
>>   	}
>>   
>> +	if (request_mask & STATX_WRITE_ATOMIC) {
>> +		unsigned int fsawu_min = 0, fsawu_max = 0;
>> +
>> +		/*
>> +		 * Get fsawu_[min|max] value which we can advertise to userspace
>> +		 * in statx call, if we support atomic writes using
>> +		 * EXT4_MF_ATOMIC_WRITE_FSAWU.
>> +		 */
>> +		if (ext4_can_atomic_write_fsawu(inode->i_sb)) {
>
> To me, it does not make sense to fill this in unless 
> EXT4_INODE_ATOMIC_WRITE is also set for the inode.
>

I was thinking advertising filesystem atomic write unit on an inode
could still be advertized. But I don't have any strong objection either.
We can advertize this values only when the inode has the atomic write
attribute enabled. I think this makes more sense. 

Thanks
-ritesh


>> +			ext4_atomic_write_fsawu(inode->i_sb, &fsawu_min,
>> +						&fsawu_max);
>> +		}
>> +
>> +		generic_fill_statx_atomic_writes(stat, fsawu_min, fsawu_max);
>> +	}
>> +
>>   	flags = ei->i_flags & EXT4_FL_USER_VISIBLE;
diff mbox series

Patch

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 023571f8dd1b..1d2bce26e616 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1817,7 +1817,8 @@  static inline int ext4_valid_inum(struct super_block *sb, unsigned long ino)
  */
 enum {
 	EXT4_MF_MNTDIR_SAMPLED,
-	EXT4_MF_FC_INELIGIBLE	/* Fast commit ineligible */
+	EXT4_MF_FC_INELIGIBLE,		/* Fast commit ineligible */
+	EXT4_MF_ATOMIC_WRITE_FSAWU	/* Atomic write via FSAWU */
 };
 
 static inline void ext4_set_mount_flag(struct super_block *sb, int bit)
@@ -3839,6 +3840,56 @@  static inline int ext4_buffer_uptodate(struct buffer_head *bh)
 	return buffer_uptodate(bh);
 }
 
+#define ext4_can_atomic_write_fsawu(sb)				\
+	ext4_test_mount_flag(sb, EXT4_MF_ATOMIC_WRITE_FSAWU)
+
+/**
+ * ext4_atomic_write_fsawu	Returns EXT4 filesystem atomic write unit.
+ *  @sb				super_block
+ *  This returns the filesystem min|max atomic write units.
+ *  For !bigalloc it is filesystem blocksize (fsawu_min)
+ *  For bigalloc it should be either blocksize or multiple of blocksize
+ *  (fsawu_min)
+ */
+static inline void ext4_atomic_write_fsawu(struct super_block *sb,
+					   unsigned int *fsawu_min,
+					   unsigned int *fsawu_max)
+{
+	u8 blkbits = sb->s_blocksize_bits;
+	unsigned int blocksize = 1U << blkbits;
+	unsigned int clustersize = blocksize;
+	struct block_device *bdev = sb->s_bdev;
+	unsigned int awu_min =
+			queue_atomic_write_unit_min_bytes(bdev->bd_queue);
+	unsigned int awu_max =
+			queue_atomic_write_unit_max_bytes(bdev->bd_queue);
+
+	if (ext4_has_feature_bigalloc(sb))
+		clustersize = 1U << (EXT4_SB(sb)->s_cluster_bits + blkbits);
+
+	/* fs min|max should respect awu_[min|max] units */
+	if (unlikely(awu_min > clustersize || awu_max < blocksize))
+		goto not_supported;
+
+	/* in case of !bigalloc fsawu_[min|max] should be same as blocksize */
+	if (!ext4_has_feature_bigalloc(sb)) {
+		*fsawu_min = blocksize;
+		*fsawu_max = blocksize;
+		return;
+	}
+
+	/* bigalloc can support write in blocksize units. So advertize it */
+	*fsawu_min = max(blocksize, awu_min);
+	*fsawu_max = min(clustersize, awu_max);
+
+	/* This should never happen, but let's keep a WARN_ON_ONCE */
+	WARN_ON_ONCE(!IS_ALIGNED(clustersize, *fsawu_min));
+	return;
+not_supported:
+	*fsawu_min = 0;
+	*fsawu_max = 0;
+}
+
 #endif	/* __KERNEL__ */
 
 #define EFSBADCRC	EBADMSG		/* Bad CRC detected */
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 2ccf3b5e3a7c..ea009ca9085d 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -5536,6 +5536,22 @@  int ext4_getattr(struct mnt_idmap *idmap, const struct path *path,
 		}
 	}
 
+	if (request_mask & STATX_WRITE_ATOMIC) {
+		unsigned int fsawu_min = 0, fsawu_max = 0;
+
+		/*
+		 * Get fsawu_[min|max] value which we can advertise to userspace
+		 * in statx call, if we support atomic writes using
+		 * EXT4_MF_ATOMIC_WRITE_FSAWU.
+		 */
+		if (ext4_can_atomic_write_fsawu(inode->i_sb)) {
+			ext4_atomic_write_fsawu(inode->i_sb, &fsawu_min,
+						&fsawu_max);
+		}
+
+		generic_fill_statx_atomic_writes(stat, fsawu_min, fsawu_max);
+	}
+
 	flags = ei->i_flags & EXT4_FL_USER_VISIBLE;
 	if (flags & EXT4_APPEND_FL)
 		stat->attributes |= STATX_ATTR_APPEND;