On 09/01/2024 23:04, Dave Chinner wrote:
--- a/include/uapi/linux/fs.h
+++ b/include/uapi/linux/fs.h
@@ -118,7 +118,8 @@ struct fsxattr {
__u32 fsx_nextents; /* nextents field value (get) */
__u32 fsx_projid; /* project identifier (get/set) */
__u32 fsx_cowextsize; /* CoW extsize field value
(get/set)*/
- unsigned char fsx_pad[8];
+ __u32 fsx_atomicwrites_size; /* unit max */
+ unsigned char fsx_pad[4];
};
/*
@@ -140,6 +141,7 @@ struct fsxattr {
#define FS_XFLAG_FILESTREAM 0x00004000 /* use filestream allocator
*/
#define FS_XFLAG_DAX 0x00008000 /* use DAX for IO */
#define FS_XFLAG_COWEXTSIZE 0x00010000 /* CoW extent size
allocator hint */
+#define FS_XFLAG_ATOMICWRITES 0x00020000
#define FS_XFLAG_HASATTR 0x80000000 /* no DIFLAG for this */
/* the read-only stuff doesn't really belong here, but any other place is
lines 1-22/22 (END)
Having FS_XFLAG_ATOMICWRITES set will lead to FMODE_CAN_ATOMIC_WRITE being
set.
So a user can issue:
xfs_io -c "atomic-writes 64K" mnt/file
xfs_io -c "atomic-writes" mnt/file
[65536] mnt/file
Where are you going to store this value in the inode? It requires a
new field in the inode and so is a change of on-disk format, right?
It would require an on-disk format change, unless we can find an
alternative way to store the value, like:
a. re-use pre-existing extsize or even cowextsize fields and 'xfs_io -c
"atomic-writes $SIZE"' would update those fields and
FS_XFLAG_ATOMICWRITES would be incompatible with FS_XFLAG_COWEXTSIZE or
FS_XFLAG_EXTSIZE
b. require FS_XFLAG_EXTSIZE and extsize be also set to enable atomic
writes, and extsize is used for atomic write unit max
I'm trying to think of ways to avoid requiring a value, but I don't see
good options, like:
- make atomic write unit max some compile-time option
- require mkfs stripe alignment/width be set and use that as basis for
atomic write unit max
We could just use the atomic write unit max which HW provides, but that
could be 1MB or more and that will hardly give efficient data usage for
small files. But maybe we don't care about that if we expect this
feature to only be used on DB files, which can be huge anyway. However I
still have concerns – we require that value to be fixed, but a disk
firmware update could increase that value and this could mean we have
what would be pre-existing mis-aligned extents.
As it is, I really don't see this as a better solution than the
original generic "force align" flag that simply makes the extent
size hint alignment a hard physical alignment requirement rather
than just a hint. This has multiple uses (DAX PMD alignment is
another), so I just don't see why something that has a single,
application specific API that implements a hard physical alignment
is desirable.
I would still hope that we will support forcealign separately for those
purposes.
Indeed, the whole reason that extent size hints are so versatile is
that they implement a generic allocation alignment/size function
that can be used for anything your imagination extends to. If they
were implemented as a "only allow RAID stripe aligned/sized
allocation" for the original use case then that functionality would
have been far less useful than it has proven to be over the past
couple of decades.
Hence history teaches us that we should be designing the API around
the generic filesystem function required (hard alignment of physical
extent allocation), not the specific use case that requires that
functionality.
I understand your concern. However I am not even sure that forcealign
even gives us everything we want to enable atomic writes. There is an
issue where we were required to pre-zero a file prior to issuing atomic
writes to ensure extents are suitably sized, so FS_XFLAG_ATOMICWRITES
would make the FS do what is required to avoid that pre-zeroing (but
that pre-zeroing requirement that does sound like a forcealign issue...)
Furthermore, there was some desire to support atomic writes on block
devices with no HW support by using a CoW-based solution, and forcealign
would not be relevant there.
Thanks,
John
