Re: [PATCH v2 2/8] zbd: fix write zone accounting

On Feb 27, 2026 / 13:53, Damien Le Moal wrote:
> On 2/16/26 16:59, Shin'ichiro Kawasaki wrote:
> > Currently, zbd_convert_to_write_zones() calls io_u_quiesce() when the
> > number of write target zones hits one of the limits of write zones. Thi=
s
> > wait by io_u_quiesce() significantly degrade the performance. While I
> > tried to remove the io_u_quiesce(), I observed that the test case 58 of
> > t/zbd/test-zbd-support failed with null_blk devices that have a
> > max_active_zones limit set.
> >=20
> > The failure cause is an incorrect write target zone accounting in
> > zbd_convert_to_write_zones(). This function checks the current write
> > target zones, and selects one of them as the next write target zone.
> > After the zone selection, it locks the zone. But when the zone is
> > locked, another job might have removed the zone from the write target
> > zones array. This caused an incorrect zone accounting and the test case
> > failure.
> >=20
> > To avoid the incorrect zone accounting, call zbd_write_zone_get() after
> > the selected zone gets locked. If the zone is removed from the write
> > target zones array, the function adds the zone back to the array.
> >=20
> > Signed-off-by: Shin'ichiro Kawasaki <[email protected]>
> > ---
> >  zbd.c | 13 +++++++++++--
> >  1 file changed, 11 insertions(+), 2 deletions(-)
> >=20
> > diff --git a/zbd.c b/zbd.c
> > index b71f842c..c511b709 100644
> > --- a/zbd.c
> > +++ b/zbd.c
> > @@ -1693,8 +1693,17 @@ retry:
> > =20
> >  		zone_lock(td, f, z);
> >  		if (zbd_zone_remainder(z) >=3D min_bs) {
> > -			need_zone_finish =3D false;
> > -			goto out;
> > +			/*
> > +			 * The zone might be already removed from
> > +			 * zbdi->write_zones[] by other jobs at this moment.
> > +			 * Even if the zone has remainder, call
> > +			 * zbd_write_zone_get() to ensure that it is in the
> > +			 * array.
> > +			 */
> > +			if (zbd_write_zone_get(td, f, z)) {
> > +				need_zone_finish =3D false;
> > +				goto out;
> > +			}
>=20
> The way I understand this is: since we do have a remainder, the zone is n=
ot
> full, so zbd_write_zone_get() cannot return false. So this looks OK, but =
is also
> very confusing. What about removing the if and instead use an assert chec=
king
> that zbd_write_zone_get() returns true ?

As to the failure of the test case 58, which has one write job and one trim=
 job,
I think your idea will work. However, I still think there is a tiny possibi=
lity
that zbd_write_zone_get() returns false due to the max_open_zone limit and =
what
other jobs do. As I describe below, the zone can be removed from the write
target array by the trim job. And if another write job exists and if it ope=
n
another zone in parallel, zbd_write_zone_get() may hit max_open_zones limit=
 and
return false.

>=20
> Also, it is not clear what the conditions are for a zone that is still no=
t full
> to be removed from the array. Can you detail that ?

As to the test case 58, the condition is that there is a trim workload runn=
ing
in parallel to a write workload. The trim workload can choose the zone in t=
he
write target array, and reset the zone. After the reset, the trim worload
removes the zone from the array, then call zone_unlock(). The zone reset by=
 the
trim zone can happen just before the zone_lock() in the hunk above. I think
write workloads with the zone_reset_threshold option can cause the same fai=
lure.

>=20
> >  		}
> >  		pthread_mutex_lock(&zbdi->mutex);
> >  	}
>=20
>=20
> --=20
> Damien Le Moal
> Western Digital Research=