[PATCH] xfs: prevent race in zoned space reservations

[PATCH] xfs: prevent race in zoned space reservations

[Hans Holmberg]

xfs_zoned_add_available() checks whether the reservation list is empty
before adding blocks to the available-space counter.  This check is not
serialized against a task adding itself to the reservation list however.

This allows the space provider to observe an empty list, after which a
reserver can enqueue itself and retry the counter before the new space is
added.  The provider then adds the space and returns without waking the
now-eligible reserver, leaving it asleep until GC or another event
provides a wakeup, potentially adding seconds to max write latency.

Take the reservation lock before updating the counter and checking the
list.  Use list_empty() because the list is now inspected under its lock.

Taking a per-mount lock when handing back space is far from ideal, but
benchmarking with null_blk showed no measurable performance regression.

Fixes: 0bb2193056b5 ("xfs: add support for zoned space reservations")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260609075655.1698743-1-hch@lst.de?part=2
Signed-off-by: Hans Holmberg <hans.holmberg@wdc.com>
---
 fs/xfs/xfs_zone_space_resv.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/xfs/xfs_zone_space_resv.c b/fs/xfs/xfs_zone_space_resv.c
index 5c6e6ef627e4..7aa3c74fb2e0 100644
--- a/fs/xfs/xfs_zone_space_resv.c
+++ b/fs/xfs/xfs_zone_space_resv.c
@@ -85,13 +85,13 @@ xfs_zoned_add_available(
 	struct xfs_zone_info		*zi = mp->m_zone_info;
 	struct xfs_zone_reservation	*reservation;
 
-	if (list_empty_careful(&zi->zi_reclaim_reservations)) {
-		xfs_add_freecounter(mp, XC_FREE_RTAVAILABLE, count_fsb);
+	spin_lock(&zi->zi_reservation_lock);
+	xfs_add_freecounter(mp, XC_FREE_RTAVAILABLE, count_fsb);
+	if (list_empty(&zi->zi_reclaim_reservations)) {
+		spin_unlock(&zi->zi_reservation_lock);
 		return;
 	}
 
-	spin_lock(&zi->zi_reservation_lock);
-	xfs_add_freecounter(mp, XC_FREE_RTAVAILABLE, count_fsb);
 	count_fsb = xfs_sum_freecounter(mp, XC_FREE_RTAVAILABLE);
 	list_for_each_entry(reservation, &zi->zi_reclaim_reservations, entry) {
 		if (reservation->count_fsb > count_fsb)
-- 
2.43.0

Re: [PATCH] xfs: prevent race in zoned space reservations

[Christoph Hellwig]

On Wed, Aug 26, 2026 at 02:32:19PM +0200, Hans Holmberg wrote:
> xfs_zoned_add_available() checks whether the reservation list is empty
> before adding blocks to the available-space counter.  This check is not
> serialized against a task adding itself to the reservation list however.
> 
> This allows the space provider to observe an empty list, after which a
> reserver can enqueue itself and retry the counter before the new space is
> added.  The provider then adds the space and returns without waking the
> now-eligible reserver, leaving it asleep until GC or another event
> provides a wakeup, potentially adding seconds to max write latency.
> 
> Take the reservation lock before updating the counter and checking the
> list.  Use list_empty() because the list is now inspected under its lock.
> 
> Taking a per-mount lock when handing back space is far from ideal, but
> benchmarking with null_blk showed no measurable performance regression.
> 

I kinda hate taking a global lock for each block free, but at least
the critical section is very small..

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH] xfs: prevent race in zoned space reservations

[Carlos Maiolino]

On Wed, Aug 26, 2026 at 02:32:19PM +0200, Hans Holmberg wrote:
> xfs_zoned_add_available() checks whether the reservation list is empty
> before adding blocks to the available-space counter.  This check is not
> serialized against a task adding itself to the reservation list however.
> 
> This allows the space provider to observe an empty list, after which a
> reserver can enqueue itself and retry the counter before the new space is
> added.  The provider then adds the space and returns without waking the
> now-eligible reserver, leaving it asleep until GC or another event
> provides a wakeup, potentially adding seconds to max write latency.
> 
> Take the reservation lock before updating the counter and checking the
> list.  Use list_empty() because the list is now inspected under its lock.
> 
> Taking a per-mount lock when handing back space is far from ideal, but
> benchmarking with null_blk showed no measurable performance regression.
> 
> Fixes: 0bb2193056b5 ("xfs: add support for zoned space reservations")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/20260609075655.1698743-1-hch@lst.de?part=2
> Signed-off-by: Hans Holmberg <hans.holmberg@wdc.com>
> ---
>  fs/xfs/xfs_zone_space_resv.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/xfs/xfs_zone_space_resv.c b/fs/xfs/xfs_zone_space_resv.c
> index 5c6e6ef627e4..7aa3c74fb2e0 100644
> --- a/fs/xfs/xfs_zone_space_resv.c
> +++ b/fs/xfs/xfs_zone_space_resv.c
> @@ -85,13 +85,13 @@ xfs_zoned_add_available(
>  	struct xfs_zone_info		*zi = mp->m_zone_info;
>  	struct xfs_zone_reservation	*reservation;
>  
> -	if (list_empty_careful(&zi->zi_reclaim_reservations)) {
> -		xfs_add_freecounter(mp, XC_FREE_RTAVAILABLE, count_fsb);
> +	spin_lock(&zi->zi_reservation_lock);
> +	xfs_add_freecounter(mp, XC_FREE_RTAVAILABLE, count_fsb);
> +	if (list_empty(&zi->zi_reclaim_reservations)) {
> +		spin_unlock(&zi->zi_reservation_lock);
>  		return;
>  	}
>  
> -	spin_lock(&zi->zi_reservation_lock);
> -	xfs_add_freecounter(mp, XC_FREE_RTAVAILABLE, count_fsb);
>  	count_fsb = xfs_sum_freecounter(mp, XC_FREE_RTAVAILABLE);
>  	list_for_each_entry(reservation, &zi->zi_reclaim_reservations, entry) {
>  		if (reservation->count_fsb > count_fsb)

Looks good to me:

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

FWIW, don't xfs_zoned_reserve_available() might have a similar problem
when decreasing the free counter? I'm not that much literate on zoned,
but a quick look seemed reserving space might hit a similar problem?!

Cheers.

> -- 
> 2.43.0
> 

Re: [PATCH] xfs: prevent race in zoned space reservations

[Hans Holmberg]

On 31/08/2026 09:19, Carlos Maiolino wrote:
> On Wed, Aug 26, 2026 at 02:32:19PM +0200, Hans Holmberg wrote:
>> xfs_zoned_add_available() checks whether the reservation list is empty
>> before adding blocks to the available-space counter.  This check is not
>> serialized against a task adding itself to the reservation list however.
>>
>> This allows the space provider to observe an empty list, after which a
>> reserver can enqueue itself and retry the counter before the new space is
>> added.  The provider then adds the space and returns without waking the
>> now-eligible reserver, leaving it asleep until GC or another event
>> provides a wakeup, potentially adding seconds to max write latency.
>>
>> Take the reservation lock before updating the counter and checking the
>> list.  Use list_empty() because the list is now inspected under its lock.
>>
>> Taking a per-mount lock when handing back space is far from ideal, but
>> benchmarking with null_blk showed no measurable performance regression.
>>
>> Fixes: 0bb2193056b5 ("xfs: add support for zoned space reservations")
>> Reported-by: Sashiko <sashiko-bot@kernel.org>
>> Closes: https://sashiko.dev/#/patchset/20260609075655.1698743-1-hch@lst.de?part=2
>> Signed-off-by: Hans Holmberg <hans.holmberg@wdc.com>
>> ---
>>  fs/xfs/xfs_zone_space_resv.c | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/xfs/xfs_zone_space_resv.c b/fs/xfs/xfs_zone_space_resv.c
>> index 5c6e6ef627e4..7aa3c74fb2e0 100644
>> --- a/fs/xfs/xfs_zone_space_resv.c
>> +++ b/fs/xfs/xfs_zone_space_resv.c
>> @@ -85,13 +85,13 @@ xfs_zoned_add_available(
>>  	struct xfs_zone_info		*zi = mp->m_zone_info;
>>  	struct xfs_zone_reservation	*reservation;
>>  
>> -	if (list_empty_careful(&zi->zi_reclaim_reservations)) {
>> -		xfs_add_freecounter(mp, XC_FREE_RTAVAILABLE, count_fsb);
>> +	spin_lock(&zi->zi_reservation_lock);
>> +	xfs_add_freecounter(mp, XC_FREE_RTAVAILABLE, count_fsb);
>> +	if (list_empty(&zi->zi_reclaim_reservations)) {
>> +		spin_unlock(&zi->zi_reservation_lock);
>>  		return;
>>  	}
>>  
>> -	spin_lock(&zi->zi_reservation_lock);
>> -	xfs_add_freecounter(mp, XC_FREE_RTAVAILABLE, count_fsb);
>>  	count_fsb = xfs_sum_freecounter(mp, XC_FREE_RTAVAILABLE);
>>  	list_for_each_entry(reservation, &zi->zi_reclaim_reservations, entry) {
>>  		if (reservation->count_fsb > count_fsb)
> 
> Looks good to me:
> 
> Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
> 
> FWIW, don't xfs_zoned_reserve_available() might have a similar problem
> when decreasing the free counter? I'm not that much literate on zoned,
> but a quick look seemed reserving space might hit a similar problem?!
> 

It's a good question, in xfs_zoned_reserve_available, a writer could
theoretically race and skip ahead of another writer that was lining up
to wait for available space.

So while xfs_zoned_reserve_available is not guaranteed to be 100% fair,
I think the alternative (adding the lock on the reserve path) is too costly. 

Re: [PATCH] xfs: prevent race in zoned space reservations

[Carlos Maiolino]

On Tue, Sep 01, 2026 at 09:07:21AM +0200, Hans Holmberg wrote:
> On 31/08/2026 09:19, Carlos Maiolino wrote:
> > On Wed, Aug 26, 2026 at 02:32:19PM +0200, Hans Holmberg wrote:
> >> xfs_zoned_add_available() checks whether the reservation list is empty
> >> before adding blocks to the available-space counter.  This check is not
> >> serialized against a task adding itself to the reservation list however.
> >>
> >> This allows the space provider to observe an empty list, after which a
> >> reserver can enqueue itself and retry the counter before the new space is
> >> added.  The provider then adds the space and returns without waking the
> >> now-eligible reserver, leaving it asleep until GC or another event
> >> provides a wakeup, potentially adding seconds to max write latency.
> >>
> >> Take the reservation lock before updating the counter and checking the
> >> list.  Use list_empty() because the list is now inspected under its lock.
> >>
> >> Taking a per-mount lock when handing back space is far from ideal, but
> >> benchmarking with null_blk showed no measurable performance regression.
> >>
> >> Fixes: 0bb2193056b5 ("xfs: add support for zoned space reservations")
> >> Reported-by: Sashiko <sashiko-bot@kernel.org>
> >> Closes: https://sashiko.dev/#/patchset/20260609075655.1698743-1-hch@lst.de?part=2
> >> Signed-off-by: Hans Holmberg <hans.holmberg@wdc.com>
> >> ---
> >>  fs/xfs/xfs_zone_space_resv.c | 8 ++++----
> >>  1 file changed, 4 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/fs/xfs/xfs_zone_space_resv.c b/fs/xfs/xfs_zone_space_resv.c
> >> index 5c6e6ef627e4..7aa3c74fb2e0 100644
> >> --- a/fs/xfs/xfs_zone_space_resv.c
> >> +++ b/fs/xfs/xfs_zone_space_resv.c
> >> @@ -85,13 +85,13 @@ xfs_zoned_add_available(
> >>  	struct xfs_zone_info		*zi = mp->m_zone_info;
> >>  	struct xfs_zone_reservation	*reservation;
> >>  
> >> -	if (list_empty_careful(&zi->zi_reclaim_reservations)) {
> >> -		xfs_add_freecounter(mp, XC_FREE_RTAVAILABLE, count_fsb);
> >> +	spin_lock(&zi->zi_reservation_lock);
> >> +	xfs_add_freecounter(mp, XC_FREE_RTAVAILABLE, count_fsb);
> >> +	if (list_empty(&zi->zi_reclaim_reservations)) {
> >> +		spin_unlock(&zi->zi_reservation_lock);
> >>  		return;
> >>  	}
> >>  
> >> -	spin_lock(&zi->zi_reservation_lock);
> >> -	xfs_add_freecounter(mp, XC_FREE_RTAVAILABLE, count_fsb);
> >>  	count_fsb = xfs_sum_freecounter(mp, XC_FREE_RTAVAILABLE);
> >>  	list_for_each_entry(reservation, &zi->zi_reclaim_reservations, entry) {
> >>  		if (reservation->count_fsb > count_fsb)
> > 
> > Looks good to me:
> > 
> > Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
> > 
> > FWIW, don't xfs_zoned_reserve_available() might have a similar problem
> > when decreasing the free counter? I'm not that much literate on zoned,
> > but a quick look seemed reserving space might hit a similar problem?!
> > 
> 
> It's a good question, in xfs_zoned_reserve_available, a writer could
> theoretically race and skip ahead of another writer that was lining up
> to wait for available space.
> 
> So while xfs_zoned_reserve_available is not guaranteed to be 100% fair,
> I think the alternative (adding the lock on the reserve path) is too costly. 

Oh, thanks for looking into it. I certainly didn't think about the costs
implied here!

Re: [PATCH] xfs: prevent race in zoned space reservations

[Carlos Maiolino]

On Wed, 26 Aug 2026 14:32:19 +0200, Hans Holmberg wrote:
> xfs_zoned_add_available() checks whether the reservation list is empty
> before adding blocks to the available-space counter.  This check is not
> serialized against a task adding itself to the reservation list however.
> 
> This allows the space provider to observe an empty list, after which a
> reserver can enqueue itself and retry the counter before the new space is
> added.  The provider then adds the space and returns without waking the
> now-eligible reserver, leaving it asleep until GC or another event
> provides a wakeup, potentially adding seconds to max write latency.
> 
> [...]

Applied to for-next, thanks!

[1/1] xfs: prevent race in zoned space reservations
      commit: d5ae1c0959420e536c9ac3a1a32f04c73f52bd09

Best regards,
-- 
Carlos Maiolino <cem@kernel.org>