Re: [PATCH RFC 02/16] fs/bdev: Add atomic write support info to statx

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

 



On 03/05/2023 22:58, Dave Chinner wrote:

Hi Dave,

+	/* Handle STATX_WRITE_ATOMIC for block devices */
+	if (request_mask & STATX_WRITE_ATOMIC) {
+		struct inode *inode = d_backing_inode(path.dentry);
+
+		if (S_ISBLK(inode->i_mode))
+			bdev_statx_atomic(inode, stat);
+	}
This duplicates STATX_DIOALIGN bdev handling.

Really, the bdev attribute handling should be completely factored
out of vfs_statx() - blockdevs are not the common fastpath for stat
operations. Somthing like:

	/*
	 * If this is a block device inode, override the filesystem
	 * attributes with the block device specific parameters
	 * that need to be obtained from the bdev backing inode.
	 */
	if (S_ISBLK(d_backing_inode(path.dentry)->i_mode))
		bdev_statx(path.dentry, stat);

And then all the overrides can go in the one function that doesn't
need to repeatedly check S_ISBLK()....

ok, that looks like a reasonable idea. We'll look to make that change.



diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 6b6f2992338c..19d33b2897b2 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1527,6 +1527,7 @@ int sync_blockdev_range(struct block_device *bdev, loff_t lstart, loff_t lend);
  int sync_blockdev_nowait(struct block_device *bdev);
  void sync_bdevs(bool wait);
  void bdev_statx_dioalign(struct inode *inode, struct kstat *stat);
+void bdev_statx_atomic(struct inode *inode, struct kstat *stat);
  void printk_all_partitions(void);
  #else
  static inline void invalidate_bdev(struct block_device *bdev)
@@ -1546,6 +1547,9 @@ static inline void sync_bdevs(bool wait)
  static inline void bdev_statx_dioalign(struct inode *inode, struct kstat *stat)
  {
  }
+static inline void bdev_statx_atomic(struct inode *inode, struct kstat *stat)
+{
+}
  static inline void printk_all_partitions(void)
  {
  }
That also gets rid of the need for all these fine grained exports
out of the bdev code for statx....

Sure


diff --git a/include/linux/stat.h b/include/linux/stat.h
index 52150570d37a..dfa69ecfaacf 100644
--- a/include/linux/stat.h
+++ b/include/linux/stat.h
@@ -53,6 +53,8 @@ struct kstat {
  	u32		dio_mem_align;
  	u32		dio_offset_align;
  	u64		change_cookie;
+	u32		atomic_write_unit_max;
+	u32		atomic_write_unit_min;
  };
/* These definitions are internal to the kernel for now. Mainly used by nfsd. */
diff --git a/include/uapi/linux/stat.h b/include/uapi/linux/stat.h
index 7cab2c65d3d7..c99d7cac2aa6 100644
--- a/include/uapi/linux/stat.h
+++ b/include/uapi/linux/stat.h
@@ -127,7 +127,10 @@ struct statx {
  	__u32	stx_dio_mem_align;	/* Memory buffer alignment for direct I/O */
  	__u32	stx_dio_offset_align;	/* File offset alignment for direct I/O */
  	/* 0xa0 */
-	__u64	__spare3[12];	/* Spare space for future expansion */
+	__u32	stx_atomic_write_unit_max;
+	__u32	stx_atomic_write_unit_min;
+	/* 0xb0 */
+	__u64	__spare3[11];	/* Spare space for future expansion */
  	/* 0x100 */
  };
No documentation on what units these are in.

It's in bytes, we're really just continuing the pattern of what we do for dio. I think that it would be reasonable to limit to u32.

Is there a statx() man
page update for this addition?

No, not yet. Is it normally expected to provide a proposed man page update in parallel? Or somewhat later, when the kernel API change has some appreciable level of agreement?

Thanks,
John




[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