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

[Hans Holmberg <hans.holmberg@wdc.com>]

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.