From: tytso@mit.edu
To: "Aneesh Kumar K. V" <aneesh.kumar@linux.vnet.ibm.com>
Cc: Ext4 Developers List <linux-ext4@vger.kernel.org>
Subject: Re: [PATCH, RFC 2/2] ext4: Convert callers of ext4_get_blocks() to use ext4_map_blocks()
Date: Tue, 4 May 2010 11:42:51 -0400 [thread overview]
Message-ID: <20100504154251.GA6344@thunk.org> (raw)
In-Reply-To: <878w80htis.fsf@linux.vnet.ibm.com>
On Tue, May 04, 2010 at 03:34:59PM +0530, Aneesh Kumar K. V wrote:
>
> bh flags are not set here. This check should be based on map.m_flags.
Good catch, thanks.
> Only thing i am worried about is we were modifying bh_flags in all
> possible confusing ways. We may want to make sure we get the update
> correct. I am still going through the patch after applying it to the
> tree. So may take more time to look at the full changeset.
I'm concerned about that as well; in fact I'm not sure what we had
before was completely correct, and I *still* have trouble reasoning
about how all the flags work and why we do some of the things that we
do based on how the flags are set --- and that scares me.
XFS doesn't jump through *nearly* as many hoops as we do --- and given
XFS's reputation for complexity, that's saying a lot! --- and I
suspect some of the things we do are mandated by the fs/buffer.c and
fs/direct_io.c, the latter of which does some very strange and
unnatural things with buffer_heads --- and some of the things we do
are based on our own, ext4-specific logic and how we route state
through the many layers of callback functions. I think I've figured
some of this out, but it's gotten very crufty over the course of
ext4's development.
One of the reasons I had for doing this cleanup, in addition to
reducing stack usage (which I have measured as around 120 bytes or so
on an 32-bit system, and I'm sure it's got to be more on a 64-bit
system) was to make explicit how we were modifying the bh_flags. At
least now we can grep for EXT4_MAP_* and see on the places where we
were mucking with things.
- Ted
next prev parent reply other threads:[~2010-05-04 15:42 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-05-03 22:51 [PATCH, RFC 1/2] ext4: Add new abstraction ext4_map_blocks() underneath ext4_get_blocks() Theodore Ts'o
2010-05-03 22:51 ` [PATCH, RFC 2/2] ext4: Convert callers of ext4_get_blocks() to use ext4_map_blocks() Theodore Ts'o
2010-05-04 10:04 ` Aneesh Kumar K. V
2010-05-04 15:42 ` tytso [this message]
2010-05-05 18:38 ` tytso
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=20100504154251.GA6344@thunk.org \
--to=tytso@mit.edu \
--cc=aneesh.kumar@linux.vnet.ibm.com \
--cc=linux-ext4@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.