linux-ext4 - Re: [PATCH 4/5] jbd2: Speedup jbd2_journal_get_[write|undo]

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 18 Jun 2015 10:52:35 +0200
From:	Jan Kara <jack@...e.cz>
To:	Jerry Lee <jerrylee@...p.com>
Cc:	Jan Kara <jack@...e.cz>, Theodore Ts'o <tytso@....edu>,
	linux-ext4@...r.kernel.org
Subject: Re: [PATCH 4/5] jbd2: Speedup jbd2_journal_get_[write|undo]_access()

On Thu 18-06-15 15:59:33, Jerry Lee wrote:
> Hi:
> 
> I found that there may be a typo in the attached patch 5/5:
> 
> +        /*
> +         * If it's in our transaction it must be in BJ_Metadata list.
> +         * The assertion is unreliable since we may see jh in
> +         * inconsistent state unless we grab bh_state lock. But this
> +         * is crutial to catch bugs so let's do a reliable check until
> +         * the lockless handling is fully proven.
> +         */
> +        if (jh->b_transaction == transaction &&
> +            jh->b_jlist != BJ_Metadata) {
> +            jbd_lock_bh_state(bh);
> +            J_ASSERT_JH(jh, jh->b_transaction != transaction ||
> +                    jh->b_jlist == BJ_Metadata);
> +            jbd_lock_bh_state(bh);
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +        }
> +        goto out;
> 
> Does the highlight part should be "jbd_unlock_bh_state(bh)"?
  Yeah, thanks for catching this. I was apparently pretty lucky in this
passing all the tests... Attached is the new version of the patch.

								Honza

> On Thu, Jun 18, 2015 at 12:56 AM, Jan Kara <jack@...e.cz> wrote:
> 
> > On Mon 08-06-15 12:47:26, Ted Tso wrote:
> > > On Thu, Apr 02, 2015 at 03:58:19PM +0200, Jan Kara wrote:
> > > > jbd2_journal_get_write_access() and jbd2_journal_get_create_access()
> > are
> > > > frequently called for buffers that are already part of the running
> > > > transaction - most frequently it is the case for bitmaps, inode table
> > > > blocks, and superblock. Since in such cases we have nothing to do, it
> > is
> > > > unfortunate we still grab reference to journal head, lock the bh, lock
> > > > bh_state only to find out there's nothing to do.
> > > >
> > > > Improving this is a bit subtle though since until we find out journal
> > > > head is attached to the running transaction, it can disappear from
> > under
> > > > us because checkpointing / commit decided it's no longer needed. We
> > deal
> > > > with this by protecting journal_head slab with RCU. We still have to be
> > > > careful about journal head being freed & reallocated within slab and
> > > > about exposing journal head in consistent state (in particular
> > > > b_modified and b_frozen_data must be in correct state before we allow
> > > > user to touch the buffer).
> > > >
> > > > FIXME: Performance data.
> > > >
> > > > Signed-off-by: Jan Kara <jack@...e.cz>
> > >
> > > Applied, so we can start getting some testing on this patch.  Did you
> > > ever get performance data?
> >
> > Yes. Here are results for reaim fserver workload for 32 core machine with
> > 128 GB of ram with ext4 on ramdisk:
> > Procs Vanilla    Patched
> > 1      20420.688  21155.556
> > 21     49684.704 178934.074
> > 41     84630.364 196647.482
> > 61    106344.284 204831.652
> > 81    120751.370 214842.428
> > 101   131585.450 208761.832
> > 121   138092.078 212741.648
> > 141   142271.578 212118.502
> > 161   146008.364 213731.388
> > 181   149569.494 216121.444
> >
> > Numbers are operations per second so larger is better. You can see that
> > for 21 processes we get increase by 260% in the number operations. Also the
> > total maximum of operations the machine is able to achieve increases by
> > 44% because of overall lower CPU overhead.
> >
> >                                                                 Honza
> > --
> > Jan Kara <jack@...e.cz>
> > SUSE Labs, CR
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> > the body of a message to majordomo@...r.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >
-- 
Jan Kara <jack@...e.cz>
SUSE Labs, CR

View attachment "0005-jbd2-Speedup-jbd2_journal_dirty_metadata.patch" of type "text/x-patch" (2671 bytes)