linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Theodore Tso <tytso@mit.edu>
To: "Jose R. Santos" <jrs@us.ibm.com>, Andreas Dilger <adilger@sun.com>
Cc: linux-ext4 <linux-ext4@vger.kernel.org>
Subject: Re: Bug in ext2fs_set_gdt_csum() and uninit_groups handling
Date: Thu, 3 Apr 2008 10:17:45 -0400	[thread overview]
Message-ID: <20080403141745.GB13486@mit.edu> (raw)
In-Reply-To: <E1JhOy1-0006g8-Oy@closure.thunk.org>

So after looking at this some more, I think I understand what had
happened.  The code as currently written is OK, because we are
initializing the inode table even if the uninit_group feature is
enabled.  There is a new COMPAT feature, lazy_bg, which causes mke2fs
to skip initializing the inode table --- but since it's not in the
allowed set of filesystem features, we're not in any danger since
currently zeroing of the inode table can't be skipped.

HOWEVER, the code in ext2fs_set_gdt_csum() which I pointed out in my
earlier message is a ticking time bomb that will go off if we ever
enable lazy_bg.

Given that uninit_groups has only recently hit the e2fsprosg mainline,
and as far as I know, no one is currently using it in production ---
and those that might be, such as CFS, are zero'ing out the entire
inode table anyway (since the feature was originally called gdt_cksum)
--- and it would seem HIGHLY uninituitive that uninit_groups, or
uninit_bg doesn't actually prevent the slow step of zeroing the inode
table, I propose that we do the following:

*)  Remove lazy_bg feature flag entirely

*)  Remove the bogus code in ext2fs_set_gdt_csum()

*)  Change mke2fs to skip initializing the inode tables if
    uninit_groups is set.

*)  Implement the kernel code for ext4 so it will zero out the inode
    table in the background.

Does this make sense?  I believe it is safe, given that all existing
filesystems using uninit_groups (what few of them exist) have the
inode table completely zero'ed out, so there is no danger in making
this change in the deifnition of what uninit_groups (or uninit_bg) means.

     	       	   	      	      		    - Ted

  reply	other threads:[~2008-04-03 14:17 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-04-03 12:54 Bug in ext2fs_set_gdt_csum() and uninit_groups handling Theodore Ts'o
2008-04-03 14:17 ` Theodore Tso [this message]
2008-04-10 17:33   ` Andreas Dilger

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=20080403141745.GB13486@mit.edu \
    --to=tytso@mit.edu \
    --cc=adilger@sun.com \
    --cc=jrs@us.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 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).