linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: tytso@mit.edu
To: Eric Sandeen <sandeen@redhat.com>
Cc: Ext4 Developers List <linux-ext4@vger.kernel.org>
Subject: Re: [PATCH 2/2] ext4: Use slab allocator for sub-page sized allocations
Date: Wed, 2 Dec 2009 15:12:34 -0500	[thread overview]
Message-ID: <20091202201234.GG6278@thunk.org> (raw)
In-Reply-To: <4B169B86.4040707@redhat.com>

On Wed, Dec 02, 2009 at 10:53:26AM -0600, Eric Sandeen wrote:
> 
> So, this undoes commit c089d490dfbf53bc0893dc9ef57cf3ee6448314d
> more or less, right:
> 
> JBD: Replace slab allocations with page allocations

Close, but not quite.  We still use getfreepage() in the case where
blocksize==pagesize.  So in the most common case of 4k blocks on
x86/x86_64, we won't be sufferring the overhead of using the
SLAB/SLUB/SLOB/SLQB machinery.

> Was alignment the only reason that commit went in?

Alignment and if debugging was enabled, it meant that you needed two
contiguous pages for every 4k shadow buffer_head used by the JBD
layer, not to mention the overhead of tracking each page in the slab
cache.

> Also, there is a race issue here I think, that we've had bugs
> on internally, but w/ the above commit it didn't matter anymore:
> 
> If you simultaneously mount several jbd(2)-based filesystems
> at once, before the slabs get created, nothing protects jbd2_slab[]
> and we can race on creation/initialization of that array.
> 
> See a proposed patch:
> 
> https://bugzilla.lustre.org/attachment.cgi?id=22906
> 
> but I think that patch is a little heavy-handed, the down_reads
> of the slab lock seem to be extraneous because if the fs is mounted,
> nobody will be tearing down that slab anyway, since that only
> happens on module unload, right?

Yeah, we only need the lock at mount time, when we are filling in the
jbd_slab[] array.  Thanks for pointing that out; I'll fix it and
resend the patch.

					- Ted

  parent reply	other threads:[~2009-12-02 20:12 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-12-02  1:14 [PATCH 1/2] jbd2: Add ENOMEM checking in and for jbd2_journal_write_metadata_buffer() Theodore Ts'o
2009-12-02  1:14 ` [PATCH 2/2] ext4: Use slab allocator for sub-page sized allocations Theodore Ts'o
2009-12-02 16:48   ` Josef Bacik
2009-12-02 16:53   ` Eric Sandeen
2009-12-02 19:31     ` Andreas Dilger
2009-12-02 20:12     ` tytso [this message]
2009-12-03 23:45       ` [PATCH -v2] " Theodore Ts'o
2009-12-05  6:43     ` [PATCH 2/2] " Eric Sandeen

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=20091202201234.GG6278@thunk.org \
    --to=tytso@mit.edu \
    --cc=linux-ext4@vger.kernel.org \
    --cc=sandeen@redhat.com \
    /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).