lkml.org 
[lkml]   [2021]   [Oct]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH 1/3] mtd: core: protect access to MTD devices while in suspend
On Mon, 11 Oct 2021 13:52:51 +0200
Sean Nyekjaer <sean@geanix.com> wrote:

> struct mtd_info {
> @@ -476,10 +478,49 @@ 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 *mtd)
> +{
> + struct mtd_info *master = mtd_get_master(mtd);

mtd_start_{access,end}() should only be called on master devices, so I
guess you can drop the mtd_get_master() call and use mtd directly.
Maybe add a WARN_ON_ONCE(mtd != mtd_get_master(mtd)) so we can
easily catch silly mistakes.

> +
> + /*
> + * 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 == 0);
> + }
> +}
> +
> +static inline void mtd_end_access(struct mtd_info *mtd)
> +{
> + struct mtd_info *master = mtd_get_master(mtd);
> +
> + 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 +528,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(mtd);
> + ret = master->_max_bad_blocks(master, mtd_get_master_ofs(mtd, ofs),
> + len);
> + mtd_end_access(mtd);

Please pass the master to those functions, there's no point walking the
parent chain again in the start/end_access() functions if you already
have the master retrieved in the caller. Oh, and there seems to be a
common pattern here, so maybe it's worth adding those macros:

#define mtd_no_suspend_void_call(master, method, ...) \
mtd_start_access(master); \
master->method(master, __VA_ARGS__); \
mtd_end_access(master);

#define mtd_no_suspend_ret_call(ret, master, method, ...) \
mtd_start_access(master); \
ret = master->method(master, __VA_ARGS__); \
mtd_end_access(master);

I don't really like the helper names, so feel free to propose something
else.

> +
> + return ret;
> }
>

\
 
 \ /
  Last update: 2021-10-15 08:55    [W:0.136 / U:0.064 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site