From: Ted Ts'o <tytso@mit.edu>
To: Amir Goldstein <amir73il@gmail.com>
Cc: Jan Kara <jack@suse.cz>, Michael Rubin <mrubin@google.com>,
Eric Sandeen <sandeen@redhat.com>,
linux-ext4@vger.kernel.org,
Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [LSF/MM TOPIC] Drop ext2/ext3 codebase? When?
Date: Mon, 14 Feb 2011 14:58:45 -0500 [thread overview]
Message-ID: <20110214195845.GD4255@thunk.org> (raw)
In-Reply-To: <AANLkTimdApPPK6FkH+CPrXdNpND6_twE2voRiNhgQOOE@mail.gmail.com>
-lsf-pc, -linux-fsdevel
On Mon, Feb 14, 2011 at 09:00:58PM +0200, Amir Goldstein wrote:
> Yes, of course. Upgraders won't be the ones using snapshots.
> My intension was to state that those people installing new systems to test
> snapshots would be functioning as testers for "ext3 mode", because:
> 1. when no snapshots exists it boils down to testing "ext3 mode".
> 2. it is unlikely that snapshots will mask "ext3 mode" bugs.
>
> So my claim is that "ext3 mode" would benefit from a transition
> period in which snapshots and (extens,delalloc) are mutually
> exclusive in ext4.
Here are the requirements that I think are critical before we do this:
1) We need to solve the testing matrix problem. Right now "ext3 mode"
in ext4 doesn't get enough testing as it is. Part of the solution is
(a) deciding on the modes that need testing, and (b) writing some
shell scripts so that xfstests can be automatically run in all of the
right modes. And then it will be having some number of people
(hopefully not just me) running said tests and reporting failures.
2) The code has to integrate in a fairly seemless and easy way.
mballoc.c is an example of code that still needs a lot of cleanup.
Coly Li has submitted some cleanups, which is great. But I suspect a
lot more is needed.
One thing that comes to mind about your question with the
e4b->alloc_semp causing problems. If the only reason why we need it
is to protect against multiple attempts to initialize different block
groups that share the same buddy bitmap, can we solve the problem by
ditching e4b->alloc_semp entirely, and simply using lock_page() on the
buddy bitmap page to protect it?
That's an example of the radical code cleanup and simplification that
parts of the ext4 codebase could really use. That isn't the
snapshot's code fault, and if we didn't really need to touch parts the
code in question, it's probably stable enough as it is.
Unfortunately, if you need to make changes, there's enough code debt
in some of the files that you need to change that any changes _has_ to
make things better, and not worse. So for example, checking to see if
the blocksize==page_size, and then skipping the down_read(alloc_smp)
call is an example of layering _more_ complexity and code hackery, and
not less.
Note what I did with patches in the ext4_da_writepages() codepath ---
about 100 lines of code removed in just 7 patches, and I expect
performance will get better as a result of the cleanup. And then
compare that to how that code looked in say, 2.6.27. We need to do
similar amounts of cleanup in other parts of ext4 --- and mballoc.c is
by no means the worse. But building on top of code which has a fair
amount of code debt, is not a receipe for long-term success; it's like
building a castle on quicksand, or in a swamp (insert obligatory Monty
Python reference here).
- Ted
next prev parent reply other threads:[~2011-02-14 19:59 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-02-03 14:40 [LSF/MM TOPIC] Drop ext2/ext3 codebase? When? Jan Kara
2011-02-03 15:08 ` Eric Sandeen
2011-02-03 19:32 ` Michael Rubin
2011-02-03 19:49 ` Eric Sandeen
2011-02-03 21:57 ` Amir Goldstein
2011-02-03 22:00 ` Eric Sandeen
2011-02-04 13:59 ` Jan Kara
2011-02-04 0:04 ` Ted Ts'o
2011-02-04 13:17 ` Jan Kara
2011-02-04 17:03 ` Ric Wheeler
2011-02-04 17:17 ` [Lsf-pc] " James Bottomley
2011-02-05 18:43 ` Trond Myklebust
2011-02-07 17:21 ` Mingming Cao
2011-02-12 11:05 ` Amir Goldstein
2011-02-14 17:25 ` Jan Kara
2011-02-14 19:00 ` Amir Goldstein
2011-02-14 19:58 ` Ted Ts'o [this message]
2011-02-14 20:59 ` Andreas Dilger
2011-02-14 21:22 ` Amir Goldstein
2011-02-15 4:28 ` Dave Chinner
2011-02-15 17:29 ` Ted Ts'o
2011-02-21 23:48 ` Dave Chinner
2011-02-04 13:03 ` Jan Kara
2011-02-04 17:36 ` Andreas Dilger
2011-02-07 16:19 ` Jan Kara
2011-02-07 16:35 ` Andreas Dilger
2011-02-11 11:16 ` Jan Kara
2011-02-11 18:44 ` Michael Rubin
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=20110214195845.GD4255@thunk.org \
--to=tytso@mit.edu \
--cc=akpm@linux-foundation.org \
--cc=amir73il@gmail.com \
--cc=jack@suse.cz \
--cc=linux-ext4@vger.kernel.org \
--cc=mrubin@google.com \
--cc=sandeen@redhat.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.