linux-kernel - Re: [PATCH v7 1/2] mtd: nand_bbt: Move BBT block selection logic out of write

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20160815172824.684a0c71@bbrezillon>
Date:	Mon, 15 Aug 2016 17:28:24 +0200
From:	Boris Brezillon <boris.brezillon@...e-electrons.com>
To:	Kyle Roeschley <kyle.roeschley@...com>
Cc:	<richard@....at>, <dwmw2@...radead.org>,
	<computersforpeace@...il.com>, <beanhuo@...ron.com>,
	<linux-mtd@...ts.infradead.org>, <linux-kernel@...r.kernel.org>,
	<nathan.sullivan@...com>, <xander.huff@...com>,
	<peterpansjtu@...il.com>
Subject: Re: [PATCH v7 1/2] mtd: nand_bbt: Move BBT block selection logic
 out of write_bbt()

On Mon, 15 Aug 2016 09:47:40 -0500
Kyle Roeschley <kyle.roeschley@...com> wrote:

> On Sat, Aug 13, 2016 at 12:37:03AM +0200, Boris Brezillon wrote:
> > On Fri, 12 Aug 2016 16:58:22 -0500
> > Kyle Roeschley <kyle.roeschley@...com> wrote:
> >   
> [...]
> > > +	while (chip < nrchips) {  
> > 
> > I'm probably missing something, but why are you turning the for loop
> > into a while loop in this patch? The commit message does not mention
> > that, and I don't see why you need it before you actually start
> > reworking the code to recover from BBT write failures (which is done in
> > patch 2).
> >   
> 
> You had changed it in patch 2 (http://code.bulix.org/e16nvo-104988) and I just
> shuffled it to the first patch since it seemed to make sense as additional code
> cleanup.

Well, this is not exactly a cleanup, it's needed because of the
rework done in patch 2: we no longer want the for loop to automatically
increment the chip variable (if we fail to write the BBT on a specific
die, we retry until we succeed or run out of free valid erase blocks).

Now, if you really want to make it part of patch 1, at least explain
why you're doing that (in preparation of BBT write failure handling).

> I'll go ahead and drop it though if you don't want it in.
> 

Note that I don't want you to completely drop this change, just put it
back in patch 2 or explain why you're doing it in patch 1 in your commit
message.