linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Brian Foster <bfoster@redhat.com>
Cc: "Darrick J. Wong" <darrick.wong@oracle.com>, linux-xfs@vger.kernel.org
Subject: Re: [RFC PATCH 0/14] xfs: Towards thin provisioning aware filesystems
Date: Tue, 7 Nov 2017 08:20:46 +1100	[thread overview]
Message-ID: <20171106212046.GG4094@dastard> (raw)
In-Reply-To: <20171106130100.GA30884@bfoster.bfoster>

On Mon, Nov 06, 2017 at 08:01:00AM -0500, Brian Foster wrote:
> On Mon, Nov 06, 2017 at 09:50:28AM +1100, Dave Chinner wrote:
> > On Fri, Nov 03, 2017 at 07:36:23AM -0400, Brian Foster wrote:
> > > On Thu, Nov 02, 2017 at 07:47:40PM -0700, Darrick J. Wong wrote:
> > > > FWIW the way I've been modelling this patch series in my head is that we
> > > > format an arbitrarily large filesystem (m_LBA_size) address space on a
> > > > thinp, feed statfs an "adjusted" size (m_usable_size)i which restricts
> > > > how much space we can allocate, and now growfs increases or decreases
> > > > the adjusted size without having to relocate anything or mess with the
> > > > address space.  If the adjusted size ever exceeds the address space
> > > > size, then we tack on more AGs like we've always done.  From that POV,
> > > > there's no need to physically shrink (i.e. relocate) anything (and we
> > > > can leave that for later/never).
> > 
> > [...]
> > 
> > > For example, suppose we had an absolute crude, barebones implementation
> > > of physical shrink right now that basically trimmmed the amount of space
> > > from the end of the fs iff those AGs were completely empty and otherwise
> > > returned -EBUSY. There is no other userspace support, etc. As such, this
> > > hypothetical feature is extremely limited to being usable immediately
> > > after a growfs and thus probably has no use case other than "undo my
> > > accidental growfs."
> > > 
> > > If we had that right now, _then_ what would the logical shrink interface
> > > look like?
> > 
> > Absolutely no different to what I'm proposing we do right now. That
> > is, the behaviour of the "shrink to size X" ioctl is determined by
> > the feature bit in the superblock.  Hence if the thinspace feature
> > is set we do a thin shrink, and if it is not set we do a physical
> > shrink. i.e. grow/shrink behaviour is defined by the kernel
> > implementation, not the user or the interface.
> > 
> 
> I don't buy that argument at all. ;) What you describe above may be
> reasonable for the current situation where shrink doesn't actually exist
> (or thin comes first),

Which is the case we are discussing here. thinspace shrink is here,
now, physical shrink is no closer than it was 10 years ago. So it's
reasonable to design changes around the needs of thinspace shrink
because physical shrink is still be years away (if ever).

> but the above example assumes that there is at
> least one simple and working physical shrink use case wired up to the
> existing interface already.

IOWs, this is a strawman argument that involves designing an API to
suit the strawman.

[....]

> In summary, my arguments here consist mostly of a collection of red
> flags that I see rather than hard incompatibilities or specific use
> cases I want to support. The problematic situations change depending on
> whether we decide to support physical shrink on thin fs or not and so
> it's not really possible or important to try and pin them all down.
> OTOH, it's also quite possible that none of them ever materialize at
> all.

And that's the point I keep making: we don't know which of the
strawmen being presented are going to matter (if at all) until we
have physical shrink designed and are deep into the implementation.

IOWs, trying to work out the future API needs of a physical shrink
is just a guessing game right now.

> If they do, I'm pretty sure we could find ways to address each one
> individually as we progress, or document potentially confusing behavior
> appropriately, etc. The larger point is that I think much of this simply
> goes away with a cleaner interface. IMO, this boils down to what I think
> is just a matter of practicing good software engineering and system/user
> interface design.

Yes, but designing based on a /guess/ is *bad engineering practice*.
It almost always ends up wrong and has to be reworked, and that
means we get stuck supporting an API we don't need or want forever
more.

