Re: [PATCH 23/43] xfs: add support for zoned space reservations

Darrick J. Wong
2025-02-13T22:09:43Z

On Thu, Feb 13, 2025 at 06:17:49AM +0100, Christoph Hellwig wrote:
> On Fri, Feb 07, 2025 at 09:52:31AM -0800, Darrick J. Wong wrote:
> > On Thu, Feb 06, 2025 at 07:44:39AM +0100, Christoph Hellwig wrote:
> > > For zoned file systems garbage collection (GC) has to take the iolock
> > > and mmaplock after moving data to a new place to synchronize with
> > > readers. This means waiting for garbage collection with the iolock can
> > > deadlock.
> > >
> > > To avoid this, the worst case required blocks have to be reserved before
> > > taking the iolock, which is done using a new RTAVAILABLE counter that
> > > tracks blocks that are free to write into and don't require garbage
> >
> > Wasn't that in the last patch? Should this sentence move there?
>
> The last patch added the counter, it really should be here. Let me
> reshuffle the patches a bit to make that happen.
>
> > > + if (flags & XFS_ZR_NOWAIT)
> > > + return -EAGAIN;
> > > +
> > > + spin_lock(&zi->zi_reservation_lock);
> > > + list_add_tail(&reservation.entry, &zi->zi_reclaim_reservations);
> >
> > I think you're supposed to have initialized reservation.entry already.
>
> What do you mean with that?

I think the reservation was declared to be initialized as zeroes, but
there was never an INIT_LIST_HEAD(&reservation.entry) to set the
pointers to each other?

> > > + int error;
> > > +
> > > + ASSERT(ac->reserved_blocks == 0);
> > > + ASSERT(ac->open_zone == NULL);
> > > +
> > > + error = xfs_dec_freecounter(mp, XC_FREE_RTEXTENTS, count_fsb,
> > > + flags & XFS_ZR_RESERVED);
> > > + if (error == -ENOSPC && (flags & XFS_ZR_GREEDY) && count_fsb > 1)
> > > + error = xfs_zoned_reserve_extents_greedy(ip, &count_fsb, flags);
> >
> > Overly long line.
>
> It's exactly 80 characters :)

There are two things I'm bad at: off by one errors.

--D