On 2020-05-02 20:04, Miquel Raynal wrote:
Miquel Raynal <miquel.raynal@xxxxxxxxxxx> wrote on Mon, 9 Mar 2020 16:22:23 +0100:Hi Rafał, Rafał Miłecki <rafal@xxxxxxxxxx> wrote on Mon, 9 Mar 2020 16:08:12 +0100: > On 09.03.2020 15:22, Miquel Raynal wrote: > > Rafał Miłecki <rafal@xxxxxxxxxx> wrote on Mon, 09 Mar 2020 15:19:10 > > +0100: > > > >> On 2020-03-09 15:04, Miquel Raynal wrote: > >>> Rafał Miłecki <zajec5@xxxxxxxxx> wrote on Mon, 9 Mar 2020 08:44:45 > >>> +0100: > >>> >>>> From: Rafał Miłecki <rafal@xxxxxxxxxx> > >>>>>> This fixes check for partitions that don't start at beginning of their > >>>> parents. Missing partition's offset in formula could result in forcing > >>>> read-only incorrectly. > >>>>>> Fixes: 6750f61a13a0 ("mtd: improve calculating partition boundaries >> when checking for alignment") > >>>> Signed-off-by: Rafał Miłecki <rafal@xxxxxxxxxx> > >>>> --- > >>>> drivers/mtd/mtdpart.c | 2 +- > >>>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>>>>> diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c > >>>> index 7328c066c5ba..c683b432cc5e 100644 > >>>> --- a/drivers/mtd/mtdpart.c > >>>> +++ b/drivers/mtd/mtdpart.c > >>>> @@ -524,7 +524,7 @@ static struct mtd_part *allocate_partition(struct >> mtd_info *parent, > >>>> part->name); > >>>> } > >>>>>> - tmp = part_absolute_offset(parent) + slave->mtd.size; > >>>> + tmp = part_absolute_offset(parent) + slave->offset + >> slave->mtd.size; > >>> > >>> I think you are doing the change at the wrong place, if you want to > >>> check where the partition *starts* you should do it a few lines above. > >>> But I think the check should be here as well, probably. > >> > >> The check where the partition *starts* is OK and I don't mean to change > >> it. The bug is about calculating absolute *end* address of partition. > > > > Can you detail a little bit then? Because I don't see the issue anymore > > even though I am convinced something is wrong here :) > > Please consider following partitions layout: > * bcm47xxsflash > ├─ boot 0x000000000000-0x000000040000 > └┬ firmware 0x000000040000-0x000001000000 > ├─ linux 0x00000000001c-0x00000018f800 > └┬ container 0x00000018f800-0x000000fc0000 > ├─ foo 0x000000000000-0x000000630800 > └─ bar 0x000000630800-0x000000e30800 (size: 0x800000) > > > Existing (correct) *start* calculation: > bar start: 0 + 0x040000 + 0x18f800 + 0x630800 = 0x800000 > > Existing (wrong) end calculation: > bar end: 0 + 0x040000 + 0x18f800 + 0x800000 = 0x9cf800 > > Fixed (correct) end calculation: > bar end: 0 + 0x040000 + 0x18f800 + 0x630800 + 0x800000 = 0x1000000 Ok I get it! I think mentioning "partitions that don't start at beginning of their parents", despite being true, was misleading to me as I understood "leaving extra space with the start of their parent". I suppose you also have the issue with "container" too? Anyway, I think the fix is fine. A better formulation for the commit log would be welcome :) (maybe adding this example is a good idea!)I don't remember having applied this fix yet, would you mind resending this patch with an enhanced commit log (your example was a good one I think).
It's not needed anymore. Please check discussion we got: -------- Original Message -------- Subject: Re: [PATCH] mtd: fix calculating partition end address Date: 2020-03-24 23:11
> I would like to apply your fix this week, do you think you can rebase > and resend soon? It's not needed anymore as you fixed this bug in your commit reworking partitions.
Great! ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
