linux-kernel - Re: [PATCH] ubi: fastmap: Reserve PEBs and init fm

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: <cc9f9239-22b8-a00b-c46f-1b9ea8cd7395@huawei.com>
Date:   Wed, 12 Apr 2023 09:53:21 +0800
From:   Zhihao Cheng <chengzhihao1@...wei.com>
To:     Xiaobing Luo <luoxiaobing0926@...il.com>, <richard@....at>,
        <miquel.raynal@...tlin.com>, <vigneshr@...com>,
        <mcoquelin.stm32@...il.com>, <alexandre.torgue@...s.st.com>
CC:     <linux-mtd@...ts.infradead.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-stm32@...md-mailman.stormreply.com>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] ubi: fastmap: Reserve PEBs and init fm_work when fastmap
 is used.

Hi Xiaobing
> Don't reserve the two fastmap PEBs when fastmap is disabled, then we can
> use the two PEBs in small ubi device.
> And don't init the fm_work when fastmap is disabled.
> 
> Signed-off-by: Xiaobing Luo <luoxiaobing0926@...il.com>
> ---
>   drivers/mtd/ubi/build.c      | 3 ++-
>   drivers/mtd/ubi/fastmap-wl.c | 2 +-
>   drivers/mtd/ubi/wl.c         | 6 +++++-
>   3 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
> index ad025b2ee417..a98a717b0e66 100644
> --- a/drivers/mtd/ubi/build.c
> +++ b/drivers/mtd/ubi/build.c
> @@ -1120,7 +1120,8 @@ int ubi_detach_mtd_dev(int ubi_num, int anyway)
>   		kthread_stop(ubi->bgt_thread);
>   
>   #ifdef CONFIG_MTD_UBI_FASTMAP
> -	cancel_work_sync(&ubi->fm_work);
> +	if (!ubi->fm_disabled)
> +		cancel_work_sync(&ubi->fm_work);
>   #endif
>   	ubi_debugfs_exit_dev(ubi);
>   	uif_close(ubi);
> diff --git a/drivers/mtd/ubi/fastmap-wl.c b/drivers/mtd/ubi/fastmap-wl.c
> index 863f571f1adb..b3df17a782c7 100644
> --- a/drivers/mtd/ubi/fastmap-wl.c
> +++ b/drivers/mtd/ubi/fastmap-wl.c
> @@ -344,7 +344,7 @@ static struct ubi_wl_entry *get_peb_for_wl(struct ubi_device *ubi)
>   		/* We cannot update the fastmap here because this
>   		 * function is called in atomic context.
>   		 * Let's fail here and refill/update it as soon as possible. */
> -		if (!ubi->fm_work_scheduled) {
> +		if (!ubi->fm_work_scheduled && !ubi->fm_disabled) {
>   			ubi->fm_work_scheduled = 1;
>   			schedule_work(&ubi->fm_work);
>   		}

I think we should keep the fm_work even fm_disabled is true. UBI is 
running with fastmap scheme as long as CONFIG_MTD_UBI_FASTMAP is 
enabled, 'ubi->fm_disabled' is just used to control whether to 
persistence fastmap metadata on flash. Free wear-leveling pebs 
consuming/producing model still depend on following process:
Consuming: get_peb_for_wl -> pool->pebs[pool->used++]
Producing: ubi->fm_work -> update_fastmap_work_fn -> ubi_refill_pools -> 
wl_pool->used = 0
If we disable 'ubi->fm_work' when ubi->fm_disabled becomes true, 
wear-leveing work will fail to get free wl peb until someone try to get 
free peb by ubi_wl_get_peb().

> diff --git a/drivers/mtd/ubi/wl.c b/drivers/mtd/ubi/wl.c
> index 26a214f016c1..8906db89808f 100644
> --- a/drivers/mtd/ubi/wl.c
> +++ b/drivers/mtd/ubi/wl.c
> @@ -1908,7 +1908,11 @@ int ubi_wl_init(struct ubi_device *ubi, struct ubi_attach_info *ai)
>   	ubi_assert(ubi->good_peb_count == found_pebs);
>   
>   	reserved_pebs = WL_RESERVED_PEBS;
> -	ubi_fastmap_init(ubi, &reserved_pebs);
> +
> +#ifdef CONFIG_MTD_UBI_FASTMAP
> +	if (!ubi->fm_disabled)
> +		ubi_fastmap_init(ubi, &reserved_pebs);
> +#endif
>   
>   	if (ubi->avail_pebs < reserved_pebs) {
>   		ubi_err(ubi, "no enough physical eraseblocks (%d, need %d)",
>