Message-ID: <a0ff2007f08b7a55e3aa58f1cac2b9314559f598.camel@elektrobit.com>
Date: Mon, 26 Feb 2024 06:47:48 +0000
From: Weiß, Simone <Simone.Weiss@...ktrobit.com>
To: "mpatocka@...hat.com" <mpatocka@...hat.com>
CC: "song@...nel.org" <song@...nel.org>, "Tomerius, Kai"
	<Kai.Tomerius@...ktrobit.com>, "simone.p.weiss@...teo.net"
	<simone.p.weiss@...teo.net>, "agk@...hat.com" <agk@...hat.com>,
	"linux-raid@...r.kernel.org" <linux-raid@...r.kernel.org>,
	"dm-devel@...ts.linux.dev" <dm-devel@...ts.linux.dev>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"yukuai3@...wei.com" <yukuai3@...wei.com>, "lukas.bulwahn@...il.com"
	<lukas.bulwahn@...il.com>, "snitzer@...nel.org" <snitzer@...nel.org>
Subject: Re: [PATCH] [RFQ] dm-integrity: Add a lazy commit mode for journal

On Fri, 2024-02-23 at 21:33 +0100, Mikulas Patocka wrote:
> CAUTION: This email originated from outside of the Elektrobit organization. Do
> not click links or open attachments unless you recognize the sender and know
> the content is safe.
> 
> 
> On Fri, 23 Feb 2024, Weiß, Simone wrote:
> 
> > On Tue, 2024-02-20 at 19:52 +0100, Mikulas Patocka wrote:
> > > CAUTION: This email originated from outside of the Elektrobit
> > > organization. Do
> > > not click links or open attachments unless you recognize the sender and
> > > know
> > > the content is safe.
> > > 
> > > 
> > > 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.
> > Hi
> > 
> > Thanks for the review of the concept. I was out this week and could only
> > think
> > about it now. I understood it right, that the proposal is to add an extra
> > value
> > to the commit ID, that is e.g. incremented when integrity_commit is
> > executed?
> > 
> > If so, I tried this quickly and looks good on first glance. Will check and
> > test
> > further next.
> > 
> > Simone
> 
> I propose to use the commit ID 0 when writing the journal for the first
> time, then 1 when the journal rolls over, 2 when it rolls over again, 3
> when it rolls over again, 4 on another roll over and so on up to
> 0x7fffffffffffffff (which will be never reached in practice).
> 
> And use the top bit as an end-of-section marker. As the commit IDs will
> never roll over, it won't happen that an old transaction would be mixed
> into a new transaction on partial journal write.
> 
> Mikulas
Hi,

I can do it this way for sure as well. Another point still in my mind is the
superblock: I would like to get rid of the build time switch and carry
information about lazy commits enabled in the superblock. As there is J, B, D
and R as mode already, a new mode L or such could be added. I will work on this
and also take a look at stuff like dmsetup to check if something would be needed
there. If there are further points for now on anyone's mind, please tell.

Best,
Simone

Powered by blists - more mailing lists