From: Jeff Garzik <jeff@garzik.org>
To: Andrew Morton <akpm@osdl.org>
Cc: Mingming Cao <cmm@us.ibm.com>,
linux-kernel@vger.kernel.org, ext2-devel@lists.sourceforge.net,
linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH 1/5] Forking ext4 filesystem from ext3 filesystem
Date: Thu, 10 Aug 2006 16:22:26 -0400 [thread overview]
Message-ID: <44DB9582.6010609@garzik.org> (raw)
In-Reply-To: <20060810111839.51c73911.akpm@osdl.org>
Andrew Morton wrote:
> On Thu, 10 Aug 2006 09:41:59 -0700
> Mingming Cao <cmm@us.ibm.com> wrote:
>
>> Andrew Morton wrote:
>>
>>> On Wed, 09 Aug 2006 18:17:02 -0700
>>> Mingming Cao <cmm@us.ibm.com> wrote:
>>>
>>>
>>>> Fork(copy) ext4 filesystem from ext3 filesystem. Rename all functions in ext4 from ext3_xxx() to ext4_xxx().
>>>
>>> It would have been nice to spend a few hours cleaning up ext3 and JBD
>>> before doing this. The code isn't toooo bad, but there are number of
>>> coding style problems, whitespace screwups, incorrect comments, missing
>>> comments, poorly-chosen variable names and all of that sort of thing.
>>>
>>> One the fs has been copied-and-pasted, it's much harder to address these
>>> things: either need to do it twice, or allow the filesystems to diverge, or
>>> not do it.
>>>
>> Andrew, thanks for taking a close look this series of changes.
>>
>> I agree with you that the timing is right, to do the clean up now rather
>> than later. I would give it a try. If I could get more help from more
>> code reviewer, it probably makes the effort a lot easier. For those
>> issues you pointed out : coding style problem___incorrect comments,
>> poorly-named variables -- do you have any specific examples in your mind?
>
> Not really, apart from the few things I identified elsewhere (such as the
> brelse thing).
>
> It's just that now is the right time for a general spring-cleaning, if we
> ever want to do that.
>
>>> Also, -mm presently has two patches pending against fs/jbd/ and nine pending
>>> against fs/ext3/. We should get all those things merged before taking the
>>> copy.
>>>
>> So probably the right thing to do is keep the ext4 patches against mm
>> tree instead of rc three?
>
> That would drive everyone nuts, I think. What I would suggest is:
>
> - get ext3 into a ready-to-copy state (merge bugfixes, spring-clean, etc)
Presumably bug fixes should go in immediately, regardless of whether
it's before or after "cp -a ext3 ext4".
I strongly disagree that ext3 should be subject to a spring cleaning.
Comments, whitespace, very very minor things, sure. Trying to get rid
of brelse() when _many_ other filesystems also use it? ext4 material.
That detracts from the idea that its the stable counterpart to the devel
filesystem (ext4).
Jeff
WARNING: multiple messages have this Message-ID (diff)
From: Jeff Garzik <jeff@garzik.org>
To: Andrew Morton <akpm@osdl.org>
Cc: linux-fsdevel@vger.kernel.org, ext2-devel@lists.sourceforge.net,
Mingming Cao <cmm@us.ibm.com>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/5] Forking ext4 filesystem from ext3 filesystem
Date: Thu, 10 Aug 2006 16:22:26 -0400 [thread overview]
Message-ID: <44DB9582.6010609@garzik.org> (raw)
In-Reply-To: <20060810111839.51c73911.akpm@osdl.org>
Andrew Morton wrote:
> On Thu, 10 Aug 2006 09:41:59 -0700
> Mingming Cao <cmm@us.ibm.com> wrote:
>
>> Andrew Morton wrote:
>>
>>> On Wed, 09 Aug 2006 18:17:02 -0700
>>> Mingming Cao <cmm@us.ibm.com> wrote:
>>>
>>>
>>>> Fork(copy) ext4 filesystem from ext3 filesystem. Rename all functions in ext4 from ext3_xxx() to ext4_xxx().
>>>
>>> It would have been nice to spend a few hours cleaning up ext3 and JBD
>>> before doing this. The code isn't toooo bad, but there are number of
>>> coding style problems, whitespace screwups, incorrect comments, missing
>>> comments, poorly-chosen variable names and all of that sort of thing.
>>>
>>> One the fs has been copied-and-pasted, it's much harder to address these
>>> things: either need to do it twice, or allow the filesystems to diverge, or
>>> not do it.
>>>
>> Andrew, thanks for taking a close look this series of changes.
>>
>> I agree with you that the timing is right, to do the clean up now rather
>> than later. I would give it a try. If I could get more help from more
>> code reviewer, it probably makes the effort a lot easier. For those
>> issues you pointed out : coding style problem___incorrect comments,
>> poorly-named variables -- do you have any specific examples in your mind?
>
> Not really, apart from the few things I identified elsewhere (such as the
> brelse thing).
>
> It's just that now is the right time for a general spring-cleaning, if we
> ever want to do that.
>
>>> Also, -mm presently has two patches pending against fs/jbd/ and nine pending
>>> against fs/ext3/. We should get all those things merged before taking the
>>> copy.
>>>
>> So probably the right thing to do is keep the ext4 patches against mm
>> tree instead of rc three?
>
> That would drive everyone nuts, I think. What I would suggest is:
>
> - get ext3 into a ready-to-copy state (merge bugfixes, spring-clean, etc)
Presumably bug fixes should go in immediately, regardless of whether
it's before or after "cp -a ext3 ext4".
I strongly disagree that ext3 should be subject to a spring cleaning.
Comments, whitespace, very very minor things, sure. Trying to get rid
of brelse() when _many_ other filesystems also use it? ext4 material.
That detracts from the idea that its the stable counterpart to the devel
filesystem (ext4).
Jeff
-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
next prev parent reply other threads:[~2006-08-10 20:23 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2006-08-10 1:17 [PATCH 1/5] Forking ext4 filesystem from ext3 filesystem Mingming Cao
2006-08-10 6:39 ` Andrew Morton
2006-08-10 6:39 ` Andrew Morton
2006-08-10 16:41 ` Mingming Cao
2006-08-10 16:41 ` Mingming Cao
2006-08-10 16:48 ` Jörn Engel
2006-08-10 16:48 ` Jörn Engel
2006-08-10 18:18 ` Andrew Morton
2006-08-10 18:18 ` Andrew Morton
2006-08-10 20:22 ` Jeff Garzik [this message]
2006-08-10 20:22 ` Jeff Garzik
2006-08-10 20:33 ` Andrew Morton
2006-08-10 20:33 ` Andrew Morton
2006-08-10 20:52 ` Jeff Garzik
2006-08-10 17:44 ` [Ext2-devel] " Theodore Tso
2006-08-10 17:44 ` Theodore Tso
2006-08-10 18:51 ` Badari Pulavarty
2006-08-10 19:23 ` Andrew Morton
2006-08-10 19:23 ` Andrew Morton
2006-08-10 19:36 ` [Ext2-devel] " Dave Kleikamp
2006-08-10 19:36 ` Dave Kleikamp
2006-08-10 19:54 ` [Ext2-devel] " Andrew Morton
2006-08-10 19:54 ` Andrew Morton
2006-08-10 20:12 ` Jeff Garzik
2006-08-10 20:12 ` Jeff Garzik
2006-08-10 20:13 ` Jeff Garzik
2006-08-10 20:13 ` Jeff Garzik
2006-08-10 20:27 ` Andrew Morton
2006-08-10 20:27 ` Andrew Morton
2006-08-10 21:00 ` Jeff Garzik
2006-08-10 21:00 ` Jeff Garzik
2006-08-10 21:11 ` [Ext2-devel] " Alex Tomas
2006-08-10 21:11 ` Alex Tomas
2006-08-10 22:18 ` [Ext2-devel] " Andrew Morton
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=44DB9582.6010609@garzik.org \
--to=jeff@garzik.org \
--cc=akpm@osdl.org \
--cc=cmm@us.ibm.com \
--cc=ext2-devel@lists.sourceforge.net \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@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 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.