From: Richard Weinberger richard@nod.at
[ Upstream commit 34653fd8c46e771585fce5975e4243f8fd401914 ]
This commit got merged along with commit 781932375ffc ("ubi: fastmap: Correctly handle interrupted erasures in EBA") upstream but only the latter has been applied to stable v4.14.54 as commit a23cf10d9abb. This resulted in a performance regression. Startup on i.MX platforms is delayed for up to a few seconds depending on the platform. This fixes ubi fastmap to be of the same performance as it has been before said fastmap changes.
Fixes: a23cf10d9abb ("ubi: fastmap: Correctly handle interrupted erasures in EBA") Signed-off-by: Richard Weinberger richard@nod.at Signed-off-by: Martin Kepplinger martin.kepplinger@ginzinger.com ---
Richard, although this fixes a major slowdown regression in -stable, do you consider this "stable" too?
This applies and is tested only for the 4.14 stable tree. It seems to be equally relevant for 4.9 and 4.4 though.
thanks martin
drivers/mtd/ubi/build.c | 1 + drivers/mtd/ubi/eba.c | 4 ++++ drivers/mtd/ubi/fastmap.c | 20 ++++++++++++++++++++ drivers/mtd/ubi/ubi.h | 11 +++++++++++ drivers/mtd/ubi/vmt.c | 1 + drivers/mtd/ubi/vtbl.c | 16 +++++++++++++++- 6 files changed, 52 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c index 18a72da759a0..6445c693d935 100644 --- a/drivers/mtd/ubi/build.c +++ b/drivers/mtd/ubi/build.c @@ -526,6 +526,7 @@ void ubi_free_internal_volumes(struct ubi_device *ubi) for (i = ubi->vtbl_slots; i < ubi->vtbl_slots + UBI_INT_VOL_COUNT; i++) { ubi_eba_replace_table(ubi->volumes[i], NULL); + ubi_fastmap_destroy_checkmap(ubi->volumes[i]); kfree(ubi->volumes[i]); } } diff --git a/drivers/mtd/ubi/eba.c b/drivers/mtd/ubi/eba.c index d0884bd9d955..c4d4b8f07630 100644 --- a/drivers/mtd/ubi/eba.c +++ b/drivers/mtd/ubi/eba.c @@ -517,6 +517,9 @@ static int check_mapping(struct ubi_device *ubi, struct ubi_volume *vol, int lnu if (!ubi->fast_attach) return 0;
+ if (!vol->checkmap || test_bit(lnum, vol->checkmap)) + return 0; + vidb = ubi_alloc_vid_buf(ubi, GFP_NOFS); if (!vidb) return -ENOMEM; @@ -551,6 +554,7 @@ static int check_mapping(struct ubi_device *ubi, struct ubi_volume *vol, int lnu goto out_free; }
+ set_bit(lnum, vol->checkmap); err = 0;
out_free: diff --git a/drivers/mtd/ubi/fastmap.c b/drivers/mtd/ubi/fastmap.c index 5a832bc79b1b..63e8527f7b65 100644 --- a/drivers/mtd/ubi/fastmap.c +++ b/drivers/mtd/ubi/fastmap.c @@ -1101,6 +1101,26 @@ int ubi_scan_fastmap(struct ubi_device *ubi, struct ubi_attach_info *ai, goto out; }
+int ubi_fastmap_init_checkmap(struct ubi_volume *vol, int leb_count) +{ + struct ubi_device *ubi = vol->ubi; + + if (!ubi->fast_attach) + return 0; + + vol->checkmap = kcalloc(BITS_TO_LONGS(leb_count), sizeof(unsigned long), + GFP_KERNEL); + if (!vol->checkmap) + return -ENOMEM; + + return 0; +} + +void ubi_fastmap_destroy_checkmap(struct ubi_volume *vol) +{ + kfree(vol->checkmap); +} + /** * ubi_write_fastmap - writes a fastmap. * @ubi: UBI device object diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h index 5fe62653995e..f5ba97c46160 100644 --- a/drivers/mtd/ubi/ubi.h +++ b/drivers/mtd/ubi/ubi.h @@ -334,6 +334,9 @@ struct ubi_eba_leb_desc { * @changing_leb: %1 if the atomic LEB change ioctl command is in progress * @direct_writes: %1 if direct writes are enabled for this volume * + * @checkmap: bitmap to remember which PEB->LEB mappings got checked, + * protected by UBI LEB lock tree. + * * The @corrupted field indicates that the volume's contents is corrupted. * Since UBI protects only static volumes, this field is not relevant to * dynamic volumes - it is user's responsibility to assure their data @@ -377,6 +380,10 @@ struct ubi_volume { unsigned int updating:1; unsigned int changing_leb:1; unsigned int direct_writes:1; + +#ifdef CONFIG_MTD_UBI_FASTMAP + unsigned long *checkmap; +#endif };
/** @@ -965,8 +972,12 @@ size_t ubi_calc_fm_size(struct ubi_device *ubi); int ubi_update_fastmap(struct ubi_device *ubi); int ubi_scan_fastmap(struct ubi_device *ubi, struct ubi_attach_info *ai, struct ubi_attach_info *scan_ai); +int ubi_fastmap_init_checkmap(struct ubi_volume *vol, int leb_count); +void ubi_fastmap_destroy_checkmap(struct ubi_volume *vol); #else static inline int ubi_update_fastmap(struct ubi_device *ubi) { return 0; } +int static inline ubi_fastmap_init_checkmap(struct ubi_volume *vol, int leb_count) { return 0; } +static inline void ubi_fastmap_destroy_checkmap(struct ubi_volume *vol) {} #endif
/* block.c */ diff --git a/drivers/mtd/ubi/vmt.c b/drivers/mtd/ubi/vmt.c index 3fd8d7ff7a02..0be516780e92 100644 --- a/drivers/mtd/ubi/vmt.c +++ b/drivers/mtd/ubi/vmt.c @@ -139,6 +139,7 @@ static void vol_release(struct device *dev) struct ubi_volume *vol = container_of(dev, struct ubi_volume, dev);
ubi_eba_replace_table(vol, NULL); + ubi_fastmap_destroy_checkmap(vol); kfree(vol); }
diff --git a/drivers/mtd/ubi/vtbl.c b/drivers/mtd/ubi/vtbl.c index 263743e7b741..94d7a865b135 100644 --- a/drivers/mtd/ubi/vtbl.c +++ b/drivers/mtd/ubi/vtbl.c @@ -534,7 +534,7 @@ static int init_volumes(struct ubi_device *ubi, const struct ubi_attach_info *ai, const struct ubi_vtbl_record *vtbl) { - int i, reserved_pebs = 0; + int i, err, reserved_pebs = 0; struct ubi_ainf_volume *av; struct ubi_volume *vol;
@@ -620,6 +620,16 @@ static int init_volumes(struct ubi_device *ubi, (long long)(vol->used_ebs - 1) * vol->usable_leb_size; vol->used_bytes += av->last_data_size; vol->last_eb_bytes = av->last_data_size; + + /* + * We use ubi->peb_count and not vol->reserved_pebs because + * we want to keep the code simple. Otherwise we'd have to + * resize/check the bitmap upon volume resize too. + * Allocating a few bytes more does not hurt. + */ + err = ubi_fastmap_init_checkmap(vol, ubi->peb_count); + if (err) + return err; }
/* And add the layout volume */ @@ -645,6 +655,9 @@ static int init_volumes(struct ubi_device *ubi, reserved_pebs += vol->reserved_pebs; ubi->vol_count += 1; vol->ubi = ubi; + err = ubi_fastmap_init_checkmap(vol, UBI_LAYOUT_VOLUME_EBS); + if (err) + return err;
if (reserved_pebs > ubi->avail_pebs) { ubi_err(ubi, "not enough PEBs, required %d, available %d", @@ -849,6 +862,7 @@ int ubi_read_volume_table(struct ubi_device *ubi, struct ubi_attach_info *ai) out_free: vfree(ubi->vtbl); for (i = 0; i < ubi->vtbl_slots + UBI_INT_VOL_COUNT; i++) { + ubi_fastmap_destroy_checkmap(ubi->volumes[i]); kfree(ubi->volumes[i]); ubi->volumes[i] = NULL; }
On Mon, Nov 26, 2018 at 11:38:42AM +0100, Martin Kepplinger wrote:
From: Richard Weinberger richard@nod.at
[ Upstream commit 34653fd8c46e771585fce5975e4243f8fd401914 ]
This commit got merged along with commit 781932375ffc ("ubi: fastmap: Correctly handle interrupted erasures in EBA") upstream but only the latter has been applied to stable v4.14.54 as commit a23cf10d9abb. This resulted in a performance regression. Startup on i.MX platforms is delayed for up to a few seconds depending on the platform. This fixes ubi fastmap to be of the same performance as it has been before said fastmap changes.
Fixes: a23cf10d9abb ("ubi: fastmap: Correctly handle interrupted erasures in EBA") Signed-off-by: Richard Weinberger richard@nod.at Signed-off-by: Martin Kepplinger martin.kepplinger@ginzinger.com
Richard, although this fixes a major slowdown regression in -stable, do you consider this "stable" too?
This applies and is tested only for the 4.14 stable tree. It seems to be equally relevant for 4.9 and 4.4 though.
Now queued up for 4.14.y, thanks.
greg k-h
Greg,
Am Donnerstag, 29. November 2018, 09:09:23 CET schrieb Greg KH:
On Mon, Nov 26, 2018 at 11:38:42AM +0100, Martin Kepplinger wrote:
From: Richard Weinberger richard@nod.at
[ Upstream commit 34653fd8c46e771585fce5975e4243f8fd401914 ]
This commit got merged along with commit 781932375ffc ("ubi: fastmap: Correctly handle interrupted erasures in EBA") upstream but only the latter has been applied to stable v4.14.54 as commit a23cf10d9abb. This resulted in a performance regression. Startup on i.MX platforms is delayed for up to a few seconds depending on the platform. This fixes ubi fastmap to be of the same performance as it has been before said fastmap changes.
Fixes: a23cf10d9abb ("ubi: fastmap: Correctly handle interrupted erasures in EBA") Signed-off-by: Richard Weinberger richard@nod.at Signed-off-by: Martin Kepplinger martin.kepplinger@ginzinger.com
Richard, although this fixes a major slowdown regression in -stable, do you consider this "stable" too?
This applies and is tested only for the 4.14 stable tree. It seems to be equally relevant for 4.9 and 4.4 though.
Now queued up for 4.14.y, thanks.
can you *please* slow a little down?
There are times (e.g. when I travel, visit customers on-site, being sick, etc...) where I don't have the resources to monitor the mailinglists in detail. Adding patches to stable on shout asks for trouble.
As Sudip points out, this patch needs a further fix patch: 25677478474a ("ubi: Initialize Fastmap checkmapping correctly")
Thanks, //richard
Hi Greg,
On Sun, Dec 2, 2018 at 8:51 AM Richard Weinberger richard@nod.at wrote:
Greg,
Am Donnerstag, 29. November 2018, 09:09:23 CET schrieb Greg KH:
On Mon, Nov 26, 2018 at 11:38:42AM +0100, Martin Kepplinger wrote:
From: Richard Weinberger richard@nod.at
[ Upstream commit 34653fd8c46e771585fce5975e4243f8fd401914 ]
<snip>
Now queued up for 4.14.y, thanks.
can you *please* slow a little down?
True. It will really help if you can have some sort of fixed schedule for stable release, like maybe stablerc is ready on Thursday or Friday and release the stable on Monday. Having a weekend in stablerc will be helpful for people like me who only get the time in weekends for upstream or stable kernel.
On Sun, Dec 02, 2018 at 11:50:33AM +0000, Sudip Mukherjee wrote:
Hi Greg,
On Sun, Dec 2, 2018 at 8:51 AM Richard Weinberger richard@nod.at wrote:
Greg,
Am Donnerstag, 29. November 2018, 09:09:23 CET schrieb Greg KH:
On Mon, Nov 26, 2018 at 11:38:42AM +0100, Martin Kepplinger wrote:
From: Richard Weinberger richard@nod.at
[ Upstream commit 34653fd8c46e771585fce5975e4243f8fd401914 ]
<snip> > > > > Now queued up for 4.14.y, thanks. > > can you *please* slow a little down?
True. It will really help if you can have some sort of fixed schedule for stable release, like maybe stablerc is ready on Thursday or Friday and release the stable on Monday. Having a weekend in stablerc will be helpful for people like me who only get the time in weekends for upstream or stable kernel.
Any sort of schedule will never work for everyone (for example, if it's part of your paid job - you don't necessarily want to review stuff over the weekend).
-- Thanks, Sasha
Sasha,
Am Sonntag, 2. Dezember 2018, 15:35:43 CET schrieb Sasha Levin:
On Sun, Dec 02, 2018 at 11:50:33AM +0000, Sudip Mukherjee wrote:
Now queued up for 4.14.y, thanks.
can you *please* slow a little down?
True. It will really help if you can have some sort of fixed schedule for stable release, like maybe stablerc is ready on Thursday or Friday and release the stable on Monday. Having a weekend in stablerc will be helpful for people like me who only get the time in weekends for upstream or stable kernel.
Any sort of schedule will never work for everyone (for example, if it's part of your paid job - you don't necessarily want to review stuff over the weekend).
a schedule is not needed, but please give maintainers at least a chance to react on stable inclusion request. In this case Martin asked for inclusion on Monday and the patch was applied two days later.
As you noted not everyone works full time on the kernel and gets paid for that. So it might take a few days to react and review such a request.
Thanks, //richard
On Sun, Dec 02, 2018 at 04:02:47PM +0100, Richard Weinberger wrote:
Sasha,
Am Sonntag, 2. Dezember 2018, 15:35:43 CET schrieb Sasha Levin:
On Sun, Dec 02, 2018 at 11:50:33AM +0000, Sudip Mukherjee wrote:
Now queued up for 4.14.y, thanks.
can you *please* slow a little down?
True. It will really help if you can have some sort of fixed schedule for stable release, like maybe stablerc is ready on Thursday or Friday and release the stable on Monday. Having a weekend in stablerc will be helpful for people like me who only get the time in weekends for upstream or stable kernel.
Any sort of schedule will never work for everyone (for example, if it's part of your paid job - you don't necessarily want to review stuff over the weekend).
a schedule is not needed, but please give maintainers at least a chance to react on stable inclusion request. In this case Martin asked for inclusion on Monday and the patch was applied two days later.
As you noted not everyone works full time on the kernel and gets paid for that. So it might take a few days to react and review such a request.
Yes, that's fair, I wasn't arguing against that.
-- Thanks, Sasha
On 02.12.18 16:02, Richard Weinberger wrote:
Sasha,
Am Sonntag, 2. Dezember 2018, 15:35:43 CET schrieb Sasha Levin:
On Sun, Dec 02, 2018 at 11:50:33AM +0000, Sudip Mukherjee wrote:
Now queued up for 4.14.y, thanks.
can you *please* slow a little down?
True. It will really help if you can have some sort of fixed schedule for stable release, like maybe stablerc is ready on Thursday or Friday and release the stable on Monday. Having a weekend in stablerc will be helpful for people like me who only get the time in weekends for upstream or stable kernel.
Any sort of schedule will never work for everyone (for example, if it's part of your paid job - you don't necessarily want to review stuff over the weekend).
a schedule is not needed, but please give maintainers at least a chance to react on stable inclusion request. In this case Martin asked for inclusion on Monday and the patch was applied two days later.
True, especially when the maintainer is asked a question as part of the patch.
I've already had the feeling that we'd need the other patch too, but in this case at least I should have searched for Fixes tags.
Greg, how about reminding people of Fixes tags in Documentation/process/stable-kernel-rules.rst ?
martin
On Tue, Dec 04, 2018 at 08:39:16AM +0100, Martin Kepplinger wrote:
On 02.12.18 16:02, Richard Weinberger wrote:
Sasha,
Am Sonntag, 2. Dezember 2018, 15:35:43 CET schrieb Sasha Levin:
On Sun, Dec 02, 2018 at 11:50:33AM +0000, Sudip Mukherjee wrote:
Now queued up for 4.14.y, thanks.
can you *please* slow a little down?
True. It will really help if you can have some sort of fixed schedule for stable release, like maybe stablerc is ready on Thursday or Friday and release the stable on Monday. Having a weekend in stablerc will be helpful for people like me who only get the time in weekends for upstream or stable kernel.
Any sort of schedule will never work for everyone (for example, if it's part of your paid job - you don't necessarily want to review stuff over the weekend).
a schedule is not needed, but please give maintainers at least a chance to react on stable inclusion request. In this case Martin asked for inclusion on Monday and the patch was applied two days later.
True, especially when the maintainer is asked a question as part of the patch.
I've already had the feeling that we'd need the other patch too, but in this case at least I should have searched for Fixes tags.
Greg, how about reminding people of Fixes tags in Documentation/process/stable-kernel-rules.rst ?
Reminding people how? Patches to that file are always gladly accepted :)
thanks,
greg k-h
On Sun, Dec 2, 2018 at 2:35 PM Sasha Levin sashal@kernel.org wrote:
On Sun, Dec 02, 2018 at 11:50:33AM +0000, Sudip Mukherjee wrote:
Hi Greg,
On Sun, Dec 2, 2018 at 8:51 AM Richard Weinberger richard@nod.at wrote:
Greg,
Am Donnerstag, 29. November 2018, 09:09:23 CET schrieb Greg KH:
On Mon, Nov 26, 2018 at 11:38:42AM +0100, Martin Kepplinger wrote:
From: Richard Weinberger richard@nod.at
[ Upstream commit 34653fd8c46e771585fce5975e4243f8fd401914 ]
<snip> > > > > Now queued up for 4.14.y, thanks. > > can you *please* slow a little down?
True. It will really help if you can have some sort of fixed schedule for stable release, like maybe stablerc is ready on Thursday or Friday and release the stable on Monday. Having a weekend in stablerc will be helpful for people like me who only get the time in weekends for upstream or stable kernel.
Any sort of schedule will never work for everyone (for example, if it's part of your paid job - you don't necessarily want to review stuff over the weekend).
Yes, exactly. But like I said if the stablerc tree is pushed on Thursday, and the stable release is released on Monday then both types of people will get time, and hopefully everyone is happy. :)
On Sun, Dec 02, 2018 at 09:51:34AM +0100, Richard Weinberger wrote:
Greg,
Am Donnerstag, 29. November 2018, 09:09:23 CET schrieb Greg KH:
On Mon, Nov 26, 2018 at 11:38:42AM +0100, Martin Kepplinger wrote:
From: Richard Weinberger richard@nod.at
[ Upstream commit 34653fd8c46e771585fce5975e4243f8fd401914 ]
This commit got merged along with commit 781932375ffc ("ubi: fastmap: Correctly handle interrupted erasures in EBA") upstream but only the latter has been applied to stable v4.14.54 as commit a23cf10d9abb. This resulted in a performance regression. Startup on i.MX platforms is delayed for up to a few seconds depending on the platform. This fixes ubi fastmap to be of the same performance as it has been before said fastmap changes.
Fixes: a23cf10d9abb ("ubi: fastmap: Correctly handle interrupted erasures in EBA") Signed-off-by: Richard Weinberger richard@nod.at Signed-off-by: Martin Kepplinger martin.kepplinger@ginzinger.com
Richard, although this fixes a major slowdown regression in -stable, do you consider this "stable" too?
This applies and is tested only for the 4.14 stable tree. It seems to be equally relevant for 4.9 and 4.4 though.
Now queued up for 4.14.y, thanks.
can you *please* slow a little down?
There are times (e.g. when I travel, visit customers on-site, being sick, etc...) where I don't have the resources to monitor the mailinglists in detail. Adding patches to stable on shout asks for trouble.
As Sudip points out, this patch needs a further fix patch: 25677478474a ("ubi: Initialize Fastmap checkmapping correctly")
This is now in 4.14.86 so all should be fine now.
As for "speed", most of the time people are complaining that I move too slow in getting fixes backported and to their patches. Rarely am I told I am moving too fast, that's a nice change :)
As for doing releases on a "regular" schedule, I've tried it, and it didn't work any better/worse than what I'm doing now as everyone who consumes these kernels have their own cadence / acceptance process and I can never get in sync with _everyone_ let alone almost _anyone_.
And due to travel and other things (like security issues coming up), trying to nail down a specific day-of-the-week doesn't work out at all either.
thanks,
greg k-h
linux-stable-mirror@lists.linaro.org
-
Greg KH -
Greg Kroah-Hartman -
Martin Kepplinger -
Richard Weinberger -
Sasha Levin -
Sudip Mukherjee