lkml.org 
[lkml]   [2021]   [Oct]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 3/3] mtd: mtdconcat: add suspend lock handling
    On Mon, 11 Oct 2021 13:52:53 +0200
    Sean Nyekjaer <sean@geanix.com> wrote:

    > Use new suspend lock handling for this special case for concatenated
    > MTD devices.
    >
    > Fixes: 013e6292aaf5 ("mtd: rawnand: Simplify the locking")
    > Signed-off-by: Sean Nyekjaer <sean@geanix.com>
    > ---
    > drivers/mtd/mtdconcat.c | 11 +++++++++--
    > 1 file changed, 9 insertions(+), 2 deletions(-)
    >
    > diff --git a/drivers/mtd/mtdconcat.c b/drivers/mtd/mtdconcat.c
    > index f685a581df48..c497c851481f 100644
    > --- a/drivers/mtd/mtdconcat.c
    > +++ b/drivers/mtd/mtdconcat.c
    > @@ -561,25 +561,32 @@ static void concat_sync(struct mtd_info *mtd)
    >
    > static int concat_suspend(struct mtd_info *mtd)
    > {
    > + struct mtd_info *master = mtd_get_master(mtd);
    > struct mtd_concat *concat = CONCAT(mtd);
    > int i, rc = 0;
    >
    > for (i = 0; i < concat->num_subdev; i++) {
    > struct mtd_info *subdev = concat->subdev[i];
    > - if ((rc = mtd_suspend(subdev)) < 0)
    > +
    > + down_write(&master->master.suspend_lock);

    You should definitely not take the concat lock here, the framework does
    it for you, so all you'll get is a deadlock.

    > + if ((rc = __mtd_suspend(subdev)) < 0)
    > return rc;

    You're returning with the lock held => DEADLOCK next time you try to
    acquire it.

    Anyway, as mentioned in my review of patch 1, I'd go for this ad-hoc
    solution:


    for (i = 0; i < concat->num_subdev; i++) {
    rc = subdev->_suspend ? subdev->_suspend(subdev) : 0;
    if (rc < 0)
    return rc;
    }

    return 0;

    > + up_write(&master->master.suspend_lock);
    > }
    > return rc;
    > }
    >
    > static void concat_resume(struct mtd_info *mtd)
    > {
    > + struct mtd_info *master = mtd_get_master(mtd);
    > struct mtd_concat *concat = CONCAT(mtd);
    > int i;
    >
    > for (i = 0; i < concat->num_subdev; i++) {
    > struct mtd_info *subdev = concat->subdev[i];
    > - mtd_resume(subdev);
    > + down_write(&master->master.suspend_lock);
    > + __mtd_resume(subdev);
    > + up_write(&master->master.suspend_lock);
    > }

    No down/up_write() needed:

    for (i = 0; i < concat->num_subdev; i++) {
    struct mtd_info *subdev = concat->subdev[i];
    if (subdev->_resume)
    subdev->_resume(subdev);

    }
    > }
    >

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