Received-SPF: pass (google.com: domain of linux-kernel+bounces-73549-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Tue, 20 Feb 2024 19:52:38 +0100 (CET)
From: Mikulas Patocka <mpatocka@redhat.com>
To: =?ISO-8859-15?Q?Simone_Wei=DF?= <simone.weiss@elektrobit.com>
cc: lukas.bulwahn@gmail.com, simone.p.weiss@posteo.net, 
    Kai Tomerius <kai.tomerius@elektrobit.com>, 
    Alasdair Kergon <agk@redhat.com>, Mike Snitzer <snitzer@kernel.org>, 
    dm-devel@lists.linux.dev, Song Liu <song@kernel.org>, 
    Yu Kuai <yukuai3@huawei.com>, linux-raid@vger.kernel.org, 
    linux-kernel@vger.kernel.org
Subject: Re: [PATCH] [RFQ] dm-integrity: Add a lazy commit mode for journal
In-Reply-To: <20240209192542.449367-1-simone.weiss@elektrobit.com>
Message-ID: <8a485b9-6dbb-78c-9a84-ed3ba65d9cb3@redhat.com>
References: <20240209192542.449367-1-simone.weiss@elektrobit.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/mixed; BOUNDARY="185210117-1528542385-1708454829=:2837266"
Content-ID: <19bdaab-220-364a-26ef-b7e1f6447b7a@redhat.com>

  This message is in MIME format.  The first part should be readable text,
  while the remaining parts are likely unreadable without MIME-aware tools.

--185210117-1528542385-1708454829=:2837266
Content-Type: text/plain; CHARSET=ISO-8859-15
Content-Transfer-Encoding: 8BIT
Content-ID: <31b6218e-52d4-9728-16-da4e5e2dce77@redhat.com>


On Fri, 9 Feb 2024, Simone Wei? wrote:

> Extend the dm-integrity driver to omit writing unused journal data sectors.
> Instead of filling up the whole journal section, mark the last used
> sector with a special commit ID. The commit ID still uses the same base value,
> but section number and sector number are inverted. At replay when commit IDs
> are analyzed this special commit ID is detected as end of valid data for this
> section. The main goal is to prolong the live times of e.g. eMMCs by avoiding
> to write the whole journal data sectors.
> 
> The change is right now to be seen as experimental and gets applied if
> CONFIG_DMINT_LAZY_COMMIT is set to y. Note please that this is NOT
> planned for a final version of the changes. I would make it configurable
> via flags passed e.g. via dmsetup and stored in the superblock.
> 
> Architectural Limitations:
> - A dm-integrity partition, that was previously used with lazy commit,
>  can't be replayed with a dm-integrity driver not using lazy commit.
> - A dm-integrity driver that uses lazy commit is expected
>  to be able to cope with a partition that was created and used without
>  lazy commit.
> - With dm-integrity lazy commit, a partially written journal (e.g. due to a
>  power cut) can cause a tag mismatch during replay if the journal entry marking
>  the end of the journal section is missing. Due to lazy commit, older journal
>  entries are not erased and might be processed if they have the same commit ID
>  as adjacent newer journal entries.

Hi

I was thinking about it and I think that this problem is a showstopper.

Suppose that a journal section contains these commit IDs:

	2	2	2	2(EOF)	3	3	3	3

The IDs "3" are left over from previous iterations. The IDs "2" contain 
the current data. And now, the journal rolls over and we attempt to write 
all 8 pages with the ID "3". However, a power failure happens and we only 
write 4 pages with the ID "3". So, the journal will look like:

	3(new)	3(new)	3(new)	3(new)	3(old)	3(old)	3(old)	3(old)

After a reboot, the journal-replay logic will falsely believe that the 
whole journal section is consistent and it will attempt to replay it.

This could be fixed by having always increasing commit IDs - the commit 
IDs have 8 bytes, so we can assume that they never roll-over and it would 
prevent us from mixing old IDs into the current transaction.

Mikulas

>  If dm-integrity detects bad sections while
>  replaying the journal, keep track about those sections and try to at least
>  replay older, good sections.
>  This is based on the assumption that most likely the newest
>  section(s) will be damaged, which might have been only partially written
>  due to a sudden reset. Previously, the whole journal would be cleared in
>  such a case.
> 
> Signed-off-by: Simone Wei? <simone.weiss@elektrobit.com>
> Signed-off-by: Kai Tomerius <kai.tomerius@elektrobit.com>
--185210117-1528542385-1708454829=:2837266--