diff mbox series

[-next,v3,2/7] md: factor out a helper to choose sync action from md_check_recovery()

Message ID 20230820090949.2874537-3-yukuai1@huaweicloud.com (mailing list archive)
State New, archived
Headers show
Series md: make rdev addition and removal independent from daemon thread | expand

Commit Message

Yu Kuai Aug. 20, 2023, 9:09 a.m. UTC 
From: Yu Kuai <yukuai3@huawei.com>

There are no functional changes, on the one hand make the code cleaner,
on the other hand prevent following checkpatch error in the next patch to
delay choosing sync action to md_start_sync().

ERROR: do not use assignment in if condition
+       } else if ((spares = remove_and_add_spares(mddev, NULL))) {

Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
 drivers/md/md.c | 70 +++++++++++++++++++++++++++++++------------------
 1 file changed, 45 insertions(+), 25 deletions(-)

Comments

Xiao Ni Aug. 22, 2023, 10:04 a.m. UTC | #1 
On Sun, Aug 20, 2023 at 5:13 PM Yu Kuai <yukuai1@huaweicloud.com> wrote:
>
> From: Yu Kuai <yukuai3@huawei.com>
>
> There are no functional changes, on the one hand make the code cleaner,
> on the other hand prevent following checkpatch error in the next patch to
> delay choosing sync action to md_start_sync().
>
> ERROR: do not use assignment in if condition
> +       } else if ((spares = remove_and_add_spares(mddev, NULL))) {
>
> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
> ---
>  drivers/md/md.c | 70 +++++++++++++++++++++++++++++++------------------
>  1 file changed, 45 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 90815be1e80f..0cb9fa703a0c 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -9246,6 +9246,50 @@ static int remove_and_add_spares(struct mddev *mddev,
>         return spares;
>  }
>
> +static bool md_choose_sync_action(struct mddev *mddev, int *spares)
> +{
> +       /* Check if reshape is in progress first. */
> +       if (mddev->reshape_position != MaxSector) {
> +               if (mddev->pers->check_reshape == NULL ||
> +                   mddev->pers->check_reshape(mddev) != 0)
> +                       return false;
> +
> +               set_bit(MD_RECOVERY_RESHAPE, &mddev->recovery);
> +               clear_bit(MD_RECOVERY_RECOVER, &mddev->recovery);
> +               return true;
> +       }
> +
> +       /*
> +        * Remove any failed drives, then add spares if possible. Spares are
> +        * also removed and re-added, to allow the personality to fail the
> +        * re-add.
> +        */
> +       *spares = remove_and_add_spares(mddev, NULL);
> +       if (*spares) {
> +               clear_bit(MD_RECOVERY_SYNC, &mddev->recovery);
> +               clear_bit(MD_RECOVERY_CHECK, &mddev->recovery);
> +               clear_bit(MD_RECOVERY_REQUESTED, &mddev->recovery);
> +
> +               /* Start new recovery. */
> +               set_bit(MD_RECOVERY_RECOVER, &mddev->recovery);
> +               return true;
> +       }
> +
> +       /* Check if recovery is in progress. */
> +       if (mddev->recovery_cp < MaxSector) {
> +               set_bit(MD_RECOVERY_SYNC, &mddev->recovery);
> +               clear_bit(MD_RECOVERY_RECOVER, &mddev->recovery);
> +               return true;
> +       }
> +
> +       /* Delay to choose resync/check/repair in md_do_sync(). */
> +       if (test_bit(MD_RECOVERY_SYNC, &mddev->recovery))
> +               return true;
> +
> +       /* Nothing to be done */
> +       return false;
> +}
> +
>  static void md_start_sync(struct work_struct *ws)
>  {
>         struct mddev *mddev = container_of(ws, struct mddev, sync_work);
> @@ -9427,32 +9471,8 @@ void md_check_recovery(struct mddev *mddev)
>                 if (!test_and_clear_bit(MD_RECOVERY_NEEDED, &mddev->recovery) ||
>                     test_bit(MD_RECOVERY_FROZEN, &mddev->recovery))
>                         goto not_running;
> -               /* no recovery is running.
> -                * remove any failed drives, then
> -                * add spares if possible.
> -                * Spares are also removed and re-added, to allow
> -                * the personality to fail the re-add.
> -                */
> -
> -               if (mddev->reshape_position != MaxSector) {
> -                       if (mddev->pers->check_reshape == NULL ||
> -                           mddev->pers->check_reshape(mddev) != 0)
> -                               /* Cannot proceed */
> -                               goto not_running;
> -                       set_bit(MD_RECOVERY_RESHAPE, &mddev->recovery);
> -                       clear_bit(MD_RECOVERY_RECOVER, &mddev->recovery);
> -               } else if ((spares = remove_and_add_spares(mddev, NULL))) {
> -                       clear_bit(MD_RECOVERY_SYNC, &mddev->recovery);
> -                       clear_bit(MD_RECOVERY_CHECK, &mddev->recovery);
> -                       clear_bit(MD_RECOVERY_REQUESTED, &mddev->recovery);
> -                       set_bit(MD_RECOVERY_RECOVER, &mddev->recovery);
> -               } else if (mddev->recovery_cp < MaxSector) {
> -                       set_bit(MD_RECOVERY_SYNC, &mddev->recovery);
> -                       clear_bit(MD_RECOVERY_RECOVER, &mddev->recovery);
> -               } else if (!test_bit(MD_RECOVERY_SYNC, &mddev->recovery))
> -                       /* nothing to be done ... */
> +               if (!md_choose_sync_action(mddev, &spares))
>                         goto not_running;
> -
>                 if (mddev->pers->sync_request) {
>                         if (spares) {
>                                 /* We are adding a device or devices to an array
> --
> 2.39.2
>

Reviewed-by: Xiao Ni <xni@redhat.com>
diff mbox series

Patch

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 90815be1e80f..0cb9fa703a0c 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -9246,6 +9246,50 @@  static int remove_and_add_spares(struct mddev *mddev,
 	return spares;
 }
 
+static bool md_choose_sync_action(struct mddev *mddev, int *spares)
+{
+	/* Check if reshape is in progress first. */
+	if (mddev->reshape_position != MaxSector) {
+		if (mddev->pers->check_reshape == NULL ||
+		    mddev->pers->check_reshape(mddev) != 0)
+			return false;
+
+		set_bit(MD_RECOVERY_RESHAPE, &mddev->recovery);
+		clear_bit(MD_RECOVERY_RECOVER, &mddev->recovery);
+		return true;
+	}
+
+	/*
+	 * Remove any failed drives, then add spares if possible. Spares are
+	 * also removed and re-added, to allow the personality to fail the
+	 * re-add.
+	 */
+	*spares = remove_and_add_spares(mddev, NULL);
+	if (*spares) {
+		clear_bit(MD_RECOVERY_SYNC, &mddev->recovery);
+		clear_bit(MD_RECOVERY_CHECK, &mddev->recovery);
+		clear_bit(MD_RECOVERY_REQUESTED, &mddev->recovery);
+
+		/* Start new recovery. */
+		set_bit(MD_RECOVERY_RECOVER, &mddev->recovery);
+		return true;
+	}
+
+	/* Check if recovery is in progress. */
+	if (mddev->recovery_cp < MaxSector) {
+		set_bit(MD_RECOVERY_SYNC, &mddev->recovery);
+		clear_bit(MD_RECOVERY_RECOVER, &mddev->recovery);
+		return true;
+	}
+
+	/* Delay to choose resync/check/repair in md_do_sync(). */
+	if (test_bit(MD_RECOVERY_SYNC, &mddev->recovery))
+		return true;
+
+	/* Nothing to be done */
+	return false;
+}
+
 static void md_start_sync(struct work_struct *ws)
 {
 	struct mddev *mddev = container_of(ws, struct mddev, sync_work);
@@ -9427,32 +9471,8 @@  void md_check_recovery(struct mddev *mddev)
 		if (!test_and_clear_bit(MD_RECOVERY_NEEDED, &mddev->recovery) ||
 		    test_bit(MD_RECOVERY_FROZEN, &mddev->recovery))
 			goto not_running;
-		/* no recovery is running.
-		 * remove any failed drives, then
-		 * add spares if possible.
-		 * Spares are also removed and re-added, to allow
-		 * the personality to fail the re-add.
-		 */
-
-		if (mddev->reshape_position != MaxSector) {
-			if (mddev->pers->check_reshape == NULL ||
-			    mddev->pers->check_reshape(mddev) != 0)
-				/* Cannot proceed */
-				goto not_running;
-			set_bit(MD_RECOVERY_RESHAPE, &mddev->recovery);
-			clear_bit(MD_RECOVERY_RECOVER, &mddev->recovery);
-		} else if ((spares = remove_and_add_spares(mddev, NULL))) {
-			clear_bit(MD_RECOVERY_SYNC, &mddev->recovery);
-			clear_bit(MD_RECOVERY_CHECK, &mddev->recovery);
-			clear_bit(MD_RECOVERY_REQUESTED, &mddev->recovery);
-			set_bit(MD_RECOVERY_RECOVER, &mddev->recovery);
-		} else if (mddev->recovery_cp < MaxSector) {
-			set_bit(MD_RECOVERY_SYNC, &mddev->recovery);
-			clear_bit(MD_RECOVERY_RECOVER, &mddev->recovery);
-		} else if (!test_bit(MD_RECOVERY_SYNC, &mddev->recovery))
-			/* nothing to be done ... */
+		if (!md_choose_sync_action(mddev, &spares))
 			goto not_running;
-
 		if (mddev->pers->sync_request) {
 			if (spares) {
 				/* We are adding a device or devices to an array