lkml.org 
[lkml]   [2021]   [Oct]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 2/4] mtd: core: protect access to MTD devices while in suspend
On Wed, 20 Oct 2021 10:45:32 +0200
Sean Nyekjaer <sean@geanix.com> wrote:

> static ssize_t mtd_otp_size(struct mtd_info *mtd, bool is_user)
> @@ -1257,6 +1259,8 @@ int mtd_erase(struct mtd_info *mtd, sruct erase_info *instr)
>
> ledtrig_mtd_activity();
>
> + mtd_start_access(master);
> +
> if (mtd->flags & MTD_SLC_ON_MLC_EMULATION) {
> adjinstr.addr = (loff_t)mtd_div_by_eb(instr->addr, mtd) *
> master->erasesize;
> @@ -1278,6 +1282,8 @@ int mtd_erase(struct mtd_info *mtd, struct erase_info *instr)
> }
> }
>
> + mtd_end_access(master);
> +

The section covered in mtd_erase() is too broad. Put the start/end
calls around the ->_erase() call.

> return ret;
> }



> @@ -1576,7 +1604,6 @@ int mtd_read_oob(struct mtd_info *mtd, loff_t from, struct mtd_oob_ops *ops)
> ret_code = mtd_read_oob_std(mtd, from, ops);
>
> mtd_update_ecc_stats(mtd, master, &old_stats);
> -

Unrelated line removal. Please drop this change.

> /*
> * In cases where ops->datbuf != NULL, mtd->_read_oob() has semantics
> * similar to mtd->_read(), returning a non-negative integer
> @@ -1615,7 +1642,9 @@ int mtd_write_oob(struct mtd_info *mtd, loff_t to,
> if (mtd->flags & MTD_SLC_ON_MLC_EMULATION)
> return mtd_io_emulated_slc(mtd, to, false, ops);
>
> - return mtd_write_oob_std(mtd, to, ops);
> + ret = mtd_write_oob_std(mtd, to, ops);
> +
> + return ret;

Ditto, you can keep the 'return mtd_write_oob_std(mtd, to, ops);' here.

> }
> EXPORT_SYMBOL_GPL(mtd_write_oob);


> +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 == 0);

Please keep the tests consistent:

wait_event(master->master.resume_wq, !master->master.suspended);

> + }
> +}
> +

\
 
 \ /
  Last update: 2021-10-20 11:14    [W:0.079 / U:1.924 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site