From: Amir Goldstein <amir73il@gmail.com>
To: "Ted Ts'o" <tytso@mit.edu>
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 23:22:11 +0200 [thread overview]
Message-ID: <AANLkTimJwBTvyYtjQfiOGXYFunC035NCfOpTCnkDQzzu@mail.gmail.com> (raw)
In-Reply-To: <20110214195845.GD4255@thunk.org>
On Mon, Feb 14, 2011 at 9:58 PM, Ted Ts'o <tytso@mit.edu> wrote:
> -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?
>
Perhaps. I imagine there is more than one elegant way to deal with that,
but using a semaphore is not one of them.
I will take a shot at evaporating e4b->alloc_semp.
> 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.
>
Fair enough. I accept the challenge.
I shall cleanup mballoc.c in the process of merging snapshots code.
If you have specific things that bug you in mballoc.c, let me know.
> 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
>
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2011-02-14 21:22 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
2011-02-14 20:59 ` Andreas Dilger
2011-02-14 21:22 ` Amir Goldstein [this message]
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=AANLkTimJwBTvyYtjQfiOGXYFunC035NCfOpTCnkDQzzu@mail.gmail.com \
--to=amir73il@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=jack@suse.cz \
--cc=linux-ext4@vger.kernel.org \
--cc=mrubin@google.com \
--cc=sandeen@redhat.com \
--cc=tytso@mit.edu \
/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).