Yes, we've categorised the risk that we might need an interface
change in future - as we should - but we don't know which of those
risks are going to materialise.  IOWs, we can't solve this interface
problem with the information or insight we currently have - we need
to implement physical shrink and determine which of these risks
actually materialise, and then we can address the interface issue
knowing that we're solving the problems that physical shrink
introduces.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

  reply	other threads:[~2017-11-06 21:20 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-26  8:33 [RFC PATCH 0/14] xfs: Towards thin provisioning aware filesystems Dave Chinner
2017-10-26  8:33 ` [PATCH 01/14] xfs: factor out AG header initialisation from growfs core Dave Chinner
2017-10-26  8:33 ` [PATCH 02/14] xfs: convert growfs AG header init to use buffer lists Dave Chinner
2017-10-26  8:33 ` [PATCH 03/14] xfs: factor ag btree reoot block initialisation Dave Chinner
2017-10-26  8:33 ` [PATCH 04/14] xfs: turn ag header initialisation into a table driven operation Dave Chinner
2017-10-26  8:33 ` [PATCH 05/14] xfs: make imaxpct changes in growfs separate Dave Chinner
2017-10-26  8:33 ` [PATCH 06/14] xfs: separate secondary sb update in growfs Dave Chinner
2017-10-26  8:33 ` [PATCH 07/14] xfs: rework secondary superblock updates " Dave Chinner
2017-10-26  8:33 ` [PATCH 08/14] xfs: move various type verifiers to common file Dave Chinner
2017-10-26  8:33 ` [PATCH 09/14] xfs: split usable space from block device size Dave Chinner
2017-10-26  8:33 ` [PATCH 10/14] xfs: hide reserved metadata space from users Dave Chinner
2017-10-26  8:33 ` [PATCH 11/14] xfs: bump XFS_IOC_FSGEOMETRY to v5 structures Dave Chinner
2017-10-26  8:33 ` [PATCH 12/14] xfs: convert remaingin xfs_sb_version_... checks to bool Dave Chinner
2017-10-26 16:03   ` Darrick J. Wong
2017-10-26  8:33 ` [PATCH 13/14] xfs: add suport for "thin space" filesystems Dave Chinner
2017-10-26  8:33 ` [PATCH 14/14] xfs: add growfs support for changing usable blocks Dave Chinner
2017-10-26 11:30   ` Amir Goldstein
2017-10-26 12:48     ` Dave Chinner
2017-10-26 13:32       ` Amir Goldstein
2017-10-27 10:26         ` Amir Goldstein
2017-10-26 11:09 ` [RFC PATCH 0/14] xfs: Towards thin provisioning aware filesystems Amir Goldstein
2017-10-26 12:35   ` Dave Chinner
2017-11-01 22:31     ` Darrick J. Wong
2017-10-30 13:31 ` Brian Foster
2017-10-30 21:09   ` Dave Chinner
2017-10-31  4:49     ` Amir Goldstein
2017-10-31 22:40       ` Dave Chinner
2017-10-31 11:24     ` Brian Foster
2017-11-01  0:45       ` Dave Chinner
2017-11-01 14:17         ` Brian Foster
2017-11-01 23:53           ` Dave Chinner
2017-11-02 11:25             ` Brian Foster
2017-11-02 23:30               ` Dave Chinner
2017-11-03  2:47                 ` Darrick J. Wong
2017-11-03 11:36                   ` Brian Foster
2017-11-05 22:50                     ` Dave Chinner
2017-11-06 13:01                       ` Brian Foster
2017-11-06 21:20                         ` Dave Chinner [this message]
2017-11-07 11:28                           ` Brian Foster
2017-11-03 11:26                 ` Brian Foster
2017-11-03 12:19                   ` Amir Goldstein
2017-11-06  1:16                     ` Dave Chinner
2017-11-06  9:48                       ` Amir Goldstein
2017-11-06 21:46                         ` Dave Chinner
2017-11-07  5:30                           ` Amir Goldstein
2017-11-05 23:51                   ` Dave Chinner
2017-11-06 13:07                     ` Brian Foster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171106212046.GG4094@dastard \
    --to=david@fromorbit.com \
    --cc=bfoster@redhat.com \
    --cc=darrick.wong@oracle.com \
    --cc=linux-xfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).