Re: [PATCH -next v3 2/7] md: factor out a helper to choose sync action from md_check_recovery()
From: Xiao Ni
Date: Tue Aug 22 2023 - 06:05:51 EST
On Sun, Aug 20, 2023 at 5:13 PM Yu Kuai <yukuai1@xxxxxxxxxxxxxxx> wrote:
>
> From: Yu Kuai <yukuai3@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxx>