* Re: [PATCH v8 05/15] xfs: ignore HW which cannot atomic write a single block
From: Christoph Hellwig @ 2025-04-23 8:33 UTC (permalink / raw)
To: John Garry
Cc: Christoph Hellwig, Darrick J. Wong, 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: <f27ea8f7-700a-4fb1-b9cd-a0cba04c9e47@oracle.com>
On Wed, Apr 23, 2025 at 09:28:14AM +0100, John Garry wrote:
>> But maybe we should just delay setting the atomic values until later so
>> that it can be done in a single pass? E.g. into xfs_setsize_buftarg
>> which then should probably be rename to something like
>> xfs_buftarg_setup.
>>
>
> How about just do away with btp->bt_bdev_awu_{min, max} struct members, and
> call bdev_atomic_write_unit_max(mp->m_ddev_targp->bt_bdev) [and same for
> RT] to later to set the mp awu max values at mountfs time? I think that
> would work..
Sounds reasonable.
^ permalink raw reply
* Re: [PATCH v4 3/3] fs: introduce getfsxattrat and setfsxattrat syscalls
From: Jan Kara @ 2025-04-23 9:53 UTC (permalink / raw)
To: Christian Brauner
Cc: Andrey Albershteyn, Richard Henderson, Matt Turner, Russell King,
Catalin Marinas, Will Deacon, Geert Uytterhoeven, Michal Simek,
Thomas Bogendoerfer, James E.J. Bottomley, Helge Deller,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
Yoshinori Sato, Rich Felker, John Paul Adrian Glaubitz,
David S. Miller, Andreas Larsson, Andy Lutomirski,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Chris Zankel, Max Filippov, Alexander Viro,
Jan Kara, Mickaël Salaün, Günther Noack,
Arnd Bergmann, Pali Rohár, Paul Moore, James Morris,
Serge E. Hallyn, linux-alpha, linux-kernel, linux-arm-kernel,
linux-m68k, linux-mips, linux-parisc, linuxppc-dev, linux-s390,
linux-sh, sparclinux, linux-fsdevel, linux-security-module,
linux-api, linux-arch, linux-xfs
In-Reply-To: <20250422-abbekommen-begierde-bcf48dd74a2e@brauner>
On Tue 22-04-25 16:59:02, Christian Brauner wrote:
> On Fri, Mar 21, 2025 at 08:48:42PM +0100, Andrey Albershteyn wrote:
> > From: Andrey Albershteyn <aalbersh@redhat.com>
> >
> > Introduce getfsxattrat and setfsxattrat syscalls to manipulate inode
> > extended attributes/flags. The syscalls take parent directory fd and
> > path to the child together with struct fsxattr.
> >
> > This is an alternative to FS_IOC_FSSETXATTR ioctl with a difference
> > that file don't need to be open as we can reference it with a path
> > instead of fd. By having this we can manipulated inode extended
> > attributes not only on regular files but also on special ones. This
> > is not possible with FS_IOC_FSSETXATTR ioctl as with special files
> > we can not call ioctl() directly on the filesystem inode using fd.
> >
> > This patch adds two new syscalls which allows userspace to get/set
> > extended inode attributes on special files by using parent directory
> > and a path - *at() like syscall.
> >
> > CC: linux-api@vger.kernel.org
> > CC: linux-fsdevel@vger.kernel.org
> > CC: linux-xfs@vger.kernel.org
> > Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com>
> > Acked-by: Arnd Bergmann <arnd@arndb.de>
...
> > + struct fsxattr __user *, ufsx, size_t, usize,
> > + unsigned int, at_flags)
> > +{
> > + struct fileattr fa = {};
> > + struct path filepath;
> > + int error;
> > + unsigned int lookup_flags = 0;
> > + struct filename *name;
> > + struct fsxattr fsx = {};
> > +
> > + BUILD_BUG_ON(sizeof(struct fsxattr) < FSXATTR_SIZE_VER0);
> > + BUILD_BUG_ON(sizeof(struct fsxattr) != FSXATTR_SIZE_LATEST);
> > +
> > + if ((at_flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) != 0)
> > + return -EINVAL;
> > +
> > + if (!(at_flags & AT_SYMLINK_NOFOLLOW))
> > + lookup_flags |= LOOKUP_FOLLOW;
> > +
> > + if (at_flags & AT_EMPTY_PATH)
> > + lookup_flags |= LOOKUP_EMPTY;
> > +
> > + if (usize > PAGE_SIZE)
> > + return -E2BIG;
> > +
> > + if (usize < FSXATTR_SIZE_VER0)
> > + return -EINVAL;
> > +
> > + name = getname_maybe_null(filename, at_flags);
> > + if (!name) {
>
> This is broken as it doesn't handle AT_FDCWD correctly. You need:
>
> name = getname_maybe_null(filename, at_flags);
> if (IS_ERR(name))
> return PTR_ERR(name);
>
> if (!name && dfd >= 0) {
> CLASS(fd, f)(dfd);
Ah, you're indeed right that if dfd == AT_FDCWD and filename == NULL, the
we should operate on cwd but we'd bail with error here. I've missed that
during my review. But as far as I've checked the same bug is there in
path_setxattrat() and path_getxattrat() so we should fix this there as
well?
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH v7 11/14] xfs: add xfs_file_dio_write_atomic()
From: Darrick J. Wong @ 2025-04-23 14:51 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: <20250423081902.GD28307@lst.de>
On Wed, Apr 23, 2025 at 10:19:02AM +0200, Christoph Hellwig wrote:
> On Wed, Apr 23, 2025 at 07:42:51AM +0200, Christoph Hellwig wrote:
> > On Mon, Apr 21, 2025 at 09:42:41AM -0700, Darrick J. Wong wrote:
> > > Well it turns out that was a stupid question -- zoned=1 can't be enabled
> > > with reflink, which means there's no cow fallback so atomic writes just
> > > plain don't work:
> >
> > Exactly. It is still on my todo list to support it, but there are a
> > few higher priority items on it as well, in addition to constant
> > interruptions for patch reviews :)
>
> Actually, for zoned we don't need reflink support - as we always write
> out place only the stuffing of multiple remaps into a single transaction
> is needed. Still no need to force John to do this work, I can look into
> this (probably fairly trivial) work once we have good enough test cases
> in xfstests that I can trust them to verify I got things right.
<nod> I think we'll need a new fstest to set an error trap on a step
midway through a multi-extent ioend completion to make sure that's
actually working properly. And probably new write commands for fsx and
fsstress to exercise RWF_ATOMIC.
(Catherine: please send the accumulated atomic writes fstests)
--D
^ permalink raw reply
* Re: [PATCH v7 11/14] xfs: add xfs_file_dio_write_atomic()
From: Darrick J. Wong @ 2025-04-23 14:53 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: <20250423054251.GA23087@lst.de>
On Wed, Apr 23, 2025 at 07:42:51AM +0200, Christoph Hellwig wrote:
> On Mon, Apr 21, 2025 at 09:42:41AM -0700, Darrick J. Wong wrote:
> > Well it turns out that was a stupid question -- zoned=1 can't be enabled
> > with reflink, which means there's no cow fallback so atomic writes just
> > plain don't work:
>
> Exactly. It is still on my todo list to support it, but there are a
> few higher priority items on it as well, in addition to constant
> interruptions for patch reviews :)
Heheh I'll try to go dig through all the stuff you've sent to the list
yesterday, though I've got an hour of paperwork ahead of me that has to
get done asap. :/
--D
> > I /think/ all you'd have to do is create an xfs_zoned_end_atomic_io
> > function that does what xfs_zoned_end_io but with a single
> > tr_atomic_ioend transaction; figure out how to convey "this is an
> > atomic out of place write" to xfs_end_ioend so that it knows to call
> > xfs_zoned_end_atomic_io; and then update the xfs_get_atomic_write*
> > helpers.
>
> Roughly, yes.
>
>
^ permalink raw reply
* Re: [PATCH v8 11/15] xfs: commit CoW-based atomic writes atomically
From: Darrick J. Wong @ 2025-04-23 14:58 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: <20250423082307.GA29539@lst.de>
On Wed, Apr 23, 2025 at 10:23:07AM +0200, Christoph Hellwig wrote:
> On Tue, Apr 22, 2025 at 12:27:35PM +0000, John Garry wrote:
> > +STATIC void
>
> Didn't we phase out STATIC for new code?
>
> > +xfs_calc_default_atomic_ioend_reservation(
> > + struct xfs_mount *mp,
> > + struct xfs_trans_resv *resp)
> > +{
> > + if (xfs_has_reflink(mp))
> > + resp->tr_atomic_ioend = resp->tr_itruncate;
> > + else
> > + memset(&resp->tr_atomic_ioend, 0,
> > + sizeof(resp->tr_atomic_ioend));
> > +}
>
> What is the point of zeroing out the structure for the non-reflink
> case? Just as a poision for not using it when not supported as no
> code should be doing that? Just thinking of this because it is a
> potentially nasty landmine for the zoned atomic support.
Yes. I thought about adding a really stupid helper:
static inline bool xfs_has_sw_atomic_write(struct xfs_mount *mp)
{
return xfs_has_reflink(mp);
}
But that seemed too stupid so I left it out. Maybe it wasn't so dumb,
since that would be where you'd enable ZNS support by changing that to:
return xfs_has_reflink(mp) || xfs_has_zoned(mp);
--D
> Otherwise looks good:
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
>
^ permalink raw reply
* Re: [PATCH v8 15/15] xfs: allow sysadmins to specify a maximum atomic write limit at mount time
From: Darrick J. Wong @ 2025-04-23 15:01 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: <20250423083209.GA30432@lst.de>
On Wed, Apr 23, 2025 at 10:32:09AM +0200, Christoph Hellwig wrote:
> On Tue, Apr 22, 2025 at 12:27:39PM +0000, John Garry wrote:
> > From: "Darrick J. Wong" <djwong@kernel.org>
> >
> > Introduce a mount option to allow sysadmins to specify the maximum size
> > of an atomic write. If the filesystem can work with the supplied value,
> > that becomes the new guaranteed maximum.
> >
> > The value mustn't be too big for the existing filesystem geometry (max
> > write size, max AG/rtgroup size). We dynamically recompute the
> > tr_atomic_write transaction reservation based on the given block size,
> > check that the current log size isn't less than the new minimum log size
> > constraints, and set a new maximum.
> >
> > The actual software atomic write max is still computed based off of
> > tr_atomic_ioend the same way it has for the past few commits.
>
> The cap is a good idea, but a mount option for something that has
> strong effects for persistent application formats is a little suboptimal.
> But adding a sb field and an incompat bit wouldn't be great either.
>
> Maybe this another use case for a trusted xattr on the root inode like
> the autofsck flag?
That would be even better, since you could set it at mkfs time and it
would persist until the next xfs_property set call.
--D
^ permalink raw reply
* Re: [PATCH v8 05/15] xfs: ignore HW which cannot atomic write a single block
From: Darrick J. Wong @ 2025-04-23 15:12 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: <20250423083317.GB30432@lst.de>
On Wed, Apr 23, 2025 at 10:33:17AM +0200, Christoph Hellwig wrote:
> On Wed, Apr 23, 2025 at 09:28:14AM +0100, John Garry wrote:
> >> But maybe we should just delay setting the atomic values until later so
> >> that it can be done in a single pass? E.g. into xfs_setsize_buftarg
> >> which then should probably be rename to something like
> >> xfs_buftarg_setup.
> >>
> >
> > How about just do away with btp->bt_bdev_awu_{min, max} struct members, and
> > call bdev_atomic_write_unit_max(mp->m_ddev_targp->bt_bdev) [and same for
> > RT] to later to set the mp awu max values at mountfs time? I think that
> > would work..
>
> Sounds reasonable.
I disagree, leaving the hardware awu_min/max in the buftarg makes more
sense to me because the buftarg is our abstraction for a block device,
and these fields describe the atomic write units that we can use with
that block device.
IOWs, I don't like dumping even more into struct xfs_mount. xfs_group
has an awu_max for the software fallback, xfs_buftarg has an awu_min/max
for hardware, and even this V8 has yet a third pair of awu_min/max in
xfs_mount which I think is just the buftarg version but possibly
truncated. I find those last two pairs confusing.
--D
^ permalink raw reply
* Re: [PATCH v8 15/15] xfs: allow sysadmins to specify a maximum atomic write limit at mount time
From: Darrick J. Wong @ 2025-04-23 15:21 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: <20250423150110.GB25675@frogsfrogsfrogs>
On Wed, Apr 23, 2025 at 08:01:10AM -0700, Darrick J. Wong wrote:
> On Wed, Apr 23, 2025 at 10:32:09AM +0200, Christoph Hellwig wrote:
> > On Tue, Apr 22, 2025 at 12:27:39PM +0000, John Garry wrote:
> > > From: "Darrick J. Wong" <djwong@kernel.org>
> > >
> > > Introduce a mount option to allow sysadmins to specify the maximum size
> > > of an atomic write. If the filesystem can work with the supplied value,
> > > that becomes the new guaranteed maximum.
> > >
> > > The value mustn't be too big for the existing filesystem geometry (max
> > > write size, max AG/rtgroup size). We dynamically recompute the
> > > tr_atomic_write transaction reservation based on the given block size,
> > > check that the current log size isn't less than the new minimum log size
> > > constraints, and set a new maximum.
> > >
> > > The actual software atomic write max is still computed based off of
> > > tr_atomic_ioend the same way it has for the past few commits.
> >
> > The cap is a good idea, but a mount option for something that has
> > strong effects for persistent application formats is a little suboptimal.
> > But adding a sb field and an incompat bit wouldn't be great either.
> >
> > Maybe this another use case for a trusted xattr on the root inode like
> > the autofsck flag?
>
> That would be even better, since you could set it at mkfs time and it
> would persist until the next xfs_property set call.
[/me hands himself another coffee]
The only problem is, setting the property while the fs is mounted does
not change the actual fs capability until the next mount since
properties are regular (and not magic) xattrs.
--D
> --D
>
^ permalink raw reply
* Re: [PATCH v8 05/15] xfs: ignore HW which cannot atomic write a single block
From: Christoph Hellwig @ 2025-04-23 15:46 UTC (permalink / raw)
To: Darrick J. Wong
Cc: Christoph Hellwig, John Garry, brauner, viro, jack, cem,
linux-fsdevel, dchinner, linux-xfs, linux-kernel, ojaswin,
ritesh.list, martin.petersen, linux-ext4, linux-block,
catherine.hoang, linux-api
In-Reply-To: <20250423151224.GC25675@frogsfrogsfrogs>
On Wed, Apr 23, 2025 at 08:12:24AM -0700, Darrick J. Wong wrote:
> I disagree, leaving the hardware awu_min/max in the buftarg makes more
> sense to me because the buftarg is our abstraction for a block device,
> and these fields describe the atomic write units that we can use with
> that block device.
Yeah. If you want to keep it I'd suggest we go back to my earlier
suggestion.
^ permalink raw reply
* Re: [PATCH v8 11/15] xfs: commit CoW-based atomic writes atomically
From: Christoph Hellwig @ 2025-04-23 15:53 UTC (permalink / raw)
To: Darrick J. Wong
Cc: Christoph Hellwig, John Garry, brauner, viro, jack, cem,
linux-fsdevel, dchinner, linux-xfs, linux-kernel, ojaswin,
ritesh.list, martin.petersen, linux-ext4, linux-block,
catherine.hoang, linux-api
In-Reply-To: <20250423145850.GA25675@frogsfrogsfrogs>
On Wed, Apr 23, 2025 at 07:58:50AM -0700, Darrick J. Wong wrote:
> > > +xfs_calc_default_atomic_ioend_reservation(
> > > + struct xfs_mount *mp,
> > > + struct xfs_trans_resv *resp)
> > > +{
> > > + if (xfs_has_reflink(mp))
> > > + resp->tr_atomic_ioend = resp->tr_itruncate;
> > > + else
> > > + memset(&resp->tr_atomic_ioend, 0,
> > > + sizeof(resp->tr_atomic_ioend));
> > > +}
> >
> > What is the point of zeroing out the structure for the non-reflink
> > case? Just as a poision for not using it when not supported as no
> > code should be doing that? Just thinking of this because it is a
> > potentially nasty landmine for the zoned atomic support.
>
> Yes. I thought about adding a really stupid helper:
Why don't we just always set up the xfs_trans_resv structure? We
do that for all kinds of other transactions not supported as well,
don't we?
> static inline bool xfs_has_sw_atomic_write(struct xfs_mount *mp)
> {
> return xfs_has_reflink(mp);
> }
>
> But that seemed too stupid so I left it out. Maybe it wasn't so dumb,
> since that would be where you'd enable ZNS support by changing that to:
>
> return xfs_has_reflink(mp) || xfs_has_zoned(mp);
But that helper might actually be useful in various places, so
independent of the above I'm in favor of it.
^ permalink raw reply
* Re: [PATCH v8 11/15] xfs: commit CoW-based atomic writes atomically
From: Darrick J. Wong @ 2025-04-23 15:58 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: <20250423155340.GA32225@lst.de>
On Wed, Apr 23, 2025 at 05:53:40PM +0200, Christoph Hellwig wrote:
> On Wed, Apr 23, 2025 at 07:58:50AM -0700, Darrick J. Wong wrote:
> > > > +xfs_calc_default_atomic_ioend_reservation(
> > > > + struct xfs_mount *mp,
> > > > + struct xfs_trans_resv *resp)
> > > > +{
> > > > + if (xfs_has_reflink(mp))
> > > > + resp->tr_atomic_ioend = resp->tr_itruncate;
> > > > + else
> > > > + memset(&resp->tr_atomic_ioend, 0,
> > > > + sizeof(resp->tr_atomic_ioend));
> > > > +}
> > >
> > > What is the point of zeroing out the structure for the non-reflink
> > > case? Just as a poision for not using it when not supported as no
> > > code should be doing that? Just thinking of this because it is a
> > > potentially nasty landmine for the zoned atomic support.
> >
> > Yes. I thought about adding a really stupid helper:
>
> Why don't we just always set up the xfs_trans_resv structure? We
> do that for all kinds of other transactions not supported as well,
> don't we?
Works for me. There's really no harm in it mirroring tr_itruncate since
it won't affect the log size calculation.
> > static inline bool xfs_has_sw_atomic_write(struct xfs_mount *mp)
> > {
> > return xfs_has_reflink(mp);
> > }
> >
> > But that seemed too stupid so I left it out. Maybe it wasn't so dumb,
> > since that would be where you'd enable ZNS support by changing that to:
> >
> > return xfs_has_reflink(mp) || xfs_has_zoned(mp);
>
> But that helper might actually be useful in various places, so
> independent of the above I'm in favor of it.
<nod> John, who should work on the next round, you or me?
--D
^ permalink raw reply
* Re: [PATCH v8 11/15] xfs: commit CoW-based atomic writes atomically
From: John Garry @ 2025-04-23 16:38 UTC (permalink / raw)
To: Darrick J. Wong, Christoph Hellwig
Cc: 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: <20250423155851.GL25700@frogsfrogsfrogs>
On 23/04/2025 16:58, Darrick J. Wong wrote:
> <nod> John, who should work on the next round, you or me?
If you can just share your changes then I can re-post. I can fix up the
smaller things, like commit messages.
Thanks,
John
^ permalink raw reply
* Re: newlines in filenames; POSIX.1-2024
From: Theodore Ts'o @ 2025-04-24 0:05 UTC (permalink / raw)
To: Alejandro Colomar; +Cc: linux-kernel, linux-api, linux-man
In-Reply-To: <3wpydsib5maytq4m4ve4b7wfbfkxwzd5m6u5woqr43qr6mickk@gw4ww6vvgyo5>
On Wed, Apr 23, 2025 at 09:31:42AM +0200, Alejandro Colomar wrote:
>
> <http://austingroupbugs.net/view.php?id=251>
Ugh. Reading through that bug, despite the fact that the original
proposal was *significantly* bared down, has greatly reduced my
respect for the Austin Group.
One of the people in that bug argued unironically that using pipes
should be deprecated. i.e., that somehow "find . ... -print0 | xargs
-0 ..." was a security problem.
<<Sigh>>
Other people pointed out that creating proscriptions that were not
implemented by many/most historical implementations would fragment the
standard and decrease the respect people would have towards the POSIX
specification. That was the "toilet paper" comment which you
referenced.
Well, they got that right.
> I think a mode for disallowing _any control characters_ (aka
> [:cntrl:], aka 0-31) would be a good choice.
As the Austin Group Bug pointed out, the problem is that the control
characters can be printable characters, depending on the code page
that you might be using. The example that was given was cp437.
The problem is that historically speaking, the kernel does *not* know
about what locale that is in use. We made an exception to handle case
folding, where we added Unicode tables into the kernel. Some would
say that was a major mistake, and it's certainly been a headache.
- Ted
^ permalink raw reply
* Re: newlines in filenames; POSIX.1-2024
From: Alejandro Colomar @ 2025-04-24 7:00 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: linux-kernel, linux-api, linux-man
In-Reply-To: <20250424000534.GB765145@mit.edu>
[-- Attachment #1: Type: text/plain, Size: 2027 bytes --]
Hi Ted,
On Wed, Apr 23, 2025 at 07:05:34PM -0500, Theodore Ts'o wrote:
> On Wed, Apr 23, 2025 at 09:31:42AM +0200, Alejandro Colomar wrote:
> >
> > <http://austingroupbugs.net/view.php?id=251>
>
> Ugh. Reading through that bug, despite the fact that the original
> proposal was *significantly* bared down, has greatly reduced my
> respect for the Austin Group.
>
> One of the people in that bug argued unironically that using pipes
> should be deprecated. i.e., that somehow "find . ... -print0 | xargs
> -0 ..." was a security problem.
Huh! I hadn't read that part.
> <<Sigh>>
>
> Other people pointed out that creating proscriptions that were not
> implemented by many/most historical implementations would fragment the
> standard and decrease the respect people would have towards the POSIX
> specification. That was the "toilet paper" comment which you
> referenced.
>
> Well, they got that right.
>
> > I think a mode for disallowing _any control characters_ (aka
> > [:cntrl:], aka 0-31) would be a good choice.
>
> As the Austin Group Bug pointed out, the problem is that the control
> characters can be printable characters, depending on the code page
> that you might be using. The example that was given was cp437.
>
> The problem is that historically speaking, the kernel does *not* know
> about what locale that is in use. We made an exception to handle case
> folding, where we added Unicode tables into the kernel. Some would
> say that was a major mistake, and it's certainly been a headache.
Hmmmm, I'm not too worried about that code page for my own system, and
most people aren't either. I still believe it would be good to have the
option to forbid 0-31, and let those users who need access file systems
with such weird conventions continue using the default (that is, not
enabling the new mode). I think ASCII has won the character wars;
especially in POSIX systems.
Have a lovely day!
Alex
--
<https://www.alejandro-colomar.es/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v4 3/3] fs: introduce getfsxattrat and setfsxattrat syscalls
From: Christian Brauner @ 2025-04-24 9:06 UTC (permalink / raw)
To: Jan Kara
Cc: Andrey Albershteyn, Richard Henderson, Matt Turner, Russell King,
Catalin Marinas, Will Deacon, Geert Uytterhoeven, Michal Simek,
Thomas Bogendoerfer, James E.J. Bottomley, Helge Deller,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
Yoshinori Sato, Rich Felker, John Paul Adrian Glaubitz,
David S. Miller, Andreas Larsson, Andy Lutomirski,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Chris Zankel, Max Filippov, Alexander Viro,
Mickaël Salaün, Günther Noack, Arnd Bergmann,
Pali Rohár, Paul Moore, James Morris, Serge E. Hallyn,
linux-alpha, linux-kernel, linux-arm-kernel, linux-m68k,
linux-mips, linux-parisc, linuxppc-dev, linux-s390, linux-sh,
sparclinux, linux-fsdevel, linux-security-module, linux-api,
linux-arch, linux-xfs
In-Reply-To: <rbzlwvecvrp4xawwp5nywdq6wp5hgjhrtrabpszv74xmfqbj4f@x7v6eqfc5gcd>
On Wed, Apr 23, 2025 at 11:53:25AM +0200, Jan Kara wrote:
> On Tue 22-04-25 16:59:02, Christian Brauner wrote:
> > On Fri, Mar 21, 2025 at 08:48:42PM +0100, Andrey Albershteyn wrote:
> > > From: Andrey Albershteyn <aalbersh@redhat.com>
> > >
> > > Introduce getfsxattrat and setfsxattrat syscalls to manipulate inode
> > > extended attributes/flags. The syscalls take parent directory fd and
> > > path to the child together with struct fsxattr.
> > >
> > > This is an alternative to FS_IOC_FSSETXATTR ioctl with a difference
> > > that file don't need to be open as we can reference it with a path
> > > instead of fd. By having this we can manipulated inode extended
> > > attributes not only on regular files but also on special ones. This
> > > is not possible with FS_IOC_FSSETXATTR ioctl as with special files
> > > we can not call ioctl() directly on the filesystem inode using fd.
> > >
> > > This patch adds two new syscalls which allows userspace to get/set
> > > extended inode attributes on special files by using parent directory
> > > and a path - *at() like syscall.
> > >
> > > CC: linux-api@vger.kernel.org
> > > CC: linux-fsdevel@vger.kernel.org
> > > CC: linux-xfs@vger.kernel.org
> > > Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com>
> > > Acked-by: Arnd Bergmann <arnd@arndb.de>
> ...
> > > + struct fsxattr __user *, ufsx, size_t, usize,
> > > + unsigned int, at_flags)
> > > +{
> > > + struct fileattr fa = {};
> > > + struct path filepath;
> > > + int error;
> > > + unsigned int lookup_flags = 0;
> > > + struct filename *name;
> > > + struct fsxattr fsx = {};
> > > +
> > > + BUILD_BUG_ON(sizeof(struct fsxattr) < FSXATTR_SIZE_VER0);
> > > + BUILD_BUG_ON(sizeof(struct fsxattr) != FSXATTR_SIZE_LATEST);
> > > +
> > > + if ((at_flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) != 0)
> > > + return -EINVAL;
> > > +
> > > + if (!(at_flags & AT_SYMLINK_NOFOLLOW))
> > > + lookup_flags |= LOOKUP_FOLLOW;
> > > +
> > > + if (at_flags & AT_EMPTY_PATH)
> > > + lookup_flags |= LOOKUP_EMPTY;
> > > +
> > > + if (usize > PAGE_SIZE)
> > > + return -E2BIG;
> > > +
> > > + if (usize < FSXATTR_SIZE_VER0)
> > > + return -EINVAL;
> > > +
> > > + name = getname_maybe_null(filename, at_flags);
> > > + if (!name) {
> >
> > This is broken as it doesn't handle AT_FDCWD correctly. You need:
> >
> > name = getname_maybe_null(filename, at_flags);
> > if (IS_ERR(name))
> > return PTR_ERR(name);
> >
> > if (!name && dfd >= 0) {
> > CLASS(fd, f)(dfd);
>
> Ah, you're indeed right that if dfd == AT_FDCWD and filename == NULL, the
> we should operate on cwd but we'd bail with error here. I've missed that
> during my review. But as far as I've checked the same bug is there in
> path_setxattrat() and path_getxattrat() so we should fix this there as
> well?
Yes, please!
^ permalink raw reply
* Re: [PATCH v4 3/3] fs: introduce getfsxattrat and setfsxattrat syscalls
From: Andrey Albershteyn @ 2025-04-24 17:45 UTC (permalink / raw)
To: Christian Brauner
Cc: Jan Kara, Richard Henderson, Matt Turner, Russell King,
Catalin Marinas, Will Deacon, Geert Uytterhoeven, Michal Simek,
Thomas Bogendoerfer, James E.J. Bottomley, Helge Deller,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
Yoshinori Sato, Rich Felker, John Paul Adrian Glaubitz,
David S. Miller, Andreas Larsson, Andy Lutomirski,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Chris Zankel, Max Filippov, Alexander Viro,
Mickaël Salaün, Günther Noack, Arnd Bergmann,
Pali Rohár, Paul Moore, James Morris, Serge E. Hallyn,
linux-alpha, linux-kernel, linux-arm-kernel, linux-m68k,
linux-mips, linux-parisc, linuxppc-dev, linux-s390, linux-sh,
sparclinux, linux-fsdevel, linux-security-module, linux-api,
linux-arch, linux-xfs
In-Reply-To: <20250424-zuspielen-luxus-3d49b600c3bf@brauner>
On 2025-04-24 11:06:07, Christian Brauner wrote:
> On Wed, Apr 23, 2025 at 11:53:25AM +0200, Jan Kara wrote:
> > On Tue 22-04-25 16:59:02, Christian Brauner wrote:
> > > On Fri, Mar 21, 2025 at 08:48:42PM +0100, Andrey Albershteyn wrote:
> > > > From: Andrey Albershteyn <aalbersh@redhat.com>
> > > >
> > > > Introduce getfsxattrat and setfsxattrat syscalls to manipulate inode
> > > > extended attributes/flags. The syscalls take parent directory fd and
> > > > path to the child together with struct fsxattr.
> > > >
> > > > This is an alternative to FS_IOC_FSSETXATTR ioctl with a difference
> > > > that file don't need to be open as we can reference it with a path
> > > > instead of fd. By having this we can manipulated inode extended
> > > > attributes not only on regular files but also on special ones. This
> > > > is not possible with FS_IOC_FSSETXATTR ioctl as with special files
> > > > we can not call ioctl() directly on the filesystem inode using fd.
> > > >
> > > > This patch adds two new syscalls which allows userspace to get/set
> > > > extended inode attributes on special files by using parent directory
> > > > and a path - *at() like syscall.
> > > >
> > > > CC: linux-api@vger.kernel.org
> > > > CC: linux-fsdevel@vger.kernel.org
> > > > CC: linux-xfs@vger.kernel.org
> > > > Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com>
> > > > Acked-by: Arnd Bergmann <arnd@arndb.de>
> > ...
> > > > + struct fsxattr __user *, ufsx, size_t, usize,
> > > > + unsigned int, at_flags)
> > > > +{
> > > > + struct fileattr fa = {};
> > > > + struct path filepath;
> > > > + int error;
> > > > + unsigned int lookup_flags = 0;
> > > > + struct filename *name;
> > > > + struct fsxattr fsx = {};
> > > > +
> > > > + BUILD_BUG_ON(sizeof(struct fsxattr) < FSXATTR_SIZE_VER0);
> > > > + BUILD_BUG_ON(sizeof(struct fsxattr) != FSXATTR_SIZE_LATEST);
> > > > +
> > > > + if ((at_flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) != 0)
> > > > + return -EINVAL;
> > > > +
> > > > + if (!(at_flags & AT_SYMLINK_NOFOLLOW))
> > > > + lookup_flags |= LOOKUP_FOLLOW;
> > > > +
> > > > + if (at_flags & AT_EMPTY_PATH)
> > > > + lookup_flags |= LOOKUP_EMPTY;
> > > > +
> > > > + if (usize > PAGE_SIZE)
> > > > + return -E2BIG;
> > > > +
> > > > + if (usize < FSXATTR_SIZE_VER0)
> > > > + return -EINVAL;
> > > > +
> > > > + name = getname_maybe_null(filename, at_flags);
> > > > + if (!name) {
> > >
> > > This is broken as it doesn't handle AT_FDCWD correctly. You need:
> > >
> > > name = getname_maybe_null(filename, at_flags);
> > > if (IS_ERR(name))
> > > return PTR_ERR(name);
> > >
> > > if (!name && dfd >= 0) {
> > > CLASS(fd, f)(dfd);
> >
> > Ah, you're indeed right that if dfd == AT_FDCWD and filename == NULL, the
> > we should operate on cwd but we'd bail with error here. I've missed that
> > during my review. But as far as I've checked the same bug is there in
> > path_setxattrat() and path_getxattrat() so we should fix this there as
> > well?
>
> Yes, please!
>
Thanks for the review, Christian. I will fix issues you noticed as
suggested. I see that Jan already sent fix for path_[s|g]etxattrat()
so won't do anything here.
--
- Andrey
^ permalink raw reply
* Re: newlines in filenames; POSIX.1-2024
From: Christoph Hellwig @ 2025-04-25 14:54 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: Alejandro Colomar, linux-kernel, linux-api, linux-man
In-Reply-To: <20250422222131.GE569616@mit.edu>
On Tue, Apr 22, 2025 at 05:21:31PM -0500, Theodore Ts'o wrote:
> Do we have any information of which implementations (if any) might
> decide to disallow new-line characters?
AFAIK: none. At least none that matters.
> Personally, I'm not convinced a newline is any different from any
> number of weird-sh*t characters, such as zero-width space Unicode
> characters, ASCII ETX or EOF characters, etc.
It isn't any different in a substantial way.
> I suppose we could add a new mount option which disallows the
> weird-sh*t characters, but I bet it will break some userspace
> programs, and it also begs the question of *which* weird-sh*t
> characters should be disallowed by the kernel.
Don't go there. The only limitations that does make some limited
sense in some limited environment is limiting to valid utf8. We've
already done that for CI, and that's causing enough problems despite
having a use case. Adding random mount options to limit random
characters has a lot of downside but absolutely no actual upside.
>
> > I guess there's no intention to change that behavior. But I should
> > ask. I thought of adding this paragraph to all pages that create
> > file names:
> >
> > +.SH CAVEATS
> > +POSIX.1-2024 encourages implementations to
> > +disallow creation of filenames containing new-line characters.
> > +Linux doesn't follow this,
> > +and allows using new-line characters.
> >
> > Are there any comments?
>
> I think this is giving the Austin Group way more attention/respect
> than they deserve, especially when it's an optional "encourage", but
> whatever...
Yeah. Don't even mention these idiotic recommendations, any attention
spent on this is too much.
^ permalink raw reply
* [PATCH v9 01/15] fs: add atomic write unit max opt to statx
From: John Garry @ 2025-04-25 16:44 UTC (permalink / raw)
To: brauner, djwong, hch, viro, jack, cem
Cc: linux-fsdevel, dchinner, linux-xfs, linux-kernel, ojaswin,
ritesh.list, martin.petersen, linux-ext4, linux-block,
catherine.hoang, linux-api, John Garry
In-Reply-To: <20250425164504.3263637-1-john.g.garry@oracle.com>
XFS will be able to support large atomic writes (atomic write > 1x block)
in future. This will be achieved by using different operating methods,
depending on the size of the write.
Specifically a new method of operation based in FS atomic extent remapping
will be supported in addition to the current HW offload-based method.
The FS method will generally be appreciably slower performing than the
HW-offload method. However the FS method will be typically able to
contribute to achieving a larger atomic write unit max limit.
XFS will support a hybrid mode, where HW offload method will be used when
possible, i.e. HW offload is used when the length of the write is
supported, and for other times FS-based atomic writes will be used.
As such, there is an atomic write length at which the user may experience
appreciably slower performance.
Advertise this limit in a new statx field, stx_atomic_write_unit_max_opt.
When zero, it means that there is no such performance boundary.
Masks STATX{_ATTR}_WRITE_ATOMIC can be used to get this new field. This is
ok for older kernels which don't support this new field, as they would
report 0 in this field (from zeroing in cp_statx()) already. Furthermore
those older kernels don't support large atomic writes - apart from block
fops, but there would be consistent performance there for atomic writes
in range [unit min, unit max].
Signed-off-by: John Garry <john.g.garry@oracle.com>
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
block/bdev.c | 3 ++-
fs/ext4/inode.c | 2 +-
fs/stat.c | 6 +++++-
fs/xfs/xfs_iops.c | 2 +-
include/linux/fs.h | 3 ++-
include/linux/stat.h | 1 +
include/uapi/linux/stat.h | 8 ++++++--
7 files changed, 18 insertions(+), 7 deletions(-)
diff --git a/block/bdev.c b/block/bdev.c
index 4844d1e27b6f..b4afc1763e8e 100644
--- a/block/bdev.c
+++ b/block/bdev.c
@@ -1301,7 +1301,8 @@ void bdev_statx(struct path *path, struct kstat *stat,
generic_fill_statx_atomic_writes(stat,
queue_atomic_write_unit_min_bytes(bd_queue),
- queue_atomic_write_unit_max_bytes(bd_queue));
+ queue_atomic_write_unit_max_bytes(bd_queue),
+ 0);
}
stat->blksize = bdev_io_min(bdev);
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 94c7d2d828a6..cdf01e60fa6d 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -5692,7 +5692,7 @@ int ext4_getattr(struct mnt_idmap *idmap, const struct path *path,
awu_max = sbi->s_awu_max;
}
- generic_fill_statx_atomic_writes(stat, awu_min, awu_max);
+ generic_fill_statx_atomic_writes(stat, awu_min, awu_max, 0);
}
flags = ei->i_flags & EXT4_FL_USER_VISIBLE;
diff --git a/fs/stat.c b/fs/stat.c
index f13308bfdc98..c41855f62d22 100644
--- a/fs/stat.c
+++ b/fs/stat.c
@@ -136,13 +136,15 @@ EXPORT_SYMBOL(generic_fill_statx_attr);
* @stat: Where to fill in the attribute flags
* @unit_min: Minimum supported atomic write length in bytes
* @unit_max: Maximum supported atomic write length in bytes
+ * @unit_max_opt: Optimised maximum supported atomic write length in bytes
*
* Fill in the STATX{_ATTR}_WRITE_ATOMIC flags in the kstat structure from
* atomic write unit_min and unit_max values.
*/
void generic_fill_statx_atomic_writes(struct kstat *stat,
unsigned int unit_min,
- unsigned int unit_max)
+ unsigned int unit_max,
+ unsigned int unit_max_opt)
{
/* Confirm that the request type is known */
stat->result_mask |= STATX_WRITE_ATOMIC;
@@ -153,6 +155,7 @@ void generic_fill_statx_atomic_writes(struct kstat *stat,
if (unit_min) {
stat->atomic_write_unit_min = unit_min;
stat->atomic_write_unit_max = unit_max;
+ stat->atomic_write_unit_max_opt = unit_max_opt;
/* Initially only allow 1x segment */
stat->atomic_write_segments_max = 1;
@@ -732,6 +735,7 @@ cp_statx(const struct kstat *stat, struct statx __user *buffer)
tmp.stx_atomic_write_unit_min = stat->atomic_write_unit_min;
tmp.stx_atomic_write_unit_max = stat->atomic_write_unit_max;
tmp.stx_atomic_write_segments_max = stat->atomic_write_segments_max;
+ tmp.stx_atomic_write_unit_max_opt = stat->atomic_write_unit_max_opt;
return copy_to_user(buffer, &tmp, sizeof(tmp)) ? -EFAULT : 0;
}
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index 756bd3ca8e00..f0e5d83195df 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -610,7 +610,7 @@ xfs_report_atomic_write(
if (xfs_inode_can_atomicwrite(ip))
unit_min = unit_max = ip->i_mount->m_sb.sb_blocksize;
- generic_fill_statx_atomic_writes(stat, unit_min, unit_max);
+ generic_fill_statx_atomic_writes(stat, unit_min, unit_max, 0);
}
STATIC int
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 016b0fe1536e..7b19d8f99aff 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -3475,7 +3475,8 @@ void generic_fillattr(struct mnt_idmap *, u32, struct inode *, struct kstat *);
void generic_fill_statx_attr(struct inode *inode, struct kstat *stat);
void generic_fill_statx_atomic_writes(struct kstat *stat,
unsigned int unit_min,
- unsigned int unit_max);
+ unsigned int unit_max,
+ unsigned int unit_max_opt);
extern int vfs_getattr_nosec(const struct path *, struct kstat *, u32, unsigned int);
extern int vfs_getattr(const struct path *, struct kstat *, u32, unsigned int);
void __inode_add_bytes(struct inode *inode, loff_t bytes);
diff --git a/include/linux/stat.h b/include/linux/stat.h
index be7496a6a0dd..e3d00e7bb26d 100644
--- a/include/linux/stat.h
+++ b/include/linux/stat.h
@@ -57,6 +57,7 @@ struct kstat {
u32 dio_read_offset_align;
u32 atomic_write_unit_min;
u32 atomic_write_unit_max;
+ u32 atomic_write_unit_max_opt;
u32 atomic_write_segments_max;
};
diff --git a/include/uapi/linux/stat.h b/include/uapi/linux/stat.h
index f78ee3670dd5..1686861aae20 100644
--- a/include/uapi/linux/stat.h
+++ b/include/uapi/linux/stat.h
@@ -182,8 +182,12 @@ struct statx {
/* File offset alignment for direct I/O reads */
__u32 stx_dio_read_offset_align;
- /* 0xb8 */
- __u64 __spare3[9]; /* Spare space for future expansion */
+ /* Optimised max atomic write unit in bytes */
+ __u32 stx_atomic_write_unit_max_opt;
+ __u32 __spare2[1];
+
+ /* 0xc0 */
+ __u64 __spare3[8]; /* Spare space for future expansion */
/* 0x100 */
};
--
2.31.1
^ permalink raw reply related
* [PATCH v9 03/15] xfs: add helpers to compute transaction reservation for finishing intent items
From: John Garry @ 2025-04-25 16:44 UTC (permalink / raw)
To: brauner, djwong, hch, viro, jack, cem
Cc: linux-fsdevel, dchinner, linux-xfs, linux-kernel, ojaswin,
ritesh.list, martin.petersen, linux-ext4, linux-block,
catherine.hoang, linux-api, John Garry
In-Reply-To: <20250425164504.3263637-1-john.g.garry@oracle.com>
From: "Darrick J. Wong" <djwong@kernel.org>
In the transaction reservation code, hoist the logic that computes the
reservation needed to finish one log intent item into separate helper
functions. These will be used in subsequent patches to estimate the
number of blocks that an online repair can commit to reaping in the same
transaction as the change committing the new data structure.
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Signed-off-by: John Garry <john.g.garry@oracle.com>
---
fs/xfs/libxfs/xfs_trans_resv.c | 165 ++++++++++++++++++++++++++-------
fs/xfs/libxfs/xfs_trans_resv.h | 18 ++++
2 files changed, 152 insertions(+), 31 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_trans_resv.c b/fs/xfs/libxfs/xfs_trans_resv.c
index 13d00c7166e1..580d00ae2857 100644
--- a/fs/xfs/libxfs/xfs_trans_resv.c
+++ b/fs/xfs/libxfs/xfs_trans_resv.c
@@ -263,6 +263,42 @@ xfs_rtalloc_block_count(
* register overflow from temporaries in the calculations.
*/
+/*
+ * Finishing a data device refcount updates (t1):
+ * the agfs of the ags containing the blocks: nr_ops * sector size
+ * the refcount btrees: nr_ops * 1 trees * (2 * max depth - 1) * block size
+ */
+inline unsigned int
+xfs_calc_finish_cui_reservation(
+ struct xfs_mount *mp,
+ unsigned int nr_ops)
+{
+ if (!xfs_has_reflink(mp))
+ return 0;
+
+ return xfs_calc_buf_res(nr_ops, mp->m_sb.sb_sectsize) +
+ xfs_calc_buf_res(xfs_refcountbt_block_count(mp, nr_ops),
+ mp->m_sb.sb_blocksize);
+}
+
+/*
+ * Realtime refcount updates (t2);
+ * the rt refcount inode
+ * the rtrefcount btrees: nr_ops * 1 trees * (2 * max depth - 1) * block size
+ */
+inline unsigned int
+xfs_calc_finish_rt_cui_reservation(
+ struct xfs_mount *mp,
+ unsigned int nr_ops)
+{
+ if (!xfs_has_rtreflink(mp))
+ return 0;
+
+ return xfs_calc_inode_res(mp, 1) +
+ xfs_calc_buf_res(xfs_rtrefcountbt_block_count(mp, nr_ops),
+ mp->m_sb.sb_blocksize);
+}
+
/*
* Compute the log reservation required to handle the refcount update
* transaction. Refcount updates are always done via deferred log items.
@@ -280,19 +316,10 @@ xfs_calc_refcountbt_reservation(
struct xfs_mount *mp,
unsigned int nr_ops)
{
- unsigned int blksz = XFS_FSB_TO_B(mp, 1);
- unsigned int t1, t2 = 0;
+ unsigned int t1, t2;
- if (!xfs_has_reflink(mp))
- return 0;
-
- t1 = xfs_calc_buf_res(nr_ops, mp->m_sb.sb_sectsize) +
- xfs_calc_buf_res(xfs_refcountbt_block_count(mp, nr_ops), blksz);
-
- if (xfs_has_realtime(mp))
- t2 = xfs_calc_inode_res(mp, 1) +
- xfs_calc_buf_res(xfs_rtrefcountbt_block_count(mp, nr_ops),
- blksz);
+ t1 = xfs_calc_finish_cui_reservation(mp, nr_ops);
+ t2 = xfs_calc_finish_rt_cui_reservation(mp, nr_ops);
return max(t1, t2);
}
@@ -379,6 +406,96 @@ xfs_calc_write_reservation_minlogsize(
return xfs_calc_write_reservation(mp, true);
}
+/*
+ * Finishing an EFI can free the blocks and bmap blocks (t2):
+ * the agf for each of the ags: nr * sector size
+ * the agfl for each of the ags: nr * sector size
+ * the super block to reflect the freed blocks: sector size
+ * worst case split in allocation btrees per extent assuming nr extents:
+ * nr exts * 2 trees * (2 * max depth - 1) * block size
+ */
+inline unsigned int
+xfs_calc_finish_efi_reservation(
+ struct xfs_mount *mp,
+ unsigned int nr)
+{
+ return xfs_calc_buf_res((2 * nr) + 1, mp->m_sb.sb_sectsize) +
+ xfs_calc_buf_res(xfs_allocfree_block_count(mp, nr),
+ mp->m_sb.sb_blocksize);
+}
+
+/*
+ * Or, if it's a realtime file (t3):
+ * the agf for each of the ags: 2 * sector size
+ * the agfl for each of the ags: 2 * sector size
+ * the super block to reflect the freed blocks: sector size
+ * the realtime bitmap:
+ * 2 exts * ((XFS_BMBT_MAX_EXTLEN / rtextsize) / NBBY) bytes
+ * the realtime summary: 2 exts * 1 block
+ * worst case split in allocation btrees per extent assuming 2 extents:
+ * 2 exts * 2 trees * (2 * max depth - 1) * block size
+ */
+inline unsigned int
+xfs_calc_finish_rt_efi_reservation(
+ struct xfs_mount *mp,
+ unsigned int nr)
+{
+ if (!xfs_has_realtime(mp))
+ return 0;
+
+ return xfs_calc_buf_res((2 * nr) + 1, mp->m_sb.sb_sectsize) +
+ xfs_calc_buf_res(xfs_rtalloc_block_count(mp, nr),
+ mp->m_sb.sb_blocksize) +
+ xfs_calc_buf_res(xfs_allocfree_block_count(mp, nr),
+ mp->m_sb.sb_blocksize);
+}
+
+/*
+ * Finishing an RUI is the same as an EFI. We can split the rmap btree twice
+ * on each end of the record, and that can cause the AGFL to be refilled or
+ * emptied out.
+ */
+inline unsigned int
+xfs_calc_finish_rui_reservation(
+ struct xfs_mount *mp,
+ unsigned int nr)
+{
+ if (!xfs_has_rmapbt(mp))
+ return 0;
+ return xfs_calc_finish_efi_reservation(mp, nr);
+}
+
+/*
+ * Finishing an RUI is the same as an EFI. We can split the rmap btree twice
+ * on each end of the record, and that can cause the AGFL to be refilled or
+ * emptied out.
+ */
+inline unsigned int
+xfs_calc_finish_rt_rui_reservation(
+ struct xfs_mount *mp,
+ unsigned int nr)
+{
+ if (!xfs_has_rtrmapbt(mp))
+ return 0;
+ return xfs_calc_finish_rt_efi_reservation(mp, nr);
+}
+
+/*
+ * In finishing a BUI, we can modify:
+ * the inode being truncated: inode size
+ * dquots
+ * the inode's bmap btree: (max depth + 1) * block size
+ */
+inline unsigned int
+xfs_calc_finish_bui_reservation(
+ struct xfs_mount *mp,
+ unsigned int nr)
+{
+ return xfs_calc_inode_res(mp, 1) + XFS_DQUOT_LOGRES +
+ xfs_calc_buf_res(XFS_BM_MAXLEVELS(mp, XFS_DATA_FORK) + 1,
+ mp->m_sb.sb_blocksize);
+}
+
/*
* In truncating a file we free up to two extents at once. We can modify (t1):
* the inode being truncated: inode size
@@ -411,16 +528,8 @@ xfs_calc_itruncate_reservation(
t1 = xfs_calc_inode_res(mp, 1) +
xfs_calc_buf_res(XFS_BM_MAXLEVELS(mp, XFS_DATA_FORK) + 1, blksz);
- t2 = xfs_calc_buf_res(9, mp->m_sb.sb_sectsize) +
- xfs_calc_buf_res(xfs_allocfree_block_count(mp, 4), blksz);
-
- if (xfs_has_realtime(mp)) {
- t3 = xfs_calc_buf_res(5, mp->m_sb.sb_sectsize) +
- xfs_calc_buf_res(xfs_rtalloc_block_count(mp, 2), blksz) +
- xfs_calc_buf_res(xfs_allocfree_block_count(mp, 2), blksz);
- } else {
- t3 = 0;
- }
+ t2 = xfs_calc_finish_efi_reservation(mp, 4);
+ t3 = xfs_calc_finish_rt_efi_reservation(mp, 2);
/*
* In the early days of reflink, we included enough reservation to log
@@ -501,9 +610,7 @@ xfs_calc_rename_reservation(
xfs_calc_buf_res(2 * XFS_DIROP_LOG_COUNT(mp),
XFS_FSB_TO_B(mp, 1));
- t2 = xfs_calc_buf_res(7, mp->m_sb.sb_sectsize) +
- xfs_calc_buf_res(xfs_allocfree_block_count(mp, 3),
- XFS_FSB_TO_B(mp, 1));
+ t2 = xfs_calc_finish_efi_reservation(mp, 3);
if (xfs_has_parent(mp)) {
unsigned int rename_overhead, exchange_overhead;
@@ -611,9 +718,7 @@ xfs_calc_link_reservation(
overhead += xfs_calc_iunlink_remove_reservation(mp);
t1 = xfs_calc_inode_res(mp, 2) +
xfs_calc_buf_res(XFS_DIROP_LOG_COUNT(mp), XFS_FSB_TO_B(mp, 1));
- t2 = xfs_calc_buf_res(3, mp->m_sb.sb_sectsize) +
- xfs_calc_buf_res(xfs_allocfree_block_count(mp, 1),
- XFS_FSB_TO_B(mp, 1));
+ t2 = xfs_calc_finish_efi_reservation(mp, 1);
if (xfs_has_parent(mp)) {
t3 = resp->tr_attrsetm.tr_logres;
@@ -676,9 +781,7 @@ xfs_calc_remove_reservation(
t1 = xfs_calc_inode_res(mp, 2) +
xfs_calc_buf_res(XFS_DIROP_LOG_COUNT(mp), XFS_FSB_TO_B(mp, 1));
- t2 = xfs_calc_buf_res(4, mp->m_sb.sb_sectsize) +
- xfs_calc_buf_res(xfs_allocfree_block_count(mp, 2),
- XFS_FSB_TO_B(mp, 1));
+ t2 = xfs_calc_finish_efi_reservation(mp, 2);
if (xfs_has_parent(mp)) {
t3 = resp->tr_attrrm.tr_logres;
diff --git a/fs/xfs/libxfs/xfs_trans_resv.h b/fs/xfs/libxfs/xfs_trans_resv.h
index 0554b9d775d2..d9d0032cbbc5 100644
--- a/fs/xfs/libxfs/xfs_trans_resv.h
+++ b/fs/xfs/libxfs/xfs_trans_resv.h
@@ -98,6 +98,24 @@ struct xfs_trans_resv {
void xfs_trans_resv_calc(struct xfs_mount *mp, struct xfs_trans_resv *resp);
uint xfs_allocfree_block_count(struct xfs_mount *mp, uint num_ops);
+unsigned int xfs_calc_finish_bui_reservation(struct xfs_mount *mp,
+ unsigned int nr_ops);
+
+unsigned int xfs_calc_finish_efi_reservation(struct xfs_mount *mp,
+ unsigned int nr_ops);
+unsigned int xfs_calc_finish_rt_efi_reservation(struct xfs_mount *mp,
+ unsigned int nr_ops);
+
+unsigned int xfs_calc_finish_rui_reservation(struct xfs_mount *mp,
+ unsigned int nr_ops);
+unsigned int xfs_calc_finish_rt_rui_reservation(struct xfs_mount *mp,
+ unsigned int nr_ops);
+
+unsigned int xfs_calc_finish_cui_reservation(struct xfs_mount *mp,
+ unsigned int nr_ops);
+unsigned int xfs_calc_finish_rt_cui_reservation(struct xfs_mount *mp,
+ unsigned int nr_ops);
+
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);
--
2.31.1
^ permalink raw reply related
* [PATCH v9 05/15] xfs: ignore HW which cannot atomic write a single block
From: John Garry @ 2025-04-25 16:44 UTC (permalink / raw)
To: brauner, djwong, hch, viro, jack, cem
Cc: linux-fsdevel, dchinner, linux-xfs, linux-kernel, ojaswin,
ritesh.list, martin.petersen, linux-ext4, linux-block,
catherine.hoang, linux-api, John Garry
In-Reply-To: <20250425164504.3263637-1-john.g.garry@oracle.com>
From: "Darrick J. Wong" <djwong@kernel.org>
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 | 41 ++++++++++++++++++++++++++++++++++-------
fs/xfs/xfs_buf.h | 3 ++-
fs/xfs/xfs_inode.h | 14 ++------------
fs/xfs/xfs_mount.c | 7 +++++++
4 files changed, 45 insertions(+), 20 deletions(-)
diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index 1a2b3f06fa71..f165446a082c 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -1772,6 +1772,40 @@ xfs_init_buftarg(
return -ENOMEM;
}
+/*
+ * Configure this buffer target for hardware-assisted atomic writes if the
+ * underlying block device supports is congruent with the filesystem geometry.
+ */
+void
+xfs_buftarg_config_atomic_writes(
+ struct xfs_buftarg *btp)
+{
+ struct xfs_mount *mp = btp->bt_mount;
+ unsigned int min_bytes, max_bytes;
+
+ ASSERT(btp->bt_bdev != NULL);
+
+ if (!bdev_can_atomic_write(btp->bt_bdev))
+ return;
+
+ min_bytes = bdev_atomic_write_unit_min_bytes(btp->bt_bdev);
+ max_bytes = bdev_atomic_write_unit_max_bytes(btp->bt_bdev);
+
+ /*
+ * Ignore atomic write geometry that is nonsense or doesn't even cover
+ * a single fsblock.
+ */
+ if (min_bytes > max_bytes ||
+ min_bytes > mp->m_sb.sb_blocksize ||
+ max_bytes < mp->m_sb.sb_blocksize) {
+ min_bytes = 0;
+ max_bytes = 0;
+ }
+
+ btp->bt_bdev_awu_min = min_bytes;
+ btp->bt_bdev_awu_max = max_bytes;
+}
+
struct xfs_buftarg *
xfs_alloc_buftarg(
struct xfs_mount *mp,
@@ -1792,13 +1826,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 d0b065a9a9f0..6f691779887f 100644
--- a/fs/xfs/xfs_buf.h
+++ b/fs/xfs/xfs_buf.h
@@ -112,7 +112,7 @@ struct xfs_buftarg {
struct percpu_counter bt_readahead_count;
struct ratelimit_state bt_ioerror_rl;
- /* Atomic write unit values */
+ /* Atomic write unit values, bytes */
unsigned int bt_bdev_awu_min;
unsigned int bt_bdev_awu_max;
@@ -375,6 +375,7 @@ extern void xfs_free_buftarg(struct xfs_buftarg *);
extern void xfs_buftarg_wait(struct xfs_buftarg *);
extern void xfs_buftarg_drain(struct xfs_buftarg *);
extern int xfs_setsize_buftarg(struct xfs_buftarg *, unsigned int);
+void xfs_buftarg_config_atomic_writes(struct xfs_buftarg *btp);
#define xfs_getsize_buftarg(buftarg) block_size((buftarg)->bt_bdev)
#define xfs_readonly_buftarg(buftarg) bdev_read_only((buftarg)->bt_bdev)
diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
index 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_mount.c b/fs/xfs/xfs_mount.c
index 00b53f479ece..1082b322e6d6 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -1082,6 +1082,13 @@ xfs_mountfs(
xfs_zone_gc_start(mp);
}
+ /* Configure hardware atomic write geometry */
+ xfs_buftarg_config_atomic_writes(mp->m_ddev_targp);
+ if (mp->m_logdev_targp && mp->m_logdev_targp != mp->m_ddev_targp)
+ xfs_buftarg_config_atomic_writes(mp->m_logdev_targp);
+ if (mp->m_rtdev_targp && mp->m_rtdev_targp != mp->m_ddev_targp)
+ xfs_buftarg_config_atomic_writes(mp->m_rtdev_targp);
+
return 0;
out_agresv:
--
2.31.1
^ permalink raw reply related
* [PATCH v9 11/15] xfs: commit CoW-based atomic writes atomically
From: John Garry @ 2025-04-25 16:45 UTC (permalink / raw)
To: brauner, djwong, hch, viro, jack, cem
Cc: linux-fsdevel, dchinner, linux-xfs, linux-kernel, ojaswin,
ritesh.list, martin.petersen, linux-ext4, linux-block,
catherine.hoang, linux-api, John Garry
In-Reply-To: <20250425164504.3263637-1-john.g.garry@oracle.com>
When completing a CoW-based write, each extent range mapping update is
covered by a separate transaction.
For a CoW-based atomic write, all mappings must be changed at once, so
change to use a single transaction.
Note that there is a limit on the amount of log intent items which can be
fit into a single transaction, but this is being ignored for now since
the count of items for a typical atomic write would be much less than is
typically supported. A typical atomic write would be expected to be 64KB
or less, which means only 16 possible extents unmaps, which is quite
small.
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
[djwong: add tr_atomic_ioend]
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: John Garry <john.g.garry@oracle.com>
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
fs/xfs/libxfs/xfs_log_rlimit.c | 4 +++
fs/xfs/libxfs/xfs_trans_resv.c | 15 +++++++++
fs/xfs/libxfs/xfs_trans_resv.h | 1 +
fs/xfs/xfs_file.c | 5 ++-
fs/xfs/xfs_reflink.c | 56 ++++++++++++++++++++++++++++++++++
fs/xfs/xfs_reflink.h | 2 ++
6 files changed, 82 insertions(+), 1 deletion(-)
diff --git a/fs/xfs/libxfs/xfs_log_rlimit.c b/fs/xfs/libxfs/xfs_log_rlimit.c
index d3bd6a86c8fe..34bba96d30ca 100644
--- a/fs/xfs/libxfs/xfs_log_rlimit.c
+++ b/fs/xfs/libxfs/xfs_log_rlimit.c
@@ -91,6 +91,7 @@ xfs_log_calc_trans_resv_for_minlogblocks(
*/
if (xfs_want_minlogsize_fixes(&mp->m_sb)) {
xfs_trans_resv_calc(mp, resv);
+ resv->tr_atomic_ioend = M_RES(mp)->tr_atomic_ioend;
return;
}
@@ -107,6 +108,9 @@ xfs_log_calc_trans_resv_for_minlogblocks(
xfs_trans_resv_calc(mp, resv);
+ /* Copy the dynamic transaction reservation types from the running fs */
+ resv->tr_atomic_ioend = M_RES(mp)->tr_atomic_ioend;
+
if (xfs_has_reflink(mp)) {
/*
* In the early days of reflink, typical log operation counts
diff --git a/fs/xfs/libxfs/xfs_trans_resv.c b/fs/xfs/libxfs/xfs_trans_resv.c
index 580d00ae2857..a841432abf83 100644
--- a/fs/xfs/libxfs/xfs_trans_resv.c
+++ b/fs/xfs/libxfs/xfs_trans_resv.c
@@ -1284,6 +1284,15 @@ xfs_calc_namespace_reservations(
resp->tr_mkdir.tr_logflags |= XFS_TRANS_PERM_LOG_RES;
}
+STATIC void
+xfs_calc_default_atomic_ioend_reservation(
+ struct xfs_mount *mp,
+ struct xfs_trans_resv *resp)
+{
+ /* Pick a default that will scale reasonably for the log size. */
+ resp->tr_atomic_ioend = resp->tr_itruncate;
+}
+
void
xfs_trans_resv_calc(
struct xfs_mount *mp,
@@ -1378,4 +1387,10 @@ xfs_trans_resv_calc(
resp->tr_itruncate.tr_logcount += logcount_adj;
resp->tr_write.tr_logcount += logcount_adj;
resp->tr_qm_dqalloc.tr_logcount += logcount_adj;
+
+ /*
+ * Now that we've finished computing the static reservations, we can
+ * compute the dynamic reservation for atomic writes.
+ */
+ xfs_calc_default_atomic_ioend_reservation(mp, resp);
}
diff --git a/fs/xfs/libxfs/xfs_trans_resv.h b/fs/xfs/libxfs/xfs_trans_resv.h
index d9d0032cbbc5..670045d417a6 100644
--- a/fs/xfs/libxfs/xfs_trans_resv.h
+++ b/fs/xfs/libxfs/xfs_trans_resv.h
@@ -48,6 +48,7 @@ struct xfs_trans_resv {
struct xfs_trans_res tr_qm_dqalloc; /* allocate quota on disk */
struct xfs_trans_res tr_sb; /* modify superblock */
struct xfs_trans_res tr_fsyncts; /* update timestamps on fsync */
+ struct xfs_trans_res tr_atomic_ioend; /* untorn write completion */
};
/* shorthand way of accessing reservation structure */
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index e8acd6ca8f27..32883ec8ca2e 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -576,7 +576,10 @@ xfs_dio_write_end_io(
nofs_flag = memalloc_nofs_save();
if (flags & IOMAP_DIO_COW) {
- error = xfs_reflink_end_cow(ip, offset, size);
+ if (iocb->ki_flags & IOCB_ATOMIC)
+ error = xfs_reflink_end_atomic_cow(ip, offset, size);
+ else
+ error = xfs_reflink_end_cow(ip, offset, size);
if (error)
goto out;
}
diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index f5d338916098..218dee76768b 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -984,6 +984,62 @@ xfs_reflink_end_cow(
return error;
}
+/*
+ * Fully remap all of the file's data fork at once, which is the critical part
+ * in achieving atomic behaviour.
+ * The regular CoW end path does not use function as to keep the block
+ * reservation per transaction as low as possible.
+ */
+int
+xfs_reflink_end_atomic_cow(
+ struct xfs_inode *ip,
+ xfs_off_t offset,
+ xfs_off_t count)
+{
+ xfs_fileoff_t offset_fsb;
+ xfs_fileoff_t end_fsb;
+ int error = 0;
+ struct xfs_mount *mp = ip->i_mount;
+ struct xfs_trans *tp;
+ unsigned int resblks;
+
+ trace_xfs_reflink_end_cow(ip, offset, count);
+
+ offset_fsb = XFS_B_TO_FSBT(mp, offset);
+ end_fsb = XFS_B_TO_FSB(mp, offset + count);
+
+ /*
+ * Each remapping operation could cause a btree split, so in the worst
+ * case that's one for each block.
+ */
+ resblks = (end_fsb - offset_fsb) *
+ XFS_NEXTENTADD_SPACE_RES(mp, 1, XFS_DATA_FORK);
+
+ error = xfs_trans_alloc(mp, &M_RES(mp)->tr_atomic_ioend, resblks, 0,
+ XFS_TRANS_RESERVE, &tp);
+ if (error)
+ return error;
+
+ xfs_ilock(ip, XFS_ILOCK_EXCL);
+ xfs_trans_ijoin(tp, ip, 0);
+
+ while (end_fsb > offset_fsb && !error) {
+ error = xfs_reflink_end_cow_extent_locked(tp, ip, &offset_fsb,
+ end_fsb);
+ }
+ if (error) {
+ trace_xfs_reflink_end_cow_error(ip, error, _RET_IP_);
+ goto out_cancel;
+ }
+ error = xfs_trans_commit(tp);
+ xfs_iunlock(ip, XFS_ILOCK_EXCL);
+ return error;
+out_cancel:
+ xfs_trans_cancel(tp);
+ xfs_iunlock(ip, XFS_ILOCK_EXCL);
+ return error;
+}
+
/*
* 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 379619f24247..412e9b6f2082 100644
--- a/fs/xfs/xfs_reflink.h
+++ b/fs/xfs/xfs_reflink.h
@@ -45,6 +45,8 @@ extern int xfs_reflink_cancel_cow_range(struct xfs_inode *ip, xfs_off_t offset,
xfs_off_t count, bool cancel_real);
extern int xfs_reflink_end_cow(struct xfs_inode *ip, xfs_off_t offset,
xfs_off_t count);
+int xfs_reflink_end_atomic_cow(struct xfs_inode *ip, xfs_off_t offset,
+ xfs_off_t count);
extern int xfs_reflink_recover_cow(struct xfs_mount *mp);
extern loff_t xfs_reflink_remap_range(struct file *file_in, loff_t pos_in,
struct file *file_out, loff_t pos_out, loff_t len,
--
2.31.1
^ permalink raw reply related
* [PATCH v9 12/15] xfs: add xfs_file_dio_write_atomic()
From: John Garry @ 2025-04-25 16:45 UTC (permalink / raw)
To: brauner, djwong, hch, viro, jack, cem
Cc: linux-fsdevel, dchinner, linux-xfs, linux-kernel, ojaswin,
ritesh.list, martin.petersen, linux-ext4, linux-block,
catherine.hoang, linux-api, John Garry
In-Reply-To: <20250425164504.3263637-1-john.g.garry@oracle.com>
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.
Signed-off-by: John Garry <john.g.garry@oracle.com>
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
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 v9 15/15] xfs: allow sysadmins to specify a maximum atomic write limit at mount time
From: John Garry @ 2025-04-25 16:45 UTC (permalink / raw)
To: brauner, djwong, hch, viro, jack, cem
Cc: linux-fsdevel, dchinner, linux-xfs, linux-kernel, ojaswin,
ritesh.list, martin.petersen, linux-ext4, linux-block,
catherine.hoang, linux-api, John Garry
In-Reply-To: <20250425164504.3263637-1-john.g.garry@oracle.com>
From: "Darrick J. Wong" <djwong@kernel.org>
Introduce a mount option to allow sysadmins to specify the maximum size
of an atomic write. If the filesystem can work with the supplied value,
that becomes the new guaranteed maximum.
The value mustn't be too big for the existing filesystem geometry (max
write size, max AG/rtgroup size). We dynamically recompute the
tr_atomic_write transaction reservation based on the given block size,
check that the current log size isn't less than the new minimum log size
constraints, and set a new maximum.
The actual software atomic write max is still computed based off of
tr_atomic_ioend the same way it has for the past few commits. Note also
that xfs_calc_atomic_write_log_geometry is non-static because mkfs will
need that.
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Signed-off-by: John Garry <john.g.garry@oracle.com>
---
Documentation/admin-guide/xfs.rst | 11 +++++
fs/xfs/libxfs/xfs_trans_resv.c | 69 ++++++++++++++++++++++++++
fs/xfs/libxfs/xfs_trans_resv.h | 4 ++
fs/xfs/xfs_mount.c | 80 ++++++++++++++++++++++++++++++-
fs/xfs/xfs_mount.h | 6 +++
fs/xfs/xfs_super.c | 58 +++++++++++++++++++++-
fs/xfs/xfs_trace.h | 33 +++++++++++++
7 files changed, 259 insertions(+), 2 deletions(-)
diff --git a/Documentation/admin-guide/xfs.rst b/Documentation/admin-guide/xfs.rst
index 5becb441c3cb..a18328a5fb93 100644
--- a/Documentation/admin-guide/xfs.rst
+++ b/Documentation/admin-guide/xfs.rst
@@ -151,6 +151,17 @@ When mounting an XFS filesystem, the following options are accepted.
optional, and the log section can be separate from the data
section or contained within it.
+ max_atomic_write=value
+ Set the maximum size of an atomic write. The size may be
+ specified in bytes, in kilobytes with a "k" suffix, in megabytes
+ with a "m" suffix, or in gigabytes with a "g" suffix. The size
+ cannot be larger than the maximum write size, larger than the
+ size of any allocation group, or larger than the size of a
+ remapping operation that the log can complete atomically.
+
+ The default value is to set the maximum I/O completion size
+ to allow each CPU to handle one at a time.
+
max_open_zones=value
Specify the max number of zones to keep open for writing on a
zoned rt device. Many open zones aids file data separation
diff --git a/fs/xfs/libxfs/xfs_trans_resv.c b/fs/xfs/libxfs/xfs_trans_resv.c
index e73c09fbd24c..86a111d0f2fc 100644
--- a/fs/xfs/libxfs/xfs_trans_resv.c
+++ b/fs/xfs/libxfs/xfs_trans_resv.c
@@ -1488,3 +1488,72 @@ xfs_calc_max_atomic_write_fsblocks(
return ret;
}
+
+/*
+ * Compute the log blocks and transaction reservation needed to complete an
+ * atomic write of a given number of blocks. Worst case, each block requires
+ * separate handling. A return value of 0 means something went wrong.
+ */
+xfs_extlen_t
+xfs_calc_atomic_write_log_geometry(
+ struct xfs_mount *mp,
+ xfs_extlen_t blockcount,
+ unsigned int *new_logres)
+{
+ struct xfs_trans_res *curr_res = &M_RES(mp)->tr_atomic_ioend;
+ uint old_logres = curr_res->tr_logres;
+ unsigned int per_intent, step_size;
+ unsigned int logres;
+ xfs_extlen_t min_logblocks;
+
+ ASSERT(blockcount > 0);
+
+ xfs_calc_default_atomic_ioend_reservation(mp, M_RES(mp));
+
+ per_intent = xfs_calc_atomic_write_ioend_geometry(mp, &step_size);
+
+ /* Check for overflows */
+ if (check_mul_overflow(blockcount, per_intent, &logres) ||
+ check_add_overflow(logres, step_size, &logres))
+ return 0;
+
+ curr_res->tr_logres = logres;
+ min_logblocks = xfs_log_calc_minimum_size(mp);
+ curr_res->tr_logres = old_logres;
+
+ trace_xfs_calc_max_atomic_write_log_geometry(mp, per_intent, step_size,
+ blockcount, min_logblocks, logres);
+
+ *new_logres = logres;
+ return min_logblocks;
+}
+
+/*
+ * Compute the transaction reservation needed to complete an out of place
+ * atomic write of a given number of blocks.
+ */
+int
+xfs_calc_atomic_write_reservation(
+ struct xfs_mount *mp,
+ xfs_extlen_t blockcount)
+{
+ unsigned int new_logres;
+ xfs_extlen_t min_logblocks;
+
+ /*
+ * If the caller doesn't ask for a specific atomic write size, then
+ * use the defaults.
+ */
+ if (blockcount == 0) {
+ xfs_calc_default_atomic_ioend_reservation(mp, M_RES(mp));
+ return 0;
+ }
+
+ min_logblocks = xfs_calc_atomic_write_log_geometry(mp, blockcount,
+ &new_logres);
+ if (!min_logblocks || min_logblocks > mp->m_sb.sb_logblocks)
+ return -EINVAL;
+
+ M_RES(mp)->tr_atomic_ioend.tr_logres = new_logres;
+ return 0;
+}
diff --git a/fs/xfs/libxfs/xfs_trans_resv.h b/fs/xfs/libxfs/xfs_trans_resv.h
index a6d303b83688..336279e0fc61 100644
--- a/fs/xfs/libxfs/xfs_trans_resv.h
+++ b/fs/xfs/libxfs/xfs_trans_resv.h
@@ -122,5 +122,9 @@ 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);
+xfs_extlen_t xfs_calc_atomic_write_log_geometry(struct xfs_mount *mp,
+ xfs_extlen_t blockcount, unsigned int *new_logres);
+int xfs_calc_atomic_write_reservation(struct xfs_mount *mp,
+ xfs_extlen_t blockcount);
#endif /* __XFS_TRANS_RESV_H__ */
diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index eebd5e7d1ab6..a6cea25b3a7f 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -740,6 +740,82 @@ xfs_calc_atomic_write_unit_max(
max_agsize, max_rgsize);
}
+/*
+ * Try to set the atomic write maximum to a new value that we got from
+ * userspace via mount option.
+ */
+int
+xfs_set_max_atomic_write_opt(
+ struct xfs_mount *mp,
+ unsigned long long new_max_bytes)
+{
+ const xfs_filblks_t new_max_fsbs = XFS_B_TO_FSBT(mp, new_max_bytes);
+ const xfs_extlen_t max_write = xfs_calc_atomic_write_max(mp);
+ const xfs_extlen_t max_group =
+ max(mp->m_groups[XG_TYPE_AG].blocks,
+ mp->m_groups[XG_TYPE_RTG].blocks);
+ const xfs_extlen_t max_group_write =
+ max(xfs_calc_perag_awu_max(mp), xfs_calc_rtgroup_awu_max(mp));
+ int error;
+
+ if (new_max_bytes == 0)
+ goto set_limit;
+
+ ASSERT(max_write <= U32_MAX);
+
+ /* generic_atomic_write_valid enforces power of two length */
+ if (!is_power_of_2(new_max_bytes)) {
+ xfs_warn(mp,
+ "max atomic write size of %llu bytes is not a power of 2",
+ new_max_bytes);
+ return -EINVAL;
+ }
+
+ if (new_max_bytes & mp->m_blockmask) {
+ xfs_warn(mp,
+ "max atomic write size of %llu bytes not aligned with fsblock",
+ new_max_bytes);
+ return -EINVAL;
+ }
+
+ if (new_max_fsbs > max_write) {
+ xfs_warn(mp,
+ "max atomic write size of %lluk cannot be larger than max write size %lluk",
+ new_max_bytes >> 10,
+ XFS_FSB_TO_B(mp, max_write) >> 10);
+ return -EINVAL;
+ }
+
+ if (new_max_fsbs > max_group) {
+ xfs_warn(mp,
+ "max atomic write size of %lluk cannot be larger than allocation group size %lluk",
+ new_max_bytes >> 10,
+ XFS_FSB_TO_B(mp, max_group) >> 10);
+ return -EINVAL;
+ }
+
+ if (new_max_fsbs > max_group_write) {
+ xfs_warn(mp,
+ "max atomic write size of %lluk cannot be larger than max allocation group write size %lluk",
+ new_max_bytes >> 10,
+ XFS_FSB_TO_B(mp, max_group_write) >> 10);
+ return -EINVAL;
+ }
+
+set_limit:
+ error = xfs_calc_atomic_write_reservation(mp, new_max_fsbs);
+ if (error) {
+ xfs_warn(mp,
+ "cannot support completing atomic writes of %lluk",
+ new_max_bytes >> 10);
+ return error;
+ }
+
+ xfs_calc_atomic_write_unit_max(mp);
+ mp->m_awu_max_bytes = new_max_bytes;
+ return 0;
+}
+
/* Compute maximum possible height for realtime btree types for this fs. */
static inline void
xfs_rtbtree_compute_maxlevels(
@@ -1168,7 +1244,9 @@ xfs_mountfs(
* derived from transaction reservations, so we must do this after the
* log is fully initialized.
*/
- xfs_calc_atomic_write_unit_max(mp);
+ error = xfs_set_max_atomic_write_opt(mp, mp->m_awu_max_bytes);
+ if (error)
+ goto out_agresv;
return 0;
diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index e2abf31438e0..5b5df70570c0 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -237,6 +237,9 @@ typedef struct xfs_mount {
unsigned int m_max_open_zones;
unsigned int m_zonegc_low_space;
+ /* max_atomic_write mount option value */
+ unsigned long long m_awu_max_bytes;
+
/*
* Bitsets of per-fs metadata that have been checked and/or are sick.
* Callers must hold m_sb_lock to access these two fields.
@@ -804,4 +807,7 @@ static inline void xfs_mod_sb_delalloc(struct xfs_mount *mp, int64_t delta)
percpu_counter_add(&mp->m_delalloc_blks, delta);
}
+int xfs_set_max_atomic_write_opt(struct xfs_mount *mp,
+ unsigned long long new_max_bytes);
+
#endif /* __XFS_MOUNT_H__ */
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index b2dd0c0bf509..4cdb161a1bb8 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -111,7 +111,7 @@ enum {
Opt_prjquota, Opt_uquota, Opt_gquota, Opt_pquota,
Opt_uqnoenforce, Opt_gqnoenforce, Opt_pqnoenforce, Opt_qnoenforce,
Opt_discard, Opt_nodiscard, Opt_dax, Opt_dax_enum, Opt_max_open_zones,
- Opt_lifetime, Opt_nolifetime,
+ Opt_lifetime, Opt_nolifetime, Opt_max_atomic_write,
};
static const struct fs_parameter_spec xfs_fs_parameters[] = {
@@ -159,6 +159,7 @@ static const struct fs_parameter_spec xfs_fs_parameters[] = {
fsparam_u32("max_open_zones", Opt_max_open_zones),
fsparam_flag("lifetime", Opt_lifetime),
fsparam_flag("nolifetime", Opt_nolifetime),
+ fsparam_string("max_atomic_write", Opt_max_atomic_write),
{}
};
@@ -241,6 +242,9 @@ xfs_fs_show_options(
if (mp->m_max_open_zones)
seq_printf(m, ",max_open_zones=%u", mp->m_max_open_zones);
+ if (mp->m_awu_max_bytes)
+ seq_printf(m, ",max_atomic_write=%lluk",
+ mp->m_awu_max_bytes >> 10);
return 0;
}
@@ -1334,6 +1338,42 @@ suffix_kstrtoint(
return ret;
}
+static int
+suffix_kstrtoull(
+ const char *s,
+ unsigned int base,
+ unsigned long long *res)
+{
+ int last, shift_left_factor = 0;
+ unsigned long long _res;
+ char *value;
+ int ret = 0;
+
+ value = kstrdup(s, GFP_KERNEL);
+ if (!value)
+ return -ENOMEM;
+
+ last = strlen(value) - 1;
+ if (value[last] == 'K' || value[last] == 'k') {
+ shift_left_factor = 10;
+ value[last] = '\0';
+ }
+ if (value[last] == 'M' || value[last] == 'm') {
+ shift_left_factor = 20;
+ value[last] = '\0';
+ }
+ if (value[last] == 'G' || value[last] == 'g') {
+ shift_left_factor = 30;
+ value[last] = '\0';
+ }
+
+ if (kstrtoull(value, base, &_res))
+ ret = -EINVAL;
+ kfree(value);
+ *res = _res << shift_left_factor;
+ return ret;
+}
+
static inline void
xfs_fs_warn_deprecated(
struct fs_context *fc,
@@ -1518,6 +1558,14 @@ xfs_fs_parse_param(
case Opt_nolifetime:
parsing_mp->m_features |= XFS_FEAT_NOLIFETIME;
return 0;
+ case Opt_max_atomic_write:
+ if (suffix_kstrtoull(param->string, 10,
+ &parsing_mp->m_awu_max_bytes)) {
+ xfs_warn(parsing_mp,
+ "max atomic write size must be positive integer");
+ return -EINVAL;
+ }
+ return 0;
default:
xfs_warn(parsing_mp, "unknown mount option [%s].", param->key);
return -EINVAL;
@@ -2114,6 +2162,14 @@ xfs_fs_reconfigure(
if (error)
return error;
+ /* Validate new max_atomic_write option before making other changes */
+ if (mp->m_awu_max_bytes != new_mp->m_awu_max_bytes) {
+ error = xfs_set_max_atomic_write_opt(mp,
+ new_mp->m_awu_max_bytes);
+ if (error)
+ return error;
+ }
+
/* inode32 -> inode64 */
if (xfs_has_small_inums(mp) && !xfs_has_small_inums(new_mp)) {
mp->m_features &= ~XFS_FEAT_SMALL_INUMS;
diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index d5ae00f8e04c..01d284a1c759 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -230,6 +230,39 @@ TRACE_EVENT(xfs_calc_max_atomic_write_fsblocks,
__entry->blockcount)
);
+TRACE_EVENT(xfs_calc_max_atomic_write_log_geometry,
+ TP_PROTO(struct xfs_mount *mp, unsigned int per_intent,
+ unsigned int step_size, unsigned int blockcount,
+ unsigned int min_logblocks, unsigned int logres),
+ TP_ARGS(mp, per_intent, step_size, blockcount, min_logblocks, logres),
+ TP_STRUCT__entry(
+ __field(dev_t, dev)
+ __field(unsigned int, per_intent)
+ __field(unsigned int, step_size)
+ __field(unsigned int, blockcount)
+ __field(unsigned int, min_logblocks)
+ __field(unsigned int, cur_logblocks)
+ __field(unsigned int, logres)
+ ),
+ TP_fast_assign(
+ __entry->dev = mp->m_super->s_dev;
+ __entry->per_intent = per_intent;
+ __entry->step_size = step_size;
+ __entry->blockcount = blockcount;
+ __entry->min_logblocks = min_logblocks;
+ __entry->cur_logblocks = mp->m_sb.sb_logblocks;
+ __entry->logres = logres;
+ ),
+ TP_printk("dev %d:%d per_intent %u step_size %u blockcount %u min_logblocks %u logblocks %u logres %u",
+ MAJOR(__entry->dev), MINOR(__entry->dev),
+ __entry->per_intent,
+ __entry->step_size,
+ __entry->blockcount,
+ __entry->min_logblocks,
+ __entry->cur_logblocks,
+ __entry->logres)
+);
+
TRACE_EVENT(xlog_intent_recovery_failed,
TP_PROTO(struct xfs_mount *mp, const struct xfs_defer_op_type *ops,
int error),
--
2.31.1
^ permalink raw reply related
* [PATCH v9 14/15] xfs: update atomic write limits
From: John Garry @ 2025-04-25 16:45 UTC (permalink / raw)
To: brauner, djwong, hch, viro, jack, cem
Cc: linux-fsdevel, dchinner, linux-xfs, linux-kernel, ojaswin,
ritesh.list, martin.petersen, linux-ext4, linux-block,
catherine.hoang, linux-api, John Garry
In-Reply-To: <20250425164504.3263637-1-john.g.garry@oracle.com>
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: John Garry <john.g.garry@oracle.com>
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
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 v9 08/15] xfs: refine atomic write size check in xfs_file_write_iter()
From: John Garry @ 2025-04-25 16:44 UTC (permalink / raw)
To: brauner, djwong, hch, viro, jack, cem
Cc: linux-fsdevel, dchinner, linux-xfs, linux-kernel, ojaswin,
ritesh.list, martin.petersen, linux-ext4, linux-block,
catherine.hoang, linux-api, John Garry
In-Reply-To: <20250425164504.3263637-1-john.g.garry@oracle.com>
Currently the size of atomic write allowed is fixed at the blocksize.
To start to lift this restriction, partly refactor
xfs_report_atomic_write() to into helpers -
xfs_get_atomic_write_{min, max}() - and use those helpers to find the
per-inode atomic write limits and check according to that.
Also add xfs_get_atomic_write_max_opt() to return the optimal limit, and
just return 0 since large atomics aren't supported yet.
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: John Garry <john.g.garry@oracle.com>
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
fs/xfs/xfs_file.c | 12 +++++-------
fs/xfs/xfs_iops.c | 36 +++++++++++++++++++++++++++++++-----
fs/xfs/xfs_iops.h | 3 +++
3 files changed, 39 insertions(+), 12 deletions(-)
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 55bdae44e42a..e8acd6ca8f27 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -1032,14 +1032,12 @@ xfs_file_write_iter(
return xfs_file_dax_write(iocb, from);
if (iocb->ki_flags & IOCB_ATOMIC) {
- /*
- * Currently only atomic writing of a single FS block is
- * supported. It would be possible to atomic write smaller than
- * a FS block, but there is no requirement to support this.
- * Note that iomap also does not support this yet.
- */
- if (ocount != ip->i_mount->m_sb.sb_blocksize)
+ if (ocount < xfs_get_atomic_write_min(ip))
return -EINVAL;
+
+ if (ocount > xfs_get_atomic_write_max(ip))
+ return -EINVAL;
+
ret = generic_atomic_write_valid(iocb, from);
if (ret)
return ret;
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index 22432c300fd7..77a0606e9dc9 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -601,16 +601,42 @@ xfs_report_dioalign(
stat->dio_offset_align = stat->dio_read_offset_align;
}
+unsigned int
+xfs_get_atomic_write_min(
+ struct xfs_inode *ip)
+{
+ if (!xfs_inode_can_hw_atomic_write(ip))
+ return 0;
+
+ return ip->i_mount->m_sb.sb_blocksize;
+}
+
+unsigned int
+xfs_get_atomic_write_max(
+ struct xfs_inode *ip)
+{
+ if (!xfs_inode_can_hw_atomic_write(ip))
+ return 0;
+
+ return ip->i_mount->m_sb.sb_blocksize;
+}
+
+unsigned int
+xfs_get_atomic_write_max_opt(
+ struct xfs_inode *ip)
+{
+ return 0;
+}
+
static void
xfs_report_atomic_write(
struct xfs_inode *ip,
struct kstat *stat)
{
- unsigned int unit_min = 0, unit_max = 0;
-
- if (xfs_inode_can_hw_atomic_write(ip))
- unit_min = unit_max = ip->i_mount->m_sb.sb_blocksize;
- generic_fill_statx_atomic_writes(stat, unit_min, unit_max, 0);
+ generic_fill_statx_atomic_writes(stat,
+ xfs_get_atomic_write_min(ip),
+ xfs_get_atomic_write_max(ip),
+ xfs_get_atomic_write_max_opt(ip));
}
STATIC int
diff --git a/fs/xfs/xfs_iops.h b/fs/xfs/xfs_iops.h
index 3c1a2605ffd2..0896f6b8b3b8 100644
--- a/fs/xfs/xfs_iops.h
+++ b/fs/xfs/xfs_iops.h
@@ -19,5 +19,8 @@ int xfs_inode_init_security(struct inode *inode, struct inode *dir,
extern void xfs_setup_inode(struct xfs_inode *ip);
extern void xfs_setup_iops(struct xfs_inode *ip);
extern void xfs_diflags_to_iflags(struct xfs_inode *ip, bool init);
+unsigned int xfs_get_atomic_write_min(struct xfs_inode *ip);
+unsigned int xfs_get_atomic_write_max(struct xfs_inode *ip);
+unsigned int xfs_get_atomic_write_max_opt(struct xfs_inode *ip);
#endif /* __XFS_IOPS_H__ */
--
2.31.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox