* Re: [PATCH v8 11/15] xfs: commit CoW-based atomic writes atomically
From: Christoph Hellwig @ 2025-04-23 8:23 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: <20250422122739.2230121-12-john.g.garry@oracle.com>
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.
Otherwise looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [PATCH v7 11/14] xfs: add xfs_file_dio_write_atomic()
From: Christoph Hellwig @ 2025-04-23 8:19 UTC (permalink / raw)
To: Darrick J. Wong
Cc: John Garry, 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: <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 :)
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.
^ permalink raw reply
* Re: [PATCH v8 10/15] xfs: add large atomic writes checks in xfs_direct_write_iomap_begin()
From: John Garry @ 2025-04-23 8:18 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: <20250423081614.GC28307@lst.de>
On 23/04/2025 09:16, Christoph Hellwig wrote:
> On Tue, Apr 22, 2025 at 12:27:34PM +0000, John Garry wrote:
>> For when large atomic writes (> 1x FS block) are supported, there will be
>> various occasions when HW offload may not be possible.
>>
>> Such instances include:
>> - unaligned extent mapping wrt write length
>> - extent mappings which do not cover the full write, e.g. the write spans
>> sparse or mixed-mapping extents
>> - the write length is greater than HW offload can support
>
> or simply no hardware support at all.
sure, will add
>
> Otherwise looks good:
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
cheers
^ permalink raw reply
* Re: [PATCH v8 10/15] xfs: add large atomic writes checks in xfs_direct_write_iomap_begin()
From: Christoph Hellwig @ 2025-04-23 8:16 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: <20250422122739.2230121-11-john.g.garry@oracle.com>
On Tue, Apr 22, 2025 at 12:27:34PM +0000, John Garry wrote:
> For when large atomic writes (> 1x FS block) are supported, there will be
> various occasions when HW offload may not be possible.
>
> Such instances include:
> - unaligned extent mapping wrt write length
> - extent mappings which do not cover the full write, e.g. the write spans
> sparse or mixed-mapping extents
> - the write length is greater than HW offload can support
or simply no hardware support at all.
Otherwise looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [PATCH v8 09/15] xfs: add xfs_atomic_write_cow_iomap_begin()
From: Christoph Hellwig @ 2025-04-23 8:12 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: <20250422122739.2230121-10-john.g.garry@oracle.com>
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [PATCH v8 05/15] xfs: ignore HW which cannot atomic write a single block
From: Christoph Hellwig @ 2025-04-23 8:10 UTC (permalink / raw)
To: John Garry
Cc: Darrick J. Wong, 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: <f467a921-e7dd-4f5b-ac9f-c6e8c043143c@oracle.com>
On Wed, Apr 23, 2025 at 08:15:43AM +0100, John Garry wrote:
> Ideally we could have not set them in the first place, but need to know the
> blocksize when xfs_alloc_buftarg() is called, but it is not yet set for
> mp/sb. Is there any neat way to know the blocksize when xfs_alloc_buftarg()
> is called?
The buftarg is needed to read the superblock, which is used to determine
the block size, so no.
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.
^ permalink raw reply
* Re: [PATCH v7 11/14] xfs: add xfs_file_dio_write_atomic()
From: Luis Chamberlain @ 2025-04-23 7:36 UTC (permalink / raw)
To: John Garry
Cc: Chris Mason, Josef Bacik, 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, Pankaj Raghav, Daniel Gomez
In-Reply-To: <cf67f166-4c65-4d76-a3a2-1ad2614e89b7@oracle.com>
On Wed, Apr 23, 2025 at 08:08:40AM +0100, John Garry wrote:
> On 23/04/2025 06:18, Luis Chamberlain wrote:
> > On Tue, Apr 22, 2025 at 07:08:32AM +0100, John Garry wrote:
> > > On 21/04/2025 22:18, Luis Chamberlain wrote:
> >
> > Sounds like a terrible predicant for those that want hw atomics and
> > reliability for it.
>
> Well from our MySQL testing performance is good.
Good to hear, thanks!
Luis
^ permalink raw reply
* Re: newlines in filenames; POSIX.1-2024
From: Alejandro Colomar @ 2025-04-23 7:31 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: linux-kernel, linux-api, linux-man
In-Reply-To: <20250422222131.GE569616@mit.edu>
[-- Attachment #1: Type: text/plain, Size: 3110 bytes --]
Hi Ted,
On Tue, Apr 22, 2025 at 05:21:31PM -0500, Theodore Ts'o wrote:
> On Wed, Apr 16, 2025 at 06:50:00PM +0200, Alejandro Colomar wrote:
> >
> > I'm updating the manual pages for POSIX.1-2024. One of the changes
> > in this revision is that POSIX now encourages implementations to
> > disallow using new-line characters in file names.
> >
> > Historically, Linux (and maybe all existing POSIX systems?) has
> > allowed new-line characters in file names.
>
> Do we have any information of which implementations (if any) might
> decide to disallow new-line characters?
Such a list doesn't exist.
<http://austingroupbugs.net/view.php?id=251>
> If the Austin Group is going to add these sorts of "encouragements"
> without engaging with us dirctly, it seems to be much like King Canute
> commanding that the tide not come in....
>
> 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.
Newline is slightly more problematic than those, especially in scripts.
But yes, other characters (mainly control characters) were also
discussed in that bug. From what I can read, it seems they were scared
that if they attempted to suggest banning all control characters at
once, there might be more opposition, and the standard would be toilet
paper.
> 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,
That's an interesting approach. Being an opt-in mount option, users
will only break at their will, and they can always go back to old mode
when they need to do some operation with weird-sh*t characters.
TBH, while I see the chances of breaking stuff (so I don't see this
being the default in a long time; maybe ever), I think an opt-in mode
would be interesting, for those that know that don't need to handle such
broken file names, to have a tighter system. I would enable such a mode
in my systems.
> and it also begs the question of *which* weird-sh*t
> characters should be disallowed by the kernel.
I think a mode for disallowing _any control characters_ (aka [:cntrl:],
aka 0-31) would be a good choice.
> > 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...
I'm not worried about that, I was more worried about the churn in the
pages. I later remembered we have a pathname(7) page, so I'll put it
there, just once.
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 v8 05/15] xfs: ignore HW which cannot atomic write a single block
From: John Garry @ 2025-04-23 7:15 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: <20250423003823.GW25675@frogsfrogsfrogs>
On 23/04/2025 01:38, Darrick J. Wong wrote:
>>
>> + /*
>> + * Set atomic write unit max for mp. Ignore devices which cannot atomic
>> + * a single block, as they would be uncommon and more difficult to
>> + * support.
>> + */
>> + if (mp->m_ddev_targp->bt_bdev_awu_min <= mp->m_sb.sb_blocksize &&
>> + mp->m_ddev_targp->bt_bdev_awu_max >= mp->m_sb.sb_blocksize)
>> + mp->m_dd_awu_hw_max = mp->m_ddev_targp->bt_bdev_awu_max;
> If we don't want to use the device's atomic write capabilities due to
> fsblock alignment problems, why not just zero out bt_bdev_awu_min/max?
> That would cut down on the number of "awu" variables around the
> codebase.
Sure, I did consider this, but thought it a bit unpleasant to zero out
structure members like this.
Ideally we could have not set them in the first place, but need to know
the blocksize when xfs_alloc_buftarg() is called, but it is not yet set
for mp/sb. Is there any neat way to know the blocksize when
xfs_alloc_buftarg() is called?
>
> /*
> * Ignore hardware atomic writes if the device can't handle a single
> * fsblock for us. Most devices set the min_awu to the LBA size, but
> * the spec allows for a functionality gap.
> */
> static void
You would call this around the same point in xfs_mountfs(), right?
> xfs_buftarg_reconcile_awu(
> struct xfs_buftarg *btp)
> {
> struct xfs_mount *mp = btp->bt_mount;
>
> if (btp->bt_bdev_awu_min > mp->m_sb.sb_blocksize ||
> btp->bt_bdev_awu_max < mp->m_sb.sb_blocksize) {
> btp->bt_bdev_awu_min = 0;
> btp->bt_bdev_awu_max = 0;
> }
> }
>
> xfs_buftarg_reconcile_awu(mp->m_ddev_targp);
> if (mp->m_rtdev_targp)
> xfs_buftarg_reconcile_awu(mp->m_rtdev_targp);
>
> Hrm?
Cheers,
John
^ permalink raw reply
* Re: [PATCH v7 11/14] xfs: add xfs_file_dio_write_atomic()
From: John Garry @ 2025-04-23 7:08 UTC (permalink / raw)
To: Luis Chamberlain
Cc: Chris Mason, Josef Bacik, 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, Pankaj Raghav, Daniel Gomez
In-Reply-To: <aAh4L9crlnEf3uuJ@bombadil.infradead.org>
On 23/04/2025 06:18, Luis Chamberlain wrote:
> On Tue, Apr 22, 2025 at 07:08:32AM +0100, John Garry wrote:
>> On 21/04/2025 22:18, Luis Chamberlain wrote:
>>>> /*
>>>> + * 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) {
>>> Based on feedback from LSFMM, due to the performance variaibility this
>>> can introduce, it sounded like some folks would like to opt-in to not
>>> have a software fallback and just require an error out.
>>>> Could an option be added to not allow the software fallback?
>>
>> I still don't see the use in this.
>
> Its not the use, its the concern for underdeterminism in performance.
Sure, we don't offer RT performance guarantees, but what does?
>
>> So consider userspace wants to write something atomically and we fail as a
>> HW-based atomic write is not possible.
>
> Sounds like a terrible predicant for those that want hw atomics and
> reliability for it.
Well from our MySQL testing performance is good.
>
>> What is userspace going to do next?
>
> It would seem that would depend on their analysis on the number of
> software fallbacks where a software atomic based solution is used and
> the impact on performance.
sorry, but I don't understand this
>
>> I heard something like "if HW-based atomics are not possible, then something
>> has not been configured properly for the FS" - that something would be
>> extent granularity and alignment, but we don't have a method to ensure this.
>> That is the whole point of having a FS fallback.
>
> We do with LBS.
Sure, but not everyone wants LBS
> Its perfectly deterministic to be aligned with a sector
> size matching the block size, even for metadata writes.
>
>>> If so, then I think the next patch would also need updating.
>>>
>>> Or are you suggesting that without the software fallback atomic writes
>>> greater than fs block size are not possible?
>>
>> Yes, as XFS has no method to guarantee extent granularity and alignment.
>
> Ah, I think the documentation for this featuer should make this clear,
> it was not clear up to this point in patch review.
>
ok, that can be added
^ permalink raw reply
* Re: [PATCH v7 11/14] xfs: add xfs_file_dio_write_atomic()
From: John Garry @ 2025-04-23 7:02 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Luis Chamberlain, Chris Mason, Josef Bacik, 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, Pankaj Raghav, Daniel Gomez
In-Reply-To: <20250423054420.GB23087@lst.de>
On 23/04/2025 06:44, Christoph Hellwig wrote:
> On Tue, Apr 22, 2025 at 07:08:32AM +0100, John Garry wrote:
>> So consider userspace wants to write something atomically and we fail as a
>> HW-based atomic write is not possible. What is userspace going to do next?
> Exactly.
>
>> I heard something like "if HW-based atomics are not possible, then
>> something has not been configured properly for the FS" - that something
>> would be extent granularity and alignment, but we don't have a method to
>> ensure this. That is the whole point of having a FS fallback.
> We now have the opt limit, right?
right
> (I'll review the reposted series
> ASAP,
ok, cheers
> but for now I'll assume it) They can just tune their applications
> to it, and trigger on a trace point for the fallback to monitor it.
^ permalink raw reply
* Re: [PATCH v7 11/14] xfs: add xfs_file_dio_write_atomic()
From: Christoph Hellwig @ 2025-04-23 5:44 UTC (permalink / raw)
To: John Garry
Cc: Luis Chamberlain, Chris Mason, Josef Bacik, 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, Pankaj Raghav, Daniel Gomez
In-Reply-To: <69302bf1-78b4-4b95-8e9b-df56dd1091c0@oracle.com>
On Tue, Apr 22, 2025 at 07:08:32AM +0100, John Garry wrote:
> So consider userspace wants to write something atomically and we fail as a
> HW-based atomic write is not possible. What is userspace going to do next?
Exactly.
>
> I heard something like "if HW-based atomics are not possible, then
> something has not been configured properly for the FS" - that something
> would be extent granularity and alignment, but we don't have a method to
> ensure this. That is the whole point of having a FS fallback.
We now have the opt limit, right? (I'll review the reposted series
ASAP, but for now I'll assume it) They can just tune their applications
to it, and trigger on a trace point for the fallback to monitor it.
^ permalink raw reply
* Re: [PATCH v7 11/14] xfs: add xfs_file_dio_write_atomic()
From: Christoph Hellwig @ 2025-04-23 5:42 UTC (permalink / raw)
To: Darrick J. Wong
Cc: John Garry, 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: <20250421164241.GD25700@frogsfrogsfrogs>
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 :)
> 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 v7 11/14] xfs: add xfs_file_dio_write_atomic()
From: Luis Chamberlain @ 2025-04-23 5:18 UTC (permalink / raw)
To: John Garry
Cc: Chris Mason, Josef Bacik, 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, Pankaj Raghav, Daniel Gomez
In-Reply-To: <69302bf1-78b4-4b95-8e9b-df56dd1091c0@oracle.com>
On Tue, Apr 22, 2025 at 07:08:32AM +0100, John Garry wrote:
> On 21/04/2025 22:18, Luis Chamberlain wrote:
> > > /*
> > > + * 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) {
> > Based on feedback from LSFMM, due to the performance variaibility this
> > can introduce, it sounded like some folks would like to opt-in to not
> > have a software fallback and just require an error out.
> > > Could an option be added to not allow the software fallback?
>
> I still don't see the use in this.
Its not the use, its the concern for underdeterminism in performance.
> So consider userspace wants to write something atomically and we fail as a
> HW-based atomic write is not possible.
Sounds like a terrible predicant for those that want hw atomics and
reliability for it.
> What is userspace going to do next?
It would seem that would depend on their analysis on the number of
software fallbacks where a software atomic based solution is used and
the impact on performance.
> I heard something like "if HW-based atomics are not possible, then something
> has not been configured properly for the FS" - that something would be
> extent granularity and alignment, but we don't have a method to ensure this.
> That is the whole point of having a FS fallback.
We do with LBS. Its perfectly deterministic to be aligned with a sector
size matching the block size, even for metadata writes.
> > If so, then I think the next patch would also need updating.
> >
> > Or are you suggesting that without the software fallback atomic writes
> > greater than fs block size are not possible?
>
> Yes, as XFS has no method to guarantee extent granularity and alignment.
Ah, I think the documentation for this featuer should make this clear,
it was not clear up to this point in patch review.
Luis
^ 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 0:38 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: <20250422122739.2230121-6-john.g.garry@oracle.com>
On Tue, Apr 22, 2025 at 12:27:29PM +0000, John Garry wrote:
> 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: John Garry <john.g.garry@oracle.com>
> ---
> fs/xfs/xfs_inode.h | 17 ++++++++---------
> fs/xfs/xfs_mount.c | 14 ++++++++++++++
> fs/xfs/xfs_mount.h | 4 ++++
> 3 files changed, 26 insertions(+), 9 deletions(-)
>
> diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
> index cff643cd03fc..725cd7c16a6e 100644
> --- a/fs/xfs/xfs_inode.h
> +++ b/fs/xfs/xfs_inode.h
> @@ -355,20 +355,19 @@ static inline bool xfs_inode_has_bigrtalloc(const struct xfs_inode *ip)
> #define xfs_inode_buftarg(ip) \
> (XFS_IS_REALTIME_INODE(ip) ? \
> (ip)->i_mount->m_rtdev_targp : (ip)->i_mount->m_ddev_targp)
> +/*
> + * Return max atomic write unit for a given inode.
> + */
> +#define xfs_inode_hw_atomicwrite_max(ip) \
> + (XFS_IS_REALTIME_INODE(ip) ? \
> + (ip)->i_mount->m_rt_awu_hw_max : \
> + (ip)->i_mount->m_dd_awu_hw_max)
>
> static inline bool
> xfs_inode_can_hw_atomicwrite(
> 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_hw_atomicwrite_max(ip);
> }
>
> /*
> diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> index 00b53f479ece..ee68c026e6cd 100644
> --- a/fs/xfs/xfs_mount.c
> +++ b/fs/xfs/xfs_mount.c
> @@ -1082,6 +1082,20 @@ xfs_mountfs(
> xfs_zone_gc_start(mp);
> }
>
> + /*
> + * Set atomic write unit max for mp. Ignore devices which cannot atomic
> + * a single block, as they would be uncommon and more difficult to
> + * support.
> + */
> + if (mp->m_ddev_targp->bt_bdev_awu_min <= mp->m_sb.sb_blocksize &&
> + mp->m_ddev_targp->bt_bdev_awu_max >= mp->m_sb.sb_blocksize)
> + mp->m_dd_awu_hw_max = mp->m_ddev_targp->bt_bdev_awu_max;
If we don't want to use the device's atomic write capabilities due to
fsblock alignment problems, why not just zero out bt_bdev_awu_min/max?
That would cut down on the number of "awu" variables around the
codebase.
/*
* Ignore hardware atomic writes if the device can't handle a single
* fsblock for us. Most devices set the min_awu to the LBA size, but
* the spec allows for a functionality gap.
*/
static void
xfs_buftarg_reconcile_awu(
struct xfs_buftarg *btp)
{
struct xfs_mount *mp = btp->bt_mount;
if (btp->bt_bdev_awu_min > mp->m_sb.sb_blocksize ||
btp->bt_bdev_awu_max < mp->m_sb.sb_blocksize) {
btp->bt_bdev_awu_min = 0;
btp->bt_bdev_awu_max = 0;
}
}
xfs_buftarg_reconcile_awu(mp->m_ddev_targp);
if (mp->m_rtdev_targp)
xfs_buftarg_reconcile_awu(mp->m_rtdev_targp);
Hrm?
--D
> +
> + if (mp->m_rtdev_targp &&
> + mp->m_rtdev_targp->bt_bdev_awu_min <= mp->m_sb.sb_blocksize &&
> + mp->m_rtdev_targp->bt_bdev_awu_max >= mp->m_sb.sb_blocksize)
> + mp->m_rt_awu_hw_max = mp->m_rtdev_targp->bt_bdev_awu_max;
> +
> return 0;
>
> out_agresv:
> diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
> index e5192c12e7ac..2819e160f0e9 100644
> --- a/fs/xfs/xfs_mount.h
> +++ b/fs/xfs/xfs_mount.h
> @@ -231,6 +231,10 @@ typedef struct xfs_mount {
> unsigned int m_max_open_zones;
> unsigned int m_zonegc_low_space;
>
> + /* ddev and rtdev HW max atomic write size */
> + unsigned int m_dd_awu_hw_max;
> + unsigned int m_rt_awu_hw_max;
> +
> /*
> * Bitsets of per-fs metadata that have been checked and/or are sick.
> * Callers must hold m_sb_lock to access these two fields.
> --
> 2.31.1
>
>
^ permalink raw reply
* Re: newlines in filenames; POSIX.1-2024
From: Theodore Ts'o @ 2025-04-22 22:21 UTC (permalink / raw)
To: Alejandro Colomar; +Cc: linux-kernel, linux-api, linux-man
In-Reply-To: <iezzxq25mqdcapusb32euu3fgvz7djtrn5n66emb72jb3bqltx@lr2545vnc55k>
On Wed, Apr 16, 2025 at 06:50:00PM +0200, Alejandro Colomar wrote:
>
> I'm updating the manual pages for POSIX.1-2024. One of the changes
> in this revision is that POSIX now encourages implementations to
> disallow using new-line characters in file names.
>
> Historically, Linux (and maybe all existing POSIX systems?) has
> allowed new-line characters in file names.
Do we have any information of which implementations (if any) might
decide to disallow new-line characters?
If the Austin Group is going to add these sorts of "encouragements"
without engaging with us dirctly, it seems to be much like King Canute
commanding that the tide not come in....
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.
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.
> 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...
- Ted
^ permalink raw reply
* Re: [PATCH v4 3/3] fs: introduce getfsxattrat and setfsxattrat syscalls
From: Christian Brauner @ 2025-04-22 15:14 UTC (permalink / raw)
To: Amir Goldstein
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-suchen-filmpreis-3573a913457c@brauner>
On Tue, Apr 22, 2025 at 04:31:29PM +0200, Christian Brauner wrote:
> On Thu, Mar 27, 2025 at 12:39:28PM +0100, Amir Goldstein wrote:
> > On Thu, Mar 27, 2025 at 10:33 AM Andrey Albershteyn <aalbersh@redhat.com> wrote:
> > >
> > > On 2025-03-23 09:56:25, Amir Goldstein wrote:
> > > > On Fri, Mar 21, 2025 at 8:49 PM Andrey Albershteyn <aalbersh@redhat.com> wrote:
> > > > >
> > > > > From: Andrey Albershteyn <aalbersh@redhat.com>
> > > > >
> > > > > Introduce getfsxattrat and setfsxattrat syscalls to manipulate inode
> > > > > extended attributes/flags. The syscalls take parent directory fd and
> > > > > path to the child together with struct fsxattr.
> > > > >
> > > > > This is an alternative to FS_IOC_FSSETXATTR ioctl with a difference
> > > > > that file don't need to be open as we can reference it with a path
> > > > > instead of fd. By having this we can manipulated inode extended
> > > > > attributes not only on regular files but also on special ones. This
> > > > > is not possible with FS_IOC_FSSETXATTR ioctl as with special files
> > > > > we can not call ioctl() directly on the filesystem inode using fd.
> > > > >
> > > > > This patch adds two new syscalls which allows userspace to get/set
> > > > > extended inode attributes on special files by using parent directory
> > > > > and a path - *at() like syscall.
> > > > >
> > > > > CC: linux-api@vger.kernel.org
> > > > > CC: linux-fsdevel@vger.kernel.org
> > > > > CC: linux-xfs@vger.kernel.org
> > > > > Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com>
> > > > > Acked-by: Arnd Bergmann <arnd@arndb.de>
> > > > > ---
> > > > ...
> > > > > +SYSCALL_DEFINE5(setfsxattrat, int, dfd, const char __user *, filename,
> > > > > + struct fsxattr __user *, ufsx, size_t, usize,
> > > > > + unsigned int, at_flags)
> > > > > +{
> > > > > + struct fileattr fa;
> > > > > + struct path filepath;
> > > > > + int error;
> > > > > + unsigned int lookup_flags = 0;
> > > > > + struct filename *name;
> > > > > + struct mnt_idmap *idmap;.
> > > >
> > > > > + struct dentry *dentry;
> > > > > + struct vfsmount *mnt;
> > > > > + struct fsxattr fsx = {};
> > > > > +
> > > > > + BUILD_BUG_ON(sizeof(struct fsxattr) < FSXATTR_SIZE_VER0);
> > > > > + BUILD_BUG_ON(sizeof(struct fsxattr) != FSXATTR_SIZE_LATEST);
> > > > > +
> > > > > + if ((at_flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) != 0)
> > > > > + return -EINVAL;
> > > > > +
> > > > > + if (!(at_flags & AT_SYMLINK_NOFOLLOW))
> > > > > + lookup_flags |= LOOKUP_FOLLOW;
> > > > > +
> > > > > + if (at_flags & AT_EMPTY_PATH)
> > > > > + lookup_flags |= LOOKUP_EMPTY;
> > > > > +
> > > > > + if (usize > PAGE_SIZE)
> > > > > + return -E2BIG;
> > > > > +
> > > > > + if (usize < FSXATTR_SIZE_VER0)
> > > > > + return -EINVAL;
> > > > > +
> > > > > + error = copy_struct_from_user(&fsx, sizeof(struct fsxattr), ufsx, usize);
> > > > > + if (error)
> > > > > + return error;
> > > > > +
> > > > > + fsxattr_to_fileattr(&fsx, &fa);
> > > > > +
> > > > > + name = getname_maybe_null(filename, at_flags);
> > > > > + if (!name) {
> > > > > + CLASS(fd, f)(dfd);
> > > > > +
> > > > > + if (fd_empty(f))
> > > > > + return -EBADF;
> > > > > +
> > > > > + idmap = file_mnt_idmap(fd_file(f));
> > > > > + dentry = file_dentry(fd_file(f));
> > > > > + mnt = fd_file(f)->f_path.mnt;
> > > > > + } else {
> > > > > + error = filename_lookup(dfd, name, lookup_flags, &filepath,
> > > > > + NULL);
> > > > > + if (error)
> > > > > + return error;
> > > > > +
> > > > > + idmap = mnt_idmap(filepath.mnt);
> > > > > + dentry = filepath.dentry;
> > > > > + mnt = filepath.mnt;
> > > > > + }
> > > > > +
> > > > > + error = mnt_want_write(mnt);
> > > > > + if (!error) {
> > > > > + error = vfs_fileattr_set(idmap, dentry, &fa);
> > > > > + if (error == -ENOIOCTLCMD)
> > > > > + error = -EOPNOTSUPP;
> > > >
> > > > This is awkward.
> > > > vfs_fileattr_set() should return -EOPNOTSUPP.
> > > > ioctl_setflags() could maybe convert it to -ENOIOCTLCMD,
> > > > but looking at similar cases ioctl_fiemap(), ioctl_fsfreeze() the
> > > > ioctl returns -EOPNOTSUPP.
> > > >
> > > > I don't think it is necessarily a bad idea to start returning
> > > > -EOPNOTSUPP instead of -ENOIOCTLCMD for the ioctl
> > > > because that really reflects the fact that the ioctl is now implemented
> > > > in vfs and not in the specific fs.
> > > >
> > > > and I think it would not be a bad idea at all to make that change
> > > > together with the merge of the syscalls as a sort of hint to userspace
> > > > that uses the ioctl, that the sycalls API exists.
> > > >
> > > > Thanks,
> > > > Amir.
> > > >
> > >
> > > Hmm, not sure what you're suggesting here. I see it as:
> > > - get/setfsxattrat should return EOPNOTSUPP as it make more sense
> > > than ENOIOCTLCMD
> > > - ioctl_setflags returns ENOIOCTLCMD which also expected
> > >
> > > Don't really see a reason to change what vfs_fileattr_set() returns
> > > and then copying this if() to other places or start returning
> > > EOPNOTSUPP.
> >
> > ENOIOCTLCMD conceptually means that the ioctl command is unknown
> > This is not the case since ->fileattr_[gs]et() became a vfs API
>
> vfs_fileattr_{g,s}et() should not return ENOIOCTLCMD. Change the return
> code to EOPNOTSUPP and then make EOPNOTSUPP be translated to ENOTTY on
> on overlayfs and to ENOIOCTLCMD in ecryptfs and in fs/ioctl.c. This way
> we get a clean VFS api while retaining current behavior. Amir can do his
> cleanup based on that.
Also this get/set dance is not something new apis should do. It should
be handled like setattr_prepare() or generic_fillattr() where the
filesystem calls a VFS helper and that does all of this based on the
current state of the inode instead of calling into the filesystem twice:
int vfs_fileattr_set(struct mnt_idmap *idmap, struct dentry *dentry,
struct fileattr *fa)
{
<snip>
inode_lock(inode);
err = vfs_fileattr_get(dentry, &old_ma);
if (!err) {
/* initialize missing bits from old_ma */
if (fa->flags_valid) {
<snip>
err = fileattr_set_prepare(inode, &old_ma, fa);
if (!err && !security_inode_setfsxattr(inode, fa))
err = inode->i_op->fileattr_set(idmap, dentry, fa);
^ permalink raw reply
* Re: [PATCH v4 3/3] fs: introduce getfsxattrat and setfsxattrat syscalls
From: Christian Brauner @ 2025-04-22 14:59 UTC (permalink / raw)
To: Andrey Albershteyn
Cc: 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: <20250321-xattrat-syscall-v4-3-3e82e6fb3264@kernel.org>
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>
> ---
> arch/alpha/kernel/syscalls/syscall.tbl | 2 +
> arch/arm/tools/syscall.tbl | 2 +
> arch/arm64/tools/syscall_32.tbl | 2 +
> arch/m68k/kernel/syscalls/syscall.tbl | 2 +
> arch/microblaze/kernel/syscalls/syscall.tbl | 2 +
> arch/mips/kernel/syscalls/syscall_n32.tbl | 2 +
> arch/mips/kernel/syscalls/syscall_n64.tbl | 2 +
> arch/mips/kernel/syscalls/syscall_o32.tbl | 2 +
> arch/parisc/kernel/syscalls/syscall.tbl | 2 +
> arch/powerpc/kernel/syscalls/syscall.tbl | 2 +
> arch/s390/kernel/syscalls/syscall.tbl | 2 +
> arch/sh/kernel/syscalls/syscall.tbl | 2 +
> arch/sparc/kernel/syscalls/syscall.tbl | 2 +
> arch/x86/entry/syscalls/syscall_32.tbl | 2 +
> arch/x86/entry/syscalls/syscall_64.tbl | 2 +
> arch/xtensa/kernel/syscalls/syscall.tbl | 2 +
> fs/inode.c | 130 ++++++++++++++++++++++++++++
> include/linux/syscalls.h | 6 ++
> include/uapi/asm-generic/unistd.h | 8 +-
> include/uapi/linux/fs.h | 3 +
> 20 files changed, 178 insertions(+), 1 deletion(-)
>
> diff --git a/arch/alpha/kernel/syscalls/syscall.tbl b/arch/alpha/kernel/syscalls/syscall.tbl
> index c59d53d6d3f3490f976ca179ddfe02e69265ae4d..4b9e687494c16b60c6fd6ca1dc4d6564706a7e25 100644
> --- a/arch/alpha/kernel/syscalls/syscall.tbl
> +++ b/arch/alpha/kernel/syscalls/syscall.tbl
> @@ -506,3 +506,5 @@
> 574 common getxattrat sys_getxattrat
> 575 common listxattrat sys_listxattrat
> 576 common removexattrat sys_removexattrat
> +577 common getfsxattrat sys_getfsxattrat
> +578 common setfsxattrat sys_setfsxattrat
> diff --git a/arch/arm/tools/syscall.tbl b/arch/arm/tools/syscall.tbl
> index 49eeb2ad8dbd8e074c6240417693f23fb328afa8..66466257f3c2debb3e2299f0b608c6740c98cab2 100644
> --- a/arch/arm/tools/syscall.tbl
> +++ b/arch/arm/tools/syscall.tbl
> @@ -481,3 +481,5 @@
> 464 common getxattrat sys_getxattrat
> 465 common listxattrat sys_listxattrat
> 466 common removexattrat sys_removexattrat
> +467 common getfsxattrat sys_getfsxattrat
> +468 common setfsxattrat sys_setfsxattrat
> diff --git a/arch/arm64/tools/syscall_32.tbl b/arch/arm64/tools/syscall_32.tbl
> index 69a829912a05eb8a3e21ed701d1030e31c0148bc..9c516118b154811d8d11d5696f32817430320dbf 100644
> --- a/arch/arm64/tools/syscall_32.tbl
> +++ b/arch/arm64/tools/syscall_32.tbl
> @@ -478,3 +478,5 @@
> 464 common getxattrat sys_getxattrat
> 465 common listxattrat sys_listxattrat
> 466 common removexattrat sys_removexattrat
> +467 common getfsxattrat sys_getfsxattrat
> +468 common setfsxattrat sys_setfsxattrat
> diff --git a/arch/m68k/kernel/syscalls/syscall.tbl b/arch/m68k/kernel/syscalls/syscall.tbl
> index f5ed71f1910d09769c845c2d062d99ee0449437c..159476387f394a92ee5e29db89b118c630372db2 100644
> --- a/arch/m68k/kernel/syscalls/syscall.tbl
> +++ b/arch/m68k/kernel/syscalls/syscall.tbl
> @@ -466,3 +466,5 @@
> 464 common getxattrat sys_getxattrat
> 465 common listxattrat sys_listxattrat
> 466 common removexattrat sys_removexattrat
> +467 common getfsxattrat sys_getfsxattrat
> +468 common setfsxattrat sys_setfsxattrat
> diff --git a/arch/microblaze/kernel/syscalls/syscall.tbl b/arch/microblaze/kernel/syscalls/syscall.tbl
> index 680f568b77f2cbefc3eacb2517f276041f229b1e..a6d59ee740b58cacf823702003cf9bad17c0d3b7 100644
> --- a/arch/microblaze/kernel/syscalls/syscall.tbl
> +++ b/arch/microblaze/kernel/syscalls/syscall.tbl
> @@ -472,3 +472,5 @@
> 464 common getxattrat sys_getxattrat
> 465 common listxattrat sys_listxattrat
> 466 common removexattrat sys_removexattrat
> +467 common getfsxattrat sys_getfsxattrat
> +468 common setfsxattrat sys_setfsxattrat
> diff --git a/arch/mips/kernel/syscalls/syscall_n32.tbl b/arch/mips/kernel/syscalls/syscall_n32.tbl
> index 0b9b7e25b69ad592642f8533bee9ccfe95ce9626..cfe38fcebe1a0279e11751378d3e71c5ec6b6569 100644
> --- a/arch/mips/kernel/syscalls/syscall_n32.tbl
> +++ b/arch/mips/kernel/syscalls/syscall_n32.tbl
> @@ -405,3 +405,5 @@
> 464 n32 getxattrat sys_getxattrat
> 465 n32 listxattrat sys_listxattrat
> 466 n32 removexattrat sys_removexattrat
> +467 n32 getfsxattrat sys_getfsxattrat
> +468 n32 setfsxattrat sys_setfsxattrat
> diff --git a/arch/mips/kernel/syscalls/syscall_n64.tbl b/arch/mips/kernel/syscalls/syscall_n64.tbl
> index c844cd5cda620b2809a397cdd6f4315ab6a1bfe2..29a0c5974d1aa2f01e33edc0252d75fb97abe230 100644
> --- a/arch/mips/kernel/syscalls/syscall_n64.tbl
> +++ b/arch/mips/kernel/syscalls/syscall_n64.tbl
> @@ -381,3 +381,5 @@
> 464 n64 getxattrat sys_getxattrat
> 465 n64 listxattrat sys_listxattrat
> 466 n64 removexattrat sys_removexattrat
> +467 n64 getfsxattrat sys_getfsxattrat
> +468 n64 setfsxattrat sys_setfsxattrat
> diff --git a/arch/mips/kernel/syscalls/syscall_o32.tbl b/arch/mips/kernel/syscalls/syscall_o32.tbl
> index 349b8aad1159f404103bd2057a1e64e9bf309f18..6c00436807c57c492ba957fcd59af1202231cf80 100644
> --- a/arch/mips/kernel/syscalls/syscall_o32.tbl
> +++ b/arch/mips/kernel/syscalls/syscall_o32.tbl
> @@ -454,3 +454,5 @@
> 464 o32 getxattrat sys_getxattrat
> 465 o32 listxattrat sys_listxattrat
> 466 o32 removexattrat sys_removexattrat
> +467 o32 getfsxattrat sys_getfsxattrat
> +468 o32 setfsxattrat sys_setfsxattrat
> diff --git a/arch/parisc/kernel/syscalls/syscall.tbl b/arch/parisc/kernel/syscalls/syscall.tbl
> index d9fc94c869657fcfbd7aca1d5f5abc9fae2fb9d8..b3578fac43d6b65167787fcc97d2d09f5a9828e7 100644
> --- a/arch/parisc/kernel/syscalls/syscall.tbl
> +++ b/arch/parisc/kernel/syscalls/syscall.tbl
> @@ -465,3 +465,5 @@
> 464 common getxattrat sys_getxattrat
> 465 common listxattrat sys_listxattrat
> 466 common removexattrat sys_removexattrat
> +467 common getfsxattrat sys_getfsxattrat
> +468 common setfsxattrat sys_setfsxattrat
> diff --git a/arch/powerpc/kernel/syscalls/syscall.tbl b/arch/powerpc/kernel/syscalls/syscall.tbl
> index d8b4ab78bef076bd50d49b87dea5060fd8c1686a..808045d82c9465c3bfa96b15947546efe5851e9a 100644
> --- a/arch/powerpc/kernel/syscalls/syscall.tbl
> +++ b/arch/powerpc/kernel/syscalls/syscall.tbl
> @@ -557,3 +557,5 @@
> 464 common getxattrat sys_getxattrat
> 465 common listxattrat sys_listxattrat
> 466 common removexattrat sys_removexattrat
> +467 common getfsxattrat sys_getfsxattrat
> +468 common setfsxattrat sys_setfsxattrat
> diff --git a/arch/s390/kernel/syscalls/syscall.tbl b/arch/s390/kernel/syscalls/syscall.tbl
> index e9115b4d8b635b846e5c9ad6ce229605323723a5..78dfc2c184d4815baf8a9e61c546c9936d58a47c 100644
> --- a/arch/s390/kernel/syscalls/syscall.tbl
> +++ b/arch/s390/kernel/syscalls/syscall.tbl
> @@ -469,3 +469,5 @@
> 464 common getxattrat sys_getxattrat sys_getxattrat
> 465 common listxattrat sys_listxattrat sys_listxattrat
> 466 common removexattrat sys_removexattrat sys_removexattrat
> +467 common getfsxattrat sys_getfsxattrat sys_getfsxattrat
> +468 common setfsxattrat sys_setfsxattrat sys_setfsxattrat
> diff --git a/arch/sh/kernel/syscalls/syscall.tbl b/arch/sh/kernel/syscalls/syscall.tbl
> index c8cad33bf250ea110de37bd1407f5a43ec5e38f2..d5a5c8339f0ed25ea07c4aba90351d352033c8a0 100644
> --- a/arch/sh/kernel/syscalls/syscall.tbl
> +++ b/arch/sh/kernel/syscalls/syscall.tbl
> @@ -470,3 +470,5 @@
> 464 common getxattrat sys_getxattrat
> 465 common listxattrat sys_listxattrat
> 466 common removexattrat sys_removexattrat
> +467 common getfsxattrat sys_getfsxattrat
> +468 common setfsxattrat sys_setfsxattrat
> diff --git a/arch/sparc/kernel/syscalls/syscall.tbl b/arch/sparc/kernel/syscalls/syscall.tbl
> index 727f99d333b304b3db0711953a3d91ece18a28eb..817dcd8603bcbffc47f3f59aa3b74b16486453d0 100644
> --- a/arch/sparc/kernel/syscalls/syscall.tbl
> +++ b/arch/sparc/kernel/syscalls/syscall.tbl
> @@ -512,3 +512,5 @@
> 464 common getxattrat sys_getxattrat
> 465 common listxattrat sys_listxattrat
> 466 common removexattrat sys_removexattrat
> +467 common getfsxattrat sys_getfsxattrat
> +468 common setfsxattrat sys_setfsxattrat
> diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
> index 4d0fb2fba7e208ae9455459afe11e277321d9f74..b4842c027c5d00c0236b2ba89387c5e2267447bd 100644
> --- a/arch/x86/entry/syscalls/syscall_32.tbl
> +++ b/arch/x86/entry/syscalls/syscall_32.tbl
> @@ -472,3 +472,5 @@
> 464 i386 getxattrat sys_getxattrat
> 465 i386 listxattrat sys_listxattrat
> 466 i386 removexattrat sys_removexattrat
> +467 i386 getfsxattrat sys_getfsxattrat
> +468 i386 setfsxattrat sys_setfsxattrat
> diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
> index 5eb708bff1c791debd6cfc5322583b2ae53f6437..b6f0a7236aaee624cf9b484239a1068085a8ffe1 100644
> --- a/arch/x86/entry/syscalls/syscall_64.tbl
> +++ b/arch/x86/entry/syscalls/syscall_64.tbl
> @@ -390,6 +390,8 @@
> 464 common getxattrat sys_getxattrat
> 465 common listxattrat sys_listxattrat
> 466 common removexattrat sys_removexattrat
> +467 common getfsxattrat sys_getfsxattrat
> +468 common setfsxattrat sys_setfsxattrat
>
> #
> # Due to a historical design error, certain syscalls are numbered differently
> diff --git a/arch/xtensa/kernel/syscalls/syscall.tbl b/arch/xtensa/kernel/syscalls/syscall.tbl
> index 37effc1b134eea061f2c350c1d68b4436b65a4dd..425d56be337d1de22f205ac503df61ff86224fee 100644
> --- a/arch/xtensa/kernel/syscalls/syscall.tbl
> +++ b/arch/xtensa/kernel/syscalls/syscall.tbl
> @@ -437,3 +437,5 @@
> 464 common getxattrat sys_getxattrat
> 465 common listxattrat sys_listxattrat
> 466 common removexattrat sys_removexattrat
> +467 common getfsxattrat sys_getfsxattrat
> +468 common setfsxattrat sys_setfsxattrat
> diff --git a/fs/inode.c b/fs/inode.c
> index 6b4c77268fc0ecace4ac78a9ca777fbffc277f4a..811debf379ab299f287ed90863277cfda27db30c 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
I really dislike the name fsxattr for a lot of reasons but it definitely
shouldn't go in inode.c. Just add a new fs/file_attr.c file and move all
the relevant helpers from fs/ioctl.c in there and then add the system
calls. Otherwise it's just all very confusing.
> @@ -23,6 +23,9 @@
> #include <linux/rw_hint.h>
> #include <linux/seq_file.h>
> #include <linux/debugfs.h>
> +#include <linux/syscalls.h>
> +#include <linux/fileattr.h>
> +#include <linux/namei.h>
> #include <trace/events/writeback.h>
> #define CREATE_TRACE_POINTS
> #include <trace/events/timestamp.h>
> @@ -2953,3 +2956,130 @@ umode_t mode_strip_sgid(struct mnt_idmap *idmap,
> return mode & ~S_ISGID;
> }
> EXPORT_SYMBOL(mode_strip_sgid);
> +
> +SYSCALL_DEFINE5(getfsxattrat, int, dfd, const char __user *, filename,
This is really misnamed. It will end up confusing userspace to no end:
getxattr()
getxattrat()
getfsxattrat()
Please name this file_setattr() and file_getattr(). There's also no need
for the *at() prefix. We've never been consistent with that. We have
plent of system calls that to fd+path without having an *at() prefix.
And here it's especially unneeded because there's no pre-existing system
calls that would even force the use of that prefix.
> + 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);
> + CLASS(fd, f)(dfd);
> +
> + if (fd_empty(f))
> + return -EBADF;
I'm pretty sure you can just do a:
path = fd_file(f_to)->f_path;
path_get(&path);
and then the vfs_fileattr_get() call and the path_put() call can be
shared between the two branches. Note that you can also use:
struct path path __free(path_put) = NULL;
and then the cleanup infrastructure will handle the path_put() for you.
> + error = vfs_fileattr_get(file_dentry(fd_file(f)), &fa);
> + } else {
> + error = filename_lookup(dfd, name, lookup_flags, &filepath,
> + NULL);
> + if (error)
> + goto out;
> + error = vfs_fileattr_get(filepath.dentry, &fa);
> + path_put(&filepath);
> + }
> + if (error == -ENOIOCTLCMD)
> + error = -EOPNOTSUPP;
> + if (!error) {
> + fileattr_to_fsxattr(&fa, &fsx);
> + error = copy_struct_to_user(ufsx, usize, &fsx,
> + sizeof(struct fsxattr), NULL);
> + }
> +out:
> + putname(name);
> + return error;
> +}
> +
> +SYSCALL_DEFINE5(setfsxattrat, int, dfd, const char __user *, filename,
> + struct fsxattr __user *, ufsx, size_t, usize,
> + unsigned int, at_flags)
> +{
> + struct fileattr fa;
> + struct path filepath;
> + int error;
> + unsigned int lookup_flags = 0;
> + struct filename *name;
> + struct mnt_idmap *idmap;
> + struct dentry *dentry;
> + struct vfsmount *mnt;
> + struct fsxattr fsx = {};
> +
> + BUILD_BUG_ON(sizeof(struct fsxattr) < FSXATTR_SIZE_VER0);
> + BUILD_BUG_ON(sizeof(struct fsxattr) != FSXATTR_SIZE_LATEST);
> +
> + if ((at_flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) != 0)
> + return -EINVAL;
> +
> + if (!(at_flags & AT_SYMLINK_NOFOLLOW))
> + lookup_flags |= LOOKUP_FOLLOW;
> +
> + if (at_flags & AT_EMPTY_PATH)
> + lookup_flags |= LOOKUP_EMPTY;
> +
> + if (usize > PAGE_SIZE)
> + return -E2BIG;
> +
> + if (usize < FSXATTR_SIZE_VER0)
> + return -EINVAL;
> +
> + error = copy_struct_from_user(&fsx, sizeof(struct fsxattr), ufsx, usize);
> + if (error)
> + return error;
> +
> + fsxattr_to_fileattr(&fsx, &fa);
> +
> + name = getname_maybe_null(filename, at_flags);
> + if (!name) {
Same comment as above.
> + CLASS(fd, f)(dfd);
> +
> + if (fd_empty(f))
> + return -EBADF;
> +
> + idmap = file_mnt_idmap(fd_file(f));
> + dentry = file_dentry(fd_file(f));
> + mnt = fd_file(f)->f_path.mnt;
This is a UAF. fd_file(f)->f_path.mnt and file_dentry(fd_file(f)) will
get auto cleaned at the end of the scope. By the time you call
vfs_fileattr_set() nothing pins them anymore...
In general, same comment about unifying the branches as for the get case
via path_get() as above. And just keep the path around don't store mount
and dentry separately.
> + } else {
> + error = filename_lookup(dfd, name, lookup_flags, &filepath,
> + NULL);
> + if (error)
> + return error;
> +
> + idmap = mnt_idmap(filepath.mnt);
> + dentry = filepath.dentry;
> + mnt = filepath.mnt;
> + }
> +
> + error = mnt_want_write(mnt);
> + if (!error) {
> + error = vfs_fileattr_set(idmap, dentry, &fa);
> + if (error == -ENOIOCTLCMD)
> + error = -EOPNOTSUPP;
> + mnt_drop_write(mnt);
> + }
> +
> + path_put(&filepath);
> + return error;
> +}
> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> index c6333204d45130eb022f6db460eea34a1f6e91db..e242ea39b3e63a8008bc777764b616fd63bd40c4 100644
> --- a/include/linux/syscalls.h
> +++ b/include/linux/syscalls.h
> @@ -371,6 +371,12 @@ asmlinkage long sys_removexattrat(int dfd, const char __user *path,
> asmlinkage long sys_lremovexattr(const char __user *path,
> const char __user *name);
> asmlinkage long sys_fremovexattr(int fd, const char __user *name);
> +asmlinkage long sys_getfsxattrat(int dfd, const char __user *filename,
> + struct fsxattr __user *ufsx, size_t usize,
> + unsigned int at_flags);
> +asmlinkage long sys_setfsxattrat(int dfd, const char __user *filename,
> + struct fsxattr __user *ufsx, size_t usize,
> + unsigned int at_flags);
> asmlinkage long sys_getcwd(char __user *buf, unsigned long size);
> asmlinkage long sys_eventfd2(unsigned int count, int flags);
> asmlinkage long sys_epoll_create1(int flags);
> diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
> index 88dc393c2bca38c0fa1b3fae579f7cfe4931223c..50be2e1007bc2779120d05c6e9512a689f86779c 100644
> --- a/include/uapi/asm-generic/unistd.h
> +++ b/include/uapi/asm-generic/unistd.h
> @@ -850,8 +850,14 @@ __SYSCALL(__NR_listxattrat, sys_listxattrat)
> #define __NR_removexattrat 466
> __SYSCALL(__NR_removexattrat, sys_removexattrat)
>
> +/* fs/inode.c */
> +#define __NR_getfsxattrat 467
> +__SYSCALL(__NR_getfsxattrat, sys_getfsxattrat)
> +#define __NR_setfsxattrat 468
> +__SYSCALL(__NR_setfsxattrat, sys_setfsxattrat)
> +
> #undef __NR_syscalls
> -#define __NR_syscalls 467
> +#define __NR_syscalls 469
>
> /*
> * 32 bit systems traditionally used different
> diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
> index 7539717707337a8cb22396a869baba3bafa08371..aed753e5d50c97da9b895a187fdaecf0477db74b 100644
> --- a/include/uapi/linux/fs.h
> +++ b/include/uapi/linux/fs.h
> @@ -139,6 +139,9 @@ struct fsxattr {
> unsigned char fsx_pad[8];
> };
>
> +#define FSXATTR_SIZE_VER0 28
> +#define FSXATTR_SIZE_LATEST FSXATTR_SIZE_VER0
> +
> /*
> * Flags for the fsx_xflags field
> */
>
> --
> 2.47.2
>
^ permalink raw reply
* Re: [PATCH v4 3/3] fs: introduce getfsxattrat and setfsxattrat syscalls
From: Christian Brauner @ 2025-04-22 14:31 UTC (permalink / raw)
To: Amir Goldstein
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: <CAOQ4uxjs=Gg-ocwx_fkzc0gxQ_dHx-P9EAgz5ZwbdbrxV0T_EA@mail.gmail.com>
On Thu, Mar 27, 2025 at 12:39:28PM +0100, Amir Goldstein wrote:
> On Thu, Mar 27, 2025 at 10:33 AM Andrey Albershteyn <aalbersh@redhat.com> wrote:
> >
> > On 2025-03-23 09:56:25, Amir Goldstein wrote:
> > > On Fri, Mar 21, 2025 at 8:49 PM Andrey Albershteyn <aalbersh@redhat.com> wrote:
> > > >
> > > > From: Andrey Albershteyn <aalbersh@redhat.com>
> > > >
> > > > Introduce getfsxattrat and setfsxattrat syscalls to manipulate inode
> > > > extended attributes/flags. The syscalls take parent directory fd and
> > > > path to the child together with struct fsxattr.
> > > >
> > > > This is an alternative to FS_IOC_FSSETXATTR ioctl with a difference
> > > > that file don't need to be open as we can reference it with a path
> > > > instead of fd. By having this we can manipulated inode extended
> > > > attributes not only on regular files but also on special ones. This
> > > > is not possible with FS_IOC_FSSETXATTR ioctl as with special files
> > > > we can not call ioctl() directly on the filesystem inode using fd.
> > > >
> > > > This patch adds two new syscalls which allows userspace to get/set
> > > > extended inode attributes on special files by using parent directory
> > > > and a path - *at() like syscall.
> > > >
> > > > CC: linux-api@vger.kernel.org
> > > > CC: linux-fsdevel@vger.kernel.org
> > > > CC: linux-xfs@vger.kernel.org
> > > > Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com>
> > > > Acked-by: Arnd Bergmann <arnd@arndb.de>
> > > > ---
> > > ...
> > > > +SYSCALL_DEFINE5(setfsxattrat, int, dfd, const char __user *, filename,
> > > > + struct fsxattr __user *, ufsx, size_t, usize,
> > > > + unsigned int, at_flags)
> > > > +{
> > > > + struct fileattr fa;
> > > > + struct path filepath;
> > > > + int error;
> > > > + unsigned int lookup_flags = 0;
> > > > + struct filename *name;
> > > > + struct mnt_idmap *idmap;.
> > >
> > > > + struct dentry *dentry;
> > > > + struct vfsmount *mnt;
> > > > + struct fsxattr fsx = {};
> > > > +
> > > > + BUILD_BUG_ON(sizeof(struct fsxattr) < FSXATTR_SIZE_VER0);
> > > > + BUILD_BUG_ON(sizeof(struct fsxattr) != FSXATTR_SIZE_LATEST);
> > > > +
> > > > + if ((at_flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) != 0)
> > > > + return -EINVAL;
> > > > +
> > > > + if (!(at_flags & AT_SYMLINK_NOFOLLOW))
> > > > + lookup_flags |= LOOKUP_FOLLOW;
> > > > +
> > > > + if (at_flags & AT_EMPTY_PATH)
> > > > + lookup_flags |= LOOKUP_EMPTY;
> > > > +
> > > > + if (usize > PAGE_SIZE)
> > > > + return -E2BIG;
> > > > +
> > > > + if (usize < FSXATTR_SIZE_VER0)
> > > > + return -EINVAL;
> > > > +
> > > > + error = copy_struct_from_user(&fsx, sizeof(struct fsxattr), ufsx, usize);
> > > > + if (error)
> > > > + return error;
> > > > +
> > > > + fsxattr_to_fileattr(&fsx, &fa);
> > > > +
> > > > + name = getname_maybe_null(filename, at_flags);
> > > > + if (!name) {
> > > > + CLASS(fd, f)(dfd);
> > > > +
> > > > + if (fd_empty(f))
> > > > + return -EBADF;
> > > > +
> > > > + idmap = file_mnt_idmap(fd_file(f));
> > > > + dentry = file_dentry(fd_file(f));
> > > > + mnt = fd_file(f)->f_path.mnt;
> > > > + } else {
> > > > + error = filename_lookup(dfd, name, lookup_flags, &filepath,
> > > > + NULL);
> > > > + if (error)
> > > > + return error;
> > > > +
> > > > + idmap = mnt_idmap(filepath.mnt);
> > > > + dentry = filepath.dentry;
> > > > + mnt = filepath.mnt;
> > > > + }
> > > > +
> > > > + error = mnt_want_write(mnt);
> > > > + if (!error) {
> > > > + error = vfs_fileattr_set(idmap, dentry, &fa);
> > > > + if (error == -ENOIOCTLCMD)
> > > > + error = -EOPNOTSUPP;
> > >
> > > This is awkward.
> > > vfs_fileattr_set() should return -EOPNOTSUPP.
> > > ioctl_setflags() could maybe convert it to -ENOIOCTLCMD,
> > > but looking at similar cases ioctl_fiemap(), ioctl_fsfreeze() the
> > > ioctl returns -EOPNOTSUPP.
> > >
> > > I don't think it is necessarily a bad idea to start returning
> > > -EOPNOTSUPP instead of -ENOIOCTLCMD for the ioctl
> > > because that really reflects the fact that the ioctl is now implemented
> > > in vfs and not in the specific fs.
> > >
> > > and I think it would not be a bad idea at all to make that change
> > > together with the merge of the syscalls as a sort of hint to userspace
> > > that uses the ioctl, that the sycalls API exists.
> > >
> > > Thanks,
> > > Amir.
> > >
> >
> > Hmm, not sure what you're suggesting here. I see it as:
> > - get/setfsxattrat should return EOPNOTSUPP as it make more sense
> > than ENOIOCTLCMD
> > - ioctl_setflags returns ENOIOCTLCMD which also expected
> >
> > Don't really see a reason to change what vfs_fileattr_set() returns
> > and then copying this if() to other places or start returning
> > EOPNOTSUPP.
>
> ENOIOCTLCMD conceptually means that the ioctl command is unknown
> This is not the case since ->fileattr_[gs]et() became a vfs API
vfs_fileattr_{g,s}et() should not return ENOIOCTLCMD. Change the return
code to EOPNOTSUPP and then make EOPNOTSUPP be translated to ENOTTY on
on overlayfs and to ENOIOCTLCMD in ecryptfs and in fs/ioctl.c. This way
we get a clean VFS api while retaining current behavior. Amir can do his
cleanup based on that.
^ permalink raw reply
* [PATCH v8 12/15] xfs: add xfs_file_dio_write_atomic()
From: John Garry @ 2025-04-22 12:27 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: <20250422122739.2230121-1-john.g.garry@oracle.com>
Add xfs_file_dio_write_atomic() for dedicated handling of atomic writes.
The function works based on two operating modes:
- HW offload, i.e. REQ_ATOMIC-based
- CoW based with out-of-places write and atomic extent remapping
The preferred method is HW offload as it will be faster. If HW offload is
not possible, then we fallback to the CoW-based method.
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 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
not possible.
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>
---
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 ba4b02abc6e4..000bbb0d1413 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_hw_atomicwrite_max(ip))
+ 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 v8 10/15] xfs: add large atomic writes checks in xfs_direct_write_iomap_begin()
From: John Garry @ 2025-04-22 12:27 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: <20250422122739.2230121-1-john.g.garry@oracle.com>
For when large atomic writes (> 1x FS block) are supported, there will be
various occasions when HW offload may not be possible.
Such instances include:
- unaligned extent mapping wrt write length
- extent mappings which do not cover the full write, e.g. the write spans
sparse or mixed-mapping extents
- the write length is greater than HW offload can support
In those cases, we need to fallback to the CoW-based atomic write mode. For
this, report special code -ENOPROTOOPT to inform the caller that HW
offload-based method is not possible.
In addition to the occasions mentioned, if the write covers an unallocated
range, we again judge that we need to rely on the CoW-based method when we
would need to allocate anything more than 1x block. This is because if we
allocate less blocks that is required for the write, then again HW
offload-based method would not be possible. So we are taking a pessimistic
approach to writes covering unallocated space.
Signed-off-by: John Garry <john.g.garry@oracle.com>
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
[djwong: don't allow too-small hw atomic writes either]
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
fs/xfs/xfs_iomap.c | 65 ++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 63 insertions(+), 2 deletions(-)
diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 049655ebc3f7..150354e5b436 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -798,6 +798,41 @@ imap_spans_range(
return true;
}
+static bool
+xfs_bmap_hw_atomic_write_possible(
+ struct xfs_inode *ip,
+ struct xfs_bmbt_irec *imap,
+ xfs_fileoff_t offset_fsb,
+ xfs_fileoff_t end_fsb)
+{
+ struct xfs_mount *mp = ip->i_mount;
+ xfs_fsize_t len = XFS_FSB_TO_B(mp, end_fsb - offset_fsb);
+
+ /*
+ * atomic writes are required to be naturally aligned for disk blocks,
+ * which ensures that we adhere to block layer rules that we won't
+ * straddle any boundary or violate write alignment requirement.
+ */
+ if (!IS_ALIGNED(imap->br_startblock, imap->br_blockcount))
+ return false;
+
+ /*
+ * Spanning multiple extents would mean that multiple BIOs would be
+ * issued, and so would lose atomicity required for REQ_ATOMIC-based
+ * atomics.
+ */
+ if (!imap_spans_range(imap, offset_fsb, end_fsb))
+ return false;
+
+ /*
+ * The ->iomap_begin caller should ensure this, but check anyway.
+ */
+ if (len > xfs_inode_hw_atomicwrite_max(ip))
+ return false;
+
+ return true;
+}
+
static int
xfs_direct_write_iomap_begin(
struct inode *inode,
@@ -812,9 +847,11 @@ xfs_direct_write_iomap_begin(
struct xfs_bmbt_irec imap, cmap;
xfs_fileoff_t offset_fsb = XFS_B_TO_FSBT(mp, offset);
xfs_fileoff_t end_fsb = xfs_iomap_end_fsb(mp, offset, length);
+ xfs_fileoff_t orig_end_fsb = end_fsb;
int nimaps = 1, error = 0;
bool shared = false;
u16 iomap_flags = 0;
+ bool needs_alloc;
unsigned int lockmode;
u64 seq;
@@ -875,13 +912,37 @@ xfs_direct_write_iomap_begin(
(flags & IOMAP_DIRECT) || IS_DAX(inode));
if (error)
goto out_unlock;
- if (shared)
+ if (shared) {
+ if ((flags & IOMAP_ATOMIC) &&
+ !xfs_bmap_hw_atomic_write_possible(ip, &cmap,
+ offset_fsb, end_fsb)) {
+ error = -ENOPROTOOPT;
+ goto out_unlock;
+ }
goto out_found_cow;
+ }
end_fsb = imap.br_startoff + imap.br_blockcount;
length = XFS_FSB_TO_B(mp, end_fsb) - offset;
}
- if (imap_needs_alloc(inode, flags, &imap, nimaps))
+ needs_alloc = imap_needs_alloc(inode, flags, &imap, nimaps);
+
+ if (flags & IOMAP_ATOMIC) {
+ error = -ENOPROTOOPT;
+ /*
+ * If we allocate less than what is required for the write
+ * then we may end up with multiple extents, which means that
+ * REQ_ATOMIC-based cannot be used, so avoid this possibility.
+ */
+ if (needs_alloc && orig_end_fsb - offset_fsb > 1)
+ goto out_unlock;
+
+ if (!xfs_bmap_hw_atomic_write_possible(ip, &imap, offset_fsb,
+ orig_end_fsb))
+ goto out_unlock;
+ }
+
+ if (needs_alloc)
goto allocate_blocks;
/*
--
2.31.1
^ permalink raw reply related
* [PATCH v8 08/15] xfs: refine atomic write size check in xfs_file_write_iter()
From: John Garry @ 2025-04-22 12:27 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: <20250422122739.2230121-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 653e42ccc0c3..1302783a7157 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 d324044a2225..3b5aa39dbfe9 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_atomicwrite(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_atomicwrite(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_atomicwrite(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 v8 13/15] xfs: add xfs_compute_atomic_write_unit_max()
From: John Garry @ 2025-04-22 12:27 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: <20250422122739.2230121-1-john.g.garry@oracle.com>
Now that CoW-based atomic writes are supported, update the max size of an
atomic write for the data device.
The limit of a CoW-based atomic write will be the limit of the number of
logitems which can fit into a single transaction.
In addition, the max atomic write size needs to be aligned to the agsize.
Limit the size of atomic writes to the greatest power-of-two factor of the
agsize so that allocations for an atomic write will always be aligned
compatibly with the alignment requirements of the storage.
Function xfs_atomic_write_logitems() is added to find the limit the number
of log items which can fit in a single transaction.
Amend the max atomic write computation to create a new transaction
reservation type, and compute the maximum size of an atomic write
completion (in fsblocks) based on this new transaction reservation.
Initially, tr_atomic_write is a clone of tr_itruncate, which provides a
reasonable level of parallelism. In the next patch, we'll add a mount
option so that sysadmins can configure their own limits.
Signed-off-by: John Garry <john.g.garry@oracle.com>
[djwong: use a new reservation type for atomic write ioends, refactor
group limit calculations]
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
[jpg: update xfs_calc_perag_awu_max() ddev check]
---
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 6c74f47f980a..708cfb4be661 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
@@ -1397,3 +1403,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 ee68c026e6cd..eb5ed61b6f99 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_dd_awu_hw_max)
+ return max_pow_of_two_factor(mp->m_sb.sb_agblocks);
+ return mp->m_ag_max_usable;
+}
+
+/*
+ * Reflink on the realtime device requires rtgroups, and atomic writes require
+ * reflink.
+ *
+ * If the realtime device advertises atomic write support, limit the size of
+ * data device atomic writes to the greatest power-of-two factor of the rtgroup
+ * size so that every atomic write unit aligns with the start of every rtgroup.
+ * This is required so that the per-rtgroup allocations for an atomic write
+ * will always be aligned compatibly with the alignment requirements of the
+ * storage.
+ *
+ * If the rt device doesn't advertise atomic writes, then there are no
+ * alignment restrictions and the largest out-of-place write we can do
+ * ourselves is the number of blocks that user files can allocate from any
+ * rtgroup.
+ */
+static inline xfs_extlen_t xfs_calc_rtgroup_awu_max(struct xfs_mount *mp)
+{
+ struct xfs_groups *rgs = &mp->m_groups[XG_TYPE_RTG];
+
+ if (mp->m_rtdev_targp && mp->m_rtdev_targp->bt_bdev_awu_min > 0)
+ return max_pow_of_two_factor(rgs->blocks);
+ return rgs->blocks;
+}
+
+/* Compute the maximum atomic write unit size for each section. */
+static inline void
+xfs_calc_atomic_write_unit_max(
+ struct xfs_mount *mp)
+{
+ struct xfs_groups *ags = &mp->m_groups[XG_TYPE_AG];
+ struct xfs_groups *rgs = &mp->m_groups[XG_TYPE_RTG];
+
+ const xfs_extlen_t max_write = xfs_calc_atomic_write_max(mp);
+ const xfs_extlen_t max_ioend = xfs_reflink_max_atomic_cow(mp);
+ const xfs_extlen_t max_agsize = xfs_calc_perag_awu_max(mp);
+ const xfs_extlen_t max_rgsize = xfs_calc_rtgroup_awu_max(mp);
+
+ ags->awu_max = min3(max_write, max_ioend, max_agsize);
+ rgs->awu_max = min3(max_write, max_ioend, max_rgsize);
+
+ trace_xfs_calc_atomic_write_unit_max(mp, max_write, max_ioend,
+ max_agsize, max_rgsize);
+}
+
/* Compute maximum possible height for realtime btree types for this fs. */
static inline void
xfs_rtbtree_compute_maxlevels(
@@ -1096,6 +1170,13 @@ xfs_mountfs(
mp->m_rtdev_targp->bt_bdev_awu_max >= mp->m_sb.sb_blocksize)
mp->m_rt_awu_hw_max = mp->m_rtdev_targp->bt_bdev_awu_max;
+ /*
+ * 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 2819e160f0e9..ba55fa1d9594 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..8c47af705f79 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_has_reflink(mp))
+ return 0;
+
+ /*
+ * Atomic write limits must always be a power-of-2, according to
+ * generic_atomic_write_valid.
+ */
+ return rounddown_pow_of_two(xfs_calc_max_atomic_write_fsblocks(mp));
+}
+
/*
* Free all CoW staging blocks that are still referenced by the ondisk refcount
* metadata. The ondisk metadata does not track which inode created the
diff --git a/fs/xfs/xfs_reflink.h b/fs/xfs/xfs_reflink.h
index 412e9b6f2082..36cda724da89 100644
--- a/fs/xfs/xfs_reflink.h
+++ b/fs/xfs/xfs_reflink.h
@@ -68,4 +68,6 @@ extern int xfs_reflink_update_dest(struct xfs_inode *dest, xfs_off_t newlen,
bool xfs_reflink_supports_rextsize(struct xfs_mount *mp, unsigned int rextsize);
+xfs_extlen_t xfs_reflink_max_atomic_cow(struct xfs_mount *mp);
+
#endif /* __XFS_REFLINK_H */
diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index 9554578c6da4..d5ae00f8e04c 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -170,6 +170,66 @@ DEFINE_ATTR_LIST_EVENT(xfs_attr_list_notfound);
DEFINE_ATTR_LIST_EVENT(xfs_attr_leaf_list);
DEFINE_ATTR_LIST_EVENT(xfs_attr_node_list);
+TRACE_EVENT(xfs_calc_atomic_write_unit_max,
+ TP_PROTO(struct xfs_mount *mp, unsigned int max_write,
+ unsigned int max_ioend, unsigned int max_agsize,
+ unsigned int max_rgsize),
+ TP_ARGS(mp, max_write, max_ioend, max_agsize, max_rgsize),
+ TP_STRUCT__entry(
+ __field(dev_t, dev)
+ __field(unsigned int, max_write)
+ __field(unsigned int, max_ioend)
+ __field(unsigned int, max_agsize)
+ __field(unsigned int, max_rgsize)
+ __field(unsigned int, data_awu_max)
+ __field(unsigned int, rt_awu_max)
+ ),
+ TP_fast_assign(
+ __entry->dev = mp->m_super->s_dev;
+ __entry->max_write = max_write;
+ __entry->max_ioend = max_ioend;
+ __entry->max_agsize = max_agsize;
+ __entry->max_rgsize = max_rgsize;
+ __entry->data_awu_max = mp->m_groups[XG_TYPE_AG].awu_max;
+ __entry->rt_awu_max = mp->m_groups[XG_TYPE_RTG].awu_max;
+ ),
+ TP_printk("dev %d:%d max_write %u max_ioend %u max_agsize %u max_rgsize %u data_awu_max %u rt_awu_max %u",
+ MAJOR(__entry->dev), MINOR(__entry->dev),
+ __entry->max_write,
+ __entry->max_ioend,
+ __entry->max_agsize,
+ __entry->max_rgsize,
+ __entry->data_awu_max,
+ __entry->rt_awu_max)
+);
+
+TRACE_EVENT(xfs_calc_max_atomic_write_fsblocks,
+ TP_PROTO(struct xfs_mount *mp, unsigned int per_intent,
+ unsigned int step_size, unsigned int logres,
+ unsigned int blockcount),
+ TP_ARGS(mp, per_intent, step_size, logres, blockcount),
+ TP_STRUCT__entry(
+ __field(dev_t, dev)
+ __field(unsigned int, per_intent)
+ __field(unsigned int, step_size)
+ __field(unsigned int, logres)
+ __field(unsigned int, blockcount)
+ ),
+ TP_fast_assign(
+ __entry->dev = mp->m_super->s_dev;
+ __entry->per_intent = per_intent;
+ __entry->step_size = step_size;
+ __entry->logres = logres;
+ __entry->blockcount = blockcount;
+ ),
+ TP_printk("dev %d:%d per_intent %u step_size %u logres %u blockcount %u",
+ MAJOR(__entry->dev), MINOR(__entry->dev),
+ __entry->per_intent,
+ __entry->step_size,
+ __entry->logres,
+ __entry->blockcount)
+);
+
TRACE_EVENT(xlog_intent_recovery_failed,
TP_PROTO(struct xfs_mount *mp, const struct xfs_defer_op_type *ops,
int error),
--
2.31.1
^ permalink raw reply related
* [PATCH v8 15/15] xfs: allow sysadmins to specify a maximum atomic write limit at mount time
From: John Garry @ 2025-04-22 12:27 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: <20250422122739.2230121-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.
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 | 55 ++++++++++++++++++++++++
fs/xfs/libxfs/xfs_trans_resv.h | 2 +
fs/xfs/xfs_mount.c | 69 ++++++++++++++++++++++++++++++-
fs/xfs/xfs_mount.h | 6 +++
fs/xfs/xfs_super.c | 58 +++++++++++++++++++++++++-
fs/xfs/xfs_trace.h | 33 +++++++++++++++
7 files changed, 232 insertions(+), 2 deletions(-)
diff --git a/Documentation/admin-guide/xfs.rst b/Documentation/admin-guide/xfs.rst
index 3e76276bd488..40b7789c8b34 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 708cfb4be661..86bc93b43dbd 100644
--- a/fs/xfs/libxfs/xfs_trans_resv.c
+++ b/fs/xfs/libxfs/xfs_trans_resv.c
@@ -1491,3 +1491,58 @@ xfs_calc_max_atomic_write_fsblocks(
return ret;
}
+
+/*
+ * Compute the log reservation needed to complete an atomic write of a given
+ * number of blocks. Worst case, each block requires separate handling.
+ */
+int
+xfs_calc_atomic_write_reservation(
+ struct xfs_mount *mp,
+ xfs_extlen_t blockcount)
+{
+ struct xfs_trans_res *curr_res = &M_RES(mp)->tr_atomic_ioend;
+ unsigned int per_intent, step_size;
+ unsigned int logres;
+ uint old_logres =
+ M_RES(mp)->tr_atomic_ioend.tr_logres;
+ int min_logblocks;
+
+ xfs_calc_default_atomic_ioend_reservation(mp, M_RES(mp));
+
+ /*
+ * If the caller doesn't ask for a specific atomic write size, then
+ * use the defaults.
+ */
+ if (blockcount == 0)
+ return 0;
+
+ /*
+ * The caller asked for a specific size but we don't support out of
+ * places writes, so this is a failure.
+ */
+ if (curr_res->tr_logres == 0)
+ return -EINVAL;
+
+ per_intent = xfs_calc_atomic_write_ioend_geometry(mp, &step_size);
+
+ /* Check for overflows */
+ if (check_mul_overflow(blockcount, per_intent, &logres))
+ return -EINVAL;
+ if (check_add_overflow(logres, step_size, &logres))
+ return -EINVAL;
+
+ curr_res->tr_logres = logres;
+ min_logblocks = xfs_log_calc_minimum_size(mp);
+
+ trace_xfs_calc_max_atomic_write_reservation(mp, per_intent, step_size,
+ blockcount, min_logblocks, curr_res->tr_logres);
+
+ if (min_logblocks > mp->m_sb.sb_logblocks) {
+ /* Log too small, revert changes. */
+ curr_res->tr_logres = old_logres;
+ return -EINVAL;
+ }
+
+ return 0;
+}
diff --git a/fs/xfs/libxfs/xfs_trans_resv.h b/fs/xfs/libxfs/xfs_trans_resv.h
index a6d303b83688..16fd90b50ccd 100644
--- a/fs/xfs/libxfs/xfs_trans_resv.h
+++ b/fs/xfs/libxfs/xfs_trans_resv.h
@@ -122,5 +122,7 @@ 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);
+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 eb5ed61b6f99..54fc06285131 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -740,6 +740,71 @@ 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(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;
+ }
+
+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(
@@ -1175,7 +1240,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 ba55fa1d9594..93e2bec24af9 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -241,6 +241,9 @@ typedef struct xfs_mount {
unsigned int m_dd_awu_hw_max;
unsigned int m_rt_awu_hw_max;
+ /* 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.
@@ -803,4 +806,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..01ef8b48a354 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_reservation,
+ 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 v8 14/15] xfs: update atomic write limits
From: John Garry @ 2025-04-22 12:27 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: <20250422122739.2230121-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>
---
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 000bbb0d1413..371f0d30f859 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_atomicwrite(XFS_I(inode)))
+ if (xfs_get_atomic_write_min(XFS_I(inode)))
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 3b5aa39dbfe9..118377d3f8d2 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_atomicwrite(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_atomicwrite(ip) || xfs_has_reflink(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_atomicwrite(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_has_reflink(mp)) {
+ if (xfs_inode_can_hw_atomicwrite(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_hw_atomicwrite_max(ip));
}
static void
--
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