From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id A1334CE7A81 for ; Mon, 25 Sep 2023 07:22:55 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231644AbjIYHW7 (ORCPT ); Mon, 25 Sep 2023 03:22:59 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43494 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230148AbjIYHW4 (ORCPT ); Mon, 25 Sep 2023 03:22:56 -0400 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id ACAEC1BF for ; Mon, 25 Sep 2023 00:22:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1695626519; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=gzztqiTccNd/WUIopmBC+PbliphbDEU0hH26bTuwFVE=; b=JstlfAO3GcJ1DmIg11x9J54fmqC3CtU4Y04dyjk9KGbytqL8UjaSKJR1Ay8a+fcSwFZJSU DS0i3880IwB7ZQjJuecXObygUvXmerRHuO/i5seq+Wt6jWFmJpFlQIM2yjJfNEhPSsay4P jdERT4Y+VLoYpjFsH+OiIHiDUICxMjM= Received: from mail-pf1-f198.google.com (mail-pf1-f198.google.com [209.85.210.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-113-kt40fQAYP5eE8GrqeIwUow-1; Mon, 25 Sep 2023 03:21:58 -0400 X-MC-Unique: kt40fQAYP5eE8GrqeIwUow-1 Received: by mail-pf1-f198.google.com with SMTP id d2e1a72fcca58-690f8b7058bso6290608b3a.1 for ; Mon, 25 Sep 2023 00:21:58 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1695626517; x=1696231317; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=gzztqiTccNd/WUIopmBC+PbliphbDEU0hH26bTuwFVE=; b=s2a/4r0uElQUyD9dQccxZ0vXgBtkOEhD9EitchYEftoGPhLWWrkkHhYkiGO+HJcakH izb3P1aEP+TMysIe0TqZCizo6uUjGkOpEttyFblLm0svuSFMA+fYTZOALib3YFI2ZWM0 GlbJ4RbSnvHmDkv64px2n8DbZks99cj89zlyDAu4UlzCtHc9/i+PPS98qLJTZT+S6FQD EqxlTpSI84usIS7d+qFqMmEvu0SC8mxsiqdP4oulW+qPSvNhrLMgtnTf/tRYV6SlNbZS m/wEql4pwZmI/IJXy6NxMJcPEZj52kUK55aGoWcTG0TV2qsc+v5DqxStepvhlvFm99vD keZA== X-Gm-Message-State: AOJu0YzXjtHV7/KkvIr7bQaPjbijA76upH5+DeFRlMsl9Wc/Nsow5GcD QGINI3QWoo8G9Rr0e3fA5TZiksy+8cN+pL6HvJKY7lyogZfNG+q2BeogfBz14QUayGmKNEofTO6 YvAEZCvIWWDjYM1y4Q203luLuP9jB05ezZdnZQw== X-Received: by 2002:a05:6a20:f392:b0:14d:396e:6e1c with SMTP id qr18-20020a056a20f39200b0014d396e6e1cmr3937736pzb.28.1695626517209; Mon, 25 Sep 2023 00:21:57 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGJGOJfpxHnsNy6HUSrcPHkZKwrs+s1jrli3TQ1B3bFrk79pIC+NE3vVlCSfd8E9+GPVWsiNkvel8RS82ZzTfM= X-Received: by 2002:a05:6a20:f392:b0:14d:396e:6e1c with SMTP id qr18-20020a056a20f39200b0014d396e6e1cmr3937728pzb.28.1695626516919; Mon, 25 Sep 2023 00:21:56 -0700 (PDT) MIME-Version: 1.0 References: <20230828020021.2489641-1-yukuai1@huaweicloud.com> <20230828020021.2489641-4-yukuai1@huaweicloud.com> In-Reply-To: <20230828020021.2489641-4-yukuai1@huaweicloud.com> From: Xiao Ni Date: Mon, 25 Sep 2023 15:21:45 +0800 Message-ID: Subject: Re: [PATCH -next v2 03/28] md: add new helpers to suspend/resume array To: Yu Kuai Cc: agk@redhat.com, snitzer@kernel.org, dm-devel@redhat.com, song@kernel.org, linux-kernel@vger.kernel.org, linux-raid@vger.kernel.org, yukuai3@huawei.com, yi.zhang@huawei.com, yangerkun@huawei.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-raid@vger.kernel.org On Mon, Aug 28, 2023 at 10:04=E2=80=AFAM Yu Kuai = wrote: > > From: Yu Kuai > > Advantages for new apis: > - reconfig_mutex is not required; > - the weird logical that suspend array hold 'reconfig_mutex' for > mddev_check_recovery() to update superblock is not needed; > - the specail handling, 'pers->prepare_suspend', for raid456 is not > needed; > - It's safe to be called at any time once mddev is allocated, and it's > designed to be used from slow path where array configuration is change= d; > > Signed-off-by: Yu Kuai > --- > drivers/md/md.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++-- > drivers/md/md.h | 3 ++ > 2 files changed, 86 insertions(+), 2 deletions(-) > > diff --git a/drivers/md/md.c b/drivers/md/md.c > index 7fa311a14317..6236e2e395c1 100644 > --- a/drivers/md/md.c > +++ b/drivers/md/md.c > @@ -443,12 +443,22 @@ void mddev_suspend(struct mddev *mddev) > lockdep_is_held(&mddev->reconfig_mutex)); > > WARN_ON_ONCE(thread && current =3D=3D thread->tsk); > - if (mddev->suspended++) > + > + /* can't concurrent with __mddev_suspend() and __mddev_resume() *= / > + mutex_lock(&mddev->suspend_mutex); > + if (mddev->suspended++) { > + mutex_unlock(&mddev->suspend_mutex); > return; > + } > + > wake_up(&mddev->sb_wait); > set_bit(MD_ALLOW_SB_UPDATE, &mddev->flags); > percpu_ref_kill(&mddev->active_io); > > + /* > + * TODO: cleanup 'pers->prepare_suspend after all callers are rep= laced > + * by __mddev_suspend(). > + */ > if (mddev->pers && mddev->pers->prepare_suspend) > mddev->pers->prepare_suspend(mddev); > > @@ -459,14 +469,21 @@ void mddev_suspend(struct mddev *mddev) > del_timer_sync(&mddev->safemode_timer); > /* restrict memory reclaim I/O during raid array is suspend */ > mddev->noio_flag =3D memalloc_noio_save(); > + > + mutex_unlock(&mddev->suspend_mutex); > } > EXPORT_SYMBOL_GPL(mddev_suspend); > > void mddev_resume(struct mddev *mddev) > { > lockdep_assert_held(&mddev->reconfig_mutex); > - if (--mddev->suspended) > + > + /* can't concurrent with __mddev_suspend() and __mddev_resume() *= / > + mutex_lock(&mddev->suspend_mutex); > + if (--mddev->suspended) { > + mutex_unlock(&mddev->suspend_mutex); > return; > + } > > /* entred the memalloc scope from mddev_suspend() */ > memalloc_noio_restore(mddev->noio_flag); > @@ -477,9 +494,72 @@ void mddev_resume(struct mddev *mddev) > set_bit(MD_RECOVERY_NEEDED, &mddev->recovery); > md_wakeup_thread(mddev->thread); > md_wakeup_thread(mddev->sync_thread); /* possibly kick off a resh= ape */ > + > + mutex_unlock(&mddev->suspend_mutex); > } > EXPORT_SYMBOL_GPL(mddev_resume); > > +void __mddev_suspend(struct mddev *mddev) > +{ > + > + /* > + * hold reconfig_mutex to wait for normal io will deadlock, becau= se > + * other context can't update super_block, and normal io can rely= on > + * updating super_block. > + */ > + lockdep_assert_not_held(&mddev->reconfig_mutex); > + > + mutex_lock(&mddev->suspend_mutex); > + > + if (mddev->suspended) { > + WRITE_ONCE(mddev->suspended, mddev->suspended + 1); > + mutex_unlock(&mddev->suspend_mutex); > + return; > + } > + > + percpu_ref_kill(&mddev->active_io); > + wait_event(mddev->sb_wait, percpu_ref_is_zero(&mddev->active_io))= ; > + > + /* > + * For raid456, io might be waiting for reshape to make progress, > + * allow new reshape to start while waiting for io to be done to > + * prevent deadlock. > + */ > + WRITE_ONCE(mddev->suspended, mddev->suspended + 1); It changes the order of setting suspended and checking active_io. suspended is used to stop I/O. Now it checks active_io first and then adds suspended, if the i/o doesn't stop, it looks like active_io can't be 0. So it will stuck at waiting active_io to be 0? Best Regards Xiao > + > + del_timer_sync(&mddev->safemode_timer); > + /* restrict memory reclaim I/O during raid array is suspend */ > + mddev->noio_flag =3D memalloc_noio_save(); > + > + mutex_unlock(&mddev->suspend_mutex); > +} > +EXPORT_SYMBOL_GPL(__mddev_suspend); > + > +void __mddev_resume(struct mddev *mddev) > +{ > + lockdep_assert_not_held(&mddev->reconfig_mutex); > + > + mutex_lock(&mddev->suspend_mutex); > + WRITE_ONCE(mddev->suspended, mddev->suspended - 1); > + if (mddev->suspended) { > + mutex_unlock(&mddev->suspend_mutex); > + return; > + } > + > + /* entred the memalloc scope from __mddev_suspend() */ > + memalloc_noio_restore(mddev->noio_flag); > + > + percpu_ref_resurrect(&mddev->active_io); > + wake_up(&mddev->sb_wait); > + > + set_bit(MD_RECOVERY_NEEDED, &mddev->recovery); > + md_wakeup_thread(mddev->thread); > + md_wakeup_thread(mddev->sync_thread); /* possibly kick off a resh= ape */ > + > + mutex_unlock(&mddev->suspend_mutex); > +} > +EXPORT_SYMBOL_GPL(__mddev_resume); > + > /* > * Generic flush handling for md > */ > @@ -667,6 +747,7 @@ int mddev_init(struct mddev *mddev) > mutex_init(&mddev->open_mutex); > mutex_init(&mddev->reconfig_mutex); > mutex_init(&mddev->sync_mutex); > + mutex_init(&mddev->suspend_mutex); > mutex_init(&mddev->bitmap_info.mutex); > INIT_LIST_HEAD(&mddev->disks); > INIT_LIST_HEAD(&mddev->all_mddevs); > diff --git a/drivers/md/md.h b/drivers/md/md.h > index fb3b123f16dd..1103e6b08ad9 100644 > --- a/drivers/md/md.h > +++ b/drivers/md/md.h > @@ -316,6 +316,7 @@ struct mddev { > unsigned long sb_flags; > > int suspended; > + struct mutex suspend_mutex; > struct percpu_ref active_io; > int ro; > int sysfs_active; /* set when sysfs d= eletes > @@ -811,6 +812,8 @@ extern void md_rdev_clear(struct md_rdev *rdev); > extern void md_handle_request(struct mddev *mddev, struct bio *bio); > extern void mddev_suspend(struct mddev *mddev); > extern void mddev_resume(struct mddev *mddev); > +extern void __mddev_suspend(struct mddev *mddev); > +extern void __mddev_resume(struct mddev *mddev); > > extern void md_reload_sb(struct mddev *mddev, int raid_disk); > extern void md_update_sb(struct mddev *mddev, int force); > -- > 2.39.2 >