On 25/02/2025 17:37, Darrick J. Wong wrote:
On Tue, Feb 25, 2025 at 10:58:56AM +0000, John Garry wrote:
On 24/02/2025 20:32, Darrick J. Wong wrote:
On Thu, Feb 13, 2025 at 01:56:14PM +0000, John Garry wrote:
For CoW-based atomic write support, always allocate a cow hole in
xfs_reflink_allocate_cow() to write the new data.
The semantics is that if @atomic is set, we will be passed a CoW fork
extent mapping for no error returned.
Signed-off-by: John Garry <john.g.garry@xxxxxxxxxx>
---
fs/xfs/xfs_iomap.c | 2 +-
fs/xfs/xfs_reflink.c | 12 +++++++-----
fs/xfs/xfs_reflink.h | 2 +-
3 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index d61460309a78..ab79f0080288 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -865,7 +865,7 @@ xfs_direct_write_iomap_begin(
/* may drop and re-acquire the ilock */
error = xfs_reflink_allocate_cow(ip, &imap, &cmap, &shared,
&lockmode,
- (flags & IOMAP_DIRECT) || IS_DAX(inode));
+ (flags & IOMAP_DIRECT) || IS_DAX(inode), false);
Now I'm /really/ think it's time for some reflink allocation flags,
because the function signature now involves two booleans...
ok, but the @convert_now arg is passed to other functions from
xfs_reflink_allocate_cow() - so would you prefer to create a bool
@convert_now inside xfs_reflink_allocate_cow() and pass that bool as before?
Or pass the flags all the way down to end users of @convert_now?
if (error)
goto out_unlock;
if (shared)
diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index 8428f7b26ee6..3dab3ba900a3 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -435,7 +435,8 @@ xfs_reflink_fill_cow_hole(
struct xfs_bmbt_irec *cmap,
bool *shared,
uint *lockmode,
- bool convert_now)
+ bool convert_now,
+ bool atomic)
...but this can come later.
Do you mean that this would just be a new flag to set?
Sorry, I meant that the double booleans -> flags conversion could be a
cleanup patch at the end of the series. But first we'd have to figure
out where we want the flags boundaries to be -- do we just pass the
IOMAP_{DIRECT,DAX,ATOMIC_*} flags directly to the reflink code and let
it figure out what to do?
We have the odd case of @convert_now being set from IS_DAX(inode) in
xfs_direct_write_iomap_begin() -> xfs_reflink_allocate_cow(), so that
thwarts the idea of passing the IOMAP flags directly. BTW, it may be
possible to clear up that IS_DAX() usage - I'm not sure, so I'll check
again.
Or do we make the xfs_iomap.c code translate
that into XFS_REFLINK_ALLOC_* flags?
That is what I was thinking of doing. But, as mentioned, it needs to be
decided if we pass XFS_REFLINK_ALLOC_* to callees of
xfs_reflink_allocate_cow(). I'm thinking 'no', as it will only create churn.
Either way, that is not something that needs to be done in this patch.
Sure
Thanks,
John
