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 [off-list ref] wrote:

quoted

On Mon 08-06-15 12:47:26, Ted Tso wrote:

quoted

On Thu, Apr 02, 2015 at 03:58:19PM +0200, Jan Kara wrote:

quoted

jbd2_journal_get_write_access() and jbd2_journal_get_create_access()

are

quoted

quoted

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

quoted

quoted

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

quoted

quoted

us because checkpointing / commit decided it's no longer needed. We

deal

quoted

quoted

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@suse.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 [off-list ref]
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Jan Kara [off-list ref]
SUSE Labs, CR