* Re: [PATCH v11 02/16] xfs: only call xfs_setsize_buftarg once per buffer target
From: John Garry @ 2025-05-05 14:48 UTC (permalink / raw)
To: Darrick J. Wong
Cc: Christoph Hellwig, 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: <20250505142234.GG1035866@frogsfrogsfrogs>
On 05/05/2025 15:22, Darrick J. Wong wrote:
>>> Yes. Or in fact just folding it into xfs_alloc_buftarg, which might
>>> be even simpler.
>> Yes, that was my next question..
>>
>>> While you're at it adding a command why we are doing
>>> the sync would also be really useful, and having it in just one place
>>> helps with that.
>> ok, there was such comment in xfs_preflush_devices().
>>
>> @Darrick, please comment on whether happy with changes discussed.
> I put the sync_blockdev calls in a separate function so that the
> EIO/ENOSPC/whatever errors that come from the block device sync don't
> get morphed into ENOMEM by xfs_alloc_buftarg before being passed up. I
> suppose we could make that function return an ERR_PTR, but I was trying
> to avoid making even more changes at the last minute, again.
It seems simpler to just have the individual sync_blockdev() calls from
xfs_alloc_buftarg(), rather than adding ERR_PTR() et al handling in both
xfs_alloc_buftarg() and xfs_open_devices().
^ permalink raw reply
* Re: [PATCH v11 14/16] xfs: add xfs_calc_atomic_write_unit_max()
From: Darrick J. Wong @ 2025-05-05 14:26 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: <9d5d7037-6ed7-4c61-afec-8422d656de37@oracle.com>
On Mon, May 05, 2025 at 09:02:31AM +0100, John Garry wrote:
> On 05/05/2025 07:08, John Garry wrote:
> > On 05/05/2025 06:25, Darrick J. Wong wrote:
> > > Ok so I even attached the reply to the WRONG VERSION. Something in
> > > these changes cause xfs/289 to barf up this UBSAN warning, even on a
> > > realtime + rtgroups volume:
>
> Could this just be from another mount (of not a realtime + rtgroups xfs
> instance)?
Quite possibly.
> > >
> > > [ 1160.539004] ------------[ cut here ]------------
> > > [ 1160.540701] UBSAN: shift-out-of-bounds in /storage/home/djwong/
> > > cdev/work/linux-djw/include/linux/log2.h:67:13
> > > [ 1160.544597] shift exponent 4294967295 is too large for 64-bit
> > > type 'long unsigned int'
> > > [ 1160.547038] CPU: 3 UID: 0 PID: 288421 Comm: mount Not tainted
> > > 6.15.0-rc5-djwx #rc5 PREEMPT(lazy)
> > > 6f606c17703b80ffff7378e7041918eca24b3e68
> > > [ 1160.547045] Hardware name: QEMU Standard PC (i440FX + PIIX,
> > > 1996), BIOS 1.16.0-4.module+el8.8.0+21164+ed375313 04/01/2014
> > > [ 1160.547047] Call Trace:
> > > [ 1160.547049] <TASK>
> > > [ 1160.547051] dump_stack_lvl+0x4f/0x60
> > > [ 1160.547060] __ubsan_handle_shift_out_of_bounds+0x1bc/0x380
> > > [ 1160.547066] xfs_set_max_atomic_write_opt.cold+0x22d/0x252 [xfs
> > > 1f657532c3dee9b1d567597a31645929273d3283]
> > > [ 1160.547249] xfs_mountfs+0xa5c/0xb50 [xfs
> > > 1f657532c3dee9b1d567597a31645929273d3283]
> > > [ 1160.547434] xfs_fs_fill_super+0x7eb/0xb30 [xfs
> > > 1f657532c3dee9b1d567597a31645929273d3283]
> > > [ 1160.547616] ? xfs_open_devices+0x240/0x240 [xfs
> > > 1f657532c3dee9b1d567597a31645929273d3283]
> > > [ 1160.547797] get_tree_bdev_flags+0x132/0x1d0
> > > [ 1160.547801] vfs_get_tree+0x17/0xa0
> > > [ 1160.547803] path_mount+0x720/0xa80
> > > [ 1160.547807] __x64_sys_mount+0x10c/0x140
> > > [ 1160.547810] do_syscall_64+0x47/0x100
> > > [ 1160.547814] entry_SYSCALL_64_after_hwframe+0x4b/0x53
> > > [ 1160.547817] RIP: 0033:0x7fde55d62e0a
> > > [ 1160.547820] Code: 48 8b 0d f9 7f 0c 00 f7 d8 64 89 01 48 83 c8 ff
> > > c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 49 89 ca b8 a5 00 00
> > > 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d c6 7f 0c 00 f7 d8 64
> > > 89 01 48
> > > [ 1160.547823] RSP: 002b:00007fff11920ce8 EFLAGS: 00000246 ORIG_RAX:
> > > 00000000000000a5
> > > [ 1160.547826] RAX: ffffffffffffffda RBX: 0000556a10cd1de0 RCX:
> > > 00007fde55d62e0a
> > > [ 1160.547828] RDX: 0000556a10cd2010 RSI: 0000556a10cd2090 RDI:
> > > 0000556a10ce2590
> > > [ 1160.547829] RBP: 0000000000000000 R08: 0000000000000000 R09:
> > > 00007fff11920d50
> > > [ 1160.547830] R10: 0000000000000000 R11: 0000000000000246 R12:
> > > 0000556a10ce2590
> > > [ 1160.547832] R13: 0000556a10cd2010 R14: 00007fde55eca264 R15:
> > > 0000556a10cd1ef8
> > > [ 1160.547834] </TASK>
> > > [ 1160.547835] ---[ end trace ]---
> > >
> > > John, can you please figure this one out, seeing as it's 10:30pm on
> > > Sunday night here?
>
>
> I could recreate this.
>
> >
>
> I think that we need this change:
>
> @@ -715,6 +716,9 @@ 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 (rgs->blocks == 0)
> + return 0;
> if (mp->m_rtdev_targp && mp->m_rtdev_targp->bt_bdev_awu_min > 0)
> return max_pow_of_two_factor(rgs->blocks);
> return rounddown_pow_of_two(rgs->blocks);
>
> My xfs/289 problem goes away with this change.
Ok good.
--D
>
> >
>
>
^ permalink raw reply
* Re: [PATCH v11 06/16] xfs: ignore HW which cannot atomic write a single block
From: Darrick J. Wong @ 2025-05-05 14:24 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: <20250505083050.GA31587@lst.de>
On Mon, May 05, 2025 at 10:30:50AM +0200, Christoph Hellwig wrote:
> On Mon, May 05, 2025 at 09:12:53AM +0100, John Garry wrote:
> > On 05/05/2025 06:45, John Garry wrote:
> >> On 05/05/2025 06:43, Christoph Hellwig wrote:
> >>> I think this subject line here is left from an earlier version and
> >>> doesn't quite seem to summarize what this patch is doing now?
> >
> > How about we just split this patch into 2 patches:
> > part 1 re-org with new helper xfs_configure_buftarg_atomic_writes()
> > part 2 ignore HW which cannot atomic write a single FS block
>
> Fine with me. Although just fixing up the subject sounds fine as well.
I don't care either way.
--D
^ permalink raw reply
* Re: [PATCH v11 02/16] xfs: only call xfs_setsize_buftarg once per buffer target
From: Darrick J. Wong @ 2025-05-05 14:22 UTC (permalink / raw)
To: John Garry
Cc: Christoph Hellwig, 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: <bb8efa28-19e6-42f5-9a26-cdc0bc48926e@oracle.com>
On Mon, May 05, 2025 at 11:55:13AM +0100, John Garry wrote:
> On 05/05/2025 11:49, Christoph Hellwig wrote:
> > On Mon, May 05, 2025 at 11:04:55AM +0100, John Garry wrote:
> > > @@ -503,6 +509,9 @@ xfs_open_devices(
> > > mp->m_logdev_targp = xfs_alloc_buftarg(mp, logdev_file);
> > > if (!mp->m_logdev_targp)
> > > goto out_free_rtdev_targ;
> > > + error = sync_blockdev(mp->m_logdev_targp->bt_bdev);
> > > + if (error)
> > > + goto out_free_rtdev_targ;
> > > } else {
> > > mp->m_logdev_targp = mp->m_ddev_targp;
> > > /* Handle won't be used, drop it */
> > >
> > >
> > > Right?
> > Yes. Or in fact just folding it into xfs_alloc_buftarg, which might
> > be even simpler.
>
> Yes, that was my next question..
>
> > While you're at it adding a command why we are doing
> > the sync would also be really useful, and having it in just one place
> > helps with that.
>
> ok, there was such comment in xfs_preflush_devices().
>
> @Darrick, please comment on whether happy with changes discussed.
I put the sync_blockdev calls in a separate function so that the
EIO/ENOSPC/whatever errors that come from the block device sync don't
get morphed into ENOMEM by xfs_alloc_buftarg before being passed up. I
suppose we could make that function return an ERR_PTR, but I was trying
to avoid making even more changes at the last minute, again.
--D
> Thanks,
> John
>
>
^ permalink raw reply
* Re: [PATCH v3 3/3] selftests: pidfd: add tests for PIDFD_SELF_*
From: Christian Brauner @ 2025-05-05 13:35 UTC (permalink / raw)
To: Peter Zijlstra
Cc: John Hubbard, 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: <20250501124646.GC4356@noisy.programming.kicks-ass.net>
On Thu, May 01, 2025 at 02:46:46PM +0200, Peter Zijlstra wrote:
> 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.
I'm completely lost as to what's happening here or whether the test here
is somehow at fault for something.
The pidfd.h head explicitly has no dependency on the pidfd uapi header
itself and I will NAK anything that makes it so. It's just a giant pain.
^ permalink raw reply
* Re: [PATCH v11 02/16] xfs: only call xfs_setsize_buftarg once per buffer target
From: John Garry @ 2025-05-05 10:55 UTC (permalink / raw)
To: Christoph Hellwig
Cc: brauner, djwong, 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: <20250505104901.GA10128@lst.de>
On 05/05/2025 11:49, Christoph Hellwig wrote:
> On Mon, May 05, 2025 at 11:04:55AM +0100, John Garry wrote:
>> @@ -503,6 +509,9 @@ xfs_open_devices(
>> mp->m_logdev_targp = xfs_alloc_buftarg(mp, logdev_file);
>> if (!mp->m_logdev_targp)
>> goto out_free_rtdev_targ;
>> + error = sync_blockdev(mp->m_logdev_targp->bt_bdev);
>> + if (error)
>> + goto out_free_rtdev_targ;
>> } else {
>> mp->m_logdev_targp = mp->m_ddev_targp;
>> /* Handle won't be used, drop it */
>>
>>
>> Right?
> Yes. Or in fact just folding it into xfs_alloc_buftarg, which might
> be even simpler.
Yes, that was my next question..
> While you're at it adding a command why we are doing
> the sync would also be really useful, and having it in just one place
> helps with that.
ok, there was such comment in xfs_preflush_devices().
@Darrick, please comment on whether happy with changes discussed.
Thanks,
John
^ permalink raw reply
* Re: [PATCH v11 02/16] xfs: only call xfs_setsize_buftarg once per buffer target
From: Christoph Hellwig @ 2025-05-05 10:49 UTC (permalink / raw)
To: John Garry
Cc: Christoph Hellwig, brauner, djwong, 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: <8ea91e81-9b96-458e-bd4e-64eada31e184@oracle.com>
On Mon, May 05, 2025 at 11:04:55AM +0100, John Garry wrote:
> @@ -503,6 +509,9 @@ xfs_open_devices(
> mp->m_logdev_targp = xfs_alloc_buftarg(mp, logdev_file);
> if (!mp->m_logdev_targp)
> goto out_free_rtdev_targ;
> + error = sync_blockdev(mp->m_logdev_targp->bt_bdev);
> + if (error)
> + goto out_free_rtdev_targ;
> } else {
> mp->m_logdev_targp = mp->m_ddev_targp;
> /* Handle won't be used, drop it */
>
>
> Right?
Yes. Or in fact just folding it into xfs_alloc_buftarg, which might
be even simpler. While you're at it adding a command why we are doing
the sync would also be really useful, and having it in just one place
helps with that.
^ permalink raw reply
* Re: [PATCH v11 02/16] xfs: only call xfs_setsize_buftarg once per buffer target
From: John Garry @ 2025-05-05 10:04 UTC (permalink / raw)
To: Christoph Hellwig
Cc: brauner, djwong, 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: <20250505054031.GA20925@lst.de>
On 05/05/2025 06:40, Christoph Hellwig wrote:
>> +/*
>> + * Flush and invalidate all devices' pagecaches before reading any metadata
>> + * because XFS doesn't use the bdev pagecache.
>> + */
>> +STATIC int
>> +xfs_preflush_devices(
>> + struct xfs_mount *mp)
>> +{
>> + int error;
>> +
>> + error = xfs_buftarg_sync(mp->m_ddev_targp);
>> + if (error)
>> + return error;
>> +
>> + if (mp->m_logdev_targp && mp->m_logdev_targp != mp->m_ddev_targp) {
>> + error = xfs_buftarg_sync(mp->m_ddev_targp);
>> + if (error)
>> + return error;
>> + }
> Why does this duplicate all the logic instead of being folded into
> xfs_open_devices?
So you mean an additive change like:
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 64fbd089ef55..9fa538938e07 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -488,6 +488,9 @@ xfs_open_devices(
mp->m_ddev_targp = xfs_alloc_buftarg(mp, sb->s_bdev_file);
if (!mp->m_ddev_targp)
goto out_close_rtdev;
+ error = sync_blockdev(mp->m_ddev_targp->bt_bdev);
+ if (error)
+ goto out_close_rtdev;
if (rtdev_file) {
@@ -495,6 +498,9 @@ xfs_open_devices(
mp->m_rtdev_targp = xfs_alloc_buftarg(mp, rtdev_file);
if (!mp->m_rtdev_targp)
goto out_free_ddev_targ;
+ error = sync_blockdev(mp->m_rtdev_targp->bt_bdev);
+ if (error)
+ goto out_free_ddev_targ;
}
if (logdev_file && file_bdev(logdev_file) != ddev) {
@@ -503,6 +509,9 @@ xfs_open_devices(
mp->m_logdev_targp = xfs_alloc_buftarg(mp, logdev_file);
if (!mp->m_logdev_targp)
goto out_free_rtdev_targ;
+ error = sync_blockdev(mp->m_logdev_targp->bt_bdev);
+ if (error)
+ goto out_free_rtdev_targ;
} else {
mp->m_logdev_targp = mp->m_ddev_targp;
/* Handle won't be used, drop it */
Right?
^ permalink raw reply related
* Re: [PATCH v11 06/16] xfs: ignore HW which cannot atomic write a single block
From: Christoph Hellwig @ 2025-05-05 8:30 UTC (permalink / raw)
To: John Garry
Cc: Christoph Hellwig, brauner, djwong, 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: <0b0d61e9-68e6-4eb0-a7bd-6e256e6d45f8@oracle.com>
On Mon, May 05, 2025 at 09:12:53AM +0100, John Garry wrote:
> On 05/05/2025 06:45, John Garry wrote:
>> On 05/05/2025 06:43, Christoph Hellwig wrote:
>>> I think this subject line here is left from an earlier version and
>>> doesn't quite seem to summarize what this patch is doing now?
>
> How about we just split this patch into 2 patches:
> part 1 re-org with new helper xfs_configure_buftarg_atomic_writes()
> part 2 ignore HW which cannot atomic write a single FS block
Fine with me. Although just fixing up the subject sounds fine as well.
^ permalink raw reply
* Re: [PATCH v11 06/16] xfs: ignore HW which cannot atomic write a single block
From: John Garry @ 2025-05-05 8:12 UTC (permalink / raw)
To: Christoph Hellwig
Cc: brauner, djwong, 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: <1d0e85d5-5e5c-4a8c-ae97-d90092c2c296@oracle.com>
On 05/05/2025 06:45, John Garry wrote:
> On 05/05/2025 06:43, Christoph Hellwig wrote:
>> I think this subject line here is left from an earlier version and
>> doesn't quite seem to summarize what this patch is doing now?
How about we just split this patch into 2 patches:
part 1 re-org with new helper xfs_configure_buftarg_atomic_writes()
part 2 ignore HW which cannot atomic write a single FS block
^ permalink raw reply
* Re: [PATCH v11 14/16] xfs: add xfs_calc_atomic_write_unit_max()
From: John Garry @ 2025-05-05 8:02 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: <2a5688e8-88ef-4224-b757-af5adfca1be1@oracle.com>
On 05/05/2025 07:08, John Garry wrote:
> On 05/05/2025 06:25, Darrick J. Wong wrote:
>> Ok so I even attached the reply to the WRONG VERSION. Something in
>> these changes cause xfs/289 to barf up this UBSAN warning, even on a
>> realtime + rtgroups volume:
Could this just be from another mount (of not a realtime + rtgroups xfs
instance)?
>>
>> [ 1160.539004] ------------[ cut here ]------------
>> [ 1160.540701] UBSAN: shift-out-of-bounds in /storage/home/djwong/
>> cdev/work/linux-djw/include/linux/log2.h:67:13
>> [ 1160.544597] shift exponent 4294967295 is too large for 64-bit type
>> 'long unsigned int'
>> [ 1160.547038] CPU: 3 UID: 0 PID: 288421 Comm: mount Not tainted
>> 6.15.0-rc5-djwx #rc5 PREEMPT(lazy)
>> 6f606c17703b80ffff7378e7041918eca24b3e68
>> [ 1160.547045] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
>> BIOS 1.16.0-4.module+el8.8.0+21164+ed375313 04/01/2014
>> [ 1160.547047] Call Trace:
>> [ 1160.547049] <TASK>
>> [ 1160.547051] dump_stack_lvl+0x4f/0x60
>> [ 1160.547060] __ubsan_handle_shift_out_of_bounds+0x1bc/0x380
>> [ 1160.547066] xfs_set_max_atomic_write_opt.cold+0x22d/0x252 [xfs
>> 1f657532c3dee9b1d567597a31645929273d3283]
>> [ 1160.547249] xfs_mountfs+0xa5c/0xb50 [xfs
>> 1f657532c3dee9b1d567597a31645929273d3283]
>> [ 1160.547434] xfs_fs_fill_super+0x7eb/0xb30 [xfs
>> 1f657532c3dee9b1d567597a31645929273d3283]
>> [ 1160.547616] ? xfs_open_devices+0x240/0x240 [xfs
>> 1f657532c3dee9b1d567597a31645929273d3283]
>> [ 1160.547797] get_tree_bdev_flags+0x132/0x1d0
>> [ 1160.547801] vfs_get_tree+0x17/0xa0
>> [ 1160.547803] path_mount+0x720/0xa80
>> [ 1160.547807] __x64_sys_mount+0x10c/0x140
>> [ 1160.547810] do_syscall_64+0x47/0x100
>> [ 1160.547814] entry_SYSCALL_64_after_hwframe+0x4b/0x53
>> [ 1160.547817] RIP: 0033:0x7fde55d62e0a
>> [ 1160.547820] Code: 48 8b 0d f9 7f 0c 00 f7 d8 64 89 01 48 83 c8 ff
>> c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 49 89 ca b8 a5 00 00
>> 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d c6 7f 0c 00 f7 d8 64 89
>> 01 48
>> [ 1160.547823] RSP: 002b:00007fff11920ce8 EFLAGS: 00000246 ORIG_RAX:
>> 00000000000000a5
>> [ 1160.547826] RAX: ffffffffffffffda RBX: 0000556a10cd1de0 RCX:
>> 00007fde55d62e0a
>> [ 1160.547828] RDX: 0000556a10cd2010 RSI: 0000556a10cd2090 RDI:
>> 0000556a10ce2590
>> [ 1160.547829] RBP: 0000000000000000 R08: 0000000000000000 R09:
>> 00007fff11920d50
>> [ 1160.547830] R10: 0000000000000000 R11: 0000000000000246 R12:
>> 0000556a10ce2590
>> [ 1160.547832] R13: 0000556a10cd2010 R14: 00007fde55eca264 R15:
>> 0000556a10cd1ef8
>> [ 1160.547834] </TASK>
>> [ 1160.547835] ---[ end trace ]---
>>
>> John, can you please figure this one out, seeing as it's 10:30pm on
>> Sunday night here?
I could recreate this.
>
I think that we need this change:
@@ -715,6 +716,9 @@ 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 (rgs->blocks == 0)
+ return 0;
if (mp->m_rtdev_targp && mp->m_rtdev_targp->bt_bdev_awu_min > 0)
return max_pow_of_two_factor(rgs->blocks);
return rounddown_pow_of_two(rgs->blocks);
My xfs/289 problem goes away with this change.
>
^ permalink raw reply
* Re: [PATCH v11 14/16] xfs: add xfs_calc_atomic_write_unit_max()
From: John Garry @ 2025-05-05 6:08 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: <20250505052534.GX25675@frogsfrogsfrogs>
On 05/05/2025 06:25, Darrick J. Wong wrote:
> Ok so I even attached the reply to the WRONG VERSION. Something in
> these changes cause xfs/289 to barf up this UBSAN warning, even on a
> realtime + rtgroups volume:
>
> [ 1160.539004] ------------[ cut here ]------------
> [ 1160.540701] UBSAN: shift-out-of-bounds in /storage/home/djwong/cdev/work/linux-djw/include/linux/log2.h:67:13
> [ 1160.544597] shift exponent 4294967295 is too large for 64-bit type 'long unsigned int'
> [ 1160.547038] CPU: 3 UID: 0 PID: 288421 Comm: mount Not tainted 6.15.0-rc5-djwx #rc5 PREEMPT(lazy) 6f606c17703b80ffff7378e7041918eca24b3e68
> [ 1160.547045] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.0-4.module+el8.8.0+21164+ed375313 04/01/2014
> [ 1160.547047] Call Trace:
> [ 1160.547049] <TASK>
> [ 1160.547051] dump_stack_lvl+0x4f/0x60
> [ 1160.547060] __ubsan_handle_shift_out_of_bounds+0x1bc/0x380
> [ 1160.547066] xfs_set_max_atomic_write_opt.cold+0x22d/0x252 [xfs 1f657532c3dee9b1d567597a31645929273d3283]
> [ 1160.547249] xfs_mountfs+0xa5c/0xb50 [xfs 1f657532c3dee9b1d567597a31645929273d3283]
> [ 1160.547434] xfs_fs_fill_super+0x7eb/0xb30 [xfs 1f657532c3dee9b1d567597a31645929273d3283]
> [ 1160.547616] ? xfs_open_devices+0x240/0x240 [xfs 1f657532c3dee9b1d567597a31645929273d3283]
> [ 1160.547797] get_tree_bdev_flags+0x132/0x1d0
> [ 1160.547801] vfs_get_tree+0x17/0xa0
> [ 1160.547803] path_mount+0x720/0xa80
> [ 1160.547807] __x64_sys_mount+0x10c/0x140
> [ 1160.547810] do_syscall_64+0x47/0x100
> [ 1160.547814] entry_SYSCALL_64_after_hwframe+0x4b/0x53
> [ 1160.547817] RIP: 0033:0x7fde55d62e0a
> [ 1160.547820] Code: 48 8b 0d f9 7f 0c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 49 89 ca b8 a5 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d c6 7f 0c 00 f7 d8 64 89 01 48
> [ 1160.547823] RSP: 002b:00007fff11920ce8 EFLAGS: 00000246 ORIG_RAX: 00000000000000a5
> [ 1160.547826] RAX: ffffffffffffffda RBX: 0000556a10cd1de0 RCX: 00007fde55d62e0a
> [ 1160.547828] RDX: 0000556a10cd2010 RSI: 0000556a10cd2090 RDI: 0000556a10ce2590
> [ 1160.547829] RBP: 0000000000000000 R08: 0000000000000000 R09: 00007fff11920d50
> [ 1160.547830] R10: 0000000000000000 R11: 0000000000000246 R12: 0000556a10ce2590
> [ 1160.547832] R13: 0000556a10cd2010 R14: 00007fde55eca264 R15: 0000556a10cd1ef8
> [ 1160.547834] </TASK>
> [ 1160.547835] ---[ end trace ]---
>
> John, can you please figure this one out, seeing as it's 10:30pm on
> Sunday night here?
I'll check it
thanks for your effort
I have known some ubsan red herrings
- I hope that this is not one of them ..
^ permalink raw reply
* Re: [PATCH v11 06/16] xfs: ignore HW which cannot atomic write a single block
From: John Garry @ 2025-05-05 5:45 UTC (permalink / raw)
To: Christoph Hellwig
Cc: brauner, djwong, 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: <20250505054310.GB20925@lst.de>
On 05/05/2025 06:43, Christoph Hellwig wrote:
> I think this subject line here is left from an earlier version and
> doesn't quite seem to summarize what this patch is doing now?
>
>> +extern int xfs_configure_buftarg(struct xfs_buftarg *, unsigned int);
>
> Please drop the extern and spell out the parameter name while you're at
> it.
I can fix all this, if Darrick does not beat me to it.
>
> Otherwise looks good:
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
>
cheers
^ permalink raw reply
* Re: [PATCH v11 06/16] xfs: ignore HW which cannot atomic write a single block
From: Christoph Hellwig @ 2025-05-05 5:43 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: <20250504085923.1895402-7-john.g.garry@oracle.com>
I think this subject line here is left from an earlier version and
doesn't quite seem to summarize what this patch is doing now?
> +extern int xfs_configure_buftarg(struct xfs_buftarg *, unsigned int);
Please drop the extern and spell out the parameter name while you're at
it.
Otherwise looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [PATCH v11 02/16] xfs: only call xfs_setsize_buftarg once per buffer target
From: Christoph Hellwig @ 2025-05-05 5:40 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: <20250504085923.1895402-3-john.g.garry@oracle.com>
On Sun, May 04, 2025 at 08:59:09AM +0000, John Garry wrote:
> +static inline int xfs_buftarg_sync(struct xfs_buftarg *btp)
> +{
> + return sync_blockdev(btp->bt_bdev);
> +}
What is the point in having this wrapper?
> +/*
> + * Flush and invalidate all devices' pagecaches before reading any metadata
> + * because XFS doesn't use the bdev pagecache.
> + */
> +STATIC int
> +xfs_preflush_devices(
> + struct xfs_mount *mp)
> +{
> + int error;
> +
> + error = xfs_buftarg_sync(mp->m_ddev_targp);
> + if (error)
> + return error;
> +
> + if (mp->m_logdev_targp && mp->m_logdev_targp != mp->m_ddev_targp) {
> + error = xfs_buftarg_sync(mp->m_ddev_targp);
> + if (error)
> + return error;
> + }
Why does this duplicate all the logic instead of being folded into
xfs_open_devices?
^ permalink raw reply
* Re: [PATCH v11 14/16] xfs: add xfs_calc_atomic_write_unit_max()
From: Darrick J. Wong @ 2025-05-05 5:25 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: <20250504085923.1895402-15-john.g.garry@oracle.com>
On Sun, May 04, 2025 at 08:59:21AM +0000, John Garry wrote:
> 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.
>
> [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: rounddown power-of-2 always]
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: John Garry <john.g.garry@oracle.com>
> ---
> 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 00b53f479ece..9c40914afabd 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 rounddown_pow_of_two(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 rounddown_pow_of_two(rgs->blocks);
Ok so I even attached the reply to the WRONG VERSION. Something in
these changes cause xfs/289 to barf up this UBSAN warning, even on a
realtime + rtgroups volume:
[ 1160.539004] ------------[ cut here ]------------
[ 1160.540701] UBSAN: shift-out-of-bounds in /storage/home/djwong/cdev/work/linux-djw/include/linux/log2.h:67:13
[ 1160.544597] shift exponent 4294967295 is too large for 64-bit type 'long unsigned int'
[ 1160.547038] CPU: 3 UID: 0 PID: 288421 Comm: mount Not tainted 6.15.0-rc5-djwx #rc5 PREEMPT(lazy) 6f606c17703b80ffff7378e7041918eca24b3e68
[ 1160.547045] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.0-4.module+el8.8.0+21164+ed375313 04/01/2014
[ 1160.547047] Call Trace:
[ 1160.547049] <TASK>
[ 1160.547051] dump_stack_lvl+0x4f/0x60
[ 1160.547060] __ubsan_handle_shift_out_of_bounds+0x1bc/0x380
[ 1160.547066] xfs_set_max_atomic_write_opt.cold+0x22d/0x252 [xfs 1f657532c3dee9b1d567597a31645929273d3283]
[ 1160.547249] xfs_mountfs+0xa5c/0xb50 [xfs 1f657532c3dee9b1d567597a31645929273d3283]
[ 1160.547434] xfs_fs_fill_super+0x7eb/0xb30 [xfs 1f657532c3dee9b1d567597a31645929273d3283]
[ 1160.547616] ? xfs_open_devices+0x240/0x240 [xfs 1f657532c3dee9b1d567597a31645929273d3283]
[ 1160.547797] get_tree_bdev_flags+0x132/0x1d0
[ 1160.547801] vfs_get_tree+0x17/0xa0
[ 1160.547803] path_mount+0x720/0xa80
[ 1160.547807] __x64_sys_mount+0x10c/0x140
[ 1160.547810] do_syscall_64+0x47/0x100
[ 1160.547814] entry_SYSCALL_64_after_hwframe+0x4b/0x53
[ 1160.547817] RIP: 0033:0x7fde55d62e0a
[ 1160.547820] Code: 48 8b 0d f9 7f 0c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 49 89 ca b8 a5 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d c6 7f 0c 00 f7 d8 64 89 01 48
[ 1160.547823] RSP: 002b:00007fff11920ce8 EFLAGS: 00000246 ORIG_RAX: 00000000000000a5
[ 1160.547826] RAX: ffffffffffffffda RBX: 0000556a10cd1de0 RCX: 00007fde55d62e0a
[ 1160.547828] RDX: 0000556a10cd2010 RSI: 0000556a10cd2090 RDI: 0000556a10ce2590
[ 1160.547829] RBP: 0000000000000000 R08: 0000000000000000 R09: 00007fff11920d50
[ 1160.547830] R10: 0000000000000000 R11: 0000000000000246 R12: 0000556a10ce2590
[ 1160.547832] R13: 0000556a10cd2010 R14: 00007fde55eca264 R15: 0000556a10cd1ef8
[ 1160.547834] </TASK>
[ 1160.547835] ---[ end trace ]---
John, can you please figure this one out, seeing as it's 10:30pm on
Sunday night here?
--D
> +}
> +
> +/* 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(
> @@ -1082,6 +1156,13 @@ xfs_mountfs(
> xfs_zone_gc_start(mp);
> }
>
> + /*
> + * 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
* Re: [PATCH v10 13/15] xfs: add xfs_calc_atomic_write_unit_max()
From: Darrick J. Wong @ 2025-05-05 5:22 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: <20250501165733.1025207-14-john.g.garry@oracle.com>
On Thu, May 01, 2025 at 04:57:31PM +0000, John Garry wrote:
> 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.
>
> [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: rounddown power-of-2 always]
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: John Garry <john.g.garry@oracle.com>
> ---
> 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 00b53f479ece..9c40914afabd 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 rounddown_pow_of_two(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 rounddown_pow_of_two(rgs->blocks);
Note: rounddown_pow_of_two returns an undefined result (and an UBSAN
warning) if rgs->blocks is zero. I'm not sure why fstests didn't barf
up errors about this until now.
Oh, that's right, I forgot to run QA on Friday night because I was too
exhausted to remember. And here it is *Sunday* night and I'm doing QA
work for ... some reason. Oh, so it won't get in the way of other work
... tomorrow? WTF is wrong with me.
--D
> +}
> +
> +/* 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(
> @@ -1082,6 +1156,13 @@ xfs_mountfs(
> xfs_zone_gc_start(mp);
> }
>
> + /*
> + * 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
* [PATCH v11 15/16] xfs: update atomic write limits
From: John Garry @ 2025-05-04 8:59 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: <20250504085923.1895402-1-john.g.garry@oracle.com>
Update the limits returned from xfs_get_atomic_write_{min, max, max_opt)().
No reflink support always means no CoW-based atomic writes.
For updating xfs_get_atomic_write_min(), we support blocksize only and that
depends on HW or reflink support.
For updating xfs_get_atomic_write_max(), for no reflink, we are limited to
blocksize but only if HW support. Otherwise we are limited to combined
limit in mp->m_atomic_write_unit_max.
For updating xfs_get_atomic_write_max_opt(), ultimately we are limited by
the bdev atomic write limit. If xfs_get_atomic_write_max() does not report
> 1x blocksize, then just continue to report 0 as before.
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
[djwong: update comments in the helper functions]
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: John Garry <john.g.garry@oracle.com>
---
fs/xfs/xfs_file.c | 2 +-
fs/xfs/xfs_iops.c | 52 +++++++++++++++++++++++++++++++++++++++++------
2 files changed, 47 insertions(+), 7 deletions(-)
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index f4a66ff85748..48254a72071b 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -1557,7 +1557,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_hw_atomic_write(XFS_I(inode)))
+ if (xfs_get_atomic_write_min(XFS_I(inode)) > 0)
file->f_mode |= FMODE_CAN_ATOMIC_WRITE;
return generic_file_open(inode, file);
}
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index 77a0606e9dc9..8cddbb7c149b 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -605,27 +605,67 @@ unsigned int
xfs_get_atomic_write_min(
struct xfs_inode *ip)
{
- if (!xfs_inode_can_hw_atomic_write(ip))
- return 0;
+ struct xfs_mount *mp = ip->i_mount;
+
+ /*
+ * If we can complete an atomic write via atomic out of place writes,
+ * then advertise a minimum size of one fsblock. Without this
+ * mechanism, we can only guarantee atomic writes up to a single LBA.
+ *
+ * If out of place writes are not available, we can guarantee an atomic
+ * write of exactly one single fsblock if the bdev will make that
+ * guarantee for us.
+ */
+ if (xfs_inode_can_hw_atomic_write(ip) || xfs_can_sw_atomic_write(mp))
+ return mp->m_sb.sb_blocksize;
- return ip->i_mount->m_sb.sb_blocksize;
+ return 0;
}
unsigned int
xfs_get_atomic_write_max(
struct xfs_inode *ip)
{
- if (!xfs_inode_can_hw_atomic_write(ip))
+ struct xfs_mount *mp = ip->i_mount;
+
+ /*
+ * If out of place writes are not available, we can guarantee an atomic
+ * write of exactly one single fsblock if the bdev will make that
+ * guarantee for us.
+ */
+ if (!xfs_can_sw_atomic_write(mp)) {
+ if (xfs_inode_can_hw_atomic_write(ip))
+ return mp->m_sb.sb_blocksize;
return 0;
+ }
- return ip->i_mount->m_sb.sb_blocksize;
+ /*
+ * If we can complete an atomic write via atomic out of place writes,
+ * then advertise a maximum size of whatever we can complete through
+ * that means. Hardware support is reported via max_opt, not here.
+ */
+ if (XFS_IS_REALTIME_INODE(ip))
+ return XFS_FSB_TO_B(mp, mp->m_groups[XG_TYPE_RTG].awu_max);
+ return XFS_FSB_TO_B(mp, mp->m_groups[XG_TYPE_AG].awu_max);
}
unsigned int
xfs_get_atomic_write_max_opt(
struct xfs_inode *ip)
{
- return 0;
+ unsigned int awu_max = xfs_get_atomic_write_max(ip);
+
+ /* if the max is 1x block, then just keep behaviour that opt is 0 */
+ if (awu_max <= ip->i_mount->m_sb.sb_blocksize)
+ return 0;
+
+ /*
+ * Advertise the maximum size of an atomic write that we can tell the
+ * block device to perform for us. In general the bdev limit will be
+ * less than our out of place write limit, but we don't want to exceed
+ * the awu_max.
+ */
+ return min(awu_max, xfs_inode_buftarg(ip)->bt_bdev_awu_max);
}
static void
--
2.31.1
^ permalink raw reply related
* [PATCH v11 13/16] xfs: add xfs_file_dio_write_atomic()
From: John Garry @ 2025-05-04 8:59 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: <20250504085923.1895402-1-john.g.garry@oracle.com>
Add xfs_file_dio_write_atomic() for dedicated handling of atomic writes.
Now HW offload will not be required to support atomic writes and is
an optional feature.
CoW-based atomic writes will be supported with out-of-places write and
atomic extent remapping.
Either mode of operation may be used for an atomic write, depending on the
circumstances.
The preferred method is HW offload as it will be faster. If HW offload is
not available then we always use the CoW-based method. If HW offload is
available but not possible to use, then again we use the CoW-based method.
If available, HW offload would not be possible for the write length
exceeding the HW offload limit, the write spanning multiple extents,
unaligned disk blocks, etc.
Apart from the write exceeding the HW offload limit, other conditions for
HW offload usage can only be detected in the iomap handling for the write.
As such, we use a fallback method to issue the write if we detect in the
->iomap_begin() handler that HW offload is not possible. Special code
-ENOPROTOOPT is returned from ->iomap_begin() to inform that HW offload is
not possible.
In other words, atomic writes are supported on any filesystem that can
perform out of place write remapping atomically (i.e. reflink) up to
some fairly large size. If the conditions are right (a single correctly
aligned overwrite mapping) then the filesystem will use any available
hardware support to avoid the filesystem metadata updates.
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: John Garry <john.g.garry@oracle.com>
---
fs/xfs/xfs_file.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 68 insertions(+)
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 32883ec8ca2e..f4a66ff85748 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -728,6 +728,72 @@ xfs_file_dio_write_zoned(
return ret;
}
+/*
+ * Handle block atomic writes
+ *
+ * Two methods of atomic writes are supported:
+ * - REQ_ATOMIC-based, which would typically use some form of HW offload in the
+ * disk
+ * - COW-based, which uses a COW fork as a staging extent for data updates
+ * before atomically updating extent mappings for the range being written
+ *
+ */
+static noinline ssize_t
+xfs_file_dio_write_atomic(
+ struct xfs_inode *ip,
+ struct kiocb *iocb,
+ struct iov_iter *from)
+{
+ unsigned int iolock = XFS_IOLOCK_SHARED;
+ ssize_t ret, ocount = iov_iter_count(from);
+ const struct iomap_ops *dops;
+
+ /*
+ * HW offload should be faster, so try that first if it is already
+ * known that the write length is not too large.
+ */
+ if (ocount > xfs_inode_buftarg(ip)->bt_bdev_awu_max)
+ dops = &xfs_atomic_write_cow_iomap_ops;
+ else
+ dops = &xfs_direct_write_iomap_ops;
+
+retry:
+ ret = xfs_ilock_iocb_for_write(iocb, &iolock);
+ if (ret)
+ return ret;
+
+ ret = xfs_file_write_checks(iocb, from, &iolock, NULL);
+ if (ret)
+ goto out_unlock;
+
+ /* Demote similar to xfs_file_dio_write_aligned() */
+ if (iolock == XFS_IOLOCK_EXCL) {
+ xfs_ilock_demote(ip, XFS_IOLOCK_EXCL);
+ iolock = XFS_IOLOCK_SHARED;
+ }
+
+ trace_xfs_file_direct_write(iocb, from);
+ ret = iomap_dio_rw(iocb, from, dops, &xfs_dio_write_ops,
+ 0, NULL, 0);
+
+ /*
+ * The retry mechanism is based on the ->iomap_begin method returning
+ * -ENOPROTOOPT, which would be when the REQ_ATOMIC-based write is not
+ * possible. The REQ_ATOMIC-based method typically not be possible if
+ * the write spans multiple extents or the disk blocks are misaligned.
+ */
+ if (ret == -ENOPROTOOPT && dops == &xfs_direct_write_iomap_ops) {
+ xfs_iunlock(ip, iolock);
+ dops = &xfs_atomic_write_cow_iomap_ops;
+ goto retry;
+ }
+
+out_unlock:
+ if (iolock)
+ xfs_iunlock(ip, iolock);
+ return ret;
+}
+
/*
* Handle block unaligned direct I/O writes
*
@@ -843,6 +909,8 @@ xfs_file_dio_write(
return xfs_file_dio_write_unaligned(ip, iocb, from);
if (xfs_is_zoned_inode(ip))
return xfs_file_dio_write_zoned(ip, iocb, from);
+ if (iocb->ki_flags & IOCB_ATOMIC)
+ return xfs_file_dio_write_atomic(ip, iocb, from);
return xfs_file_dio_write_aligned(ip, iocb, from,
&xfs_direct_write_iomap_ops, &xfs_dio_write_ops, NULL);
}
--
2.31.1
^ permalink raw reply related
* [PATCH v11 06/16] xfs: ignore HW which cannot atomic write a single block
From: John Garry @ 2025-05-04 8:59 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: <20250504085923.1895402-1-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>
Signed-off-by: John Garry <john.g.garry@oracle.com>
---
fs/xfs/xfs_buf.c | 44 ++++++++++++++++++++++++++++++++++++--------
fs/xfs/xfs_buf.h | 4 ++--
fs/xfs/xfs_inode.h | 14 ++------------
fs/xfs/xfs_super.c | 6 +++---
4 files changed, 43 insertions(+), 25 deletions(-)
diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index 292891d6ff69..770dc4ca79e4 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -1714,13 +1714,45 @@ xfs_free_buftarg(
kfree(btp);
}
+/*
+ * Configure this buffer target for hardware-assisted atomic writes if the
+ * underlying block device supports is congruent with the filesystem geometry.
+ */
+static inline void
+xfs_configure_buftarg_atomic_writes(
+ struct xfs_buftarg *btp)
+{
+ struct xfs_mount *mp = btp->bt_mount;
+ unsigned int min_bytes, max_bytes;
+
+ 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;
+}
+
+/* Configure a buffer target that abstracts a block device. */
int
-xfs_setsize_buftarg(
+xfs_configure_buftarg(
struct xfs_buftarg *btp,
unsigned int sectorsize)
{
int error;
+ ASSERT(btp->bt_bdev != NULL);
+
/* Set up metadata sector size info */
btp->bt_meta_sectorsize = sectorsize;
btp->bt_meta_sectormask = sectorsize - 1;
@@ -1733,6 +1765,9 @@ xfs_setsize_buftarg(
return -EINVAL;
}
+ if (bdev_can_atomic_write(btp->bt_bdev))
+ xfs_configure_buftarg_atomic_writes(btp);
+
return 0;
}
@@ -1795,13 +1830,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_buf.h b/fs/xfs/xfs_buf.h
index 132210705602..7759fe35d93e 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;
@@ -374,7 +374,7 @@ struct xfs_buftarg *xfs_alloc_buftarg(struct xfs_mount *mp,
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);
+extern int xfs_configure_buftarg(struct xfs_buftarg *, unsigned int);
#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 bdbbff0d8d99..d7e2b902ef5c 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_super.c b/fs/xfs/xfs_super.c
index 45e188466e51..04e361664710 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -558,7 +558,7 @@ xfs_setup_devices(
{
int error;
- error = xfs_setsize_buftarg(mp->m_ddev_targp, mp->m_sb.sb_sectsize);
+ error = xfs_configure_buftarg(mp->m_ddev_targp, mp->m_sb.sb_sectsize);
if (error)
return error;
@@ -567,7 +567,7 @@ xfs_setup_devices(
if (xfs_has_sector(mp))
log_sector_size = mp->m_sb.sb_logsectsize;
- error = xfs_setsize_buftarg(mp->m_logdev_targp,
+ error = xfs_configure_buftarg(mp->m_logdev_targp,
log_sector_size);
if (error)
return error;
@@ -581,7 +581,7 @@ xfs_setup_devices(
}
mp->m_rtdev_targp = mp->m_ddev_targp;
} else if (mp->m_rtname) {
- error = xfs_setsize_buftarg(mp->m_rtdev_targp,
+ error = xfs_configure_buftarg(mp->m_rtdev_targp,
mp->m_sb.sb_sectsize);
if (error)
return error;
--
2.31.1
^ permalink raw reply related
* [PATCH v11 07/16] xfs: allow block allocator to take an alignment hint
From: John Garry @ 2025-05-04 8:59 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: <20250504085923.1895402-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: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: John Garry <john.g.garry@oracle.com>
---
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 v11 05/16] xfs: rename xfs_inode_can_atomicwrite() -> xfs_inode_can_hw_atomic_write()
From: John Garry @ 2025-05-04 8:59 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: <20250504085923.1895402-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().
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>
Signed-off-by: John Garry <john.g.garry@oracle.com>
---
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 v11 09/16] xfs: refine atomic write size check in xfs_file_write_iter()
From: John Garry @ 2025-05-04 8:59 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: <20250504085923.1895402-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: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: John Garry <john.g.garry@oracle.com>
---
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
* [PATCH v11 10/16] xfs: add xfs_atomic_write_cow_iomap_begin()
From: John Garry @ 2025-05-04 8:59 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: <20250504085923.1895402-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().
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>
Signed-off-by: John Garry <john.g.garry@oracle.com>
---
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 v11 14/16] xfs: add xfs_calc_atomic_write_unit_max()
From: John Garry @ 2025-05-04 8:59 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: <20250504085923.1895402-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.
[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: rounddown power-of-2 always]
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: John Garry <john.g.garry@oracle.com>
---
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 00b53f479ece..9c40914afabd 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 rounddown_pow_of_two(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 rounddown_pow_of_two(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(
@@ -1082,6 +1156,13 @@ xfs_mountfs(
xfs_zone_gc_start(mp);
}
+ /*
+ * 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
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