Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Andreas Dilger <adilger@dilger.ca>
Message-Id: <C091902C-F3C9-4A8C-8704-DDB321FAA317@dilger.ca>
Content-Type: multipart/signed;
 boundary="Apple-Mail=_145085AF-F63A-4E2D-B223-D502EFE71B08";
 protocol="application/pgp-signature"; micalg=pgp-sha256
Mime-Version: 1.0 (Mac OS X Mail 10.3 \(3273\))
Subject: Re: [RFC PATCH v4 3/4] ext4: data=journal: fixes for
 ext4_page_mkwrite()
Date:   Mon, 28 Sep 2020 20:34:09 -0600
In-Reply-To: <20200928194103.244692-4-mfo@canonical.com>
Cc:     Jan Kara <jack@suse.cz>, linux-ext4@vger.kernel.org,
        dann frazier <dann.frazier@canonical.com>
To:     Mauricio Faria de Oliveira <mfo@canonical.com>
References: <20200928194103.244692-1-mfo@canonical.com>
 <20200928194103.244692-4-mfo@canonical.com>
Precedence: bulk


--Apple-Mail=_145085AF-F63A-4E2D-B223-D502EFE71B08
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=us-ascii

On Sep 28, 2020, at 1:41 PM, Mauricio Faria de Oliveira =
<mfo@canonical.com> wrote:
>=20
> These are two fixes for data journalling required by
> the next patch, discovered while testing it.
>=20
> First, the optimization to return early if all buffers
> are mapped is not appropriate for the next patch:
>=20
> The inode _must_ be added to the transaction's list in
> data=3Djournal mode (so to write-protect pages on commit)
> thus we cannot return early there.
>=20
> Second, once that optimization to reduce transactions
> was disabled for data=3Djournal mode, more transactions
> happened, and occasionally hit this warning message:
> 'JBD2: Spotted dirty metadata buffer'.
>=20
> Reason is, block_page_mkwrite() will set_buffer_dirty()
> before do_journal_get_write_access() that is there to
> prevent it. This issue was masked by the optimization.
>=20
> So, on data=3Djournal use __block_write_begin() instead.
> This also requires page locking and len recalculation.
> (see block_page_mkwrite() for implementation details.)
>=20
> Finally, as Jan noted there is little sharing between
> data=3Djournal and other modes in ext4_page_mkwrite().
>=20
> However, a prototype of ext4_journalled_page_mkwrite()
> showed there still would be lots of duplicated lines
> (tens of) that didn't seem worth it.
>=20
> Thus this patch ends up with an ugly goto to skip all
> non-data journalling code (to avoid long indentations,
> but that can be changed..) in the beginning, and just
> a conditional in the transaction section.
>=20
> Well, we skip a common part to data journalling which
> is the page truncated check, but we do it again after
> ext4_journal_start() when we re-acquire the page lock
> (so not to acquire the page lock twice needlessly for
> data journalling.)
>=20
> Signed-off-by: Mauricio Faria de Oliveira <mfo@canonical.com>
> Suggested-by: Jan Kara <jack@suse.cz>

Thanks for the clear commit message.  It definitely would not
be clear why the patch was structured in this way without it.

Reviewed-by: Andreas Dilger <adilger@dilger.ca>


Cheers, Andreas


--Apple-Mail=_145085AF-F63A-4E2D-B223-D502EFE71B08
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
	filename=signature.asc
Content-Type: application/pgp-signature;
	name=signature.asc
Content-Description: Message signed with OpenPGP

-----BEGIN PGP SIGNATURE-----
Comment: GPGTools - http://gpgtools.org

iQIzBAEBCAAdFiEEDb73u6ZejP5ZMprvcqXauRfMH+AFAl9ynSEACgkQcqXauRfM
H+DlCg//ZgnzNQ7dy71hUok2KxfEiA3ktZyLPKjccr5Gt+OBsuQKmQBGzs9+LlzA
7+KXFJz8pmQ52pqpw3pMmhFvgYnfrRW2FyEoV1wIL/Mkk8XmGCE5sZVmmwZDFA5b
CGWWYRHPt0w81pLKO2Kr8Hmw1Off3p1EGJtBpPjCYLg08LZ8rHEIrGNSn7FDsRV6
3akvhIFN5wBBAxidM5si6GFc3lzaBJs3UfMnG0qVPxZElwnXcpTIMdsPwHKNj5p4
09ofsxfFaCSI351wt90MsO1UsWQ6GR0MOMzxPoC69H45vOCEK5bG86eKEu69FSs1
b/06vc6s870IQecdLV5UdSLtwF+Lzw+6BN7+wd4MbvBPbSCLPjLAB3wfhmTMeCRE
DQHnR+zuuKVCNi+miwhEmS7r+MIk22QZdF6dM7TLR18Js0RTXUVfUGVA8Xe8CPQs
qgzKhF3177ZcYX5AR6dUr3uX/dcAhYjbSxucGthqbSz+BrO6nUufqFDnh+veOtWC
VMmryNhs78QPpgObi46o0Il2TZ8KVXLOI0MV9qdb9yj2COt7PSc9S9zAJCH+2eGB
sY7kbn1eNdSpkSXHXODPwLTAKZYp7fgep9e4uDZdKrACsL1HK4htgMYYUg+2zUJd
TJaNJleDJcP/PUjHrmFRbUwxd37ehYFDvrZRUio6Z90catZJH/o=
=Bsxp
-----END PGP SIGNATURE-----

--Apple-Mail=_145085AF-F63A-4E2D-B223-D502EFE71B08--