| Message ID | 1550493012-5959-3-git-send-email-amit.pundir@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [for-4.14.y+] mtd: keep original flags for every struct mtd_info | expand |
On 2019-02-18 13:30, Amit Pundir wrote: > From: Rafał Miłecki <rafal@milecki.pl> > > commit 1186af457cc186c5ed01708da71b1ffbdf0a2638 upstream. > > When allocating a new partition mtd subsystem runs internal tests in > the > allocate_partition(). They may result in modifying specified flags > (e.g. > dropping some /features/ like write access). > > Those constraints don't have to be necessary true for subpartitions. It > may happen parent partition isn't block aligned (effectively disabling > write access) while subpartition may fit blocks nicely. In such case > all > checks should be run again (starting with original flags value). > > Signed-off-by: Rafał Miłecki <rafal@milecki.pl> > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com> > Signed-off-by: Amit Pundir <amit.pundir@linaro.org> > --- > I understand that it doesn't exactly fit into Stable rules but I didn't > want to assume anything, so sending it up for review. > Cherry-picked from lede tree https://git.lede-project.org/?p=source.git > and build tested on v4.14.97 and v4.19.19 for ARCH=arm/arm64 defconfig. This was a pre-requirement for some later-added *feature*. I see to reason to pick this one for stable. We won't be picking that feature (whatever it was), so what's the point?
On Mon, 18 Feb 2019 at 18:31, Rafał Miłecki <rafal@milecki.pl> wrote: > > On 2019-02-18 13:30, Amit Pundir wrote: > > From: Rafał Miłecki <rafal@milecki.pl> > > > > commit 1186af457cc186c5ed01708da71b1ffbdf0a2638 upstream. > > > > When allocating a new partition mtd subsystem runs internal tests in > > the > > allocate_partition(). They may result in modifying specified flags > > (e.g. > > dropping some /features/ like write access). > > > > Those constraints don't have to be necessary true for subpartitions. It > > may happen parent partition isn't block aligned (effectively disabling > > write access) while subpartition may fit blocks nicely. In such case > > all > > checks should be run again (starting with original flags value). > > > > Signed-off-by: Rafał Miłecki <rafal@milecki.pl> > > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com> > > Signed-off-by: Amit Pundir <amit.pundir@linaro.org> > > --- > > I understand that it doesn't exactly fit into Stable rules but I didn't > > want to assume anything, so sending it up for review. > > Cherry-picked from lede tree https://git.lede-project.org/?p=source.git > > and build tested on v4.14.97 and v4.19.19 for ARCH=arm/arm64 defconfig. > > This was a pre-requirement for some later-added *feature*. I see to > reason to pick this one for stable. We won't be picking that feature > (whatever it was), so what's the point? Thank you for the review. I knew I was missing something. I sent it out of curiosity based on commit log, which talks about re-running all checks as a fix for internal tests likely dropping write access to new partition. And I didn't read it is a pre-requisite for later-added feature. I'm sure lede/openwrt would have picked/backported the whole feature, but as you mentioned it doesn't make sense for Stable. Thank you again for the feedback. Regards, Amit Pundir
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c index 97ac219c082e..2d24701255e5 100644 --- a/drivers/mtd/mtdcore.c +++ b/drivers/mtd/mtdcore.c @@ -665,6 +665,8 @@ static void mtd_set_dev_defaults(struct mtd_info *mtd) } else { pr_debug("mtd device won't show a device symlink in sysfs\n"); } + + mtd->orig_flags = mtd->flags; } /** diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c index 99c460facd5e..2b6e53af47da 100644 --- a/drivers/mtd/mtdpart.c +++ b/drivers/mtd/mtdpart.c @@ -346,7 +346,8 @@ static struct mtd_part *allocate_partition(struct mtd_info *parent, /* set up the MTD object for this partition */ slave->mtd.type = parent->type; - slave->mtd.flags = parent->flags & ~part->mask_flags; + slave->mtd.flags = parent->orig_flags & ~part->mask_flags; + slave->mtd.orig_flags = slave->mtd.flags; slave->mtd.size = part->size; slave->mtd.writesize = parent->writesize; slave->mtd.writebufsize = parent->writebufsize; diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h index cd0be91bdefa..b491a08e87e5 100644 --- a/include/linux/mtd/mtd.h +++ b/include/linux/mtd/mtd.h @@ -207,6 +207,7 @@ struct mtd_debug_info { struct mtd_info { u_char type; uint32_t flags; + uint32_t orig_flags; /* Flags as before running mtd checks */ uint64_t size; // Total size of the MTD /* "Major" erase size for the device. Naïve users may take this