* Re: [PATCH v3 3/3] selftests: pidfd: add tests for PIDFD_SELF_*
From: Peter Zijlstra @ 2025-05-01 12:46 UTC (permalink / raw)
To: John Hubbard
Cc: Lorenzo Stoakes, Shuah Khan, Christian Brauner, Shuah Khan,
Liam R . Howlett, Suren Baghdasaryan, Vlastimil Babka,
pedro.falcato, linux-kselftest, linux-mm, linux-fsdevel,
linux-api, linux-kernel, Oliver Sang, seanjc
In-Reply-To: <20250501114235.GP4198@noisy.programming.kicks-ass.net>
On Thu, May 01, 2025 at 01:42:35PM +0200, Peter Zijlstra wrote:
> On Wed, Oct 16, 2024 at 07:14:34PM -0700, John Hubbard wrote:
> > On 10/16/24 3:06 PM, Lorenzo Stoakes wrote:
> > > On Wed, Oct 16, 2024 at 02:00:27PM -0600, Shuah Khan wrote:
> > > > On 10/16/24 04:20, Lorenzo Stoakes wrote:
> > ...
> > > > > diff --git a/tools/testing/selftests/pidfd/pidfd.h b/tools/testing/selftests/pidfd/pidfd.h
> > > > > index 88d6830ee004..1640b711889b 100644
> > > > > --- a/tools/testing/selftests/pidfd/pidfd.h
> > > > > +++ b/tools/testing/selftests/pidfd/pidfd.h
> > > > > @@ -50,6 +50,14 @@
> > > > > #define PIDFD_NONBLOCK O_NONBLOCK
> > > > > #endif
> > > > > +/* System header file may not have this available. */
> > > > > +#ifndef PIDFD_SELF_THREAD
> > > > > +#define PIDFD_SELF_THREAD -100
> > > > > +#endif
> > > > > +#ifndef PIDFD_SELF_THREAD_GROUP
> > > > > +#define PIDFD_SELF_THREAD_GROUP -200
> > > > > +#endif
> > > > > +
> > > >
> > > > As mentioned in my response to v1 patch:
> > > >
> > > > kselftest has dependency on "make headers" and tests include
> > > > headers from linux/ directory
> > >
> > > Right but that assumes you install the kernel headers on the build system,
> > > which is quite a painful thing to have to do when you are quickly iterating
> > > on a qemu setup.
> > >
> > > This is a use case I use all the time so not at all theoretical.
> > >
> >
> > This is turning out to be a fairly typical reaction from kernel
> > developers, when presented with the "you must first run make headers"
> > requirement for kselftests.
> >
> > Peter Zijlstra's "NAK NAK NAK" response [1] last year was the most
> > colorful, so I'll helpfully cite it here. :)
>
> Let me re-try this.
>
> This is driving me insane. I've spend the past _TWO_ days trying to
> build KVM selftests and I'm still failing.
>
> This is absolute atrocious crap and is costing me valuable time.
>
> Please fix this fucking selftests shit to just build. This is unusable
> garbage.
So after spending more time trying to remember how to debug Makefiles (I
hate my life), I found that not only do I need this headers shit, the
kvm selftests Makefile is actively broken if you use: make O=foo
-INSTALL_HDR_PATH = $(top_srcdir)/usr
+INSTALL_HDR_PATH = $(top_srcdir)/$(O)/usr
And then finally, I can do:
make O=foo headers_install
make O=foo -C tools/testing/selftests/kvm/
So yeah, thank you very much for wasting my time *AGAIN*.
Seriously, I want to be able to do:
cd tools/testing/selftests/foo; make
and have it just work. I would strongly suggest every subsystem to
reclaim their selftests and make it so again.
And on that, let me go merge the fixes I need to have x86 and futex
build without this headers shit.
^ permalink raw reply
* Re: [PATCH v3 3/3] selftests: pidfd: add tests for PIDFD_SELF_*
From: Peter Zijlstra @ 2025-05-01 11:42 UTC (permalink / raw)
To: John Hubbard
Cc: Lorenzo Stoakes, Shuah Khan, Christian Brauner, Shuah Khan,
Liam R . Howlett, Suren Baghdasaryan, Vlastimil Babka,
pedro.falcato, linux-kselftest, linux-mm, linux-fsdevel,
linux-api, linux-kernel, Oliver Sang, seanjc
In-Reply-To: <5b0b8e1e-6f50-4e18-bf46-39b00376c26e@nvidia.com>
On Wed, Oct 16, 2024 at 07:14:34PM -0700, John Hubbard wrote:
> On 10/16/24 3:06 PM, Lorenzo Stoakes wrote:
> > On Wed, Oct 16, 2024 at 02:00:27PM -0600, Shuah Khan wrote:
> > > On 10/16/24 04:20, Lorenzo Stoakes wrote:
> ...
> > > > diff --git a/tools/testing/selftests/pidfd/pidfd.h b/tools/testing/selftests/pidfd/pidfd.h
> > > > index 88d6830ee004..1640b711889b 100644
> > > > --- a/tools/testing/selftests/pidfd/pidfd.h
> > > > +++ b/tools/testing/selftests/pidfd/pidfd.h
> > > > @@ -50,6 +50,14 @@
> > > > #define PIDFD_NONBLOCK O_NONBLOCK
> > > > #endif
> > > > +/* System header file may not have this available. */
> > > > +#ifndef PIDFD_SELF_THREAD
> > > > +#define PIDFD_SELF_THREAD -100
> > > > +#endif
> > > > +#ifndef PIDFD_SELF_THREAD_GROUP
> > > > +#define PIDFD_SELF_THREAD_GROUP -200
> > > > +#endif
> > > > +
> > >
> > > As mentioned in my response to v1 patch:
> > >
> > > kselftest has dependency on "make headers" and tests include
> > > headers from linux/ directory
> >
> > Right but that assumes you install the kernel headers on the build system,
> > which is quite a painful thing to have to do when you are quickly iterating
> > on a qemu setup.
> >
> > This is a use case I use all the time so not at all theoretical.
> >
>
> This is turning out to be a fairly typical reaction from kernel
> developers, when presented with the "you must first run make headers"
> requirement for kselftests.
>
> Peter Zijlstra's "NAK NAK NAK" response [1] last year was the most
> colorful, so I'll helpfully cite it here. :)
Let me re-try this.
This is driving me insane. I've spend the past _TWO_ days trying to
build KVM selftests and I'm still failing.
This is absolute atrocious crap and is costing me valuable time.
Please fix this fucking selftests shit to just build. This is unusable
garbage.
^ permalink raw reply
* Re: [PATCH v9 00/15] large atomic writes for xfs
From: John Garry @ 2025-05-01 5:04 UTC (permalink / raw)
To: Darrick J. Wong
Cc: hch, linux-fsdevel, dchinner, linux-xfs, linux-kernel, ojaswin,
ritesh.list, martin.petersen, linux-ext4, linux-block,
catherine.hoang, linux-api, brauner, viro, jack, cem
In-Reply-To: <20250501043151.GE1035866@frogsfrogsfrogs>
On 01/05/2025 05:31, Darrick J. Wong wrote:
> On Wed, Apr 30, 2025 at 03:14:04PM +0100, John Garry wrote:
>> Hi Christoph,
>>
>> At this point, is your only issue now with
>> 05/15 xfs: ignore HW which cannot atomic write a single block
>> ?
>>
>> I am not sure on 15/15...
>
> This series still needs reviews of patches 2, 3, 5, and 15, if I'm not
> mistaken.
I can review 2 and 3, but I am not familiar with the code so I don't
think that there is much value in that. I feel that very few are. So who
else will?
^ permalink raw reply
* Re: [PATCH v9 13/15] xfs: add xfs_compute_atomic_write_unit_max()
From: John Garry @ 2025-05-01 5:00 UTC (permalink / raw)
To: Darrick J. Wong
Cc: brauner, hch, viro, jack, cem, linux-fsdevel, dchinner, linux-xfs,
linux-kernel, ojaswin, ritesh.list, martin.petersen, linux-ext4,
linux-block, catherine.hoang, linux-api
In-Reply-To: <20250501043053.GD1035866@frogsfrogsfrogs>
On 01/05/2025 05:30, Darrick J. Wong wrote:
> On Wed, Apr 30, 2025 at 08:52:00AM +0100, John Garry wrote:
>> On 25/04/2025 17:45, John Garry wrote:
>>> +static inline xfs_extlen_t xfs_calc_perag_awu_max(struct xfs_mount *mp)
>>> +{
>>> + if (mp->m_ddev_targp->bt_bdev_awu_min > 0)
>>> + return max_pow_of_two_factor(mp->m_sb.sb_agblocks);
>>> + return mp->m_ag_max_usable;
>> I think that this should be rounddown_pow_of_two(mp->m_ag_max_usable)
>>
>> ditto for rt
>>
>> I will fix (unless disagree).
> I don't think this needs fixing. If there's no hardware support on the
> device, then we can do any size of atomic write that we want.
Check man pages for statx:
stx_atomic_write_unit_min
stx_atomic_write_unit_max
... These values are each guaranteed to be
a power-of-2.
Same is enforced for size for RWF_ATOMIC.
^ permalink raw reply
* Re: [PATCH v9 00/15] large atomic writes for xfs
From: Darrick J. Wong @ 2025-05-01 4:31 UTC (permalink / raw)
To: John Garry
Cc: hch, linux-fsdevel, dchinner, linux-xfs, linux-kernel, ojaswin,
ritesh.list, martin.petersen, linux-ext4, linux-block,
catherine.hoang, linux-api, brauner, viro, jack, cem
In-Reply-To: <972bd2fc-4dc9-42d5-ab05-dab29fd0e444@oracle.com>
On Wed, Apr 30, 2025 at 03:14:04PM +0100, John Garry wrote:
> Hi Christoph,
>
> At this point, is your only issue now with
> 05/15 xfs: ignore HW which cannot atomic write a single block
> ?
>
> I am not sure on 15/15...
This series still needs reviews of patches 2, 3, 5, and 15, if I'm not
mistaken.
--D
> Thanks,
> John
>
^ permalink raw reply
* Re: [PATCH v9 13/15] xfs: add xfs_compute_atomic_write_unit_max()
From: Darrick J. Wong @ 2025-05-01 4:30 UTC (permalink / raw)
To: John Garry
Cc: brauner, hch, viro, jack, cem, linux-fsdevel, dchinner, linux-xfs,
linux-kernel, ojaswin, ritesh.list, martin.petersen, linux-ext4,
linux-block, catherine.hoang, linux-api
In-Reply-To: <a8ef548a-b83e-4910-9178-7b3fd35bca14@oracle.com>
On Wed, Apr 30, 2025 at 08:52:00AM +0100, John Garry wrote:
> On 25/04/2025 17:45, John Garry wrote:
> > +static inline xfs_extlen_t xfs_calc_perag_awu_max(struct xfs_mount *mp)
> > +{
> > + if (mp->m_ddev_targp->bt_bdev_awu_min > 0)
> > + return max_pow_of_two_factor(mp->m_sb.sb_agblocks);
> > + return mp->m_ag_max_usable;
>
> I think that this should be rounddown_pow_of_two(mp->m_ag_max_usable)
>
> ditto for rt
>
> I will fix (unless disagree).
I don't think this needs fixing. If there's no hardware support on the
device, then we can do any size of atomic write that we want.
--D
^ permalink raw reply
* Re: [PATCH v9 00/15] large atomic writes for xfs
From: John Garry @ 2025-04-30 14:14 UTC (permalink / raw)
To: djwong, hch
Cc: linux-fsdevel, dchinner, linux-xfs, linux-kernel, ojaswin,
ritesh.list, martin.petersen, linux-ext4, linux-block,
catherine.hoang, linux-api, brauner, viro, jack, cem
In-Reply-To: <20250425164504.3263637-1-john.g.garry@oracle.com>
Hi Christoph,
At this point, is your only issue now with
05/15 xfs: ignore HW which cannot atomic write a single block
?
I am not sure on 15/15...
Thanks,
John
^ permalink raw reply
* Re: [PATCH v9 05/15] xfs: ignore HW which cannot atomic write a single block
From: Christoph Hellwig @ 2025-04-30 12:59 UTC (permalink / raw)
To: Darrick J. Wong
Cc: Christoph Hellwig, John Garry, brauner, viro, jack, cem,
linux-fsdevel, dchinner, linux-xfs, linux-kernel, ojaswin,
ritesh.list, martin.petersen, linux-ext4, linux-block,
catherine.hoang, linux-api
In-Reply-To: <20250429144446.GD25655@frogsfrogsfrogs>
On Tue, Apr 29, 2025 at 07:44:46AM -0700, Darrick J. Wong wrote:
> > So this can't be merged into xfs_setsize_buftarg as suggeted last round
> > instead of needing yet another per-device call into the buftarg code?
>
> Oh, heh, I forgot that xfs_setsize_buftarg is called a second time by
> xfs_setup_devices at the end of fill_super.
That's actually the real call. The first is just a dummy to have
bt_meta_sectorsize/bt_meta_sectormask initialized because if we didn't
do that some assert in the block layer triggered. We should probably
remove that call and open code the two assignments..
> I don't like the idea of merging the hw atomic write detection into
> xfs_setsize_buftarg itself because (a) it gets called for the data
> device before we've read the fs blocksize so the validation is
> meaningless and (b) that makes xfs_setsize_buftarg's purpose less
> cohesive.
As explained last round this came up I'd of course rename it if
we did that. But I can do that later.
^ permalink raw reply
* Re: [PATCH v9 13/15] xfs: add xfs_compute_atomic_write_unit_max()
From: John Garry @ 2025-04-30 7:52 UTC (permalink / raw)
To: brauner, djwong, hch, viro, jack, cem
Cc: linux-fsdevel, dchinner, linux-xfs, linux-kernel, ojaswin,
ritesh.list, martin.petersen, linux-ext4, linux-block,
catherine.hoang, linux-api
In-Reply-To: <20250425164504.3263637-14-john.g.garry@oracle.com>
On 25/04/2025 17:45, John Garry wrote:
> +static inline xfs_extlen_t xfs_calc_perag_awu_max(struct xfs_mount *mp)
> +{
> + if (mp->m_ddev_targp->bt_bdev_awu_min > 0)
> + return max_pow_of_two_factor(mp->m_sb.sb_agblocks);
> + return mp->m_ag_max_usable;
I think that this should be rounddown_pow_of_two(mp->m_ag_max_usable)
ditto for rt
I will fix (unless disagree).
^ permalink raw reply
* [PATCH v9.1 05/15] xfs: ignore HW which cannot atomic write a single block
From: Darrick J. Wong @ 2025-04-30 5:18 UTC (permalink / raw)
To: John Garry
Cc: brauner, hch, viro, jack, cem, linux-fsdevel, dchinner, linux-xfs,
linux-kernel, ojaswin, ritesh.list, martin.petersen, linux-ext4,
linux-block, catherine.hoang, linux-api
In-Reply-To: <20250425164504.3263637-6-john.g.garry@oracle.com>
From: Darrick J. Wong <djwong@kernel.org>
Currently only HW which can write at least 1x block is supported.
For supporting atomic writes > 1x block, a CoW-based method will also be
used and this will not be resticted to using HW which can write >= 1x
block.
However for deciding if HW-based atomic writes can be used, we need to
start adding checks for write length < HW min, which complicates the
code. Indeed, a statx field similar to unit_max_opt should also be
added for this minimum, which is undesirable.
HW which can only write > 1x blocks would be uncommon and quite weird,
so let's just not support it.
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
v9.1: move the atomic hw geometry calls to xfs_setup_devices
---
fs/xfs/xfs_buf.h | 3 ++-
fs/xfs/xfs_inode.h | 14 ++------------
fs/xfs/xfs_buf.c | 41 ++++++++++++++++++++++++++++++++++-------
fs/xfs/xfs_super.c | 6 +++++-
4 files changed, 43 insertions(+), 21 deletions(-)
diff --git a/fs/xfs/xfs_buf.h b/fs/xfs/xfs_buf.h
index d0b065a9a9f0d2..6f691779887f77 100644
--- a/fs/xfs/xfs_buf.h
+++ b/fs/xfs/xfs_buf.h
@@ -112,7 +112,7 @@ struct xfs_buftarg {
struct percpu_counter bt_readahead_count;
struct ratelimit_state bt_ioerror_rl;
- /* Atomic write unit values */
+ /* Atomic write unit values, bytes */
unsigned int bt_bdev_awu_min;
unsigned int bt_bdev_awu_max;
@@ -375,6 +375,7 @@ extern void xfs_free_buftarg(struct xfs_buftarg *);
extern void xfs_buftarg_wait(struct xfs_buftarg *);
extern void xfs_buftarg_drain(struct xfs_buftarg *);
extern int xfs_setsize_buftarg(struct xfs_buftarg *, unsigned int);
+void xfs_buftarg_config_atomic_writes(struct xfs_buftarg *btp);
#define xfs_getsize_buftarg(buftarg) block_size((buftarg)->bt_bdev)
#define xfs_readonly_buftarg(buftarg) bdev_read_only((buftarg)->bt_bdev)
diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
index bdbbff0d8d9920..d7e2b902ef5c97 100644
--- a/fs/xfs/xfs_inode.h
+++ b/fs/xfs/xfs_inode.h
@@ -356,19 +356,9 @@ static inline bool xfs_inode_has_bigrtalloc(const struct xfs_inode *ip)
(XFS_IS_REALTIME_INODE(ip) ? \
(ip)->i_mount->m_rtdev_targp : (ip)->i_mount->m_ddev_targp)
-static inline bool
-xfs_inode_can_hw_atomic_write(
- struct xfs_inode *ip)
+static inline bool xfs_inode_can_hw_atomic_write(const struct xfs_inode *ip)
{
- struct xfs_mount *mp = ip->i_mount;
- struct xfs_buftarg *target = xfs_inode_buftarg(ip);
-
- if (mp->m_sb.sb_blocksize < target->bt_bdev_awu_min)
- return false;
- if (mp->m_sb.sb_blocksize > target->bt_bdev_awu_max)
- return false;
-
- return true;
+ return xfs_inode_buftarg(ip)->bt_bdev_awu_max > 0;
}
/*
diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index 5ae77ffdc947b1..c1bd5654c3afa8 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -1779,6 +1779,40 @@ xfs_init_buftarg(
return -ENOMEM;
}
+/*
+ * Configure this buffer target for hardware-assisted atomic writes if the
+ * underlying block device supports is congruent with the filesystem geometry.
+ */
+void
+xfs_buftarg_config_atomic_writes(
+ struct xfs_buftarg *btp)
+{
+ struct xfs_mount *mp = btp->bt_mount;
+ unsigned int min_bytes, max_bytes;
+
+ ASSERT(btp->bt_bdev != NULL);
+
+ if (!bdev_can_atomic_write(btp->bt_bdev))
+ return;
+
+ min_bytes = bdev_atomic_write_unit_min_bytes(btp->bt_bdev);
+ max_bytes = bdev_atomic_write_unit_max_bytes(btp->bt_bdev);
+
+ /*
+ * Ignore atomic write geometry that is nonsense or doesn't even cover
+ * a single fsblock.
+ */
+ if (min_bytes > max_bytes ||
+ min_bytes > mp->m_sb.sb_blocksize ||
+ max_bytes < mp->m_sb.sb_blocksize) {
+ min_bytes = 0;
+ max_bytes = 0;
+ }
+
+ btp->bt_bdev_awu_min = min_bytes;
+ btp->bt_bdev_awu_max = max_bytes;
+}
+
struct xfs_buftarg *
xfs_alloc_buftarg(
struct xfs_mount *mp,
@@ -1799,13 +1833,6 @@ xfs_alloc_buftarg(
btp->bt_daxdev = fs_dax_get_by_bdev(btp->bt_bdev, &btp->bt_dax_part_off,
mp, ops);
- if (bdev_can_atomic_write(btp->bt_bdev)) {
- btp->bt_bdev_awu_min = bdev_atomic_write_unit_min_bytes(
- btp->bt_bdev);
- btp->bt_bdev_awu_max = bdev_atomic_write_unit_max_bytes(
- btp->bt_bdev);
- }
-
/*
* When allocating the buftargs we have not yet read the super block and
* thus don't know the file system sector size yet.
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index b2dd0c0bf50979..af4c541251d859 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -520,7 +520,8 @@ xfs_open_devices(
}
/*
- * Setup xfs_mount buffer target pointers based on superblock
+ * Setup xfs_mount buffer target pointers based on superblock, and configure
+ * the atomic write capabilities now that we've validated the blocksize.
*/
STATIC int
xfs_setup_devices(
@@ -531,6 +532,7 @@ xfs_setup_devices(
error = xfs_setsize_buftarg(mp->m_ddev_targp, mp->m_sb.sb_sectsize);
if (error)
return error;
+ xfs_buftarg_config_atomic_writes(mp->m_ddev_targp);
if (mp->m_logdev_targp && mp->m_logdev_targp != mp->m_ddev_targp) {
unsigned int log_sector_size = BBSIZE;
@@ -541,6 +543,7 @@ xfs_setup_devices(
log_sector_size);
if (error)
return error;
+ xfs_buftarg_config_atomic_writes(mp->m_logdev_targp);
}
if (mp->m_sb.sb_rtstart) {
@@ -555,6 +558,7 @@ xfs_setup_devices(
mp->m_sb.sb_sectsize);
if (error)
return error;
+ xfs_buftarg_config_atomic_writes(mp->m_rtdev_targp);
}
return 0;
^ permalink raw reply related
* Re: [PATCH v9 05/15] xfs: ignore HW which cannot atomic write a single block
From: Darrick J. Wong @ 2025-04-29 14:44 UTC (permalink / raw)
To: Christoph Hellwig
Cc: John Garry, brauner, viro, jack, cem, linux-fsdevel, dchinner,
linux-xfs, linux-kernel, ojaswin, ritesh.list, martin.petersen,
linux-ext4, linux-block, catherine.hoang, linux-api
In-Reply-To: <20250429122105.GA12603@lst.de>
On Tue, Apr 29, 2025 at 02:21:05PM +0200, Christoph Hellwig wrote:
> On Fri, Apr 25, 2025 at 04:44:54PM +0000, John Garry wrote:
> > + /* Configure hardware atomic write geometry */
> > + xfs_buftarg_config_atomic_writes(mp->m_ddev_targp);
> > + if (mp->m_logdev_targp && mp->m_logdev_targp != mp->m_ddev_targp)
> > + xfs_buftarg_config_atomic_writes(mp->m_logdev_targp);
> > + if (mp->m_rtdev_targp && mp->m_rtdev_targp != mp->m_ddev_targp)
> > + xfs_buftarg_config_atomic_writes(mp->m_rtdev_targp);
>
> So this can't be merged into xfs_setsize_buftarg as suggeted last round
> instead of needing yet another per-device call into the buftarg code?
Oh, heh, I forgot that xfs_setsize_buftarg is called a second time by
xfs_setup_devices at the end of fill_super. xfs_setup_devices is a
better place to call xfs_buftarg_config_atomic_writes.
I don't like the idea of merging the hw atomic write detection into
xfs_setsize_buftarg itself because (a) it gets called for the data
device before we've read the fs blocksize so the validation is
meaningless and (b) that makes xfs_setsize_buftarg's purpose less
cohesive.
--D
^ permalink raw reply
* Re: [PATCH v9 15/15] xfs: allow sysadmins to specify a maximum atomic write limit at mount time
From: Darrick J. Wong @ 2025-04-29 14:38 UTC (permalink / raw)
To: Christoph Hellwig
Cc: John Garry, brauner, viro, jack, cem, linux-fsdevel, dchinner,
linux-xfs, linux-kernel, ojaswin, ritesh.list, martin.petersen,
linux-ext4, linux-block, catherine.hoang, linux-api
In-Reply-To: <20250429122211.GB12603@lst.de>
On Tue, Apr 29, 2025 at 02:22:11PM +0200, Christoph Hellwig wrote:
> So I guess the property variant didn't work out based on the replies
> from Darrick last round?
Correct. Calling setxattr on the property wouldn't call
xfs_set_max_atomic_write_opt to change the operational limit, and I
don't want to start up the whole magic xattrs discussion again by making
xfs_xattr_set do extra things if name=="trusted.xfs:max_atomic_write".
(Though iirc you were the most opposed to magic xattrs, and my
opposition is 45% yuck factor and 55% "hch will nak it anyway" ;) )
--D
^ permalink raw reply
* Re: [PATCH v9 15/15] xfs: allow sysadmins to specify a maximum atomic write limit at mount time
From: Christoph Hellwig @ 2025-04-29 12:22 UTC (permalink / raw)
To: John Garry
Cc: brauner, djwong, hch, viro, jack, cem, linux-fsdevel, dchinner,
linux-xfs, linux-kernel, ojaswin, ritesh.list, martin.petersen,
linux-ext4, linux-block, catherine.hoang, linux-api
In-Reply-To: <20250425164504.3263637-16-john.g.garry@oracle.com>
So I guess the property variant didn't work out based on the replies
from Darrick last round?
^ permalink raw reply
* Re: [PATCH v9 05/15] xfs: ignore HW which cannot atomic write a single block
From: Christoph Hellwig @ 2025-04-29 12:21 UTC (permalink / raw)
To: John Garry
Cc: brauner, djwong, hch, viro, jack, cem, linux-fsdevel, dchinner,
linux-xfs, linux-kernel, ojaswin, ritesh.list, martin.petersen,
linux-ext4, linux-block, catherine.hoang, linux-api
In-Reply-To: <20250425164504.3263637-6-john.g.garry@oracle.com>
On Fri, Apr 25, 2025 at 04:44:54PM +0000, John Garry wrote:
> + /* Configure hardware atomic write geometry */
> + xfs_buftarg_config_atomic_writes(mp->m_ddev_targp);
> + if (mp->m_logdev_targp && mp->m_logdev_targp != mp->m_ddev_targp)
> + xfs_buftarg_config_atomic_writes(mp->m_logdev_targp);
> + if (mp->m_rtdev_targp && mp->m_rtdev_targp != mp->m_ddev_targp)
> + xfs_buftarg_config_atomic_writes(mp->m_rtdev_targp);
So this can't be merged into xfs_setsize_buftarg as suggeted last round
instead of needing yet another per-device call into the buftarg code?
^ permalink raw reply
* Re: [PATCH v4 3/3] fs: introduce getfsxattrat and setfsxattrat syscalls
From: Christian Brauner @ 2025-04-28 9:17 UTC (permalink / raw)
To: Andrey Albershteyn
Cc: Amir Goldstein, Richard Henderson, Matt Turner, Russell King,
Catalin Marinas, Will Deacon, Geert Uytterhoeven, Michal Simek,
Thomas Bogendoerfer, James E.J. Bottomley, Helge Deller,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
Yoshinori Sato, Rich Felker, John Paul Adrian Glaubitz,
David S. Miller, Andreas Larsson, Andy Lutomirski,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Chris Zankel, Max Filippov, Alexander Viro,
Jan Kara, Mickaël Salaün, Günther Noack,
Arnd Bergmann, Pali Rohár, Paul Moore, James Morris,
Serge E. Hallyn, linux-alpha, linux-kernel, linux-arm-kernel,
linux-m68k, linux-mips, linux-parisc, linuxppc-dev, linux-s390,
linux-sh, sparclinux, linux-fsdevel, linux-security-module,
linux-api, linux-arch, linux-xfs
In-Reply-To: <l33napyvz5fwbcdju4otllbu4zr6faaz6mufz652alpxnjjfvl@h7j4hu4uwqwv>
On Fri, Apr 25, 2025 at 08:16:48PM +0200, Andrey Albershteyn wrote:
> On 2025-04-22 17:14:10, Christian Brauner wrote:
> > On Tue, Apr 22, 2025 at 04:31:29PM +0200, Christian Brauner wrote:
> > > On Thu, Mar 27, 2025 at 12:39:28PM +0100, Amir Goldstein wrote:
> > > > On Thu, Mar 27, 2025 at 10:33 AM Andrey Albershteyn <aalbersh@redhat.com> wrote:
> > > > >
> > > > > On 2025-03-23 09:56:25, Amir Goldstein wrote:
> > > > > > On Fri, Mar 21, 2025 at 8:49 PM Andrey Albershteyn <aalbersh@redhat.com> wrote:
> > > > > > >
> > > > > > > From: Andrey Albershteyn <aalbersh@redhat.com>
> > > > > > >
> > > > > > > Introduce getfsxattrat and setfsxattrat syscalls to manipulate inode
> > > > > > > extended attributes/flags. The syscalls take parent directory fd and
> > > > > > > path to the child together with struct fsxattr.
> > > > > > >
> > > > > > > This is an alternative to FS_IOC_FSSETXATTR ioctl with a difference
> > > > > > > that file don't need to be open as we can reference it with a path
> > > > > > > instead of fd. By having this we can manipulated inode extended
> > > > > > > attributes not only on regular files but also on special ones. This
> > > > > > > is not possible with FS_IOC_FSSETXATTR ioctl as with special files
> > > > > > > we can not call ioctl() directly on the filesystem inode using fd.
> > > > > > >
> > > > > > > This patch adds two new syscalls which allows userspace to get/set
> > > > > > > extended inode attributes on special files by using parent directory
> > > > > > > and a path - *at() like syscall.
> > > > > > >
> > > > > > > CC: linux-api@vger.kernel.org
> > > > > > > CC: linux-fsdevel@vger.kernel.org
> > > > > > > CC: linux-xfs@vger.kernel.org
> > > > > > > Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com>
> > > > > > > Acked-by: Arnd Bergmann <arnd@arndb.de>
> > > > > > > ---
> > > > > > ...
> > > > > > > +SYSCALL_DEFINE5(setfsxattrat, int, dfd, const char __user *, filename,
> > > > > > > + struct fsxattr __user *, ufsx, size_t, usize,
> > > > > > > + unsigned int, at_flags)
> > > > > > > +{
> > > > > > > + struct fileattr fa;
> > > > > > > + struct path filepath;
> > > > > > > + int error;
> > > > > > > + unsigned int lookup_flags = 0;
> > > > > > > + struct filename *name;
> > > > > > > + struct mnt_idmap *idmap;.
> > > > > >
> > > > > > > + struct dentry *dentry;
> > > > > > > + struct vfsmount *mnt;
> > > > > > > + struct fsxattr fsx = {};
> > > > > > > +
> > > > > > > + BUILD_BUG_ON(sizeof(struct fsxattr) < FSXATTR_SIZE_VER0);
> > > > > > > + BUILD_BUG_ON(sizeof(struct fsxattr) != FSXATTR_SIZE_LATEST);
> > > > > > > +
> > > > > > > + if ((at_flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) != 0)
> > > > > > > + return -EINVAL;
> > > > > > > +
> > > > > > > + if (!(at_flags & AT_SYMLINK_NOFOLLOW))
> > > > > > > + lookup_flags |= LOOKUP_FOLLOW;
> > > > > > > +
> > > > > > > + if (at_flags & AT_EMPTY_PATH)
> > > > > > > + lookup_flags |= LOOKUP_EMPTY;
> > > > > > > +
> > > > > > > + if (usize > PAGE_SIZE)
> > > > > > > + return -E2BIG;
> > > > > > > +
> > > > > > > + if (usize < FSXATTR_SIZE_VER0)
> > > > > > > + return -EINVAL;
> > > > > > > +
> > > > > > > + error = copy_struct_from_user(&fsx, sizeof(struct fsxattr), ufsx, usize);
> > > > > > > + if (error)
> > > > > > > + return error;
> > > > > > > +
> > > > > > > + fsxattr_to_fileattr(&fsx, &fa);
> > > > > > > +
> > > > > > > + name = getname_maybe_null(filename, at_flags);
> > > > > > > + if (!name) {
> > > > > > > + CLASS(fd, f)(dfd);
> > > > > > > +
> > > > > > > + if (fd_empty(f))
> > > > > > > + return -EBADF;
> > > > > > > +
> > > > > > > + idmap = file_mnt_idmap(fd_file(f));
> > > > > > > + dentry = file_dentry(fd_file(f));
> > > > > > > + mnt = fd_file(f)->f_path.mnt;
> > > > > > > + } else {
> > > > > > > + error = filename_lookup(dfd, name, lookup_flags, &filepath,
> > > > > > > + NULL);
> > > > > > > + if (error)
> > > > > > > + return error;
> > > > > > > +
> > > > > > > + idmap = mnt_idmap(filepath.mnt);
> > > > > > > + dentry = filepath.dentry;
> > > > > > > + mnt = filepath.mnt;
> > > > > > > + }
> > > > > > > +
> > > > > > > + error = mnt_want_write(mnt);
> > > > > > > + if (!error) {
> > > > > > > + error = vfs_fileattr_set(idmap, dentry, &fa);
> > > > > > > + if (error == -ENOIOCTLCMD)
> > > > > > > + error = -EOPNOTSUPP;
> > > > > >
> > > > > > This is awkward.
> > > > > > vfs_fileattr_set() should return -EOPNOTSUPP.
> > > > > > ioctl_setflags() could maybe convert it to -ENOIOCTLCMD,
> > > > > > but looking at similar cases ioctl_fiemap(), ioctl_fsfreeze() the
> > > > > > ioctl returns -EOPNOTSUPP.
> > > > > >
> > > > > > I don't think it is necessarily a bad idea to start returning
> > > > > > -EOPNOTSUPP instead of -ENOIOCTLCMD for the ioctl
> > > > > > because that really reflects the fact that the ioctl is now implemented
> > > > > > in vfs and not in the specific fs.
> > > > > >
> > > > > > and I think it would not be a bad idea at all to make that change
> > > > > > together with the merge of the syscalls as a sort of hint to userspace
> > > > > > that uses the ioctl, that the sycalls API exists.
> > > > > >
> > > > > > Thanks,
> > > > > > Amir.
> > > > > >
> > > > >
> > > > > Hmm, not sure what you're suggesting here. I see it as:
> > > > > - get/setfsxattrat should return EOPNOTSUPP as it make more sense
> > > > > than ENOIOCTLCMD
> > > > > - ioctl_setflags returns ENOIOCTLCMD which also expected
> > > > >
> > > > > Don't really see a reason to change what vfs_fileattr_set() returns
> > > > > and then copying this if() to other places or start returning
> > > > > EOPNOTSUPP.
> > > >
> > > > ENOIOCTLCMD conceptually means that the ioctl command is unknown
> > > > This is not the case since ->fileattr_[gs]et() became a vfs API
> > >
> > > vfs_fileattr_{g,s}et() should not return ENOIOCTLCMD. Change the return
> > > code to EOPNOTSUPP and then make EOPNOTSUPP be translated to ENOTTY on
> > > on overlayfs and to ENOIOCTLCMD in ecryptfs and in fs/ioctl.c. This way
> > > we get a clean VFS api while retaining current behavior. Amir can do his
> > > cleanup based on that.
> >
> > Also this get/set dance is not something new apis should do. It should
> > be handled like setattr_prepare() or generic_fillattr() where the
> > filesystem calls a VFS helper and that does all of this based on the
> > current state of the inode instead of calling into the filesystem twice:
> >
> > int vfs_fileattr_set(struct mnt_idmap *idmap, struct dentry *dentry,
> > struct fileattr *fa)
> > {
> > <snip>
> > inode_lock(inode);
> > err = vfs_fileattr_get(dentry, &old_ma);
> > if (!err) {
> > /* initialize missing bits from old_ma */
> > if (fa->flags_valid) {
> > <snip>
> > err = fileattr_set_prepare(inode, &old_ma, fa);
> > if (!err && !security_inode_setfsxattr(inode, fa))
> > err = inode->i_op->fileattr_set(idmap, dentry, fa);
> >
>
> You mean something like this? (not all fs are done)
Yes, possibly. But don't bother with this now as that'll need some more
thinking and it'll just stall your work for no good reason. Let's just
get the syscalls in mergable shape now.
^ permalink raw reply
* Re: [PATCH v4 3/3] fs: introduce getfsxattrat and setfsxattrat syscalls
From: Andrey Albershteyn @ 2025-04-25 18:16 UTC (permalink / raw)
To: Christian Brauner
Cc: Amir Goldstein, Richard Henderson, Matt Turner, Russell King,
Catalin Marinas, Will Deacon, Geert Uytterhoeven, Michal Simek,
Thomas Bogendoerfer, James E.J. Bottomley, Helge Deller,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
Yoshinori Sato, Rich Felker, John Paul Adrian Glaubitz,
David S. Miller, Andreas Larsson, Andy Lutomirski,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Chris Zankel, Max Filippov, Alexander Viro,
Jan Kara, Mickaël Salaün, Günther Noack,
Arnd Bergmann, Pali Rohár, Paul Moore, James Morris,
Serge E. Hallyn, linux-alpha, linux-kernel, linux-arm-kernel,
linux-m68k, linux-mips, linux-parisc, linuxppc-dev, linux-s390,
linux-sh, sparclinux, linux-fsdevel, linux-security-module,
linux-api, linux-arch, linux-xfs
In-Reply-To: <20250422-gefressen-faucht-8ded2c9a5375@brauner>
On 2025-04-22 17:14:10, Christian Brauner wrote:
> On Tue, Apr 22, 2025 at 04:31:29PM +0200, Christian Brauner wrote:
> > On Thu, Mar 27, 2025 at 12:39:28PM +0100, Amir Goldstein wrote:
> > > On Thu, Mar 27, 2025 at 10:33 AM Andrey Albershteyn <aalbersh@redhat.com> wrote:
> > > >
> > > > On 2025-03-23 09:56:25, Amir Goldstein wrote:
> > > > > On Fri, Mar 21, 2025 at 8:49 PM Andrey Albershteyn <aalbersh@redhat.com> wrote:
> > > > > >
> > > > > > From: Andrey Albershteyn <aalbersh@redhat.com>
> > > > > >
> > > > > > Introduce getfsxattrat and setfsxattrat syscalls to manipulate inode
> > > > > > extended attributes/flags. The syscalls take parent directory fd and
> > > > > > path to the child together with struct fsxattr.
> > > > > >
> > > > > > This is an alternative to FS_IOC_FSSETXATTR ioctl with a difference
> > > > > > that file don't need to be open as we can reference it with a path
> > > > > > instead of fd. By having this we can manipulated inode extended
> > > > > > attributes not only on regular files but also on special ones. This
> > > > > > is not possible with FS_IOC_FSSETXATTR ioctl as with special files
> > > > > > we can not call ioctl() directly on the filesystem inode using fd.
> > > > > >
> > > > > > This patch adds two new syscalls which allows userspace to get/set
> > > > > > extended inode attributes on special files by using parent directory
> > > > > > and a path - *at() like syscall.
> > > > > >
> > > > > > CC: linux-api@vger.kernel.org
> > > > > > CC: linux-fsdevel@vger.kernel.org
> > > > > > CC: linux-xfs@vger.kernel.org
> > > > > > Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com>
> > > > > > Acked-by: Arnd Bergmann <arnd@arndb.de>
> > > > > > ---
> > > > > ...
> > > > > > +SYSCALL_DEFINE5(setfsxattrat, int, dfd, const char __user *, filename,
> > > > > > + struct fsxattr __user *, ufsx, size_t, usize,
> > > > > > + unsigned int, at_flags)
> > > > > > +{
> > > > > > + struct fileattr fa;
> > > > > > + struct path filepath;
> > > > > > + int error;
> > > > > > + unsigned int lookup_flags = 0;
> > > > > > + struct filename *name;
> > > > > > + struct mnt_idmap *idmap;.
> > > > >
> > > > > > + struct dentry *dentry;
> > > > > > + struct vfsmount *mnt;
> > > > > > + struct fsxattr fsx = {};
> > > > > > +
> > > > > > + BUILD_BUG_ON(sizeof(struct fsxattr) < FSXATTR_SIZE_VER0);
> > > > > > + BUILD_BUG_ON(sizeof(struct fsxattr) != FSXATTR_SIZE_LATEST);
> > > > > > +
> > > > > > + if ((at_flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) != 0)
> > > > > > + return -EINVAL;
> > > > > > +
> > > > > > + if (!(at_flags & AT_SYMLINK_NOFOLLOW))
> > > > > > + lookup_flags |= LOOKUP_FOLLOW;
> > > > > > +
> > > > > > + if (at_flags & AT_EMPTY_PATH)
> > > > > > + lookup_flags |= LOOKUP_EMPTY;
> > > > > > +
> > > > > > + if (usize > PAGE_SIZE)
> > > > > > + return -E2BIG;
> > > > > > +
> > > > > > + if (usize < FSXATTR_SIZE_VER0)
> > > > > > + return -EINVAL;
> > > > > > +
> > > > > > + error = copy_struct_from_user(&fsx, sizeof(struct fsxattr), ufsx, usize);
> > > > > > + if (error)
> > > > > > + return error;
> > > > > > +
> > > > > > + fsxattr_to_fileattr(&fsx, &fa);
> > > > > > +
> > > > > > + name = getname_maybe_null(filename, at_flags);
> > > > > > + if (!name) {
> > > > > > + CLASS(fd, f)(dfd);
> > > > > > +
> > > > > > + if (fd_empty(f))
> > > > > > + return -EBADF;
> > > > > > +
> > > > > > + idmap = file_mnt_idmap(fd_file(f));
> > > > > > + dentry = file_dentry(fd_file(f));
> > > > > > + mnt = fd_file(f)->f_path.mnt;
> > > > > > + } else {
> > > > > > + error = filename_lookup(dfd, name, lookup_flags, &filepath,
> > > > > > + NULL);
> > > > > > + if (error)
> > > > > > + return error;
> > > > > > +
> > > > > > + idmap = mnt_idmap(filepath.mnt);
> > > > > > + dentry = filepath.dentry;
> > > > > > + mnt = filepath.mnt;
> > > > > > + }
> > > > > > +
> > > > > > + error = mnt_want_write(mnt);
> > > > > > + if (!error) {
> > > > > > + error = vfs_fileattr_set(idmap, dentry, &fa);
> > > > > > + if (error == -ENOIOCTLCMD)
> > > > > > + error = -EOPNOTSUPP;
> > > > >
> > > > > This is awkward.
> > > > > vfs_fileattr_set() should return -EOPNOTSUPP.
> > > > > ioctl_setflags() could maybe convert it to -ENOIOCTLCMD,
> > > > > but looking at similar cases ioctl_fiemap(), ioctl_fsfreeze() the
> > > > > ioctl returns -EOPNOTSUPP.
> > > > >
> > > > > I don't think it is necessarily a bad idea to start returning
> > > > > -EOPNOTSUPP instead of -ENOIOCTLCMD for the ioctl
> > > > > because that really reflects the fact that the ioctl is now implemented
> > > > > in vfs and not in the specific fs.
> > > > >
> > > > > and I think it would not be a bad idea at all to make that change
> > > > > together with the merge of the syscalls as a sort of hint to userspace
> > > > > that uses the ioctl, that the sycalls API exists.
> > > > >
> > > > > Thanks,
> > > > > Amir.
> > > > >
> > > >
> > > > Hmm, not sure what you're suggesting here. I see it as:
> > > > - get/setfsxattrat should return EOPNOTSUPP as it make more sense
> > > > than ENOIOCTLCMD
> > > > - ioctl_setflags returns ENOIOCTLCMD which also expected
> > > >
> > > > Don't really see a reason to change what vfs_fileattr_set() returns
> > > > and then copying this if() to other places or start returning
> > > > EOPNOTSUPP.
> > >
> > > ENOIOCTLCMD conceptually means that the ioctl command is unknown
> > > This is not the case since ->fileattr_[gs]et() became a vfs API
> >
> > vfs_fileattr_{g,s}et() should not return ENOIOCTLCMD. Change the return
> > code to EOPNOTSUPP and then make EOPNOTSUPP be translated to ENOTTY on
> > on overlayfs and to ENOIOCTLCMD in ecryptfs and in fs/ioctl.c. This way
> > we get a clean VFS api while retaining current behavior. Amir can do his
> > cleanup based on that.
>
> Also this get/set dance is not something new apis should do. It should
> be handled like setattr_prepare() or generic_fillattr() where the
> filesystem calls a VFS helper and that does all of this based on the
> current state of the inode instead of calling into the filesystem twice:
>
> int vfs_fileattr_set(struct mnt_idmap *idmap, struct dentry *dentry,
> struct fileattr *fa)
> {
> <snip>
> inode_lock(inode);
> err = vfs_fileattr_get(dentry, &old_ma);
> if (!err) {
> /* initialize missing bits from old_ma */
> if (fa->flags_valid) {
> <snip>
> err = fileattr_set_prepare(inode, &old_ma, fa);
> if (!err && !security_inode_setfsxattr(inode, fa))
> err = inode->i_op->fileattr_set(idmap, dentry, fa);
>
You mean something like this? (not all fs are done)
--
From 421445f054ccad3116d55ae22c8995a48bb753fd Mon Sep 17 00:00:00 2001
From: Andrey Albershteyn <aalbersh@kernel.org>
Date: Fri, 25 Apr 2025 17:20:42 +0200
Subject: [PATCH] fs: push retrieval of fileattr down to filesystems
Currently, vfs_fileattr_set() calls twice to the file system. Firstly,
to retrieve current state of the inode extended attributes and secondly
to set the new ones.
This patch refactors this in a way that filesystem firstly gets current
inode attribute state and then calls VFS helper to verify them. This way
vfs_fileattr_set() will call filesystem just once.
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
---
fs/ext2/ioctl.c | 9 ++++++
fs/ext4/ioctl.c | 9 ++++++
fs/f2fs/file.c | 12 +++++++-
fs/file_attr.c | 62 ++++++++++++++++++++++++----------------
fs/gfs2/file.c | 9 ++++++
fs/hfsplus/inode.c | 9 ++++++
fs/jfs/ioctl.c | 9 +++++-
fs/ntfs3/file.c | 12 +++++++-
fs/orangefs/inode.c | 9 ++++++
fs/ubifs/ioctl.c | 12 +++++++-
fs/xfs/xfs_ioctl.c | 6 ++++
include/linux/fileattr.h | 2 ++
mm/shmem.c | 8 ++++++
13 files changed, 140 insertions(+), 28 deletions(-)
diff --git a/fs/ext2/ioctl.c b/fs/ext2/ioctl.c
index 44e04484e570..3a45ed9c12b7 100644
--- a/fs/ext2/ioctl.c
+++ b/fs/ext2/ioctl.c
@@ -32,6 +32,15 @@ int ext2_fileattr_set(struct mnt_idmap *idmap,
{
struct inode *inode = d_inode(dentry);
struct ext2_inode_info *ei = EXT2_I(inode);
+ struct fileattr cfa;
+ int err;
+
+ err = ext2_fileattr_get(dentry, &cfa);
+ if (err)
+ return err;
+ err = vfs_fileattr_set_prepare(idmap, dentry, &cfa, fa);
+ if (err)
+ return err;
if (fileattr_has_fsx(fa))
return -EOPNOTSUPP;
diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index d17207386ead..f988ff4d7256 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -1002,6 +1002,15 @@ int ext4_fileattr_set(struct mnt_idmap *idmap,
struct inode *inode = d_inode(dentry);
u32 flags = fa->flags;
int err = -EOPNOTSUPP;
+ struct fileattr cfa;
+
+ err = ext4_fileattr_get(dentry, &cfa);
+ if (err)
+ return err;
+
+ err = vfs_fileattr_set_prepare(idmap, dentry, &cfa, fa);
+ if (err)
+ return err;
if (flags & ~EXT4_FL_USER_VISIBLE)
goto out;
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index abbcbb5865a3..f196a07f1f17 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -3371,14 +3371,24 @@ int f2fs_fileattr_set(struct mnt_idmap *idmap,
struct dentry *dentry, struct fileattr *fa)
{
struct inode *inode = d_inode(dentry);
- u32 fsflags = fa->flags, mask = F2FS_SETTABLE_FS_FL;
+ u32 fsflags, mask = F2FS_SETTABLE_FS_FL;
u32 iflags;
+ struct fileattr cfa;
int err;
if (unlikely(f2fs_cp_error(F2FS_I_SB(inode))))
return -EIO;
if (!f2fs_is_checkpoint_ready(F2FS_I_SB(inode)))
return -ENOSPC;
+
+ err = f2fs_fileattr_get(dentry, &cfa);
+ if (err)
+ return err;
+ err = vfs_fileattr_set_prepare(idmap, dentry, &cfa, fa);
+ if (err)
+ return err;
+ fsflags = fa->flags;
+
if (fsflags & ~F2FS_GETTABLE_FS_FL)
return -EOPNOTSUPP;
fsflags &= F2FS_SETTABLE_FS_FL;
diff --git a/fs/file_attr.c b/fs/file_attr.c
index 5e51c5b851ef..d0a01377bca8 100644
--- a/fs/file_attr.c
+++ b/fs/file_attr.c
@@ -7,6 +7,8 @@
#include <linux/fileattr.h>
#include <linux/namei.h>
+#include "internal.h"
+
/**
* fileattr_fill_xflags - initialize fileattr with xflags
* @fa: fileattr pointer
@@ -225,6 +227,36 @@ static int fileattr_set_prepare(struct inode *inode,
return 0;
}
+/**
+ * vfs_fileattr_set_prepare - merge new filettr state and check for validity
+ * @idmap: idmap of the mount
+ * @dentry: the object to change
+ * @cfa: current fileattr state
+ * @fa: fileattr pointer with new values
+ *
+ * Return: 0 on success, or a negative error on failure.
+ */
+int vfs_fileattr_set_prepare(struct mnt_idmap *idmap, struct dentry *dentry,
+ struct fileattr *cfa, struct fileattr *fa)
+{
+ int err;
+
+ /* initialize missing bits from cfa */
+ if (fa->flags_valid) {
+ fa->fsx_xflags |= cfa->fsx_xflags & ~FS_XFLAG_COMMON;
+ fa->fsx_extsize = cfa->fsx_extsize;
+ fa->fsx_nextents = cfa->fsx_nextents;
+ fa->fsx_projid = cfa->fsx_projid;
+ fa->fsx_cowextsize = cfa->fsx_cowextsize;
+ } else {
+ fa->flags |= cfa->flags & ~FS_COMMON_FL;
+ }
+
+ err = fileattr_set_prepare(d_inode(dentry), cfa, fa);
+ return err;
+}
+EXPORT_SYMBOL(vfs_fileattr_set_prepare);
+
/**
* vfs_fileattr_set - change miscellaneous file attributes
* @idmap: idmap of the mount
@@ -245,7 +277,6 @@ int vfs_fileattr_set(struct mnt_idmap *idmap, struct dentry *dentry,
struct fileattr *fa)
{
struct inode *inode = d_inode(dentry);
- struct fileattr old_ma = {};
int err;
if (!inode->i_op->fileattr_set)
@@ -255,29 +286,12 @@ int vfs_fileattr_set(struct mnt_idmap *idmap, struct dentry *dentry,
return -EPERM;
inode_lock(inode);
- err = vfs_fileattr_get(dentry, &old_ma);
- if (!err) {
- /* initialize missing bits from old_ma */
- if (fa->flags_valid) {
- fa->fsx_xflags |= old_ma.fsx_xflags & ~FS_XFLAG_COMMON;
- fa->fsx_extsize = old_ma.fsx_extsize;
- fa->fsx_nextents = old_ma.fsx_nextents;
- fa->fsx_projid = old_ma.fsx_projid;
- fa->fsx_cowextsize = old_ma.fsx_cowextsize;
- } else {
- fa->flags |= old_ma.flags & ~FS_COMMON_FL;
- }
-
- err = fileattr_set_prepare(inode, &old_ma, fa);
- if (err)
- goto out;
- err = security_inode_file_setattr(dentry, fa);
- if (err)
- goto out;
- err = inode->i_op->fileattr_set(idmap, dentry, fa);
- if (err)
- goto out;
- }
+ err = security_inode_file_setattr(dentry, fa);
+ if (err)
+ goto out;
+ err = inode->i_op->fileattr_set(idmap, dentry, fa);
+ if (err)
+ goto out;
out:
inode_unlock(inode);
diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
index fd1147aa3891..cf796fa73af2 100644
--- a/fs/gfs2/file.c
+++ b/fs/gfs2/file.c
@@ -282,10 +282,19 @@ int gfs2_fileattr_set(struct mnt_idmap *idmap,
u32 fsflags = fa->flags, gfsflags = 0;
u32 mask;
int i;
+ struct fileattr cfa;
+ int error;
if (d_is_special(dentry))
return -ENOTTY;
+ error = gfs2_fileattr_get(dentry, &cfa);
+ if (error)
+ return error;
+ error = vfs_fileattr_set_prepare(idmap, dentry, &cfa, fa);
+ if (error)
+ return error;
+
if (fileattr_has_fsx(fa))
return -EOPNOTSUPP;
diff --git a/fs/hfsplus/inode.c b/fs/hfsplus/inode.c
index f331e9574217..cdb11d00faea 100644
--- a/fs/hfsplus/inode.c
+++ b/fs/hfsplus/inode.c
@@ -678,6 +678,15 @@ int hfsplus_fileattr_set(struct mnt_idmap *idmap,
struct inode *inode = d_inode(dentry);
struct hfsplus_inode_info *hip = HFSPLUS_I(inode);
unsigned int new_fl = 0;
+ struct fileattr cfa;
+ int err;
+
+ err = hfsplus_fileattr_get(dentry, &cfa);
+ if (err)
+ return err;
+ err = vfs_fileattr_set_prepare(idmap, dentry, &cfa, fa);
+ if (err)
+ return err;
if (fileattr_has_fsx(fa))
return -EOPNOTSUPP;
diff --git a/fs/jfs/ioctl.c b/fs/jfs/ioctl.c
index f7bd7e8f5be4..4c62c14d15b0 100644
--- a/fs/jfs/ioctl.c
+++ b/fs/jfs/ioctl.c
@@ -75,11 +75,18 @@ int jfs_fileattr_set(struct mnt_idmap *idmap,
{
struct inode *inode = d_inode(dentry);
struct jfs_inode_info *jfs_inode = JFS_IP(inode);
- unsigned int flags;
+ unsigned int flags = jfs_inode->mode2 & JFS_FL_USER_VISIBLE;
+ struct fileattr cfa;
+ int err;
if (d_is_special(dentry))
return -ENOTTY;
+ fileattr_fill_flags(&cfa, jfs_map_ext2(flags, 0));
+ err = vfs_fileattr_set_prepare(idmap, dentry, &cfa, fa);
+ if (err)
+ return err;
+
if (fileattr_has_fsx(fa))
return -EOPNOTSUPP;
diff --git a/fs/ntfs3/file.c b/fs/ntfs3/file.c
index 9b6a3f8d2e7c..bc7ee7595b70 100644
--- a/fs/ntfs3/file.c
+++ b/fs/ntfs3/file.c
@@ -83,12 +83,22 @@ int ntfs_fileattr_set(struct mnt_idmap *idmap, struct dentry *dentry,
{
struct inode *inode = d_inode(dentry);
struct ntfs_inode *ni = ntfs_i(inode);
- u32 flags = fa->flags;
+ u32 flags;
unsigned int new_fl = 0;
+ struct fileattr cfa;
+ int err;
+
+ err = ntfs_fileattr_get(dentry, &cfa);
+ if (err)
+ return err;
+ err = vfs_fileattr_set_prepare(idmap, dentry, &cfa, fa);
+ if (err)
+ return err;
if (fileattr_has_fsx(fa))
return -EOPNOTSUPP;
+ flags = fa->flags;
if (flags & ~(FS_IMMUTABLE_FL | FS_APPEND_FL | FS_COMPR_FL))
return -EOPNOTSUPP;
diff --git a/fs/orangefs/inode.c b/fs/orangefs/inode.c
index 5ac743c6bc2e..aecb61146443 100644
--- a/fs/orangefs/inode.c
+++ b/fs/orangefs/inode.c
@@ -910,6 +910,15 @@ static int orangefs_fileattr_set(struct mnt_idmap *idmap,
struct dentry *dentry, struct fileattr *fa)
{
u64 val = 0;
+ struct fileattr cfa;
+ int error = 0;
+
+ error = orangefs_fileattr_get(dentry, &cfa);
+ if (error)
+ return error;
+ error = vfs_fileattr_set_prepare(idmap, dentry, &cfa, fa);
+ if (error)
+ return error;
gossip_debug(GOSSIP_FILE_DEBUG, "%s: called on %pd\n", __func__,
dentry);
diff --git a/fs/ubifs/ioctl.c b/fs/ubifs/ioctl.c
index 2c99349cf537..e71e362c786b 100644
--- a/fs/ubifs/ioctl.c
+++ b/fs/ubifs/ioctl.c
@@ -148,14 +148,24 @@ int ubifs_fileattr_set(struct mnt_idmap *idmap,
struct dentry *dentry, struct fileattr *fa)
{
struct inode *inode = d_inode(dentry);
- int flags = fa->flags;
+ int flags;
+ struct fileattr cfa;
+ int err;
if (d_is_special(dentry))
return -ENOTTY;
+ err = ubifs_fileattr_get(dentry, &cfa);
+ if (err)
+ return err;
+ err = vfs_fileattr_set_prepare(idmap, dentry, &cfa, fa);
+ if (err)
+ return err;
+
if (fileattr_has_fsx(fa))
return -EOPNOTSUPP;
+ flags = fa->flags;
if (flags & ~UBIFS_GETTABLE_IOCTL_FLAGS)
return -EOPNOTSUPP;
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index d250f7f74e3b..c861dc1c3cf0 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -733,12 +733,18 @@ xfs_fileattr_set(
struct xfs_dquot *pdqp = NULL;
struct xfs_dquot *olddquot = NULL;
int error;
+ struct fileattr cfa;
trace_xfs_ioctl_setattr(ip);
if (d_is_special(dentry))
return -ENOTTY;
+ xfs_fill_fsxattr(ip, XFS_DATA_FORK, &cfa);
+ error = vfs_fileattr_set_prepare(idmap, dentry, &cfa, fa);
+ if (error)
+ return error;
+
if (!fa->fsx_valid) {
if (fa->flags & ~(FS_IMMUTABLE_FL | FS_APPEND_FL |
FS_NOATIME_FL | FS_NODUMP_FL |
diff --git a/include/linux/fileattr.h b/include/linux/fileattr.h
index f62a5143eb2d..aba76d897533 100644
--- a/include/linux/fileattr.h
+++ b/include/linux/fileattr.h
@@ -75,6 +75,8 @@ static inline bool fileattr_has_fsx(const struct fileattr *fa)
}
int vfs_fileattr_get(struct dentry *dentry, struct fileattr *fa);
+int vfs_fileattr_set_prepare(struct mnt_idmap *idmap, struct dentry *dentry,
+ struct fileattr *cfa, struct fileattr *fa);
int vfs_fileattr_set(struct mnt_idmap *idmap, struct dentry *dentry,
struct fileattr *fa);
int ioctl_getflags(struct file *file, unsigned int __user *argp);
diff --git a/mm/shmem.c b/mm/shmem.c
index 99327c30507c..c2a5991f944f 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -4199,6 +4199,14 @@ static int shmem_fileattr_set(struct mnt_idmap *idmap,
struct inode *inode = d_inode(dentry);
struct shmem_inode_info *info = SHMEM_I(inode);
int ret, flags;
+ struct fileattr cfa;
+
+ ret = shmem_fileattr_get(dentry, &cfa);
+ if (ret)
+ return ret;
+ ret = vfs_fileattr_set_prepare(idmap, dentry, &cfa, fa);
+ if (ret)
+ return ret;
if (fileattr_has_fsx(fa))
return -EOPNOTSUPP;
--
2.47.2
^ permalink raw reply related
* [PATCH v9 02/15] xfs: add helpers to compute log item overhead
From: John Garry @ 2025-04-25 16:44 UTC (permalink / raw)
To: brauner, djwong, hch, viro, jack, cem
Cc: linux-fsdevel, dchinner, linux-xfs, linux-kernel, ojaswin,
ritesh.list, martin.petersen, linux-ext4, linux-block,
catherine.hoang, linux-api, John Garry
In-Reply-To: <20250425164504.3263637-1-john.g.garry@oracle.com>
From: "Darrick J. Wong" <djwong@kernel.org>
Add selected helpers to estimate the transaction reservation required to
write various log intent and buffer items to the log. These helpers
will be used by the online repair code for more precise estimations of
how much work can be done in a single transaction.
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Signed-off-by: John Garry <john.g.garry@oracle.com>
---
fs/xfs/xfs_bmap_item.c | 10 ++++++++++
fs/xfs/xfs_bmap_item.h | 3 +++
fs/xfs/xfs_buf_item.c | 19 +++++++++++++++++++
fs/xfs/xfs_buf_item.h | 3 +++
fs/xfs/xfs_extfree_item.c | 10 ++++++++++
fs/xfs/xfs_extfree_item.h | 3 +++
fs/xfs/xfs_log_cil.c | 4 +---
fs/xfs/xfs_log_priv.h | 13 +++++++++++++
fs/xfs/xfs_refcount_item.c | 10 ++++++++++
fs/xfs/xfs_refcount_item.h | 3 +++
fs/xfs/xfs_rmap_item.c | 10 ++++++++++
fs/xfs/xfs_rmap_item.h | 3 +++
12 files changed, 88 insertions(+), 3 deletions(-)
diff --git a/fs/xfs/xfs_bmap_item.c b/fs/xfs/xfs_bmap_item.c
index 3d52e9d7ad57..646c515ee355 100644
--- a/fs/xfs/xfs_bmap_item.c
+++ b/fs/xfs/xfs_bmap_item.c
@@ -77,6 +77,11 @@ xfs_bui_item_size(
*nbytes += xfs_bui_log_format_sizeof(buip->bui_format.bui_nextents);
}
+unsigned int xfs_bui_log_space(unsigned int nr)
+{
+ return xlog_item_space(1, xfs_bui_log_format_sizeof(nr));
+}
+
/*
* This is called to fill in the vector of log iovecs for the
* given bui log item. We use only 1 iovec, and we point that
@@ -168,6 +173,11 @@ xfs_bud_item_size(
*nbytes += sizeof(struct xfs_bud_log_format);
}
+unsigned int xfs_bud_log_space(void)
+{
+ return xlog_item_space(1, sizeof(struct xfs_bud_log_format));
+}
+
/*
* This is called to fill in the vector of log iovecs for the
* given bud log item. We use only 1 iovec, and we point that
diff --git a/fs/xfs/xfs_bmap_item.h b/fs/xfs/xfs_bmap_item.h
index 6fee6a508343..b42fee06899d 100644
--- a/fs/xfs/xfs_bmap_item.h
+++ b/fs/xfs/xfs_bmap_item.h
@@ -72,4 +72,7 @@ struct xfs_bmap_intent;
void xfs_bmap_defer_add(struct xfs_trans *tp, struct xfs_bmap_intent *bi);
+unsigned int xfs_bui_log_space(unsigned int nr);
+unsigned int xfs_bud_log_space(void);
+
#endif /* __XFS_BMAP_ITEM_H__ */
diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
index 19eb0b7a3e58..90139e0f3271 100644
--- a/fs/xfs/xfs_buf_item.c
+++ b/fs/xfs/xfs_buf_item.c
@@ -103,6 +103,25 @@ xfs_buf_item_size_segment(
return;
}
+/*
+ * Compute the worst case log item overhead for an invalidated buffer with the
+ * given map count and block size.
+ */
+unsigned int
+xfs_buf_inval_log_space(
+ unsigned int map_count,
+ unsigned int blocksize)
+{
+ unsigned int chunks = DIV_ROUND_UP(blocksize, XFS_BLF_CHUNK);
+ unsigned int bitmap_size = DIV_ROUND_UP(chunks, NBWORD);
+ unsigned int ret =
+ offsetof(struct xfs_buf_log_format, blf_data_map) +
+ (bitmap_size * sizeof_field(struct xfs_buf_log_format,
+ blf_data_map[0]));
+
+ return ret * map_count;
+}
+
/*
* Return the number of log iovecs and space needed to log the given buf log
* item.
diff --git a/fs/xfs/xfs_buf_item.h b/fs/xfs/xfs_buf_item.h
index 8cde85259a58..e10e324cd245 100644
--- a/fs/xfs/xfs_buf_item.h
+++ b/fs/xfs/xfs_buf_item.h
@@ -64,6 +64,9 @@ static inline void xfs_buf_dquot_iodone(struct xfs_buf *bp)
void xfs_buf_iodone(struct xfs_buf *);
bool xfs_buf_log_check_iovec(struct xfs_log_iovec *iovec);
+unsigned int xfs_buf_inval_log_space(unsigned int map_count,
+ unsigned int blocksize);
+
extern struct kmem_cache *xfs_buf_item_cache;
#endif /* __XFS_BUF_ITEM_H__ */
diff --git a/fs/xfs/xfs_extfree_item.c b/fs/xfs/xfs_extfree_item.c
index 777438b853da..d574f5f639fa 100644
--- a/fs/xfs/xfs_extfree_item.c
+++ b/fs/xfs/xfs_extfree_item.c
@@ -83,6 +83,11 @@ xfs_efi_item_size(
*nbytes += xfs_efi_log_format_sizeof(efip->efi_format.efi_nextents);
}
+unsigned int xfs_efi_log_space(unsigned int nr)
+{
+ return xlog_item_space(1, xfs_efi_log_format_sizeof(nr));
+}
+
/*
* This is called to fill in the vector of log iovecs for the
* given efi log item. We use only 1 iovec, and we point that
@@ -254,6 +259,11 @@ xfs_efd_item_size(
*nbytes += xfs_efd_log_format_sizeof(efdp->efd_format.efd_nextents);
}
+unsigned int xfs_efd_log_space(unsigned int nr)
+{
+ return xlog_item_space(1, xfs_efd_log_format_sizeof(nr));
+}
+
/*
* This is called to fill in the vector of log iovecs for the
* given efd log item. We use only 1 iovec, and we point that
diff --git a/fs/xfs/xfs_extfree_item.h b/fs/xfs/xfs_extfree_item.h
index 41b7c4306079..c8402040410b 100644
--- a/fs/xfs/xfs_extfree_item.h
+++ b/fs/xfs/xfs_extfree_item.h
@@ -94,4 +94,7 @@ void xfs_extent_free_defer_add(struct xfs_trans *tp,
struct xfs_extent_free_item *xefi,
struct xfs_defer_pending **dfpp);
+unsigned int xfs_efi_log_space(unsigned int nr);
+unsigned int xfs_efd_log_space(unsigned int nr);
+
#endif /* __XFS_EXTFREE_ITEM_H__ */
diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c
index 1ca406ec1b40..f66d2d430e4f 100644
--- a/fs/xfs/xfs_log_cil.c
+++ b/fs/xfs/xfs_log_cil.c
@@ -309,9 +309,7 @@ xlog_cil_alloc_shadow_bufs(
* Then round nbytes up to 64-bit alignment so that the initial
* buffer alignment is easy to calculate and verify.
*/
- nbytes += niovecs *
- (sizeof(uint64_t) + sizeof(struct xlog_op_header));
- nbytes = round_up(nbytes, sizeof(uint64_t));
+ nbytes = xlog_item_space(niovecs, nbytes);
/*
* The data buffer needs to start 64-bit aligned, so round up
diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h
index f3d78869e5e5..39a102cc1b43 100644
--- a/fs/xfs/xfs_log_priv.h
+++ b/fs/xfs/xfs_log_priv.h
@@ -698,4 +698,17 @@ xlog_kvmalloc(
return p;
}
+/*
+ * Given a count of iovecs and space for a log item, compute the space we need
+ * in the log to store that data plus the log headers.
+ */
+static inline unsigned int
+xlog_item_space(
+ unsigned int niovecs,
+ unsigned int nbytes)
+{
+ nbytes += niovecs * (sizeof(uint64_t) + sizeof(struct xlog_op_header));
+ return round_up(nbytes, sizeof(uint64_t));
+}
+
#endif /* __XFS_LOG_PRIV_H__ */
diff --git a/fs/xfs/xfs_refcount_item.c b/fs/xfs/xfs_refcount_item.c
index fe2d7aab8554..076501123d89 100644
--- a/fs/xfs/xfs_refcount_item.c
+++ b/fs/xfs/xfs_refcount_item.c
@@ -78,6 +78,11 @@ xfs_cui_item_size(
*nbytes += xfs_cui_log_format_sizeof(cuip->cui_format.cui_nextents);
}
+unsigned int xfs_cui_log_space(unsigned int nr)
+{
+ return xlog_item_space(1, xfs_cui_log_format_sizeof(nr));
+}
+
/*
* This is called to fill in the vector of log iovecs for the
* given cui log item. We use only 1 iovec, and we point that
@@ -179,6 +184,11 @@ xfs_cud_item_size(
*nbytes += sizeof(struct xfs_cud_log_format);
}
+unsigned int xfs_cud_log_space(void)
+{
+ return xlog_item_space(1, sizeof(struct xfs_cud_log_format));
+}
+
/*
* This is called to fill in the vector of log iovecs for the
* given cud log item. We use only 1 iovec, and we point that
diff --git a/fs/xfs/xfs_refcount_item.h b/fs/xfs/xfs_refcount_item.h
index bfee8f30c63c..0fc3f493342b 100644
--- a/fs/xfs/xfs_refcount_item.h
+++ b/fs/xfs/xfs_refcount_item.h
@@ -76,4 +76,7 @@ struct xfs_refcount_intent;
void xfs_refcount_defer_add(struct xfs_trans *tp,
struct xfs_refcount_intent *ri);
+unsigned int xfs_cui_log_space(unsigned int nr);
+unsigned int xfs_cud_log_space(void);
+
#endif /* __XFS_REFCOUNT_ITEM_H__ */
diff --git a/fs/xfs/xfs_rmap_item.c b/fs/xfs/xfs_rmap_item.c
index 89decffe76c8..c99700318ec2 100644
--- a/fs/xfs/xfs_rmap_item.c
+++ b/fs/xfs/xfs_rmap_item.c
@@ -77,6 +77,11 @@ xfs_rui_item_size(
*nbytes += xfs_rui_log_format_sizeof(ruip->rui_format.rui_nextents);
}
+unsigned int xfs_rui_log_space(unsigned int nr)
+{
+ return xlog_item_space(1, xfs_rui_log_format_sizeof(nr));
+}
+
/*
* This is called to fill in the vector of log iovecs for the
* given rui log item. We use only 1 iovec, and we point that
@@ -180,6 +185,11 @@ xfs_rud_item_size(
*nbytes += sizeof(struct xfs_rud_log_format);
}
+unsigned int xfs_rud_log_space(void)
+{
+ return xlog_item_space(1, sizeof(struct xfs_rud_log_format));
+}
+
/*
* This is called to fill in the vector of log iovecs for the
* given rud log item. We use only 1 iovec, and we point that
diff --git a/fs/xfs/xfs_rmap_item.h b/fs/xfs/xfs_rmap_item.h
index 40d331555675..3a99f0117f2d 100644
--- a/fs/xfs/xfs_rmap_item.h
+++ b/fs/xfs/xfs_rmap_item.h
@@ -75,4 +75,7 @@ struct xfs_rmap_intent;
void xfs_rmap_defer_add(struct xfs_trans *tp, struct xfs_rmap_intent *ri);
+unsigned int xfs_rui_log_space(unsigned int nr);
+unsigned int xfs_rud_log_space(void);
+
#endif /* __XFS_RMAP_ITEM_H__ */
--
2.31.1
^ permalink raw reply related
* [PATCH v9 00/15] large atomic writes for xfs
From: John Garry @ 2025-04-25 16:44 UTC (permalink / raw)
To: brauner, djwong, hch, viro, jack, cem
Cc: linux-fsdevel, dchinner, linux-xfs, linux-kernel, ojaswin,
ritesh.list, martin.petersen, linux-ext4, linux-block,
catherine.hoang, linux-api, John Garry
Currently atomic write support for xfs is limited to writing a single
block as we have no way to guarantee alignment and that the write covers
a single extent.
This series introduces a method to issue atomic writes via a
software-based method.
The software-based method is used as a fallback for when attempting to
issue an atomic write over misaligned or multiple extents.
For xfs, this support is based on reflink CoW support.
The basic idea of this CoW method is to alloc a range in the CoW fork,
write the data, and atomically update the mapping.
Initial mysql performance testing has shown this method to perform ok.
However, there we are only using 16K atomic writes (and 4K block size),
so typically - and thankfully - this software fallback method won't be
used often.
For other FSes which want large atomics writes and don't support CoW, I
think that they can follow the example in [0].
Catherine is currently working on further xfstests for this feature,
which we hope to share soon.
Based on f0447f80aec8 (xfs/next-rc, xfs/for-next) xfs: remove
duplicate Zoned Filesystems sections in admin-guide
[0] https://lore.kernel.org/linux-xfs/20250310183946.932054-1-john.g.garry@oracle.com/
Differences to v8:
- Darrick reworked patch for mount option
- Darrick reworked patch to ignore HW
- Minor changes and cleanups from Darrick
- Rework some commit messages (Christoph)
- Pick up RB tags from Christoph (thanks!)
Differences to v7:
- Add patch for mp hw awu max and min
- Fixed for awu max mount option (Darrick)
Differences to v6:
- log item sizes updates (Darrick)
- rtvol support (Darrick)
- mount option for atomic writes (Darrick)
- Add RB tags from Darrick and Christoph (Thanks!)
Darrick J. Wong (4):
xfs: add helpers to compute log item overhead
xfs: add helpers to compute transaction reservation for finishing
intent items
xfs: ignore HW which cannot atomic write a single block
xfs: allow sysadmins to specify a maximum atomic write limit at mount
time
John Garry (11):
fs: add atomic write unit max opt to statx
xfs: rename xfs_inode_can_atomicwrite() ->
xfs_inode_can_hw_atomic_write()
xfs: allow block allocator to take an alignment hint
xfs: refactor xfs_reflink_end_cow_extent()
xfs: refine atomic write size check in xfs_file_write_iter()
xfs: add xfs_atomic_write_cow_iomap_begin()
xfs: add large atomic writes checks in xfs_direct_write_iomap_begin()
xfs: commit CoW-based atomic writes atomically
xfs: add xfs_file_dio_write_atomic()
xfs: add xfs_compute_atomic_write_unit_max()
xfs: update atomic write limits
Documentation/admin-guide/xfs.rst | 11 +
block/bdev.c | 3 +-
fs/ext4/inode.c | 2 +-
fs/stat.c | 6 +-
fs/xfs/libxfs/xfs_bmap.c | 5 +
fs/xfs/libxfs/xfs_bmap.h | 6 +-
fs/xfs/libxfs/xfs_log_rlimit.c | 4 +
fs/xfs/libxfs/xfs_trans_resv.c | 343 +++++++++++++++++++++++++++---
fs/xfs/libxfs/xfs_trans_resv.h | 25 +++
fs/xfs/xfs_bmap_item.c | 10 +
fs/xfs/xfs_bmap_item.h | 3 +
fs/xfs/xfs_buf.c | 41 +++-
fs/xfs/xfs_buf.h | 3 +-
fs/xfs/xfs_buf_item.c | 19 ++
fs/xfs/xfs_buf_item.h | 3 +
fs/xfs/xfs_extfree_item.c | 10 +
fs/xfs/xfs_extfree_item.h | 3 +
fs/xfs/xfs_file.c | 87 +++++++-
fs/xfs/xfs_inode.h | 14 +-
fs/xfs/xfs_iomap.c | 190 ++++++++++++++++-
fs/xfs/xfs_iomap.h | 1 +
fs/xfs/xfs_iops.c | 76 ++++++-
fs/xfs/xfs_iops.h | 3 +
fs/xfs/xfs_log_cil.c | 4 +-
fs/xfs/xfs_log_priv.h | 13 ++
fs/xfs/xfs_mount.c | 166 +++++++++++++++
fs/xfs/xfs_mount.h | 17 ++
fs/xfs/xfs_refcount_item.c | 10 +
fs/xfs/xfs_refcount_item.h | 3 +
fs/xfs/xfs_reflink.c | 146 ++++++++++---
fs/xfs/xfs_reflink.h | 6 +
fs/xfs/xfs_rmap_item.c | 10 +
fs/xfs/xfs_rmap_item.h | 3 +
fs/xfs/xfs_super.c | 58 ++++-
fs/xfs/xfs_trace.h | 115 ++++++++++
include/linux/fs.h | 3 +-
include/linux/stat.h | 1 +
include/uapi/linux/stat.h | 8 +-
38 files changed, 1322 insertions(+), 109 deletions(-)
--
2.31.1
^ permalink raw reply
* [PATCH v9 07/15] xfs: refactor xfs_reflink_end_cow_extent()
From: John Garry @ 2025-04-25 16:44 UTC (permalink / raw)
To: brauner, djwong, hch, viro, jack, cem
Cc: linux-fsdevel, dchinner, linux-xfs, linux-kernel, ojaswin,
ritesh.list, martin.petersen, linux-ext4, linux-block,
catherine.hoang, linux-api, John Garry
In-Reply-To: <20250425164504.3263637-1-john.g.garry@oracle.com>
Refactor xfs_reflink_end_cow_extent() into separate parts which process
the CoW range and commit the transaction.
This refactoring will be used in future for when it is required to commit
a range of extents as a single transaction, similar to how it was done
pre-commit d6f215f359637.
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: John Garry <john.g.garry@oracle.com>
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
fs/xfs/xfs_reflink.c | 72 ++++++++++++++++++++++++++------------------
1 file changed, 42 insertions(+), 30 deletions(-)
diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index cc3b4df88110..bd711c5bb6bb 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -786,35 +786,19 @@ xfs_reflink_update_quota(
* requirements as low as possible.
*/
STATIC int
-xfs_reflink_end_cow_extent(
+xfs_reflink_end_cow_extent_locked(
+ struct xfs_trans *tp,
struct xfs_inode *ip,
xfs_fileoff_t *offset_fsb,
xfs_fileoff_t end_fsb)
{
struct xfs_iext_cursor icur;
struct xfs_bmbt_irec got, del, data;
- struct xfs_mount *mp = ip->i_mount;
- struct xfs_trans *tp;
struct xfs_ifork *ifp = xfs_ifork_ptr(ip, XFS_COW_FORK);
- unsigned int resblks;
int nmaps;
bool isrt = XFS_IS_REALTIME_INODE(ip);
int error;
- resblks = XFS_EXTENTADD_SPACE_RES(mp, XFS_DATA_FORK);
- error = xfs_trans_alloc(mp, &M_RES(mp)->tr_write, resblks, 0,
- XFS_TRANS_RESERVE, &tp);
- if (error)
- return error;
-
- /*
- * Lock the inode. We have to ijoin without automatic unlock because
- * the lead transaction is the refcountbt record deletion; the data
- * fork update follows as a deferred log item.
- */
- xfs_ilock(ip, XFS_ILOCK_EXCL);
- xfs_trans_ijoin(tp, ip, 0);
-
/*
* In case of racing, overlapping AIO writes no COW extents might be
* left by the time I/O completes for the loser of the race. In that
@@ -823,7 +807,7 @@ xfs_reflink_end_cow_extent(
if (!xfs_iext_lookup_extent(ip, ifp, *offset_fsb, &icur, &got) ||
got.br_startoff >= end_fsb) {
*offset_fsb = end_fsb;
- goto out_cancel;
+ return 0;
}
/*
@@ -837,7 +821,7 @@ xfs_reflink_end_cow_extent(
if (!xfs_iext_next_extent(ifp, &icur, &got) ||
got.br_startoff >= end_fsb) {
*offset_fsb = end_fsb;
- goto out_cancel;
+ return 0;
}
}
del = got;
@@ -846,14 +830,14 @@ xfs_reflink_end_cow_extent(
error = xfs_iext_count_extend(tp, ip, XFS_DATA_FORK,
XFS_IEXT_REFLINK_END_COW_CNT);
if (error)
- goto out_cancel;
+ return error;
/* Grab the corresponding mapping in the data fork. */
nmaps = 1;
error = xfs_bmapi_read(ip, del.br_startoff, del.br_blockcount, &data,
&nmaps, 0);
if (error)
- goto out_cancel;
+ return error;
/* We can only remap the smaller of the two extent sizes. */
data.br_blockcount = min(data.br_blockcount, del.br_blockcount);
@@ -882,7 +866,7 @@ xfs_reflink_end_cow_extent(
error = xfs_bunmapi(NULL, ip, data.br_startoff,
data.br_blockcount, 0, 1, &done);
if (error)
- goto out_cancel;
+ return error;
ASSERT(done);
}
@@ -899,17 +883,45 @@ xfs_reflink_end_cow_extent(
/* Remove the mapping from the CoW fork. */
xfs_bmap_del_extent_cow(ip, &icur, &got, &del);
- error = xfs_trans_commit(tp);
- xfs_iunlock(ip, XFS_ILOCK_EXCL);
- if (error)
- return error;
-
/* Update the caller about how much progress we made. */
*offset_fsb = del.br_startoff + del.br_blockcount;
return 0;
+}
-out_cancel:
- xfs_trans_cancel(tp);
+/*
+ * Remap part of the CoW fork into the data fork.
+ *
+ * We aim to remap the range starting at @offset_fsb and ending at @end_fsb
+ * into the data fork; this function will remap what it can (at the end of the
+ * range) and update @end_fsb appropriately. Each remap gets its own
+ * transaction because we can end up merging and splitting bmbt blocks for
+ * every remap operation and we'd like to keep the block reservation
+ * requirements as low as possible.
+ */
+STATIC int
+xfs_reflink_end_cow_extent(
+ struct xfs_inode *ip,
+ xfs_fileoff_t *offset_fsb,
+ xfs_fileoff_t end_fsb)
+{
+ struct xfs_mount *mp = ip->i_mount;
+ struct xfs_trans *tp;
+ unsigned int resblks;
+ int error;
+
+ resblks = XFS_EXTENTADD_SPACE_RES(mp, XFS_DATA_FORK);
+ error = xfs_trans_alloc(mp, &M_RES(mp)->tr_write, resblks, 0,
+ XFS_TRANS_RESERVE, &tp);
+ if (error)
+ return error;
+ xfs_ilock(ip, XFS_ILOCK_EXCL);
+ xfs_trans_ijoin(tp, ip, 0);
+
+ error = xfs_reflink_end_cow_extent_locked(tp, ip, offset_fsb, end_fsb);
+ if (error)
+ xfs_trans_cancel(tp);
+ else
+ error = xfs_trans_commit(tp);
xfs_iunlock(ip, XFS_ILOCK_EXCL);
return error;
}
--
2.31.1
^ permalink raw reply related
* [PATCH v9 09/15] xfs: add xfs_atomic_write_cow_iomap_begin()
From: John Garry @ 2025-04-25 16:44 UTC (permalink / raw)
To: brauner, djwong, hch, viro, jack, cem
Cc: linux-fsdevel, dchinner, linux-xfs, linux-kernel, ojaswin,
ritesh.list, martin.petersen, linux-ext4, linux-block,
catherine.hoang, linux-api, John Garry
In-Reply-To: <20250425164504.3263637-1-john.g.garry@oracle.com>
For CoW-based atomic writes, reuse the infrastructure for reflink CoW fork
support.
Add ->iomap_begin() callback xfs_atomic_write_cow_iomap_begin() to create
staging mappings in the CoW fork for atomic write updates.
The general steps in the function are as follows:
- find extent mapping in the CoW fork for the FS block range being written
- if part or full extent is found, proceed to process found extent
- if no extent found, map in new blocks to the CoW fork
- convert unwritten blocks in extent if required
- update iomap extent mapping and return
The bulk of this function is quite similar to the processing in
xfs_reflink_allocate_cow(), where we try to find an extent mapping; if
none exists, then allocate a new extent in the CoW fork, convert unwritten
blocks, and return a mapping.
Performance testing has shown the XFS_ILOCK_EXCL locking to be quite
a bottleneck, so this is an area which could be optimised in future.
Christoph Hellwig contributed almost all of the code in
xfs_atomic_write_cow_iomap_begin().
Signed-off-by: John Garry <john.g.garry@oracle.com>
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
[djwong: add a new xfs_can_sw_atomic_write to convey intent better]
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/xfs_iomap.c | 128 +++++++++++++++++++++++++++++++++++++++++++
fs/xfs/xfs_iomap.h | 1 +
fs/xfs/xfs_mount.h | 5 ++
fs/xfs/xfs_reflink.c | 2 +-
fs/xfs/xfs_reflink.h | 2 +
fs/xfs/xfs_trace.h | 22 ++++++++
6 files changed, 159 insertions(+), 1 deletion(-)
diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index cb23c8871f81..166fba2ff1ef 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -1022,6 +1022,134 @@ const struct iomap_ops xfs_zoned_direct_write_iomap_ops = {
};
#endif /* CONFIG_XFS_RT */
+static int
+xfs_atomic_write_cow_iomap_begin(
+ struct inode *inode,
+ loff_t offset,
+ loff_t length,
+ unsigned flags,
+ struct iomap *iomap,
+ struct iomap *srcmap)
+{
+ struct xfs_inode *ip = XFS_I(inode);
+ struct xfs_mount *mp = ip->i_mount;
+ const xfs_fileoff_t offset_fsb = XFS_B_TO_FSBT(mp, offset);
+ xfs_fileoff_t end_fsb = xfs_iomap_end_fsb(mp, offset, length);
+ xfs_filblks_t count_fsb = end_fsb - offset_fsb;
+ int nmaps = 1;
+ xfs_filblks_t resaligned;
+ struct xfs_bmbt_irec cmap;
+ struct xfs_iext_cursor icur;
+ struct xfs_trans *tp;
+ unsigned int dblocks = 0, rblocks = 0;
+ int error;
+ u64 seq;
+
+ ASSERT(flags & IOMAP_WRITE);
+ ASSERT(flags & IOMAP_DIRECT);
+
+ if (xfs_is_shutdown(mp))
+ return -EIO;
+
+ if (!xfs_can_sw_atomic_write(mp)) {
+ ASSERT(xfs_can_sw_atomic_write(mp));
+ return -EINVAL;
+ }
+
+ /* blocks are always allocated in this path */
+ if (flags & IOMAP_NOWAIT)
+ return -EAGAIN;
+
+ trace_xfs_iomap_atomic_write_cow(ip, offset, length);
+
+ xfs_ilock(ip, XFS_ILOCK_EXCL);
+
+ if (!ip->i_cowfp) {
+ ASSERT(!xfs_is_reflink_inode(ip));
+ xfs_ifork_init_cow(ip);
+ }
+
+ if (!xfs_iext_lookup_extent(ip, ip->i_cowfp, offset_fsb, &icur, &cmap))
+ cmap.br_startoff = end_fsb;
+ if (cmap.br_startoff <= offset_fsb) {
+ xfs_trim_extent(&cmap, offset_fsb, count_fsb);
+ goto found;
+ }
+
+ end_fsb = cmap.br_startoff;
+ count_fsb = end_fsb - offset_fsb;
+
+ resaligned = xfs_aligned_fsb_count(offset_fsb, count_fsb,
+ xfs_get_cowextsz_hint(ip));
+ xfs_iunlock(ip, XFS_ILOCK_EXCL);
+
+ if (XFS_IS_REALTIME_INODE(ip)) {
+ dblocks = XFS_DIOSTRAT_SPACE_RES(mp, 0);
+ rblocks = resaligned;
+ } else {
+ dblocks = XFS_DIOSTRAT_SPACE_RES(mp, resaligned);
+ rblocks = 0;
+ }
+
+ error = xfs_trans_alloc_inode(ip, &M_RES(mp)->tr_write, dblocks,
+ rblocks, false, &tp);
+ if (error)
+ return error;
+
+ /* extent layout could have changed since the unlock, so check again */
+ if (!xfs_iext_lookup_extent(ip, ip->i_cowfp, offset_fsb, &icur, &cmap))
+ cmap.br_startoff = end_fsb;
+ if (cmap.br_startoff <= offset_fsb) {
+ xfs_trim_extent(&cmap, offset_fsb, count_fsb);
+ xfs_trans_cancel(tp);
+ goto found;
+ }
+
+ /*
+ * Allocate the entire reservation as unwritten blocks.
+ *
+ * Use XFS_BMAPI_EXTSZALIGN to hint at aligning new extents according to
+ * extszhint, such that there will be a greater chance that future
+ * atomic writes to that same range will be aligned (and don't require
+ * this COW-based method).
+ */
+ error = xfs_bmapi_write(tp, ip, offset_fsb, count_fsb,
+ XFS_BMAPI_COWFORK | XFS_BMAPI_PREALLOC |
+ XFS_BMAPI_EXTSZALIGN, 0, &cmap, &nmaps);
+ if (error) {
+ xfs_trans_cancel(tp);
+ goto out_unlock;
+ }
+
+ xfs_inode_set_cowblocks_tag(ip);
+ error = xfs_trans_commit(tp);
+ if (error)
+ goto out_unlock;
+
+found:
+ if (cmap.br_state != XFS_EXT_NORM) {
+ error = xfs_reflink_convert_cow_locked(ip, offset_fsb,
+ count_fsb);
+ if (error)
+ goto out_unlock;
+ cmap.br_state = XFS_EXT_NORM;
+ }
+
+ length = XFS_FSB_TO_B(mp, cmap.br_startoff + cmap.br_blockcount);
+ trace_xfs_iomap_found(ip, offset, length - offset, XFS_COW_FORK, &cmap);
+ seq = xfs_iomap_inode_sequence(ip, IOMAP_F_SHARED);
+ xfs_iunlock(ip, XFS_ILOCK_EXCL);
+ return xfs_bmbt_to_iomap(ip, iomap, &cmap, flags, IOMAP_F_SHARED, seq);
+
+out_unlock:
+ xfs_iunlock(ip, XFS_ILOCK_EXCL);
+ return error;
+}
+
+const struct iomap_ops xfs_atomic_write_cow_iomap_ops = {
+ .iomap_begin = xfs_atomic_write_cow_iomap_begin,
+};
+
static int
xfs_dax_write_iomap_end(
struct inode *inode,
diff --git a/fs/xfs/xfs_iomap.h b/fs/xfs/xfs_iomap.h
index d330c4a581b1..674f8ac1b9bd 100644
--- a/fs/xfs/xfs_iomap.h
+++ b/fs/xfs/xfs_iomap.h
@@ -56,5 +56,6 @@ extern const struct iomap_ops xfs_read_iomap_ops;
extern const struct iomap_ops xfs_seek_iomap_ops;
extern const struct iomap_ops xfs_xattr_iomap_ops;
extern const struct iomap_ops xfs_dax_write_iomap_ops;
+extern const struct iomap_ops xfs_atomic_write_cow_iomap_ops;
#endif /* __XFS_IOMAP_H__*/
diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index e5192c12e7ac..e67bc3e91f98 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -464,6 +464,11 @@ static inline bool xfs_has_nonzoned(const struct xfs_mount *mp)
return !xfs_has_zoned(mp);
}
+static inline bool xfs_can_sw_atomic_write(struct xfs_mount *mp)
+{
+ return xfs_has_reflink(mp);
+}
+
/*
* Some features are always on for v5 file systems, allow the compiler to
* eliminiate dead code when building without v4 support.
diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index bd711c5bb6bb..f5d338916098 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -293,7 +293,7 @@ xfs_bmap_trim_cow(
return xfs_reflink_trim_around_shared(ip, imap, shared);
}
-static int
+int
xfs_reflink_convert_cow_locked(
struct xfs_inode *ip,
xfs_fileoff_t offset_fsb,
diff --git a/fs/xfs/xfs_reflink.h b/fs/xfs/xfs_reflink.h
index cc4e92278279..379619f24247 100644
--- a/fs/xfs/xfs_reflink.h
+++ b/fs/xfs/xfs_reflink.h
@@ -35,6 +35,8 @@ int xfs_reflink_allocate_cow(struct xfs_inode *ip, struct xfs_bmbt_irec *imap,
bool convert_now);
extern int xfs_reflink_convert_cow(struct xfs_inode *ip, xfs_off_t offset,
xfs_off_t count);
+int xfs_reflink_convert_cow_locked(struct xfs_inode *ip,
+ xfs_fileoff_t offset_fsb, xfs_filblks_t count_fsb);
extern int xfs_reflink_cancel_cow_blocks(struct xfs_inode *ip,
struct xfs_trans **tpp, xfs_fileoff_t offset_fsb,
diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index e56ba1963160..9554578c6da4 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -1657,6 +1657,28 @@ DEFINE_RW_EVENT(xfs_file_direct_write);
DEFINE_RW_EVENT(xfs_file_dax_write);
DEFINE_RW_EVENT(xfs_reflink_bounce_dio_write);
+TRACE_EVENT(xfs_iomap_atomic_write_cow,
+ TP_PROTO(struct xfs_inode *ip, xfs_off_t offset, ssize_t count),
+ TP_ARGS(ip, offset, count),
+ TP_STRUCT__entry(
+ __field(dev_t, dev)
+ __field(xfs_ino_t, ino)
+ __field(xfs_off_t, offset)
+ __field(ssize_t, count)
+ ),
+ TP_fast_assign(
+ __entry->dev = VFS_I(ip)->i_sb->s_dev;
+ __entry->ino = ip->i_ino;
+ __entry->offset = offset;
+ __entry->count = count;
+ ),
+ TP_printk("dev %d:%d ino 0x%llx pos 0x%llx bytecount 0x%zx",
+ MAJOR(__entry->dev), MINOR(__entry->dev),
+ __entry->ino,
+ __entry->offset,
+ __entry->count)
+)
+
DECLARE_EVENT_CLASS(xfs_imap_class,
TP_PROTO(struct xfs_inode *ip, xfs_off_t offset, ssize_t count,
int whichfork, struct xfs_bmbt_irec *irec),
--
2.31.1
^ permalink raw reply related
* [PATCH v9 13/15] xfs: add xfs_compute_atomic_write_unit_max()
From: John Garry @ 2025-04-25 16:45 UTC (permalink / raw)
To: brauner, djwong, hch, viro, jack, cem
Cc: linux-fsdevel, dchinner, linux-xfs, linux-kernel, ojaswin,
ritesh.list, martin.petersen, linux-ext4, linux-block,
catherine.hoang, linux-api, John Garry
In-Reply-To: <20250425164504.3263637-1-john.g.garry@oracle.com>
Now that CoW-based atomic writes are supported, update the max size of an
atomic write for the data device.
The limit of a CoW-based atomic write will be the limit of the number of
logitems which can fit into a single transaction.
In addition, the max atomic write size needs to be aligned to the agsize.
Limit the size of atomic writes to the greatest power-of-two factor of the
agsize so that allocations for an atomic write will always be aligned
compatibly with the alignment requirements of the storage.
Function xfs_atomic_write_logitems() is added to find the limit the number
of log items which can fit in a single transaction.
Amend the max atomic write computation to create a new transaction
reservation type, and compute the maximum size of an atomic write
completion (in fsblocks) based on this new transaction reservation.
Initially, tr_atomic_write is a clone of tr_itruncate, which provides a
reasonable level of parallelism. In the next patch, we'll add a mount
option so that sysadmins can configure their own limits.
Signed-off-by: John Garry <john.g.garry@oracle.com>
[djwong: use a new reservation type for atomic write ioends, refactor
group limit calculations]
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
[jpg: update xfs_calc_perag_awu_max() ddev check]
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/libxfs/xfs_trans_resv.c | 94 ++++++++++++++++++++++++++++++++++
fs/xfs/libxfs/xfs_trans_resv.h | 2 +
fs/xfs/xfs_mount.c | 81 +++++++++++++++++++++++++++++
fs/xfs/xfs_mount.h | 6 +++
fs/xfs/xfs_reflink.c | 16 ++++++
fs/xfs/xfs_reflink.h | 2 +
fs/xfs/xfs_trace.h | 60 ++++++++++++++++++++++
7 files changed, 261 insertions(+)
diff --git a/fs/xfs/libxfs/xfs_trans_resv.c b/fs/xfs/libxfs/xfs_trans_resv.c
index a841432abf83..e73c09fbd24c 100644
--- a/fs/xfs/libxfs/xfs_trans_resv.c
+++ b/fs/xfs/libxfs/xfs_trans_resv.c
@@ -22,6 +22,12 @@
#include "xfs_rtbitmap.h"
#include "xfs_attr_item.h"
#include "xfs_log.h"
+#include "xfs_defer.h"
+#include "xfs_bmap_item.h"
+#include "xfs_extfree_item.h"
+#include "xfs_rmap_item.h"
+#include "xfs_refcount_item.h"
+#include "xfs_trace.h"
#define _ALLOC true
#define _FREE false
@@ -1394,3 +1400,91 @@ xfs_trans_resv_calc(
*/
xfs_calc_default_atomic_ioend_reservation(mp, resp);
}
+
+/*
+ * Return the per-extent and fixed transaction reservation sizes needed to
+ * complete an atomic write.
+ */
+STATIC unsigned int
+xfs_calc_atomic_write_ioend_geometry(
+ struct xfs_mount *mp,
+ unsigned int *step_size)
+{
+ const unsigned int efi = xfs_efi_log_space(1);
+ const unsigned int efd = xfs_efd_log_space(1);
+ const unsigned int rui = xfs_rui_log_space(1);
+ const unsigned int rud = xfs_rud_log_space();
+ const unsigned int cui = xfs_cui_log_space(1);
+ const unsigned int cud = xfs_cud_log_space();
+ const unsigned int bui = xfs_bui_log_space(1);
+ const unsigned int bud = xfs_bud_log_space();
+
+ /*
+ * Maximum overhead to complete an atomic write ioend in software:
+ * remove data fork extent + remove cow fork extent + map extent into
+ * data fork.
+ *
+ * tx0: Creates a BUI and a CUI and that's all it needs.
+ *
+ * tx1: Roll to finish the BUI. Need space for the BUD, an RUI, and
+ * enough space to relog the CUI (== CUI + CUD).
+ *
+ * tx2: Roll again to finish the RUI. Need space for the RUD and space
+ * to relog the CUI.
+ *
+ * tx3: Roll again, need space for the CUD and possibly a new EFI.
+ *
+ * tx4: Roll again, need space for an EFD.
+ *
+ * If the extent referenced by the pair of BUI/CUI items is not the one
+ * being currently processed, then we need to reserve space to relog
+ * both items.
+ */
+ const unsigned int tx0 = bui + cui;
+ const unsigned int tx1 = bud + rui + cui + cud;
+ const unsigned int tx2 = rud + cui + cud;
+ const unsigned int tx3 = cud + efi;
+ const unsigned int tx4 = efd;
+ const unsigned int relog = bui + bud + cui + cud;
+
+ const unsigned int per_intent = max(max3(tx0, tx1, tx2),
+ max3(tx3, tx4, relog));
+
+ /* Overhead to finish one step of each intent item type */
+ const unsigned int f1 = xfs_calc_finish_efi_reservation(mp, 1);
+ const unsigned int f2 = xfs_calc_finish_rui_reservation(mp, 1);
+ const unsigned int f3 = xfs_calc_finish_cui_reservation(mp, 1);
+ const unsigned int f4 = xfs_calc_finish_bui_reservation(mp, 1);
+
+ /* We only finish one item per transaction in a chain */
+ *step_size = max(f4, max3(f1, f2, f3));
+
+ return per_intent;
+}
+
+/*
+ * Compute the maximum size (in fsblocks) of atomic writes that we can complete
+ * given the existing log reservations.
+ */
+xfs_extlen_t
+xfs_calc_max_atomic_write_fsblocks(
+ struct xfs_mount *mp)
+{
+ const struct xfs_trans_res *resv = &M_RES(mp)->tr_atomic_ioend;
+ unsigned int per_intent = 0;
+ unsigned int step_size = 0;
+ unsigned int ret = 0;
+
+ if (resv->tr_logres > 0) {
+ per_intent = xfs_calc_atomic_write_ioend_geometry(mp,
+ &step_size);
+
+ if (resv->tr_logres >= step_size)
+ ret = (resv->tr_logres - step_size) / per_intent;
+ }
+
+ trace_xfs_calc_max_atomic_write_fsblocks(mp, per_intent, step_size,
+ resv->tr_logres, ret);
+
+ return ret;
+}
diff --git a/fs/xfs/libxfs/xfs_trans_resv.h b/fs/xfs/libxfs/xfs_trans_resv.h
index 670045d417a6..a6d303b83688 100644
--- a/fs/xfs/libxfs/xfs_trans_resv.h
+++ b/fs/xfs/libxfs/xfs_trans_resv.h
@@ -121,4 +121,6 @@ unsigned int xfs_calc_itruncate_reservation_minlogsize(struct xfs_mount *mp);
unsigned int xfs_calc_write_reservation_minlogsize(struct xfs_mount *mp);
unsigned int xfs_calc_qm_dqalloc_reservation_minlogsize(struct xfs_mount *mp);
+xfs_extlen_t xfs_calc_max_atomic_write_fsblocks(struct xfs_mount *mp);
+
#endif /* __XFS_TRANS_RESV_H__ */
diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index 1082b322e6d6..eebd5e7d1ab6 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -666,6 +666,80 @@ xfs_agbtree_compute_maxlevels(
mp->m_agbtree_maxlevels = max(levels, mp->m_refc_maxlevels);
}
+/* Maximum atomic write IO size that the kernel allows. */
+static inline xfs_extlen_t xfs_calc_atomic_write_max(struct xfs_mount *mp)
+{
+ return rounddown_pow_of_two(XFS_B_TO_FSB(mp, MAX_RW_COUNT));
+}
+
+static inline unsigned int max_pow_of_two_factor(const unsigned int nr)
+{
+ return 1 << (ffs(nr) - 1);
+}
+
+/*
+ * If the data device advertises atomic write support, limit the size of data
+ * device atomic writes to the greatest power-of-two factor of the AG size so
+ * that every atomic write unit aligns with the start of every AG. This is
+ * required so that the per-AG allocations for an atomic write will always be
+ * aligned compatibly with the alignment requirements of the storage.
+ *
+ * If the data device doesn't advertise atomic writes, then there are no
+ * alignment restrictions and the largest out-of-place write we can do
+ * ourselves is the number of blocks that user files can allocate from any AG.
+ */
+static inline xfs_extlen_t xfs_calc_perag_awu_max(struct xfs_mount *mp)
+{
+ if (mp->m_ddev_targp->bt_bdev_awu_min > 0)
+ return max_pow_of_two_factor(mp->m_sb.sb_agblocks);
+ return mp->m_ag_max_usable;
+}
+
+/*
+ * Reflink on the realtime device requires rtgroups, and atomic writes require
+ * reflink.
+ *
+ * If the realtime device advertises atomic write support, limit the size of
+ * data device atomic writes to the greatest power-of-two factor of the rtgroup
+ * size so that every atomic write unit aligns with the start of every rtgroup.
+ * This is required so that the per-rtgroup allocations for an atomic write
+ * will always be aligned compatibly with the alignment requirements of the
+ * storage.
+ *
+ * If the rt device doesn't advertise atomic writes, then there are no
+ * alignment restrictions and the largest out-of-place write we can do
+ * ourselves is the number of blocks that user files can allocate from any
+ * rtgroup.
+ */
+static inline xfs_extlen_t xfs_calc_rtgroup_awu_max(struct xfs_mount *mp)
+{
+ struct xfs_groups *rgs = &mp->m_groups[XG_TYPE_RTG];
+
+ if (mp->m_rtdev_targp && mp->m_rtdev_targp->bt_bdev_awu_min > 0)
+ return max_pow_of_two_factor(rgs->blocks);
+ return rgs->blocks;
+}
+
+/* Compute the maximum atomic write unit size for each section. */
+static inline void
+xfs_calc_atomic_write_unit_max(
+ struct xfs_mount *mp)
+{
+ struct xfs_groups *ags = &mp->m_groups[XG_TYPE_AG];
+ struct xfs_groups *rgs = &mp->m_groups[XG_TYPE_RTG];
+
+ const xfs_extlen_t max_write = xfs_calc_atomic_write_max(mp);
+ const xfs_extlen_t max_ioend = xfs_reflink_max_atomic_cow(mp);
+ const xfs_extlen_t max_agsize = xfs_calc_perag_awu_max(mp);
+ const xfs_extlen_t max_rgsize = xfs_calc_rtgroup_awu_max(mp);
+
+ ags->awu_max = min3(max_write, max_ioend, max_agsize);
+ rgs->awu_max = min3(max_write, max_ioend, max_rgsize);
+
+ trace_xfs_calc_atomic_write_unit_max(mp, max_write, max_ioend,
+ max_agsize, max_rgsize);
+}
+
/* Compute maximum possible height for realtime btree types for this fs. */
static inline void
xfs_rtbtree_compute_maxlevels(
@@ -1089,6 +1163,13 @@ xfs_mountfs(
if (mp->m_rtdev_targp && mp->m_rtdev_targp != mp->m_ddev_targp)
xfs_buftarg_config_atomic_writes(mp->m_rtdev_targp);
+ /*
+ * Pre-calculate atomic write unit max. This involves computations
+ * derived from transaction reservations, so we must do this after the
+ * log is fully initialized.
+ */
+ xfs_calc_atomic_write_unit_max(mp);
+
return 0;
out_agresv:
diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index e67bc3e91f98..e2abf31438e0 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -119,6 +119,12 @@ struct xfs_groups {
* SMR hard drives.
*/
xfs_fsblock_t start_fsb;
+
+ /*
+ * Maximum length of an atomic write for files stored in this
+ * collection of allocation groups, in fsblocks.
+ */
+ xfs_extlen_t awu_max;
};
struct xfs_freecounter {
diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index 218dee76768b..ad3bcb76d805 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -1040,6 +1040,22 @@ xfs_reflink_end_atomic_cow(
return error;
}
+/* Compute the largest atomic write that we can complete through software. */
+xfs_extlen_t
+xfs_reflink_max_atomic_cow(
+ struct xfs_mount *mp)
+{
+ /* We cannot do any atomic writes without out of place writes. */
+ if (!xfs_can_sw_atomic_write(mp))
+ return 0;
+
+ /*
+ * Atomic write limits must always be a power-of-2, according to
+ * generic_atomic_write_valid.
+ */
+ return rounddown_pow_of_two(xfs_calc_max_atomic_write_fsblocks(mp));
+}
+
/*
* Free all CoW staging blocks that are still referenced by the ondisk refcount
* metadata. The ondisk metadata does not track which inode created the
diff --git a/fs/xfs/xfs_reflink.h b/fs/xfs/xfs_reflink.h
index 412e9b6f2082..36cda724da89 100644
--- a/fs/xfs/xfs_reflink.h
+++ b/fs/xfs/xfs_reflink.h
@@ -68,4 +68,6 @@ extern int xfs_reflink_update_dest(struct xfs_inode *dest, xfs_off_t newlen,
bool xfs_reflink_supports_rextsize(struct xfs_mount *mp, unsigned int rextsize);
+xfs_extlen_t xfs_reflink_max_atomic_cow(struct xfs_mount *mp);
+
#endif /* __XFS_REFLINK_H */
diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index 9554578c6da4..d5ae00f8e04c 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -170,6 +170,66 @@ DEFINE_ATTR_LIST_EVENT(xfs_attr_list_notfound);
DEFINE_ATTR_LIST_EVENT(xfs_attr_leaf_list);
DEFINE_ATTR_LIST_EVENT(xfs_attr_node_list);
+TRACE_EVENT(xfs_calc_atomic_write_unit_max,
+ TP_PROTO(struct xfs_mount *mp, unsigned int max_write,
+ unsigned int max_ioend, unsigned int max_agsize,
+ unsigned int max_rgsize),
+ TP_ARGS(mp, max_write, max_ioend, max_agsize, max_rgsize),
+ TP_STRUCT__entry(
+ __field(dev_t, dev)
+ __field(unsigned int, max_write)
+ __field(unsigned int, max_ioend)
+ __field(unsigned int, max_agsize)
+ __field(unsigned int, max_rgsize)
+ __field(unsigned int, data_awu_max)
+ __field(unsigned int, rt_awu_max)
+ ),
+ TP_fast_assign(
+ __entry->dev = mp->m_super->s_dev;
+ __entry->max_write = max_write;
+ __entry->max_ioend = max_ioend;
+ __entry->max_agsize = max_agsize;
+ __entry->max_rgsize = max_rgsize;
+ __entry->data_awu_max = mp->m_groups[XG_TYPE_AG].awu_max;
+ __entry->rt_awu_max = mp->m_groups[XG_TYPE_RTG].awu_max;
+ ),
+ TP_printk("dev %d:%d max_write %u max_ioend %u max_agsize %u max_rgsize %u data_awu_max %u rt_awu_max %u",
+ MAJOR(__entry->dev), MINOR(__entry->dev),
+ __entry->max_write,
+ __entry->max_ioend,
+ __entry->max_agsize,
+ __entry->max_rgsize,
+ __entry->data_awu_max,
+ __entry->rt_awu_max)
+);
+
+TRACE_EVENT(xfs_calc_max_atomic_write_fsblocks,
+ TP_PROTO(struct xfs_mount *mp, unsigned int per_intent,
+ unsigned int step_size, unsigned int logres,
+ unsigned int blockcount),
+ TP_ARGS(mp, per_intent, step_size, logres, blockcount),
+ TP_STRUCT__entry(
+ __field(dev_t, dev)
+ __field(unsigned int, per_intent)
+ __field(unsigned int, step_size)
+ __field(unsigned int, logres)
+ __field(unsigned int, blockcount)
+ ),
+ TP_fast_assign(
+ __entry->dev = mp->m_super->s_dev;
+ __entry->per_intent = per_intent;
+ __entry->step_size = step_size;
+ __entry->logres = logres;
+ __entry->blockcount = blockcount;
+ ),
+ TP_printk("dev %d:%d per_intent %u step_size %u logres %u blockcount %u",
+ MAJOR(__entry->dev), MINOR(__entry->dev),
+ __entry->per_intent,
+ __entry->step_size,
+ __entry->logres,
+ __entry->blockcount)
+);
+
TRACE_EVENT(xlog_intent_recovery_failed,
TP_PROTO(struct xfs_mount *mp, const struct xfs_defer_op_type *ops,
int error),
--
2.31.1
^ permalink raw reply related
* [PATCH v9 06/15] xfs: allow block allocator to take an alignment hint
From: John Garry @ 2025-04-25 16:44 UTC (permalink / raw)
To: brauner, djwong, hch, viro, jack, cem
Cc: linux-fsdevel, dchinner, linux-xfs, linux-kernel, ojaswin,
ritesh.list, martin.petersen, linux-ext4, linux-block,
catherine.hoang, linux-api, John Garry
In-Reply-To: <20250425164504.3263637-1-john.g.garry@oracle.com>
Add a BMAPI flag to provide a hint to the block allocator to align extents
according to the extszhint.
This will be useful for atomic writes to ensure that we are not being
allocated extents which are not suitable (for atomic writes).
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: John Garry <john.g.garry@oracle.com>
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
fs/xfs/libxfs/xfs_bmap.c | 5 +++++
fs/xfs/libxfs/xfs_bmap.h | 6 +++++-
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 63255820b58a..d954f9b8071f 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -3312,6 +3312,11 @@ xfs_bmap_compute_alignments(
align = xfs_get_cowextsz_hint(ap->ip);
else if (ap->datatype & XFS_ALLOC_USERDATA)
align = xfs_get_extsz_hint(ap->ip);
+
+ /* Try to align start block to any minimum allocation alignment */
+ if (align > 1 && (ap->flags & XFS_BMAPI_EXTSZALIGN))
+ args->alignment = align;
+
if (align) {
if (xfs_bmap_extsize_align(mp, &ap->got, &ap->prev, align, 0,
ap->eof, 0, ap->conv, &ap->offset,
diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
index b4d9c6e0f3f9..d5f2729305fa 100644
--- a/fs/xfs/libxfs/xfs_bmap.h
+++ b/fs/xfs/libxfs/xfs_bmap.h
@@ -87,6 +87,9 @@ struct xfs_bmalloca {
/* Do not update the rmap btree. Used for reconstructing bmbt from rmapbt. */
#define XFS_BMAPI_NORMAP (1u << 10)
+/* Try to align allocations to the extent size hint */
+#define XFS_BMAPI_EXTSZALIGN (1u << 11)
+
#define XFS_BMAPI_FLAGS \
{ XFS_BMAPI_ENTIRE, "ENTIRE" }, \
{ XFS_BMAPI_METADATA, "METADATA" }, \
@@ -98,7 +101,8 @@ struct xfs_bmalloca {
{ XFS_BMAPI_REMAP, "REMAP" }, \
{ XFS_BMAPI_COWFORK, "COWFORK" }, \
{ XFS_BMAPI_NODISCARD, "NODISCARD" }, \
- { XFS_BMAPI_NORMAP, "NORMAP" }
+ { XFS_BMAPI_NORMAP, "NORMAP" },\
+ { XFS_BMAPI_EXTSZALIGN, "EXTSZALIGN" }
static inline int xfs_bmapi_aflag(int w)
--
2.31.1
^ permalink raw reply related
* [PATCH v9 04/15] xfs: rename xfs_inode_can_atomicwrite() -> xfs_inode_can_hw_atomic_write()
From: John Garry @ 2025-04-25 16:44 UTC (permalink / raw)
To: brauner, djwong, hch, viro, jack, cem
Cc: linux-fsdevel, dchinner, linux-xfs, linux-kernel, ojaswin,
ritesh.list, martin.petersen, linux-ext4, linux-block,
catherine.hoang, linux-api, John Garry
In-Reply-To: <20250425164504.3263637-1-john.g.garry@oracle.com>
In future we will want to be able to check if specifically HW offload-based
atomic writes are possible, so rename xfs_inode_can_atomicwrite() ->
xfs_inode_can_hw_atomicwrite().
Signed-off-by: John Garry <john.g.garry@oracle.com>
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
[djwong: add an underscore to be consistent with everything else]
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
fs/xfs/xfs_file.c | 2 +-
fs/xfs/xfs_inode.h | 2 +-
fs/xfs/xfs_iops.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 84f08c976ac4..55bdae44e42a 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -1488,7 +1488,7 @@ xfs_file_open(
if (xfs_is_shutdown(XFS_M(inode->i_sb)))
return -EIO;
file->f_mode |= FMODE_NOWAIT | FMODE_CAN_ODIRECT;
- if (xfs_inode_can_atomicwrite(XFS_I(inode)))
+ if (xfs_inode_can_hw_atomic_write(XFS_I(inode)))
file->f_mode |= FMODE_CAN_ATOMIC_WRITE;
return generic_file_open(inode, file);
}
diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
index eae0159983ca..bdbbff0d8d99 100644
--- a/fs/xfs/xfs_inode.h
+++ b/fs/xfs/xfs_inode.h
@@ -357,7 +357,7 @@ static inline bool xfs_inode_has_bigrtalloc(const struct xfs_inode *ip)
(ip)->i_mount->m_rtdev_targp : (ip)->i_mount->m_ddev_targp)
static inline bool
-xfs_inode_can_atomicwrite(
+xfs_inode_can_hw_atomic_write(
struct xfs_inode *ip)
{
struct xfs_mount *mp = ip->i_mount;
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index f0e5d83195df..22432c300fd7 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -608,7 +608,7 @@ xfs_report_atomic_write(
{
unsigned int unit_min = 0, unit_max = 0;
- if (xfs_inode_can_atomicwrite(ip))
+ if (xfs_inode_can_hw_atomic_write(ip))
unit_min = unit_max = ip->i_mount->m_sb.sb_blocksize;
generic_fill_statx_atomic_writes(stat, unit_min, unit_max, 0);
}
--
2.31.1
^ permalink raw reply related
* [PATCH v9 10/15] xfs: add large atomic writes checks in xfs_direct_write_iomap_begin()
From: John Garry @ 2025-04-25 16:44 UTC (permalink / raw)
To: brauner, djwong, hch, viro, jack, cem
Cc: linux-fsdevel, dchinner, linux-xfs, linux-kernel, ojaswin,
ritesh.list, martin.petersen, linux-ext4, linux-block,
catherine.hoang, linux-api, John Garry
In-Reply-To: <20250425164504.3263637-1-john.g.garry@oracle.com>
For when large atomic writes (> 1x FS block) are supported, there will be
various occasions when HW offload may not be possible.
Such instances include:
- unaligned extent mapping wrt write length
- extent mappings which do not cover the full write, e.g. the write spans
sparse or mixed-mapping extents
- the write length is greater than HW offload can support
- no hardware support at all
In those cases, we need to fallback to the CoW-based atomic write mode. For
this, report special code -ENOPROTOOPT to inform the caller that HW
offload-based method is not possible.
In addition to the occasions mentioned, if the write covers an unallocated
range, we again judge that we need to rely on the CoW-based method when we
would need to allocate anything more than 1x block. This is because if we
allocate less blocks that is required for the write, then again HW
offload-based method would not be possible. So we are taking a pessimistic
approach to writes covering unallocated space.
Signed-off-by: John Garry <john.g.garry@oracle.com>
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
[djwong: various cleanups]
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/xfs_iomap.c | 62 ++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 60 insertions(+), 2 deletions(-)
diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 166fba2ff1ef..ff05e6b1b0bb 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -798,6 +798,38 @@ imap_spans_range(
return true;
}
+static bool
+xfs_bmap_hw_atomic_write_possible(
+ struct xfs_inode *ip,
+ struct xfs_bmbt_irec *imap,
+ xfs_fileoff_t offset_fsb,
+ xfs_fileoff_t end_fsb)
+{
+ struct xfs_mount *mp = ip->i_mount;
+ xfs_fsize_t len = XFS_FSB_TO_B(mp, end_fsb - offset_fsb);
+
+ /*
+ * atomic writes are required to be naturally aligned for disk blocks,
+ * which ensures that we adhere to block layer rules that we won't
+ * straddle any boundary or violate write alignment requirement.
+ */
+ if (!IS_ALIGNED(imap->br_startblock, imap->br_blockcount))
+ return false;
+
+ /*
+ * Spanning multiple extents would mean that multiple BIOs would be
+ * issued, and so would lose atomicity required for REQ_ATOMIC-based
+ * atomics.
+ */
+ if (!imap_spans_range(imap, offset_fsb, end_fsb))
+ return false;
+
+ /*
+ * The ->iomap_begin caller should ensure this, but check anyway.
+ */
+ return len <= xfs_inode_buftarg(ip)->bt_bdev_awu_max;
+}
+
static int
xfs_direct_write_iomap_begin(
struct inode *inode,
@@ -812,9 +844,11 @@ xfs_direct_write_iomap_begin(
struct xfs_bmbt_irec imap, cmap;
xfs_fileoff_t offset_fsb = XFS_B_TO_FSBT(mp, offset);
xfs_fileoff_t end_fsb = xfs_iomap_end_fsb(mp, offset, length);
+ xfs_fileoff_t orig_end_fsb = end_fsb;
int nimaps = 1, error = 0;
bool shared = false;
u16 iomap_flags = 0;
+ bool needs_alloc;
unsigned int lockmode;
u64 seq;
@@ -875,13 +909,37 @@ xfs_direct_write_iomap_begin(
(flags & IOMAP_DIRECT) || IS_DAX(inode));
if (error)
goto out_unlock;
- if (shared)
+ if (shared) {
+ if ((flags & IOMAP_ATOMIC) &&
+ !xfs_bmap_hw_atomic_write_possible(ip, &cmap,
+ offset_fsb, end_fsb)) {
+ error = -ENOPROTOOPT;
+ goto out_unlock;
+ }
goto out_found_cow;
+ }
end_fsb = imap.br_startoff + imap.br_blockcount;
length = XFS_FSB_TO_B(mp, end_fsb) - offset;
}
- if (imap_needs_alloc(inode, flags, &imap, nimaps))
+ needs_alloc = imap_needs_alloc(inode, flags, &imap, nimaps);
+
+ if (flags & IOMAP_ATOMIC) {
+ error = -ENOPROTOOPT;
+ /*
+ * If we allocate less than what is required for the write
+ * then we may end up with multiple extents, which means that
+ * REQ_ATOMIC-based cannot be used, so avoid this possibility.
+ */
+ if (needs_alloc && orig_end_fsb - offset_fsb > 1)
+ goto out_unlock;
+
+ if (!xfs_bmap_hw_atomic_write_possible(ip, &imap, offset_fsb,
+ orig_end_fsb))
+ goto out_unlock;
+ }
+
+ if (needs_alloc)
goto allocate_blocks;
/*
--
2.31.1
^ permalink raw reply related
* [PATCH v9 08/15] xfs: refine atomic write size check in xfs_file_write_iter()
From: John Garry @ 2025-04-25 16:44 UTC (permalink / raw)
To: brauner, djwong, hch, viro, jack, cem
Cc: linux-fsdevel, dchinner, linux-xfs, linux-kernel, ojaswin,
ritesh.list, martin.petersen, linux-ext4, linux-block,
catherine.hoang, linux-api, John Garry
In-Reply-To: <20250425164504.3263637-1-john.g.garry@oracle.com>
Currently the size of atomic write allowed is fixed at the blocksize.
To start to lift this restriction, partly refactor
xfs_report_atomic_write() to into helpers -
xfs_get_atomic_write_{min, max}() - and use those helpers to find the
per-inode atomic write limits and check according to that.
Also add xfs_get_atomic_write_max_opt() to return the optimal limit, and
just return 0 since large atomics aren't supported yet.
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: John Garry <john.g.garry@oracle.com>
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
fs/xfs/xfs_file.c | 12 +++++-------
fs/xfs/xfs_iops.c | 36 +++++++++++++++++++++++++++++++-----
fs/xfs/xfs_iops.h | 3 +++
3 files changed, 39 insertions(+), 12 deletions(-)
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 55bdae44e42a..e8acd6ca8f27 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -1032,14 +1032,12 @@ xfs_file_write_iter(
return xfs_file_dax_write(iocb, from);
if (iocb->ki_flags & IOCB_ATOMIC) {
- /*
- * Currently only atomic writing of a single FS block is
- * supported. It would be possible to atomic write smaller than
- * a FS block, but there is no requirement to support this.
- * Note that iomap also does not support this yet.
- */
- if (ocount != ip->i_mount->m_sb.sb_blocksize)
+ if (ocount < xfs_get_atomic_write_min(ip))
return -EINVAL;
+
+ if (ocount > xfs_get_atomic_write_max(ip))
+ return -EINVAL;
+
ret = generic_atomic_write_valid(iocb, from);
if (ret)
return ret;
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index 22432c300fd7..77a0606e9dc9 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -601,16 +601,42 @@ xfs_report_dioalign(
stat->dio_offset_align = stat->dio_read_offset_align;
}
+unsigned int
+xfs_get_atomic_write_min(
+ struct xfs_inode *ip)
+{
+ if (!xfs_inode_can_hw_atomic_write(ip))
+ return 0;
+
+ return ip->i_mount->m_sb.sb_blocksize;
+}
+
+unsigned int
+xfs_get_atomic_write_max(
+ struct xfs_inode *ip)
+{
+ if (!xfs_inode_can_hw_atomic_write(ip))
+ return 0;
+
+ return ip->i_mount->m_sb.sb_blocksize;
+}
+
+unsigned int
+xfs_get_atomic_write_max_opt(
+ struct xfs_inode *ip)
+{
+ return 0;
+}
+
static void
xfs_report_atomic_write(
struct xfs_inode *ip,
struct kstat *stat)
{
- unsigned int unit_min = 0, unit_max = 0;
-
- if (xfs_inode_can_hw_atomic_write(ip))
- unit_min = unit_max = ip->i_mount->m_sb.sb_blocksize;
- generic_fill_statx_atomic_writes(stat, unit_min, unit_max, 0);
+ generic_fill_statx_atomic_writes(stat,
+ xfs_get_atomic_write_min(ip),
+ xfs_get_atomic_write_max(ip),
+ xfs_get_atomic_write_max_opt(ip));
}
STATIC int
diff --git a/fs/xfs/xfs_iops.h b/fs/xfs/xfs_iops.h
index 3c1a2605ffd2..0896f6b8b3b8 100644
--- a/fs/xfs/xfs_iops.h
+++ b/fs/xfs/xfs_iops.h
@@ -19,5 +19,8 @@ int xfs_inode_init_security(struct inode *inode, struct inode *dir,
extern void xfs_setup_inode(struct xfs_inode *ip);
extern void xfs_setup_iops(struct xfs_inode *ip);
extern void xfs_diflags_to_iflags(struct xfs_inode *ip, bool init);
+unsigned int xfs_get_atomic_write_min(struct xfs_inode *ip);
+unsigned int xfs_get_atomic_write_max(struct xfs_inode *ip);
+unsigned int xfs_get_atomic_write_max_opt(struct xfs_inode *ip);
#endif /* __XFS_IOPS_H__ */
--
2.31.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox