From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from shadbolt.e.decadent.org.uk ([88.96.1.126]:34354 "EHLO shadbolt.e.decadent.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755611AbbI0NoW (ORCPT ); Sun, 27 Sep 2015 09:44:22 -0400 Message-ID: <1443361449.2004.20.camel@decadent.org.uk> Subject: Re: [PATCH 3.2.x] jbd2: add mutex_lock on j_checkpoint_mutex in jbd2_journal_flush From: Ben Hutchings To: zerg2000@astral.org.pl Cc: stable , Jan Kara Date: Sun, 27 Sep 2015 14:44:09 +0100 In-Reply-To: <1668256.Geort4rEnn@zealot> References: <1668256.Geort4rEnn@zealot> Content-Type: multipart/signed; micalg="pgp-sha512"; protocol="application/pgp-signature"; boundary="=-qD0zBrMtRjhaXRbUMrd+" Mime-Version: 1.0 Sender: stable-owner@vger.kernel.org List-ID: --=-qD0zBrMtRjhaXRbUMrd+ Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Sun, 2015-09-06 at 03:25 +0200, Bartosz Kwitniewski wrote: > Commit a3ceb22921615827bfed39d7612a9a370bff0edb (upstream=20 > 79feb521a44705262d15cc819a4117a447b11ea7) in 3.2.x tree introduced=20 > __jbd2_update_log_tail which requires j_checkpoint_mutex, but locking of= =20 > j_checkpoint_mutex in jbd2_journal_flush was not backported from upstream= . Oops. > Fixes kernel BUG at fs/jbd2/journal.c:832 (__jbd2_update_log_tail): > [] ? jbd2_cleanup_journal_tail+0x5d/0x61 [jbd2] > [] ? jbd2_journal_flush+0xc2/0x156 [jbd2] > [] ? ext4_freeze+0x2f/0x71 [ext4] > [] ? filemap_write_and_wait+0x26/0x32 > [] ? freeze_super+0x8c/0xdd > [] ? freeze_bdev+0x5b/0xa1 > [] ? start_cow_session+0xb3/0x2d6 [hcpdriver] > [] ? printk+0x40/0x49 > [] ? alloc_cts_session+0x2e/0x33 [hcpdriver] > [] ? ioctl_start_hcp_session+0x131/0x20d [hcpdriver] > [] ? handle_ioctlStartHC2+0x95/0x1ab [hcpdriver] > [] ? cow_ioctl_unlocked+0x13/0x18 [hcpdriver] > [] ? do_vfs_ioctl+0x55a/0x5a9 > [] ? pax_randomize_kstack+0x4c/0x60 > [] ? sysret_check+0x20/0x62 > [] ? do_sys_open+0x11e/0x130 > [] ? sys_ioctl+0x3c/0x5f > [] ? system_call_fastpath+0x16/0x1b >=20 > Signed-off-by: Bartosz Kwitniewski > Cc: stable@vger.kernel.org > --- > --- fs/jbd2/journal.c.orig> > 2015-08-12 16:33:24.000000000 +0200 > +++ fs/jbd2/journal.c> > 2015-09-06 00:57:56.890894891 +0200 > @@ -1828,10 +1828,13 @@ int jbd2_journal_flush(journal_t *journa > > > if (is_journal_aborted(journal)) > > > > return -EIO; > =20 > +> > mutex_lock(&journal->j_checkpoint_mutex); > > > if (!err) { > > > > err =3D jbd2_cleanup_journal_tail(journal); > -> > > if (err < 0) > +> > > if (err < 0) { > +> > > > mutex_unlock(&journal->j_checkpoint_mutex); > > > > > goto out; > +> > > } > > > > err =3D 0; > > > } > =20 > @@ -1841,6 +1844,7 @@ int jbd2_journal_flush(journal_t *journa > > > * commits of data to the journal will restore the current > > > * s_start value. */ > > > jbd2_mark_journal_empty(journal); > +> > mutex_unlock(&journal->j_checkpoint_mutex); > > > write_lock(&journal->j_state_lock); > > > J_ASSERT(!journal->j_running_transaction); > > > J_ASSERT(!journal->j_committing_transaction); Why is it sufficient to add locking of j_checkpoint_mutex only in this one function? Shouldn't I cherry-pick commits 24bcc89c7e7c ("jbd2: split updating of journal superblock and marking journal empty") and a78bb11d7acd ("jbd2: protect all log tail updates with j_checkpoint_mutex") as well? Ben. --=20 Ben Hutchings I say we take off; nuke the site from orbit. It's the only way to be sure. --=-qD0zBrMtRjhaXRbUMrd+ Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIUAwUAVgfyqee/yOyVhhEJAQoUUQ/43/qwBWD+lJhnDuFHzHtweB1kWkumo/n6 IVeWvNK8zaR07P9E6whBAdEGC3FVNcjXSs/bNGgBB6ScrUZi2yFK8qZi+Vnd/SU+ fFIL5Bl+rztoP+U9/4oVK+vDymLa6Mz9b5WjMZ3P0FL/N67MvsNGh8SAkZI99hdd +hV8hSlJ0Y4W9zi1+nNtMCY5FwIKtQEQqIb/F+a7O8dJOJ1j/sDkvYthuZRqOTog 20n5Z7Cc/586C+uLg9B4xrQnxG1Qr3jP3VcLDYiD7g9TdvWj/btrqOxa1wzwrY5m hl/1cfCmNmFcSnOWzzF3DcrhNpJ6fCC5rd++Mra+B/6NT2AheZJZ4SGrHY/1PMOs brq8CMkZyWPoxhZsagxlAQsnVZuItW3joOUlhyIqrCs87hiuI+dF2L+Frmi+C/mP pC00pBtBU1kMvjppaw2x1QcHmTB8hWqKkk+A4uM97DqtVNlkly93ceBV60D6VWBo 7aq9dG2fPSfPInODCYwR4/nhRDGo4SIYWstoah1S7Ny9jByGJoZZS4vCs3CdIY6v 58ul6YU/A5CGHNKo09+Fwh7zMZ3bHAeb0M5wiZx7Q14vnrURP3KPve3oZYxPtCi8 sPy/I35zwqBHz6pjBTI0HsSs9r97auHtasM5Iob6zPomBTjiSy3ylScvKY2QZU// LNsWTAJ9BQ== =cE/8 -----END PGP SIGNATURE----- --=-qD0zBrMtRjhaXRbUMrd+--