Re: [PATCH V14 00/16] Bail out if transaction can cause extent count to overflow

From: Dave Chinner <david@fromorbit.com>
To: Amir Goldstein <amir73il@gmail.com>
Cc: Chandan Babu R <chandanrlinux@gmail.com>,
	linux-xfs <linux-xfs@vger.kernel.org>,
	"Darrick J . Wong" <darrick.wong@oracle.com>,
	"Darrick J. Wong" <djwong@kernel.org>,
	Christoph Hellwig <hch@lst.de>,
	Allison Henderson <allison.henderson@oracle.com>,
	"Luis R. Rodriguez" <mcgrof@kernel.org>,
	Theodore Tso <tytso@mit.edu>
Subject: Re: [PATCH V14 00/16] Bail out if transaction can cause extent count to overflow
Date: Wed, 25 May 2022 18:38:14 +1000	[thread overview]
Message-ID: <20220525083814.GH1098723@dread.disaster.area> (raw)
In-Reply-To: <CAOQ4uxi=jZt_FnNktw8u5L90WUcEAtX4jymM126hLTVbw74f=g@mail.gmail.com>

On Wed, May 25, 2022 at 10:48:09AM +0300, Amir Goldstein wrote:
> On Wed, May 25, 2022 at 10:33 AM Dave Chinner <david@fromorbit.com> wrote:
> >
> > On Tue, May 24, 2022 at 08:36:50AM +0300, Amir Goldstein wrote:
> > > On Tue, May 24, 2022 at 1:43 AM Dave Chinner <david@fromorbit.com> wrote:
> > > >
> > > > On Mon, May 23, 2022 at 02:15:44PM +0300, Amir Goldstein wrote:
> > > > > On Sun, Jan 10, 2021 at 6:10 PM Chandan Babu R <chandanrlinux@gmail.com> wrote:
> > >
> > > I am not sure I follow this argument.
> > > Users can create large attributes, can they not?
> >
> > Sure. But *nobody does*, and there are good reasons we don't see
> > people doing this.
> >
> > The reality is that apps don't use xattrs heavily because
> > filesystems are traditionally very bad at storing even moderate
> > numbers of xattrs. XFS is the exception to the rule. Hence nobody is
> > trying to use a few million xattrs per inode right now, and it's
> > unlikely anyone will unless they specifically target XFS.  In which
> > case, they are going to want the large extent count stuff that just
> > got merged into the for-next tree, and this whole discussion is
> > moot....
> 
> With all the barriers to large extents count that you mentioned
> I wonder how large extent counters feature mitigates those,
> but that is irrelevant to the question at hand.

They don't. That's the point I'm trying to make - these patches
don't actually fix any problems with large data fork extent counts -
they just allow them to get bigger.

As I said earlier - the primary driver for these changes is not
growing the number of data extents or reflink - it's growing the
amount of data we can store in the attribute fork. We need to grow
that from 2^16 extents to 2^32 extents because we want to be able to
store hundreds of millions of xattrs per file for internal
filesystem purposes.

Extending the data fork to 2^48 extents at the same time just makes
sense from an on-disk format perspective, not because the current
code can scale effectively to 2^32 extents, but because we're
already changing all that code to support a different attr fork
extent size. We will probably need >2^32 extents in the next decade,
so we're making the change now while we are touching the code....

There are future mods planned that will make large extent counts
bearable, but we don't have any idea how to solve problems like
making reflink go from O(n) to O(log n) to make reflink of
billion extent files an every day occurrence....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com