linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ted Ts'o <tytso@mit.edu>
To: Andreas Dilger <adilger@dilger.ca>
Cc: Ext4 Developers List <linux-ext4@vger.kernel.org>
Subject: Re: [PATCH 1/3] ext4: remove block bitmap initialization in ext4_new_inode()
Date: Mon, 16 Jan 2012 10:52:06 -0500	[thread overview]
Message-ID: <20120116155206.GB4667@thunk.org> (raw)
In-Reply-To: <00C2AC6D-7D45-4FFF-B509-77BFD21BC093@dilger.ca>

On Fri, Jan 13, 2012 at 10:02:46PM -0700, Andreas Dilger wrote:
> On 2012-01-13, at 2:30 PM, Theodore Ts'o wrote:
> > This is old code from the very early days that is just
> > confusing things, and also has the problem of modifying the block
> > group descriptor without obeying the ext4_journal_get_write_access() /
> > ext4_handle_dirty_metadata() modification protocols.
> 
> The group descriptor was already modified earlier on when the inode was
> being allocated from the group, so the group descriptor block was already
> accounted for in the transaction credits after "repeat_in_this_group:"

That's true, but we also have to modify the block bitmap block itself;
and that's a journal credit which is currently not accounted for.
Fortunately we're over conservative enough in how we calculate the
number of journal credits necessary that in practice we don't BUG_ON
due to running out ouf journal credits.

> I'm not wholly against removing this code, but we have to do it with the
> clear understanding that we will have inodes in use for which the block
> bitmap is showing that the in-use blocks are free.  This doesn't seem like
> a great situation to me.

We have backup block groups descriptors and (in the case of meta_bg,
which is needed for 64-bit resize) primary block group descriptors
which are also only accounted for by the fields in the block group
descriptors.

We can't allocate blocks in that block group without initializing the
block bitmap, and ext4_init_block_bitmap() will reserve the
appropriate fields; furthermore, if the block group checksum is
incorrect, it will lock out the block group by marking all blocks as
used and setting the number of free inodes and blocks to zero.

So it should be a safe thing to do.

						- Ted

  parent reply	other threads:[~2012-01-16 15:52 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-13 21:30 [PATCH 0/3] Clean up bitmap loading Theodore Ts'o
2012-01-13 21:30 ` [PATCH 1/3] ext4: remove block bitmap initialization in ext4_new_inode() Theodore Ts'o
2012-01-14  5:02   ` Andreas Dilger
2012-01-14 18:41     ` Amir Goldstein
2012-01-15 17:25       ` Andreas Dilger
2012-01-16 15:52     ` Ted Ts'o [this message]
2012-01-13 21:30 ` [PATCH 2/3] ext4: fold ext4_claim_inode into ext4_new_inode Theodore Ts'o
2012-01-13 21:30 ` [PATCH 3/3] ext4: fix race when setting bitmap_uptdate flag Theodore Ts'o

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=20120116155206.GB4667@thunk.org \
    --to=tytso@mit.edu \
    --cc=adilger@dilger.ca \
    --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 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).