All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marek Szyprowski <m.szyprowski@samsung.com>
To: Sean Nyekjaer <sean@geanix.com>,
	Boris Brezillon <boris.brezillon@collabora.com>
Cc: Miquel Raynal <miquel.raynal@bootlin.com>,
	Richard Weinberger <richard@nod.at>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Boris Brezillon <bbrezillon@kernel.org>,
	linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5 3/4] mtd: core: protect access to MTD devices while in suspend
Date: Tue, 23 Nov 2021 13:03:52 +0100	[thread overview]
Message-ID: <21092c0f-e11b-ac16-ab96-2a0556c0e30a@samsung.com> (raw)
In-Reply-To: <20211102110204.3334609-4-sean@geanix.com>

Hi,

On 02.11.2021 12:02, Sean Nyekjaer wrote:
> Prevent MTD access while in a suspended state. Also
> prevent suspending a device which is still currently in use.
>
> Commit 013e6292aaf5 ("mtd: rawnand: Simplify the locking") allows the
> rawnand layer to return errors rather than waiting in a blocking wait.
>
> Tested on a iMX6ULL.
>
> Suggested-by: Boris Brezillon <boris.brezillon@collabora.com>
> Signed-off-by: Sean Nyekjaer <sean@geanix.com>

This patch landed recently in linux-next as commit 9d6abd489e70 ("mtd: 
core: protect access to MTD devices while in suspend"). I found that it 
triggers the following warning on my test systems:

INFO: trying to register non-static key.
The code is fine but needs lockdep annotation, or maybe
you didn't initialize this object before use?
turning off the locking correctness validator.
CPU: 1 PID: 1606 Comm: reboot Not tainted 5.16.0-rc1+ #4165
Hardware name: linux,dummy-virt (DT)
Call trace:
  dump_backtrace+0x0/0x1ac
  show_stack+0x18/0x24
  dump_stack_lvl+0x8c/0xb8
  dump_stack+0x18/0x34
  register_lock_class+0x4a0/0x4cc
  __lock_acquire+0x78/0x20cc
  lock_acquire.part.0+0xe0/0x230
  lock_acquire+0x68/0x84
  down_write+0x64/0xe0
  physmap_flash_shutdown+0x60/0x140
  platform_shutdown+0x28/0x40
  device_shutdown+0x160/0x340
  __do_sys_reboot+0x1f8/0x2a0
  __arm64_sys_reboot+0x28/0x34
  invoke_syscall+0x48/0x114
  el0_svc_common.constprop.0+0x60/0x11c
  do_el0_svc_compat+0x1c/0x50
  el0_svc_compat+0x4c/0x100
  el0t_32_sync_handler+0x90/0x140
  el0t_32_sync+0x1a4/0x1a8
Flash device refused suspend due to active operation (state 20)
------------[ cut here ]------------
DEBUG_RWSEMS_WARN_ON(sem->magic != sem): count = 0x1, magic = 0x0, owner 
= 0xffff588b4547c740, curr 0xffff588b4547c740, list not empty
WARNING: CPU: 1 PID: 1606 at kernel/locking/rwsem.c:1322 
up_write+0x144/0x1a0
Modules linked in: bluetooth ecdh_generic ecc rfkill ipv6
CPU: 1 PID: 1606 Comm: reboot Not tainted 5.16.0-rc1+ #4165
Hardware name: linux,dummy-virt (DT)
pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : up_write+0x144/0x1a0
lr : up_write+0x144/0x1a0
sp : ffff8000109ebbd0
x29: ffff8000109ebbd0 x28: ffff588b4547c740 x27: 0000000000000000
x26: ffffce0238d56470 x25: 0000000000000008 x24: ffffce0239bba030
x23: ffff588b451938b0 x22: 0000000000000000 x21: ffff588b44046c80
x20: ffffce02397a2000 x19: ffff588b451938b0 x18: 0000000000000030
x17: 0000000000000000 x16: 0000000000000000 x15: ffffffffffffffff
x14: 0000000000000005 x13: ffffce02397c5198 x12: 0000000000000390
x11: 0000000000000130 x10: ffffce023981d198 x9 : 00000000fffff000
x8 : ffffce02397c5198 x7 : ffffce023981d198 x6 : 0000000000000000
x5 : 000000000000bff4 x4 : 0000000000000000 x3 : 0000000000000000
x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff588b4547c740
Call trace:
  up_write+0x144/0x1a0
  physmap_flash_shutdown+0x13c/0x140
  platform_shutdown+0x28/0x40
  device_shutdown+0x160/0x340
  __do_sys_reboot+0x1f8/0x2a0
  __arm64_sys_reboot+0x28/0x34
  invoke_syscall+0x48/0x114
  el0_svc_common.constprop.0+0x60/0x11c
  do_el0_svc_compat+0x1c/0x50
  el0_svc_compat+0x4c/0x100
  el0t_32_sync_handler+0x90/0x140
  el0t_32_sync+0x1a4/0x1a8
irq event stamp: 2541
hardirqs last  enabled at (2541): [<ffffce02382d94c8>] 
_raw_spin_unlock_irq+0x44/0x90
hardirqs last disabled at (2540): [<ffffce02382d98cc>] 
_raw_spin_lock_irq+0xac/0xb0
softirqs last  enabled at (2278): [<ffffce0237210470>] _stext+0x470/0x5e8
softirqs last disabled at (2273): [<ffffce023729d904>] 
__irq_exit_rcu+0x180/0x1ac
---[ end trace d06160a193b668c2 ]---
Flash device refused suspend due to active operation (state 20)


Reverting $subject patch on top of linux-next hides the warning. The 
above log has been gathered on QEMU/arm64 'virt' machine, during the 
reboot operation. If you need more information how to reproduce it, let 
me know.


> ---
>   drivers/mtd/mtdcore.c   | 115 +++++++++++++++++++++++++++++++++++-----
>   include/linux/mtd/mtd.h |  81 +++++++++++++++++++++++-----
>   2 files changed, 169 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index 153229198947..f02b602b3fa9 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -777,6 +777,8 @@ static void mtd_set_dev_defaults(struct mtd_info *mtd)
>   	INIT_LIST_HEAD(&mtd->partitions);
>   	mutex_init(&mtd->master.partitions_lock);
>   	mutex_init(&mtd->master.chrdev_lock);
> +	init_waitqueue_head(&mtd->master.resume_wq);
> +	init_rwsem(&mtd->master.suspend_lock);
>   }
>   
>   static ssize_t mtd_otp_size(struct mtd_info *mtd, bool is_user)
> @@ -1267,7 +1269,9 @@ int mtd_erase(struct mtd_info *mtd, struct erase_info *instr)
>   
>   	adjinstr.addr += mst_ofs;
>   
> +	mtd_start_access(master);
>   	ret = master->_erase(master, &adjinstr);
> +	mtd_end_access(master);
>   
>   	if (adjinstr.fail_addr != MTD_FAIL_ADDR_UNKNOWN) {
>   		instr->fail_addr = adjinstr.fail_addr - mst_ofs;
> @@ -1289,6 +1293,7 @@ int mtd_point(struct mtd_info *mtd, loff_t from, size_t len, size_t *retlen,
>   	      void **virt, resource_size_t *phys)
>   {
>   	struct mtd_info *master = mtd_get_master(mtd);
> +	int ret;
>   
>   	*retlen = 0;
>   	*virt = NULL;
> @@ -1301,8 +1306,12 @@ int mtd_point(struct mtd_info *mtd, loff_t from, size_t len, size_t *retlen,
>   	if (!len)
>   		return 0;
>   
> +	mtd_start_access(master);
>   	from = mtd_get_master_ofs(mtd, from);
> -	return master->_point(master, from, len, retlen, virt, phys);
> +	ret = master->_point(master, from, len, retlen, virt, phys);
> +	mtd_end_access(master);
> +
> +	return ret;
>   }
>   EXPORT_SYMBOL_GPL(mtd_point);
>   
> @@ -1310,6 +1319,7 @@ EXPORT_SYMBOL_GPL(mtd_point);
>   int mtd_unpoint(struct mtd_info *mtd, loff_t from, size_t len)
>   {
>   	struct mtd_info *master = mtd_get_master(mtd);
> +	int ret;
>   
>   	if (!master->_unpoint)
>   		return -EOPNOTSUPP;
> @@ -1317,7 +1327,12 @@ int mtd_unpoint(struct mtd_info *mtd, loff_t from, size_t len)
>   		return -EINVAL;
>   	if (!len)
>   		return 0;
> -	return master->_unpoint(master, mtd_get_master_ofs(mtd, from), len);
> +
> +	mtd_start_access(master);
> +	ret =  master->_unpoint(master, mtd_get_master_ofs(mtd, from), len);
> +	mtd_end_access(master);
> +
> +	return ret;
>   }
>   EXPORT_SYMBOL_GPL(mtd_unpoint);
>   
> @@ -1372,6 +1387,7 @@ int mtd_read(struct mtd_info *mtd, loff_t from, size_t len, size_t *retlen,
>   	};
>   	int ret;
>   
> +	/* mtd_read_oob_std handles mtd access protection */
>   	ret = mtd_read_oob(mtd, from, &ops);
>   	*retlen = ops.retlen;
>   
> @@ -1388,6 +1404,7 @@ int mtd_write(struct mtd_info *mtd, loff_t to, size_t len, size_t *retlen,
>   	};
>   	int ret;
>   
> +	/* mtd_write_oob_std handles mtd access protection */
>   	ret = mtd_write_oob(mtd, to, &ops);
>   	*retlen = ops.retlen;
>   
> @@ -1464,11 +1481,13 @@ static int mtd_read_oob_std(struct mtd_info *mtd, loff_t from,
>   	int ret;
>   
>   	from = mtd_get_master_ofs(mtd, from);
> +	mtd_start_access(master);
>   	if (master->_read_oob)
>   		ret = master->_read_oob(master, from, ops);
>   	else
>   		ret = master->_read(master, from, ops->len, &ops->retlen,
>   				    ops->datbuf);
> +	mtd_end_access(master);
>   
>   	return ret;
>   }
> @@ -1480,11 +1499,13 @@ static int mtd_write_oob_std(struct mtd_info *mtd, loff_t to,
>   	int ret;
>   
>   	to = mtd_get_master_ofs(mtd, to);
> +	mtd_start_access(master);
>   	if (master->_write_oob)
>   		ret = master->_write_oob(master, to, ops);
>   	else
>   		ret = master->_write(master, to, ops->len, &ops->retlen,
>   				     ops->datbuf);
> +	mtd_end_access(master);
>   
>   	return ret;
>   }
> @@ -1992,12 +2013,18 @@ int mtd_get_fact_prot_info(struct mtd_info *mtd, size_t len, size_t *retlen,
>   			   struct otp_info *buf)
>   {
>   	struct mtd_info *master = mtd_get_master(mtd);
> +	int ret;
>   
>   	if (!master->_get_fact_prot_info)
>   		return -EOPNOTSUPP;
>   	if (!len)
>   		return 0;
> -	return master->_get_fact_prot_info(master, len, retlen, buf);
> +
> +	mtd_start_access(master);
> +	ret = master->_get_fact_prot_info(master, len, retlen, buf);
> +	mtd_end_access(master);
> +
> +	return ret;
>   }
>   EXPORT_SYMBOL_GPL(mtd_get_fact_prot_info);
>   
> @@ -2005,13 +2032,19 @@ int mtd_read_fact_prot_reg(struct mtd_info *mtd, loff_t from, size_t len,
>   			   size_t *retlen, u_char *buf)
>   {
>   	struct mtd_info *master = mtd_get_master(mtd);
> +	int ret;
>   
>   	*retlen = 0;
>   	if (!master->_read_fact_prot_reg)
>   		return -EOPNOTSUPP;
>   	if (!len)
>   		return 0;
> -	return master->_read_fact_prot_reg(master, from, len, retlen, buf);
> +
> +	mtd_start_access(master);
> +	ret = master->_read_fact_prot_reg(master, from, len, retlen, buf);
> +	mtd_end_access(master);
> +
> +	return ret;
>   }
>   EXPORT_SYMBOL_GPL(mtd_read_fact_prot_reg);
>   
> @@ -2019,12 +2052,18 @@ int mtd_get_user_prot_info(struct mtd_info *mtd, size_t len, size_t *retlen,
>   			   struct otp_info *buf)
>   {
>   	struct mtd_info *master = mtd_get_master(mtd);
> +	int ret;
>   
>   	if (!master->_get_user_prot_info)
>   		return -EOPNOTSUPP;
>   	if (!len)
>   		return 0;
> -	return master->_get_user_prot_info(master, len, retlen, buf);
> +
> +	mtd_start_access(master);
> +	ret =  master->_get_user_prot_info(master, len, retlen, buf);
> +	mtd_end_access(master);
> +
> +	return ret;
>   }
>   EXPORT_SYMBOL_GPL(mtd_get_user_prot_info);
>   
> @@ -2032,13 +2071,19 @@ int mtd_read_user_prot_reg(struct mtd_info *mtd, loff_t from, size_t len,
>   			   size_t *retlen, u_char *buf)
>   {
>   	struct mtd_info *master = mtd_get_master(mtd);
> +	int ret;
>   
>   	*retlen = 0;
>   	if (!master->_read_user_prot_reg)
>   		return -EOPNOTSUPP;
>   	if (!len)
>   		return 0;
> -	return master->_read_user_prot_reg(master, from, len, retlen, buf);
> +
> +	mtd_start_access(master);
> +	ret = master->_read_user_prot_reg(master, from, len, retlen, buf);
> +	mtd_end_access(master);
> +
> +	return ret;
>   }
>   EXPORT_SYMBOL_GPL(mtd_read_user_prot_reg);
>   
> @@ -2053,7 +2098,11 @@ int mtd_write_user_prot_reg(struct mtd_info *mtd, loff_t to, size_t len,
>   		return -EOPNOTSUPP;
>   	if (!len)
>   		return 0;
> +
> +	mtd_start_access(master);
>   	ret = master->_write_user_prot_reg(master, to, len, retlen, buf);
> +	mtd_end_access(master);
> +
>   	if (ret)
>   		return ret;
>   
> @@ -2068,24 +2117,36 @@ EXPORT_SYMBOL_GPL(mtd_write_user_prot_reg);
>   int mtd_lock_user_prot_reg(struct mtd_info *mtd, loff_t from, size_t len)
>   {
>   	struct mtd_info *master = mtd_get_master(mtd);
> +	int ret;
>   
>   	if (!master->_lock_user_prot_reg)
>   		return -EOPNOTSUPP;
>   	if (!len)
>   		return 0;
> -	return master->_lock_user_prot_reg(master, from, len);
> +
> +	mtd_start_access(master);
> +	ret = master->_lock_user_prot_reg(master, from, len);
> +	mtd_end_access(master);
> +
> +	return ret;
>   }
>   EXPORT_SYMBOL_GPL(mtd_lock_user_prot_reg);
>   
>   int mtd_erase_user_prot_reg(struct mtd_info *mtd, loff_t from, size_t len)
>   {
>   	struct mtd_info *master = mtd_get_master(mtd);
> +	int ret;
>   
>   	if (!master->_erase_user_prot_reg)
>   		return -EOPNOTSUPP;
>   	if (!len)
>   		return 0;
> -	return master->_erase_user_prot_reg(master, from, len);
> +
> +	mtd_start_access(master);
> +	ret = master->_erase_user_prot_reg(master, from, len);
> +	mtd_end_access(master);
> +
> +	return ret;
>   }
>   EXPORT_SYMBOL_GPL(mtd_erase_user_prot_reg);
>   
> @@ -2093,6 +2154,7 @@ EXPORT_SYMBOL_GPL(mtd_erase_user_prot_reg);
>   int mtd_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
>   {
>   	struct mtd_info *master = mtd_get_master(mtd);
> +	int ret;
>   
>   	if (!master->_lock)
>   		return -EOPNOTSUPP;
> @@ -2106,13 +2168,18 @@ int mtd_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
>   		len = (u64)mtd_div_by_eb(len, mtd) * master->erasesize;
>   	}
>   
> -	return master->_lock(master, mtd_get_master_ofs(mtd, ofs), len);
> +	mtd_start_access(master);
> +	ret = master->_lock(master, mtd_get_master_ofs(mtd, ofs), len);
> +	mtd_end_access(master);
> +
> +	return ret;
>   }
>   EXPORT_SYMBOL_GPL(mtd_lock);
>   
>   int mtd_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
>   {
>   	struct mtd_info *master = mtd_get_master(mtd);
> +	int ret;
>   
>   	if (!master->_unlock)
>   		return -EOPNOTSUPP;
> @@ -2126,13 +2193,18 @@ int mtd_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
>   		len = (u64)mtd_div_by_eb(len, mtd) * master->erasesize;
>   	}
>   
> -	return master->_unlock(master, mtd_get_master_ofs(mtd, ofs), len);
> +	mtd_start_access(master);
> +	ret = master->_unlock(master, mtd_get_master_ofs(mtd, ofs), len);
> +	mtd_end_access(master);
> +
> +	return ret;
>   }
>   EXPORT_SYMBOL_GPL(mtd_unlock);
>   
>   int mtd_is_locked(struct mtd_info *mtd, loff_t ofs, uint64_t len)
>   {
>   	struct mtd_info *master = mtd_get_master(mtd);
> +	int ret;
>   
>   	if (!master->_is_locked)
>   		return -EOPNOTSUPP;
> @@ -2146,13 +2218,18 @@ int mtd_is_locked(struct mtd_info *mtd, loff_t ofs, uint64_t len)
>   		len = (u64)mtd_div_by_eb(len, mtd) * master->erasesize;
>   	}
>   
> -	return master->_is_locked(master, mtd_get_master_ofs(mtd, ofs), len);
> +	mtd_start_access(master);
> +	ret = master->_is_locked(master, mtd_get_master_ofs(mtd, ofs), len);
> +	mtd_end_access(master);
> +
> +	return ret;
>   }
>   EXPORT_SYMBOL_GPL(mtd_is_locked);
>   
>   int mtd_block_isreserved(struct mtd_info *mtd, loff_t ofs)
>   {
>   	struct mtd_info *master = mtd_get_master(mtd);
> +	int ret;
>   
>   	if (ofs < 0 || ofs >= mtd->size)
>   		return -EINVAL;
> @@ -2162,13 +2239,18 @@ int mtd_block_isreserved(struct mtd_info *mtd, loff_t ofs)
>   	if (mtd->flags & MTD_SLC_ON_MLC_EMULATION)
>   		ofs = (loff_t)mtd_div_by_eb(ofs, mtd) * master->erasesize;
>   
> -	return master->_block_isreserved(master, mtd_get_master_ofs(mtd, ofs));
> +	mtd_start_access(master);
> +	ret = master->_block_isreserved(master, mtd_get_master_ofs(mtd, ofs));
> +	mtd_end_access(master);
> +
> +	return ret;
>   }
>   EXPORT_SYMBOL_GPL(mtd_block_isreserved);
>   
>   int mtd_block_isbad(struct mtd_info *mtd, loff_t ofs)
>   {
>   	struct mtd_info *master = mtd_get_master(mtd);
> +	int ret;
>   
>   	if (ofs < 0 || ofs >= mtd->size)
>   		return -EINVAL;
> @@ -2178,7 +2260,11 @@ int mtd_block_isbad(struct mtd_info *mtd, loff_t ofs)
>   	if (mtd->flags & MTD_SLC_ON_MLC_EMULATION)
>   		ofs = (loff_t)mtd_div_by_eb(ofs, mtd) * master->erasesize;
>   
> -	return master->_block_isbad(master, mtd_get_master_ofs(mtd, ofs));
> +	mtd_start_access(master);
> +	ret = master->_block_isbad(master, mtd_get_master_ofs(mtd, ofs));
> +	mtd_end_access(master);
> +
> +	return ret;
>   }
>   EXPORT_SYMBOL_GPL(mtd_block_isbad);
>   
> @@ -2197,7 +2283,10 @@ int mtd_block_markbad(struct mtd_info *mtd, loff_t ofs)
>   	if (mtd->flags & MTD_SLC_ON_MLC_EMULATION)
>   		ofs = (loff_t)mtd_div_by_eb(ofs, mtd) * master->erasesize;
>   
> +	mtd_start_access(master);
>   	ret = master->_block_markbad(master, mtd_get_master_ofs(mtd, ofs));
> +	mtd_end_access(master);
> +
>   	if (ret)
>   		return ret;
>   
> diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
> index 88227044fc86..b074106e2d8e 100644
> --- a/include/linux/mtd/mtd.h
> +++ b/include/linux/mtd/mtd.h
> @@ -231,6 +231,8 @@ struct mtd_master {
>   	struct mutex partitions_lock;
>   	struct mutex chrdev_lock;
>   	unsigned int suspended : 1;
> +	wait_queue_head_t resume_wq;
> +	struct rw_semaphore suspend_lock;
>   };
>   
>   struct mtd_info {
> @@ -476,10 +478,47 @@ static inline u32 mtd_oobavail(struct mtd_info *mtd, struct mtd_oob_ops *ops)
>   	return ops->mode == MTD_OPS_AUTO_OOB ? mtd->oobavail : mtd->oobsize;
>   }
>   
> +static inline void mtd_start_access(struct mtd_info *master)
> +{
> +	WARN_ON_ONCE(master != mtd_get_master(master));
> +
> +	/*
> +	 * Don't take the suspend_lock on devices that don't
> +	 * implement the suspend hook. Otherwise, lockdep will
> +	 * complain about nested locks when trying to suspend MTD
> +	 * partitions or MTD devices created by gluebi which are
> +	 * backed by real devices.
> +	 */
> +	if (!master->_suspend)
> +		return;
> +
> +	/*
> +	 * Wait until the device is resumed. Should we have a
> +	 * non-blocking mode here?
> +	 */
> +	while (1) {
> +		down_read(&master->master.suspend_lock);
> +		if (!master->master.suspended)
> +			return;
> +
> +		up_read(&master->master.suspend_lock);
> +		wait_event(master->master.resume_wq, !master->master.suspended);
> +	}
> +}
> +
> +static inline void mtd_end_access(struct mtd_info *master)
> +{
> +	if (!master->_suspend)
> +		return;
> +
> +	up_read(&master->master.suspend_lock);
> +}
> +
>   static inline int mtd_max_bad_blocks(struct mtd_info *mtd,
>   				     loff_t ofs, size_t len)
>   {
>   	struct mtd_info *master = mtd_get_master(mtd);
> +	int ret;
>   
>   	if (!master->_max_bad_blocks)
>   		return -ENOTSUPP;
> @@ -487,8 +526,12 @@ static inline int mtd_max_bad_blocks(struct mtd_info *mtd,
>   	if (mtd->size < (len + ofs) || ofs < 0)
>   		return -EINVAL;
>   
> -	return master->_max_bad_blocks(master, mtd_get_master_ofs(mtd, ofs),
> -				       len);
> +	mtd_start_access(master);
> +	ret = master->_max_bad_blocks(master, mtd_get_master_ofs(mtd, ofs),
> +				      len);
> +	mtd_end_access(master);
> +
> +	return ret;
>   }
>   
>   int mtd_wunit_to_pairing_info(struct mtd_info *mtd, int wunit,
> @@ -546,30 +589,40 @@ int mtd_block_markbad(struct mtd_info *mtd, loff_t ofs);
>   static inline int mtd_suspend(struct mtd_info *mtd)
>   {
>   	struct mtd_info *master = mtd_get_master(mtd);
> -	int ret;
> -
> -	if (master->master.suspended)
> -		return 0;
> +	int ret = 0;
>   
> -	ret = master->_suspend ? master->_suspend(master) : 0;
> -	if (ret)
> +	if (!master->_suspend)
>   		return ret;
>   
> -	master->master.suspended = 1;
> -	return 0;
> +	down_write(&master->master.suspend_lock);
> +	if (!master->master.suspended) {
> +		ret = master->_suspend(master);
> +		if (!ret)
> +			master->master.suspended = 1;
> +	}
> +	up_write(&master->master.suspend_lock);
> +
> +	return ret;
>   }
>   
>   static inline void mtd_resume(struct mtd_info *mtd)
>   {
>   	struct mtd_info *master = mtd_get_master(mtd);
>   
> -	if (!master->master.suspended)
> +	if (!master->_suspend)
>   		return;
>   
> -	if (master->_resume)
> -		master->_resume(master);
> +	down_write(&master->master.suspend_lock);
> +	if (master->master.suspended) {
> +		if (master->_resume)
> +			master->_resume(master);
> +
> +		master->master.suspended = 0;
>   
> -	master->master.suspended = 0;
> +		/* The MTD dev has been resumed, wake up all waiters. */
> +		wake_up_all(&master->master.resume_wq);
> +	}
> +	up_write(&master->master.suspend_lock);
>   }
>   
>   static inline uint32_t mtd_div_by_eb(uint64_t sz, struct mtd_info *mtd)

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


WARNING: multiple messages have this Message-ID (diff)
From: Marek Szyprowski <m.szyprowski@samsung.com>
To: Sean Nyekjaer <sean@geanix.com>,
	Boris Brezillon <boris.brezillon@collabora.com>
Cc: Miquel Raynal <miquel.raynal@bootlin.com>,
	Richard Weinberger <richard@nod.at>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Boris Brezillon <bbrezillon@kernel.org>,
	linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5 3/4] mtd: core: protect access to MTD devices while in suspend
Date: Tue, 23 Nov 2021 13:03:52 +0100	[thread overview]
Message-ID: <21092c0f-e11b-ac16-ab96-2a0556c0e30a@samsung.com> (raw)
In-Reply-To: <20211102110204.3334609-4-sean@geanix.com>

Hi,

On 02.11.2021 12:02, Sean Nyekjaer wrote:
> Prevent MTD access while in a suspended state. Also
> prevent suspending a device which is still currently in use.
>
> Commit 013e6292aaf5 ("mtd: rawnand: Simplify the locking") allows the
> rawnand layer to return errors rather than waiting in a blocking wait.
>
> Tested on a iMX6ULL.
>
> Suggested-by: Boris Brezillon <boris.brezillon@collabora.com>
> Signed-off-by: Sean Nyekjaer <sean@geanix.com>

This patch landed recently in linux-next as commit 9d6abd489e70 ("mtd: 
core: protect access to MTD devices while in suspend"). I found that it 
triggers the following warning on my test systems:

INFO: trying to register non-static key.
The code is fine but needs lockdep annotation, or maybe
you didn't initialize this object before use?
turning off the locking correctness validator.
CPU: 1 PID: 1606 Comm: reboot Not tainted 5.16.0-rc1+ #4165
Hardware name: linux,dummy-virt (DT)
Call trace:
  dump_backtrace+0x0/0x1ac
  show_stack+0x18/0x24
  dump_stack_lvl+0x8c/0xb8
  dump_stack+0x18/0x34
  register_lock_class+0x4a0/0x4cc
  __lock_acquire+0x78/0x20cc
  lock_acquire.part.0+0xe0/0x230
  lock_acquire+0x68/0x84
  down_write+0x64/0xe0
  physmap_flash_shutdown+0x60/0x140
  platform_shutdown+0x28/0x40
  device_shutdown+0x160/0x340
  __do_sys_reboot+0x1f8/0x2a0
  __arm64_sys_reboot+0x28/0x34
  invoke_syscall+0x48/0x114
  el0_svc_common.constprop.0+0x60/0x11c
  do_el0_svc_compat+0x1c/0x50
  el0_svc_compat+0x4c/0x100
  el0t_32_sync_handler+0x90/0x140
  el0t_32_sync+0x1a4/0x1a8
Flash device refused suspend due to active operation (state 20)
------------[ cut here ]------------
DEBUG_RWSEMS_WARN_ON(sem->magic != sem): count = 0x1, magic = 0x0, owner 
= 0xffff588b4547c740, curr 0xffff588b4547c740, list not empty
WARNING: CPU: 1 PID: 1606 at kernel/locking/rwsem.c:1322 
up_write+0x144/0x1a0
Modules linked in: bluetooth ecdh_generic ecc rfkill ipv6
CPU: 1 PID: 1606 Comm: reboot Not tainted 5.16.0-rc1+ #4165
Hardware name: linux,dummy-virt (DT)
pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : up_write+0x144/0x1a0
lr : up_write+0x144/0x1a0
sp : ffff8000109ebbd0
x29: ffff8000109ebbd0 x28: ffff588b4547c740 x27: 0000000000000000
x26: ffffce0238d56470 x25: 0000000000000008 x24: ffffce0239bba030
x23: ffff588b451938b0 x22: 0000000000000000 x21: ffff588b44046c80
x20: ffffce02397a2000 x19: ffff588b451938b0 x18: 0000000000000030
x17: 0000000000000000 x16: 0000000000000000 x15: ffffffffffffffff
x14: 0000000000000005 x13: ffffce02397c5198 x12: 0000000000000390
x11: 0000000000000130 x10: ffffce023981d198 x9 : 00000000fffff000
x8 : ffffce02397c5198 x7 : ffffce023981d198 x6 : 0000000000000000
x5 : 000000000000bff4 x4 : 0000000000000000 x3 : 0000000000000000
x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff588b4547c740
Call trace:
  up_write+0x144/0x1a0
  physmap_flash_shutdown+0x13c/0x140
  platform_shutdown+0x28/0x40
  device_shutdown+0x160/0x340
  __do_sys_reboot+0x1f8/0x2a0
  __arm64_sys_reboot+0x28/0x34
  invoke_syscall+0x48/0x114
  el0_svc_common.constprop.0+0x60/0x11c
  do_el0_svc_compat+0x1c/0x50
  el0_svc_compat+0x4c/0x100
  el0t_32_sync_handler+0x90/0x140
  el0t_32_sync+0x1a4/0x1a8
irq event stamp: 2541
hardirqs last  enabled at (2541): [<ffffce02382d94c8>] 
_raw_spin_unlock_irq+0x44/0x90
hardirqs last disabled at (2540): [<ffffce02382d98cc>] 
_raw_spin_lock_irq+0xac/0xb0
softirqs last  enabled at (2278): [<ffffce0237210470>] _stext+0x470/0x5e8
softirqs last disabled at (2273): [<ffffce023729d904>] 
__irq_exit_rcu+0x180/0x1ac
---[ end trace d06160a193b668c2 ]---
Flash device refused suspend due to active operation (state 20)


Reverting $subject patch on top of linux-next hides the warning. The 
above log has been gathered on QEMU/arm64 'virt' machine, during the 
reboot operation. If you need more information how to reproduce it, let 
me know.


> ---
>   drivers/mtd/mtdcore.c   | 115 +++++++++++++++++++++++++++++++++++-----
>   include/linux/mtd/mtd.h |  81 +++++++++++++++++++++++-----
>   2 files changed, 169 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index 153229198947..f02b602b3fa9 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -777,6 +777,8 @@ static void mtd_set_dev_defaults(struct mtd_info *mtd)
>   	INIT_LIST_HEAD(&mtd->partitions);
>   	mutex_init(&mtd->master.partitions_lock);
>   	mutex_init(&mtd->master.chrdev_lock);
> +	init_waitqueue_head(&mtd->master.resume_wq);
> +	init_rwsem(&mtd->master.suspend_lock);
>   }
>   
>   static ssize_t mtd_otp_size(struct mtd_info *mtd, bool is_user)
> @@ -1267,7 +1269,9 @@ int mtd_erase(struct mtd_info *mtd, struct erase_info *instr)
>   
>   	adjinstr.addr += mst_ofs;
>   
> +	mtd_start_access(master);
>   	ret = master->_erase(master, &adjinstr);
> +	mtd_end_access(master);
>   
>   	if (adjinstr.fail_addr != MTD_FAIL_ADDR_UNKNOWN) {
>   		instr->fail_addr = adjinstr.fail_addr - mst_ofs;
> @@ -1289,6 +1293,7 @@ int mtd_point(struct mtd_info *mtd, loff_t from, size_t len, size_t *retlen,
>   	      void **virt, resource_size_t *phys)
>   {
>   	struct mtd_info *master = mtd_get_master(mtd);
> +	int ret;
>   
>   	*retlen = 0;
>   	*virt = NULL;
> @@ -1301,8 +1306,12 @@ int mtd_point(struct mtd_info *mtd, loff_t from, size_t len, size_t *retlen,
>   	if (!len)
>   		return 0;
>   
> +	mtd_start_access(master);
>   	from = mtd_get_master_ofs(mtd, from);
> -	return master->_point(master, from, len, retlen, virt, phys);
> +	ret = master->_point(master, from, len, retlen, virt, phys);
> +	mtd_end_access(master);
> +
> +	return ret;
>   }
>   EXPORT_SYMBOL_GPL(mtd_point);
>   
> @@ -1310,6 +1319,7 @@ EXPORT_SYMBOL_GPL(mtd_point);
>   int mtd_unpoint(struct mtd_info *mtd, loff_t from, size_t len)
>   {
>   	struct mtd_info *master = mtd_get_master(mtd);
> +	int ret;
>   
>   	if (!master->_unpoint)
>   		return -EOPNOTSUPP;
> @@ -1317,7 +1327,12 @@ int mtd_unpoint(struct mtd_info *mtd, loff_t from, size_t len)
>   		return -EINVAL;
>   	if (!len)
>   		return 0;
> -	return master->_unpoint(master, mtd_get_master_ofs(mtd, from), len);
> +
> +	mtd_start_access(master);
> +	ret =  master->_unpoint(master, mtd_get_master_ofs(mtd, from), len);
> +	mtd_end_access(master);
> +
> +	return ret;
>   }
>   EXPORT_SYMBOL_GPL(mtd_unpoint);
>   
> @@ -1372,6 +1387,7 @@ int mtd_read(struct mtd_info *mtd, loff_t from, size_t len, size_t *retlen,
>   	};
>   	int ret;
>   
> +	/* mtd_read_oob_std handles mtd access protection */
>   	ret = mtd_read_oob(mtd, from, &ops);
>   	*retlen = ops.retlen;
>   
> @@ -1388,6 +1404,7 @@ int mtd_write(struct mtd_info *mtd, loff_t to, size_t len, size_t *retlen,
>   	};
>   	int ret;
>   
> +	/* mtd_write_oob_std handles mtd access protection */
>   	ret = mtd_write_oob(mtd, to, &ops);
>   	*retlen = ops.retlen;
>   
> @@ -1464,11 +1481,13 @@ static int mtd_read_oob_std(struct mtd_info *mtd, loff_t from,
>   	int ret;
>   
>   	from = mtd_get_master_ofs(mtd, from);
> +	mtd_start_access(master);
>   	if (master->_read_oob)
>   		ret = master->_read_oob(master, from, ops);
>   	else
>   		ret = master->_read(master, from, ops->len, &ops->retlen,
>   				    ops->datbuf);
> +	mtd_end_access(master);
>   
>   	return ret;
>   }
> @@ -1480,11 +1499,13 @@ static int mtd_write_oob_std(struct mtd_info *mtd, loff_t to,
>   	int ret;
>   
>   	to = mtd_get_master_ofs(mtd, to);
> +	mtd_start_access(master);
>   	if (master->_write_oob)
>   		ret = master->_write_oob(master, to, ops);
>   	else
>   		ret = master->_write(master, to, ops->len, &ops->retlen,
>   				     ops->datbuf);
> +	mtd_end_access(master);
>   
>   	return ret;
>   }
> @@ -1992,12 +2013,18 @@ int mtd_get_fact_prot_info(struct mtd_info *mtd, size_t len, size_t *retlen,
>   			   struct otp_info *buf)
>   {
>   	struct mtd_info *master = mtd_get_master(mtd);
> +	int ret;
>   
>   	if (!master->_get_fact_prot_info)
>   		return -EOPNOTSUPP;
>   	if (!len)
>   		return 0;
> -	return master->_get_fact_prot_info(master, len, retlen, buf);
> +
> +	mtd_start_access(master);
> +	ret = master->_get_fact_prot_info(master, len, retlen, buf);
> +	mtd_end_access(master);
> +
> +	return ret;
>   }
>   EXPORT_SYMBOL_GPL(mtd_get_fact_prot_info);
>   
> @@ -2005,13 +2032,19 @@ int mtd_read_fact_prot_reg(struct mtd_info *mtd, loff_t from, size_t len,
>   			   size_t *retlen, u_char *buf)
>   {
>   	struct mtd_info *master = mtd_get_master(mtd);
> +	int ret;
>   
>   	*retlen = 0;
>   	if (!master->_read_fact_prot_reg)
>   		return -EOPNOTSUPP;
>   	if (!len)
>   		return 0;
> -	return master->_read_fact_prot_reg(master, from, len, retlen, buf);
> +
> +	mtd_start_access(master);
> +	ret = master->_read_fact_prot_reg(master, from, len, retlen, buf);
> +	mtd_end_access(master);
> +
> +	return ret;
>   }
>   EXPORT_SYMBOL_GPL(mtd_read_fact_prot_reg);
>   
> @@ -2019,12 +2052,18 @@ int mtd_get_user_prot_info(struct mtd_info *mtd, size_t len, size_t *retlen,
>   			   struct otp_info *buf)
>   {
>   	struct mtd_info *master = mtd_get_master(mtd);
> +	int ret;
>   
>   	if (!master->_get_user_prot_info)
>   		return -EOPNOTSUPP;
>   	if (!len)
>   		return 0;
> -	return master->_get_user_prot_info(master, len, retlen, buf);
> +
> +	mtd_start_access(master);
> +	ret =  master->_get_user_prot_info(master, len, retlen, buf);
> +	mtd_end_access(master);
> +
> +	return ret;
>   }
>   EXPORT_SYMBOL_GPL(mtd_get_user_prot_info);
>   
> @@ -2032,13 +2071,19 @@ int mtd_read_user_prot_reg(struct mtd_info *mtd, loff_t from, size_t len,
>   			   size_t *retlen, u_char *buf)
>   {
>   	struct mtd_info *master = mtd_get_master(mtd);
> +	int ret;
>   
>   	*retlen = 0;
>   	if (!master->_read_user_prot_reg)
>   		return -EOPNOTSUPP;
>   	if (!len)
>   		return 0;
> -	return master->_read_user_prot_reg(master, from, len, retlen, buf);
> +
> +	mtd_start_access(master);
> +	ret = master->_read_user_prot_reg(master, from, len, retlen, buf);
> +	mtd_end_access(master);
> +
> +	return ret;
>   }
>   EXPORT_SYMBOL_GPL(mtd_read_user_prot_reg);
>   
> @@ -2053,7 +2098,11 @@ int mtd_write_user_prot_reg(struct mtd_info *mtd, loff_t to, size_t len,
>   		return -EOPNOTSUPP;
>   	if (!len)
>   		return 0;
> +
> +	mtd_start_access(master);
>   	ret = master->_write_user_prot_reg(master, to, len, retlen, buf);
> +	mtd_end_access(master);
> +
>   	if (ret)
>   		return ret;
>   
> @@ -2068,24 +2117,36 @@ EXPORT_SYMBOL_GPL(mtd_write_user_prot_reg);
>   int mtd_lock_user_prot_reg(struct mtd_info *mtd, loff_t from, size_t len)
>   {
>   	struct mtd_info *master = mtd_get_master(mtd);
> +	int ret;
>   
>   	if (!master->_lock_user_prot_reg)
>   		return -EOPNOTSUPP;
>   	if (!len)
>   		return 0;
> -	return master->_lock_user_prot_reg(master, from, len);
> +
> +	mtd_start_access(master);
> +	ret = master->_lock_user_prot_reg(master, from, len);
> +	mtd_end_access(master);
> +
> +	return ret;
>   }
>   EXPORT_SYMBOL_GPL(mtd_lock_user_prot_reg);
>   
>   int mtd_erase_user_prot_reg(struct mtd_info *mtd, loff_t from, size_t len)
>   {
>   	struct mtd_info *master = mtd_get_master(mtd);
> +	int ret;
>   
>   	if (!master->_erase_user_prot_reg)
>   		return -EOPNOTSUPP;
>   	if (!len)
>   		return 0;
> -	return master->_erase_user_prot_reg(master, from, len);
> +
> +	mtd_start_access(master);
> +	ret = master->_erase_user_prot_reg(master, from, len);
> +	mtd_end_access(master);
> +
> +	return ret;
>   }
>   EXPORT_SYMBOL_GPL(mtd_erase_user_prot_reg);
>   
> @@ -2093,6 +2154,7 @@ EXPORT_SYMBOL_GPL(mtd_erase_user_prot_reg);
>   int mtd_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
>   {
>   	struct mtd_info *master = mtd_get_master(mtd);
> +	int ret;
>   
>   	if (!master->_lock)
>   		return -EOPNOTSUPP;
> @@ -2106,13 +2168,18 @@ int mtd_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
>   		len = (u64)mtd_div_by_eb(len, mtd) * master->erasesize;
>   	}
>   
> -	return master->_lock(master, mtd_get_master_ofs(mtd, ofs), len);
> +	mtd_start_access(master);
> +	ret = master->_lock(master, mtd_get_master_ofs(mtd, ofs), len);
> +	mtd_end_access(master);
> +
> +	return ret;
>   }
>   EXPORT_SYMBOL_GPL(mtd_lock);
>   
>   int mtd_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
>   {
>   	struct mtd_info *master = mtd_get_master(mtd);
> +	int ret;
>   
>   	if (!master->_unlock)
>   		return -EOPNOTSUPP;
> @@ -2126,13 +2193,18 @@ int mtd_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
>   		len = (u64)mtd_div_by_eb(len, mtd) * master->erasesize;
>   	}
>   
> -	return master->_unlock(master, mtd_get_master_ofs(mtd, ofs), len);
> +	mtd_start_access(master);
> +	ret = master->_unlock(master, mtd_get_master_ofs(mtd, ofs), len);
> +	mtd_end_access(master);
> +
> +	return ret;
>   }
>   EXPORT_SYMBOL_GPL(mtd_unlock);
>   
>   int mtd_is_locked(struct mtd_info *mtd, loff_t ofs, uint64_t len)
>   {
>   	struct mtd_info *master = mtd_get_master(mtd);
> +	int ret;
>   
>   	if (!master->_is_locked)
>   		return -EOPNOTSUPP;
> @@ -2146,13 +2218,18 @@ int mtd_is_locked(struct mtd_info *mtd, loff_t ofs, uint64_t len)
>   		len = (u64)mtd_div_by_eb(len, mtd) * master->erasesize;
>   	}
>   
> -	return master->_is_locked(master, mtd_get_master_ofs(mtd, ofs), len);
> +	mtd_start_access(master);
> +	ret = master->_is_locked(master, mtd_get_master_ofs(mtd, ofs), len);
> +	mtd_end_access(master);
> +
> +	return ret;
>   }
>   EXPORT_SYMBOL_GPL(mtd_is_locked);
>   
>   int mtd_block_isreserved(struct mtd_info *mtd, loff_t ofs)
>   {
>   	struct mtd_info *master = mtd_get_master(mtd);
> +	int ret;
>   
>   	if (ofs < 0 || ofs >= mtd->size)
>   		return -EINVAL;
> @@ -2162,13 +2239,18 @@ int mtd_block_isreserved(struct mtd_info *mtd, loff_t ofs)
>   	if (mtd->flags & MTD_SLC_ON_MLC_EMULATION)
>   		ofs = (loff_t)mtd_div_by_eb(ofs, mtd) * master->erasesize;
>   
> -	return master->_block_isreserved(master, mtd_get_master_ofs(mtd, ofs));
> +	mtd_start_access(master);
> +	ret = master->_block_isreserved(master, mtd_get_master_ofs(mtd, ofs));
> +	mtd_end_access(master);
> +
> +	return ret;
>   }
>   EXPORT_SYMBOL_GPL(mtd_block_isreserved);
>   
>   int mtd_block_isbad(struct mtd_info *mtd, loff_t ofs)
>   {
>   	struct mtd_info *master = mtd_get_master(mtd);
> +	int ret;
>   
>   	if (ofs < 0 || ofs >= mtd->size)
>   		return -EINVAL;
> @@ -2178,7 +2260,11 @@ int mtd_block_isbad(struct mtd_info *mtd, loff_t ofs)
>   	if (mtd->flags & MTD_SLC_ON_MLC_EMULATION)
>   		ofs = (loff_t)mtd_div_by_eb(ofs, mtd) * master->erasesize;
>   
> -	return master->_block_isbad(master, mtd_get_master_ofs(mtd, ofs));
> +	mtd_start_access(master);
> +	ret = master->_block_isbad(master, mtd_get_master_ofs(mtd, ofs));
> +	mtd_end_access(master);
> +
> +	return ret;
>   }
>   EXPORT_SYMBOL_GPL(mtd_block_isbad);
>   
> @@ -2197,7 +2283,10 @@ int mtd_block_markbad(struct mtd_info *mtd, loff_t ofs)
>   	if (mtd->flags & MTD_SLC_ON_MLC_EMULATION)
>   		ofs = (loff_t)mtd_div_by_eb(ofs, mtd) * master->erasesize;
>   
> +	mtd_start_access(master);
>   	ret = master->_block_markbad(master, mtd_get_master_ofs(mtd, ofs));
> +	mtd_end_access(master);
> +
>   	if (ret)
>   		return ret;
>   
> diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
> index 88227044fc86..b074106e2d8e 100644
> --- a/include/linux/mtd/mtd.h
> +++ b/include/linux/mtd/mtd.h
> @@ -231,6 +231,8 @@ struct mtd_master {
>   	struct mutex partitions_lock;
>   	struct mutex chrdev_lock;
>   	unsigned int suspended : 1;
> +	wait_queue_head_t resume_wq;
> +	struct rw_semaphore suspend_lock;
>   };
>   
>   struct mtd_info {
> @@ -476,10 +478,47 @@ static inline u32 mtd_oobavail(struct mtd_info *mtd, struct mtd_oob_ops *ops)
>   	return ops->mode == MTD_OPS_AUTO_OOB ? mtd->oobavail : mtd->oobsize;
>   }
>   
> +static inline void mtd_start_access(struct mtd_info *master)
> +{
> +	WARN_ON_ONCE(master != mtd_get_master(master));
> +
> +	/*
> +	 * Don't take the suspend_lock on devices that don't
> +	 * implement the suspend hook. Otherwise, lockdep will
> +	 * complain about nested locks when trying to suspend MTD
> +	 * partitions or MTD devices created by gluebi which are
> +	 * backed by real devices.
> +	 */
> +	if (!master->_suspend)
> +		return;
> +
> +	/*
> +	 * Wait until the device is resumed. Should we have a
> +	 * non-blocking mode here?
> +	 */
> +	while (1) {
> +		down_read(&master->master.suspend_lock);
> +		if (!master->master.suspended)
> +			return;
> +
> +		up_read(&master->master.suspend_lock);
> +		wait_event(master->master.resume_wq, !master->master.suspended);
> +	}
> +}
> +
> +static inline void mtd_end_access(struct mtd_info *master)
> +{
> +	if (!master->_suspend)
> +		return;
> +
> +	up_read(&master->master.suspend_lock);
> +}
> +
>   static inline int mtd_max_bad_blocks(struct mtd_info *mtd,
>   				     loff_t ofs, size_t len)
>   {
>   	struct mtd_info *master = mtd_get_master(mtd);
> +	int ret;
>   
>   	if (!master->_max_bad_blocks)
>   		return -ENOTSUPP;
> @@ -487,8 +526,12 @@ static inline int mtd_max_bad_blocks(struct mtd_info *mtd,
>   	if (mtd->size < (len + ofs) || ofs < 0)
>   		return -EINVAL;
>   
> -	return master->_max_bad_blocks(master, mtd_get_master_ofs(mtd, ofs),
> -				       len);
> +	mtd_start_access(master);
> +	ret = master->_max_bad_blocks(master, mtd_get_master_ofs(mtd, ofs),
> +				      len);
> +	mtd_end_access(master);
> +
> +	return ret;
>   }
>   
>   int mtd_wunit_to_pairing_info(struct mtd_info *mtd, int wunit,
> @@ -546,30 +589,40 @@ int mtd_block_markbad(struct mtd_info *mtd, loff_t ofs);
>   static inline int mtd_suspend(struct mtd_info *mtd)
>   {
>   	struct mtd_info *master = mtd_get_master(mtd);
> -	int ret;
> -
> -	if (master->master.suspended)
> -		return 0;
> +	int ret = 0;
>   
> -	ret = master->_suspend ? master->_suspend(master) : 0;
> -	if (ret)
> +	if (!master->_suspend)
>   		return ret;
>   
> -	master->master.suspended = 1;
> -	return 0;
> +	down_write(&master->master.suspend_lock);
> +	if (!master->master.suspended) {
> +		ret = master->_suspend(master);
> +		if (!ret)
> +			master->master.suspended = 1;
> +	}
> +	up_write(&master->master.suspend_lock);
> +
> +	return ret;
>   }
>   
>   static inline void mtd_resume(struct mtd_info *mtd)
>   {
>   	struct mtd_info *master = mtd_get_master(mtd);
>   
> -	if (!master->master.suspended)
> +	if (!master->_suspend)
>   		return;
>   
> -	if (master->_resume)
> -		master->_resume(master);
> +	down_write(&master->master.suspend_lock);
> +	if (master->master.suspended) {
> +		if (master->_resume)
> +			master->_resume(master);
> +
> +		master->master.suspended = 0;
>   
> -	master->master.suspended = 0;
> +		/* The MTD dev has been resumed, wake up all waiters. */
> +		wake_up_all(&master->master.resume_wq);
> +	}
> +	up_write(&master->master.suspend_lock);
>   }
>   
>   static inline uint32_t mtd_div_by_eb(uint64_t sz, struct mtd_info *mtd)

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

  parent reply	other threads:[~2021-11-23 12:03 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-02 11:02 [PATCH v5 0/4] mtd: core: protect access to mtd devices while in suspend Sean Nyekjaer
2021-11-02 11:02 ` Sean Nyekjaer
2021-11-02 11:02 ` [PATCH v5 1/4] mtd: rawnand: nand_bbt: hide suspend/resume hooks while scanning bbt Sean Nyekjaer
2021-11-02 11:02   ` Sean Nyekjaer
2021-11-19 18:35   ` Miquel Raynal
2021-11-19 18:35     ` Miquel Raynal
2021-11-02 11:02 ` [PATCH v5 2/4] mtd: mtdconcat: don't use mtd_{suspend,resume}() Sean Nyekjaer
2021-11-02 11:02   ` Sean Nyekjaer
2021-11-19 18:35   ` Miquel Raynal
2021-11-19 18:35     ` Miquel Raynal
2021-11-02 11:02 ` [PATCH v5 3/4] mtd: core: protect access to MTD devices while in suspend Sean Nyekjaer
2021-11-02 11:02   ` Sean Nyekjaer
2021-11-19 18:35   ` Miquel Raynal
2021-11-19 18:35     ` Miquel Raynal
     [not found]   ` <CGME20211123120353eucas1p2fb2561b7cfddd8d6e7decaef8b504f4c@eucas1p2.samsung.com>
2021-11-23 12:03     ` Marek Szyprowski [this message]
2021-11-23 12:03       ` Marek Szyprowski
2021-11-23 12:50       ` Sean Nyekjaer
2021-11-23 12:50         ` Sean Nyekjaer
2021-11-23 13:07         ` Boris Brezillon
2021-11-23 13:07           ` Boris Brezillon
2021-11-29  9:19           ` Miquel Raynal
2021-11-29  9:19             ` Miquel Raynal
2021-11-29  9:41             ` Sean Nyekjaer
2021-11-29  9:41               ` Sean Nyekjaer
2021-11-29 11:17               ` Marek Szyprowski
2021-11-29 11:17                 ` Marek Szyprowski
2021-11-30 12:41                 ` Sean Nyekjaer
2021-11-30 12:41                   ` Sean Nyekjaer
2021-11-30 13:15                   ` Boris Brezillon
2021-11-30 13:15                     ` Boris Brezillon
2021-11-30 13:29                     ` Sean Nyekjaer
2021-11-30 13:29                       ` Sean Nyekjaer
2021-11-30 13:37                       ` Boris Brezillon
2021-11-30 13:37                         ` Boris Brezillon
2021-12-03 13:39                         ` Miquel Raynal
2021-12-03 13:39                           ` Miquel Raynal
2021-12-09 14:07                           ` Sean Nyekjaer
2021-12-09 14:07                             ` Sean Nyekjaer
2021-12-09 14:28                             ` Miquel Raynal
2021-12-09 14:28                               ` Miquel Raynal
2021-12-10 13:25                               ` Sean Nyekjaer
2021-12-10 13:25                                 ` Sean Nyekjaer
2021-12-13  9:10                                 ` Miquel Raynal
2021-12-13  9:10                                   ` Miquel Raynal
2021-12-13  9:28                                   ` Boris Brezillon
2021-12-13  9:28                                     ` Boris Brezillon
2021-12-13  9:33                                     ` Miquel Raynal
2021-12-13  9:33                                       ` Miquel Raynal
2021-12-13  9:53                                       ` Boris Brezillon
2021-12-13  9:53                                         ` Boris Brezillon
2021-12-13 10:50                                         ` Sean Nyekjaer
2021-12-13 10:50                                           ` Sean Nyekjaer
2021-12-13 11:06                                           ` Boris Brezillon
2021-12-13 11:06                                             ` Boris Brezillon
2021-11-02 11:02 ` [PATCH v5 4/4] mtd: rawnand: remove suspended check Sean Nyekjaer
2021-11-02 11:02   ` Sean Nyekjaer
2021-11-19 18:35   ` Miquel Raynal
2021-11-19 18:35     ` Miquel Raynal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=21092c0f-e11b-ac16-ab96-2a0556c0e30a@samsung.com \
    --to=m.szyprowski@samsung.com \
    --cc=bbrezillon@kernel.org \
    --cc=boris.brezillon@collabora.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=miquel.raynal@bootlin.com \
    --cc=richard@nod.at \
    --cc=sean@geanix.com \
    --cc=vigneshr@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.