Re: [PATCH 1/7] xfs: fix under-reservation of blocks when repairing sf directories

["Darrick J. Wong" <djwong@kernel.org>]

On Thu, Sep 10, 2026 at 07:31:37AM +0200, Carlos Maiolino wrote:
> On Thu, Sep 10, 2026 at 07:00:31AM +0200, Christoph Hellwig wrote:
> > On Tue, Sep 08, 2026 at 11:04:35PM -0700, Darrick J. Wong wrote:
> > > +static inline unsigned int
> > > +xrep_tempexch_estimate_sf_resblks(
> > > +	struct xfs_scrub	*sc,
> > > +	int			whichfork)
> > > +{
> > > +	if (whichfork == XFS_ATTR_FORK)
> > > +		return sc->mp->m_attr_geo->fsbcount;
> > > +	if (S_ISDIR(VFS_I(sc->ip)->i_mode))
> > > +		return sc->mp->m_dir_geo->fsbcount;
> > > +	return 1;
> > 
> > geo->fsbcount for attrs is a always 1 and a lot of code relies on that,
> > so this feels a bit overcomplicated and misleading (but still correct).
> > 
> > What about the more usual:
> > 
> > 	if (S_ISDIR(VFS_I(sc->ip)->i_mode) && whichfork == XFS_DATA_FORK)
> > 		return sc->mp->m_dir_geo->fsbcount;
> > 	return 1;

I prefer not to leave a logic bomb in the scrub code in case we ever
have multi-fsblock attr blocks.

Though I guess I'd be ok with

	ASSERT(mp->m_attr_geo->fsbcount == 1);

	if (S_ISDIR(VFS_I(sc->ip)->i_mode) && whichfork == XFS_DATA_FORK)
		return sc->mp->m_dir_geo->fsbcount;
	return 1;

or something like that?

> Either are fine to me so feel free to add:
> 
> Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

But only if Carlos really wants me to add that.

--D

> 
> > 
> > ?
> > 
> > Otherwise looks good:
> > 
> > Reviewed-by: Christoph Hellwig <hch@lst.de>
> >