[PATCH md-6.10 1/9] md: rearrange recovery_flage

Yu Kuai posted 9 patches 1 year, 7 months ago
[PATCH md-6.10 1/9] md: rearrange recovery_flage
Posted by Yu Kuai 1 year, 7 months ago
From: Yu Kuai <yukuai3@huawei.com>

Currently there are lots of flags and the names are confusing, since
there are two main types of flags, sync thread runnng status and sync
thread action, rearrange and update comment to improve code readability,
there are no functional changes.

Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
 drivers/md/md.h | 52 ++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 38 insertions(+), 14 deletions(-)

diff --git a/drivers/md/md.h b/drivers/md/md.h
index 029dd0491a36..2a1cb7b889e5 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -551,22 +551,46 @@ struct mddev {
 };
 
 enum recovery_flags {
+	/* flags for sync thread running status */
+
+	/*
+	 * set when one of sync action is set and new sync thread need to be
+	 * registered, or just add/remove spares from conf.
+	 */
+	MD_RECOVERY_NEEDED,
+	/* sync thread is running, or about to be started */
+	MD_RECOVERY_RUNNING,
+	/* sync thread needs to be aborted for some reason */
+	MD_RECOVERY_INTR,
+	/* sync thread is done and is waiting to be unregistered */
+	MD_RECOVERY_DONE,
+	/* running sync thread must abort immediately, and not restart */
+	MD_RECOVERY_FROZEN,
+	/* waiting for pers->start() to finish */
+	MD_RECOVERY_WAIT,
+	/* interrupted because io-error */
+	MD_RECOVERY_ERROR,
+
+	/* flags determines sync action */
+
+	/* if just this flag is set, action is resync. */
+	MD_RECOVERY_SYNC,
+	/*
+	 * paired with MD_RECOVERY_SYNC, if MD_RECOVERY_CHECK is not set,
+	 * action is repair, means user requested resync.
+	 */
+	MD_RECOVERY_REQUESTED,
 	/*
-	 * If neither SYNC or RESHAPE are set, then it is a recovery.
+	 * paired with MD_RECOVERY_SYNC and MD_RECOVERY_REQUESTED, action is
+	 * check.
 	 */
-	MD_RECOVERY_RUNNING,	/* a thread is running, or about to be started */
-	MD_RECOVERY_SYNC,	/* actually doing a resync, not a recovery */
-	MD_RECOVERY_RECOVER,	/* doing recovery, or need to try it. */
-	MD_RECOVERY_INTR,	/* resync needs to be aborted for some reason */
-	MD_RECOVERY_DONE,	/* thread is done and is waiting to be reaped */
-	MD_RECOVERY_NEEDED,	/* we might need to start a resync/recover */
-	MD_RECOVERY_REQUESTED,	/* user-space has requested a sync (used with SYNC) */
-	MD_RECOVERY_CHECK,	/* user-space request for check-only, no repair */
-	MD_RECOVERY_RESHAPE,	/* A reshape is happening */
-	MD_RECOVERY_FROZEN,	/* User request to abort, and not restart, any action */
-	MD_RECOVERY_ERROR,	/* sync-action interrupted because io-error */
-	MD_RECOVERY_WAIT,	/* waiting for pers->start() to finish */
-	MD_RESYNCING_REMOTE,	/* remote node is running resync thread */
+	MD_RECOVERY_CHECK,
+	/* recovery, or need to try it */
+	MD_RECOVERY_RECOVER,
+	/* reshape */
+	MD_RECOVERY_RESHAPE,
+	/* remote node is running resync thread */
+	MD_RESYNCING_REMOTE,
 };
 
 enum md_ro_state {
-- 
2.39.2
Re: [PATCH md-6.10 1/9] md: rearrange recovery_flage
Posted by Xiao Ni 1 year, 7 months ago
On Mon, May 13, 2024 at 9:57 AM Yu Kuai <yukuai1@huaweicloud.com> wrote:
>
> From: Yu Kuai <yukuai3@huawei.com>
>
> Currently there are lots of flags and the names are confusing, since
> there are two main types of flags, sync thread runnng status and sync
> thread action, rearrange and update comment to improve code readability,
> there are no functional changes.
>
> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
> ---
>  drivers/md/md.h | 52 ++++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 38 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/md/md.h b/drivers/md/md.h
> index 029dd0491a36..2a1cb7b889e5 100644
> --- a/drivers/md/md.h
> +++ b/drivers/md/md.h
> @@ -551,22 +551,46 @@ struct mddev {
>  };
>
>  enum recovery_flags {
> +       /* flags for sync thread running status */
> +
> +       /*
> +        * set when one of sync action is set and new sync thread need to be
> +        * registered, or just add/remove spares from conf.
> +        */
> +       MD_RECOVERY_NEEDED,
> +       /* sync thread is running, or about to be started */
> +       MD_RECOVERY_RUNNING,
> +       /* sync thread needs to be aborted for some reason */
> +       MD_RECOVERY_INTR,
> +       /* sync thread is done and is waiting to be unregistered */
> +       MD_RECOVERY_DONE,
> +       /* running sync thread must abort immediately, and not restart */
> +       MD_RECOVERY_FROZEN,
> +       /* waiting for pers->start() to finish */
> +       MD_RECOVERY_WAIT,
> +       /* interrupted because io-error */
> +       MD_RECOVERY_ERROR,
> +
> +       /* flags determines sync action */
> +
> +       /* if just this flag is set, action is resync. */
> +       MD_RECOVERY_SYNC,
> +       /*
> +        * paired with MD_RECOVERY_SYNC, if MD_RECOVERY_CHECK is not set,
> +        * action is repair, means user requested resync.
> +        */
> +       MD_RECOVERY_REQUESTED,
>         /*
> -        * If neither SYNC or RESHAPE are set, then it is a recovery.
> +        * paired with MD_RECOVERY_SYNC and MD_RECOVERY_REQUESTED, action is
> +        * check.

Hi Kuai

I did a test as follows:

echo check > /sys/block/md0/md/sync_action
I added some logs in md_do_sync to check these bits. It only prints
MD_RECOVERY_SYNC and MD_RECOVERY_CHECK without MD_RECOVERY_SYNC. So
the comment is not right?

Best Regards
Xiao

>          */
> -       MD_RECOVERY_RUNNING,    /* a thread is running, or about to be started */
> -       MD_RECOVERY_SYNC,       /* actually doing a resync, not a recovery */
> -       MD_RECOVERY_RECOVER,    /* doing recovery, or need to try it. */
> -       MD_RECOVERY_INTR,       /* resync needs to be aborted for some reason */
> -       MD_RECOVERY_DONE,       /* thread is done and is waiting to be reaped */
> -       MD_RECOVERY_NEEDED,     /* we might need to start a resync/recover */
> -       MD_RECOVERY_REQUESTED,  /* user-space has requested a sync (used with SYNC) */
> -       MD_RECOVERY_CHECK,      /* user-space request for check-only, no repair */
> -       MD_RECOVERY_RESHAPE,    /* A reshape is happening */
> -       MD_RECOVERY_FROZEN,     /* User request to abort, and not restart, any action */
> -       MD_RECOVERY_ERROR,      /* sync-action interrupted because io-error */
> -       MD_RECOVERY_WAIT,       /* waiting for pers->start() to finish */
> -       MD_RESYNCING_REMOTE,    /* remote node is running resync thread */
> +       MD_RECOVERY_CHECK,
> +       /* recovery, or need to try it */
> +       MD_RECOVERY_RECOVER,
> +       /* reshape */
> +       MD_RECOVERY_RESHAPE,
> +       /* remote node is running resync thread */
> +       MD_RESYNCING_REMOTE,
>  };
>
>  enum md_ro_state {
> --
> 2.39.2
>
Re: [PATCH md-6.10 1/9] md: rearrange recovery_flage
Posted by Yu Kuai 1 year, 7 months ago
Hi,

在 2024/05/14 13:51, Xiao Ni 写道:
> On Mon, May 13, 2024 at 9:57 AM Yu Kuai <yukuai1@huaweicloud.com> wrote:
>>
>> From: Yu Kuai <yukuai3@huawei.com>
>>
>> Currently there are lots of flags and the names are confusing, since
>> there are two main types of flags, sync thread runnng status and sync
>> thread action, rearrange and update comment to improve code readability,
>> there are no functional changes.
>>
>> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
>> ---
>>   drivers/md/md.h | 52 ++++++++++++++++++++++++++++++++++++-------------
>>   1 file changed, 38 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/md/md.h b/drivers/md/md.h
>> index 029dd0491a36..2a1cb7b889e5 100644
>> --- a/drivers/md/md.h
>> +++ b/drivers/md/md.h
>> @@ -551,22 +551,46 @@ struct mddev {
>>   };
>>
>>   enum recovery_flags {
>> +       /* flags for sync thread running status */
>> +
>> +       /*
>> +        * set when one of sync action is set and new sync thread need to be
>> +        * registered, or just add/remove spares from conf.
>> +        */
>> +       MD_RECOVERY_NEEDED,
>> +       /* sync thread is running, or about to be started */
>> +       MD_RECOVERY_RUNNING,
>> +       /* sync thread needs to be aborted for some reason */
>> +       MD_RECOVERY_INTR,
>> +       /* sync thread is done and is waiting to be unregistered */
>> +       MD_RECOVERY_DONE,
>> +       /* running sync thread must abort immediately, and not restart */
>> +       MD_RECOVERY_FROZEN,
>> +       /* waiting for pers->start() to finish */
>> +       MD_RECOVERY_WAIT,
>> +       /* interrupted because io-error */
>> +       MD_RECOVERY_ERROR,
>> +
>> +       /* flags determines sync action */
>> +
>> +       /* if just this flag is set, action is resync. */
>> +       MD_RECOVERY_SYNC,
>> +       /*
>> +        * paired with MD_RECOVERY_SYNC, if MD_RECOVERY_CHECK is not set,
>> +        * action is repair, means user requested resync.
>> +        */
>> +       MD_RECOVERY_REQUESTED,
>>          /*
>> -        * If neither SYNC or RESHAPE are set, then it is a recovery.
>> +        * paired with MD_RECOVERY_SYNC and MD_RECOVERY_REQUESTED, action is
>> +        * check.
> 
> Hi Kuai
> 
> I did a test as follows:
> 
> echo check > /sys/block/md0/md/sync_action
> I added some logs in md_do_sync to check these bits. It only prints
> MD_RECOVERY_SYNC and MD_RECOVERY_CHECK without MD_RECOVERY_SYNC. So
> the comment is not right?

There is a typo, I'm not sure what you mean yet. Can you show how you
add your logs? I think comment is right.

 From action_store:

                 if (cmd_match(page, "check"))
                         set_bit(MD_RECOVERY_CHECK, &mddev->recovery);
                 else if (!cmd_match(page, "repair"))
                         return -EINVAL;
                 clear_bit(MD_RECOVERY_FROZEN, &mddev->recovery);
                 set_bit(MD_RECOVERY_REQUESTED, &mddev->recovery);
                 set_bit(MD_RECOVERY_SYNC, &mddev->recovery);

 From md_do_sync:

         if (test_bit(MD_RECOVERY_SYNC, &mddev->recovery)) {
                 if (test_bit(MD_RECOVERY_CHECK, &mddev->recovery)) {
                         desc = "data-check";
                         action = "check";
                 } else if (test_bit(MD_RECOVERY_REQUESTED, 
&mddev->recovery)) {
                         desc = "requested-resync";
                         action = "repair";
                 } else
                         desc = "resync";

Thanks,
Kuai

> 
> Best Regards
> Xiao
> 
>>           */
>> -       MD_RECOVERY_RUNNING,    /* a thread is running, or about to be started */
>> -       MD_RECOVERY_SYNC,       /* actually doing a resync, not a recovery */
>> -       MD_RECOVERY_RECOVER,    /* doing recovery, or need to try it. */
>> -       MD_RECOVERY_INTR,       /* resync needs to be aborted for some reason */
>> -       MD_RECOVERY_DONE,       /* thread is done and is waiting to be reaped */
>> -       MD_RECOVERY_NEEDED,     /* we might need to start a resync/recover */
>> -       MD_RECOVERY_REQUESTED,  /* user-space has requested a sync (used with SYNC) */
>> -       MD_RECOVERY_CHECK,      /* user-space request for check-only, no repair */
>> -       MD_RECOVERY_RESHAPE,    /* A reshape is happening */
>> -       MD_RECOVERY_FROZEN,     /* User request to abort, and not restart, any action */
>> -       MD_RECOVERY_ERROR,      /* sync-action interrupted because io-error */
>> -       MD_RECOVERY_WAIT,       /* waiting for pers->start() to finish */
>> -       MD_RESYNCING_REMOTE,    /* remote node is running resync thread */
>> +       MD_RECOVERY_CHECK,
>> +       /* recovery, or need to try it */
>> +       MD_RECOVERY_RECOVER,
>> +       /* reshape */
>> +       MD_RECOVERY_RESHAPE,
>> +       /* remote node is running resync thread */
>> +       MD_RESYNCING_REMOTE,
>>   };
>>
>>   enum md_ro_state {
>> --
>> 2.39.2
>>
> 
> 
> .
> 

Re: [PATCH md-6.10 1/9] md: rearrange recovery_flage
Posted by Xiao Ni 1 year, 7 months ago
On Tue, May 14, 2024 at 2:16 PM Yu Kuai <yukuai1@huaweicloud.com> wrote:
>
> Hi,
>
> 在 2024/05/14 13:51, Xiao Ni 写道:
> > On Mon, May 13, 2024 at 9:57 AM Yu Kuai <yukuai1@huaweicloud.com> wrote:
> >>
> >> From: Yu Kuai <yukuai3@huawei.com>
> >>
> >> Currently there are lots of flags and the names are confusing, since
> >> there are two main types of flags, sync thread runnng status and sync
> >> thread action, rearrange and update comment to improve code readability,
> >> there are no functional changes.
> >>
> >> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
> >> ---
> >>   drivers/md/md.h | 52 ++++++++++++++++++++++++++++++++++++-------------
> >>   1 file changed, 38 insertions(+), 14 deletions(-)
> >>
> >> diff --git a/drivers/md/md.h b/drivers/md/md.h
> >> index 029dd0491a36..2a1cb7b889e5 100644
> >> --- a/drivers/md/md.h
> >> +++ b/drivers/md/md.h
> >> @@ -551,22 +551,46 @@ struct mddev {
> >>   };
> >>
> >>   enum recovery_flags {
> >> +       /* flags for sync thread running status */
> >> +
> >> +       /*
> >> +        * set when one of sync action is set and new sync thread need to be
> >> +        * registered, or just add/remove spares from conf.
> >> +        */
> >> +       MD_RECOVERY_NEEDED,
> >> +       /* sync thread is running, or about to be started */
> >> +       MD_RECOVERY_RUNNING,
> >> +       /* sync thread needs to be aborted for some reason */
> >> +       MD_RECOVERY_INTR,
> >> +       /* sync thread is done and is waiting to be unregistered */
> >> +       MD_RECOVERY_DONE,
> >> +       /* running sync thread must abort immediately, and not restart */
> >> +       MD_RECOVERY_FROZEN,
> >> +       /* waiting for pers->start() to finish */
> >> +       MD_RECOVERY_WAIT,
> >> +       /* interrupted because io-error */
> >> +       MD_RECOVERY_ERROR,
> >> +
> >> +       /* flags determines sync action */
> >> +
> >> +       /* if just this flag is set, action is resync. */
> >> +       MD_RECOVERY_SYNC,
> >> +       /*
> >> +        * paired with MD_RECOVERY_SYNC, if MD_RECOVERY_CHECK is not set,
> >> +        * action is repair, means user requested resync.
> >> +        */
> >> +       MD_RECOVERY_REQUESTED,
> >>          /*
> >> -        * If neither SYNC or RESHAPE are set, then it is a recovery.
> >> +        * paired with MD_RECOVERY_SYNC and MD_RECOVERY_REQUESTED, action is
> >> +        * check.
> >
> > Hi Kuai
> >
> > I did a test as follows:
> >
> > echo check > /sys/block/md0/md/sync_action
> > I added some logs in md_do_sync to check these bits. It only prints
> > MD_RECOVERY_SYNC and MD_RECOVERY_CHECK without MD_RECOVERY_SYNC. So
> > the comment is not right?
>
> There is a typo, I'm not sure what you mean yet. Can you show how you
> add your logs? I think comment is right.

Sorry. I added logs in the wrong places. You're right. Data check has
the three bits. So, the comments are right.


>
>  From action_store:
>
>                  if (cmd_match(page, "check"))
>                          set_bit(MD_RECOVERY_CHECK, &mddev->recovery);
>                  else if (!cmd_match(page, "repair"))
>                          return -EINVAL;
>                  clear_bit(MD_RECOVERY_FROZEN, &mddev->recovery);
>                  set_bit(MD_RECOVERY_REQUESTED, &mddev->recovery);
>                  set_bit(MD_RECOVERY_SYNC, &mddev->recovery);

Yes.

Best Regards
Xiao

>
>  From md_do_sync:
>
>          if (test_bit(MD_RECOVERY_SYNC, &mddev->recovery)) {
>                  if (test_bit(MD_RECOVERY_CHECK, &mddev->recovery)) {
>                          desc = "data-check";
>                          action = "check";
>                  } else if (test_bit(MD_RECOVERY_REQUESTED,
> &mddev->recovery)) {
>                          desc = "requested-resync";
>                          action = "repair";
>                  } else
>                          desc = "resync";
>
> Thanks,
> Kuai
>
> >
> > Best Regards
> > Xiao
> >
> >>           */
> >> -       MD_RECOVERY_RUNNING,    /* a thread is running, or about to be started */
> >> -       MD_RECOVERY_SYNC,       /* actually doing a resync, not a recovery */
> >> -       MD_RECOVERY_RECOVER,    /* doing recovery, or need to try it. */
> >> -       MD_RECOVERY_INTR,       /* resync needs to be aborted for some reason */
> >> -       MD_RECOVERY_DONE,       /* thread is done and is waiting to be reaped */
> >> -       MD_RECOVERY_NEEDED,     /* we might need to start a resync/recover */
> >> -       MD_RECOVERY_REQUESTED,  /* user-space has requested a sync (used with SYNC) */
> >> -       MD_RECOVERY_CHECK,      /* user-space request for check-only, no repair */
> >> -       MD_RECOVERY_RESHAPE,    /* A reshape is happening */
> >> -       MD_RECOVERY_FROZEN,     /* User request to abort, and not restart, any action */
> >> -       MD_RECOVERY_ERROR,      /* sync-action interrupted because io-error */
> >> -       MD_RECOVERY_WAIT,       /* waiting for pers->start() to finish */
> >> -       MD_RESYNCING_REMOTE,    /* remote node is running resync thread */
> >> +       MD_RECOVERY_CHECK,
> >> +       /* recovery, or need to try it */
> >> +       MD_RECOVERY_RECOVER,
> >> +       /* reshape */
> >> +       MD_RECOVERY_RESHAPE,
> >> +       /* remote node is running resync thread */
> >> +       MD_RESYNCING_REMOTE,
> >>   };
> >>
> >>   enum md_ro_state {
> >> --
> >> 2.39.2
> >>
> >
> >
> > .
> >
>
>
Re: [PATCH md-6.10 1/9] md: rearrange recovery_flage
Posted by Mariusz Tkaczyk 1 year, 7 months ago
On Thu,  9 May 2024 09:18:52 +0800
Yu Kuai <yukuai1@huaweicloud.com> wrote:

There is typo in subject.

> From: Yu Kuai <yukuai3@huawei.com>
> 
> Currently there are lots of flags and the names are confusing, since
> there are two main types of flags, sync thread runnng status and sync
> thread action, rearrange and update comment to improve code readability,
> there are no functional changes.
> 
> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
> ---
>  drivers/md/md.h | 52 ++++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 38 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/md/md.h b/drivers/md/md.h
> index 029dd0491a36..2a1cb7b889e5 100644
> --- a/drivers/md/md.h
> +++ b/drivers/md/md.h
> @@ -551,22 +551,46 @@ struct mddev {
>  };
>  
>  enum recovery_flags {
> +	/* flags for sync thread running status */
> +
> +	/*
> +	 * set when one of sync action is set and new sync thread need to be
> +	 * registered, or just add/remove spares from conf.
> +	 */
> +	MD_RECOVERY_NEEDED,
> +	/* sync thread is running, or about to be started */
> +	MD_RECOVERY_RUNNING,
> +	/* sync thread needs to be aborted for some reason */
> +	MD_RECOVERY_INTR,
> +	/* sync thread is done and is waiting to be unregistered */
> +	MD_RECOVERY_DONE,
> +	/* running sync thread must abort immediately, and not restart */
> +	MD_RECOVERY_FROZEN,
> +	/* waiting for pers->start() to finish */
> +	MD_RECOVERY_WAIT,
> +	/* interrupted because io-error */
> +	MD_RECOVERY_ERROR,
> +
> +	/* flags determines sync action */
> +
> +	/* if just this flag is set, action is resync. */
> +	MD_RECOVERY_SYNC,
> +	/*
> +	 * paired with MD_RECOVERY_SYNC, if MD_RECOVERY_CHECK is not set,
> +	 * action is repair, means user requested resync.
> +	 */
> +	MD_RECOVERY_REQUESTED,
>  	/*
> -	 * If neither SYNC or RESHAPE are set, then it is a recovery.
> +	 * paired with MD_RECOVERY_SYNC and MD_RECOVERY_REQUESTED, action is
> +	 * check.
>  	 */
> -	MD_RECOVERY_RUNNING,	/* a thread is running, or about to be
> started */
> -	MD_RECOVERY_SYNC,	/* actually doing a resync, not a recovery
> */
> -	MD_RECOVERY_RECOVER,	/* doing recovery, or need to try it. */
> -	MD_RECOVERY_INTR,	/* resync needs to be aborted for some
> reason */
> -	MD_RECOVERY_DONE,	/* thread is done and is waiting to be
> reaped */
> -	MD_RECOVERY_NEEDED,	/* we might need to start a
> resync/recover */
> -	MD_RECOVERY_REQUESTED,	/* user-space has requested a sync
> (used with SYNC) */
> -	MD_RECOVERY_CHECK,	/* user-space request for check-only, no
> repair */
> -	MD_RECOVERY_RESHAPE,	/* A reshape is happening */
> -	MD_RECOVERY_FROZEN,	/* User request to abort, and not
> restart, any action */
> -	MD_RECOVERY_ERROR,	/* sync-action interrupted because
> io-error */
> -	MD_RECOVERY_WAIT,	/* waiting for pers->start() to finish */
> -	MD_RESYNCING_REMOTE,	/* remote node is running resync thread
> */
> +	MD_RECOVERY_CHECK,
> +	/* recovery, or need to try it */
> +	MD_RECOVERY_RECOVER,
> +	/* reshape */
> +	MD_RECOVERY_RESHAPE,
> +	/* remote node is running resync thread */
> +	MD_RESYNCING_REMOTE,
>  };
>  
>  enum md_ro_state {

I don't know if it is better readable but I know that Kernel coding style comes
with different approach. I used it for enum mddev_flags in md.h please take a
look.

Also, I get used to comment above, not below enum values but I don't have strong
justification here.

Thanks,
Mariusz
Re: [PATCH md-6.10 1/9] md: rearrange recovery_flage
Posted by Yu Kuai 1 year, 7 months ago
Hi,

在 2024/05/13 23:12, Mariusz Tkaczyk 写道:
> On Thu,  9 May 2024 09:18:52 +0800
> Yu Kuai <yukuai1@huaweicloud.com> wrote:
> 
> There is typo in subject.

Yes, :)
> 
>> From: Yu Kuai <yukuai3@huawei.com>
>>
>> Currently there are lots of flags and the names are confusing, since
>> there are two main types of flags, sync thread runnng status and sync
>> thread action, rearrange and update comment to improve code readability,
>> there are no functional changes.
>>
>> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
>> ---
>>   drivers/md/md.h | 52 ++++++++++++++++++++++++++++++++++++-------------
>>   1 file changed, 38 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/md/md.h b/drivers/md/md.h
>> index 029dd0491a36..2a1cb7b889e5 100644
>> --- a/drivers/md/md.h
>> +++ b/drivers/md/md.h
>> @@ -551,22 +551,46 @@ struct mddev {
>>   };
>>   
>>   enum recovery_flags {
>> +	/* flags for sync thread running status */
>> +
>> +	/*
>> +	 * set when one of sync action is set and new sync thread need to be
>> +	 * registered, or just add/remove spares from conf.
>> +	 */
>> +	MD_RECOVERY_NEEDED,
>> +	/* sync thread is running, or about to be started */
>> +	MD_RECOVERY_RUNNING,
>> +	/* sync thread needs to be aborted for some reason */
>> +	MD_RECOVERY_INTR,
>> +	/* sync thread is done and is waiting to be unregistered */
>> +	MD_RECOVERY_DONE,
>> +	/* running sync thread must abort immediately, and not restart */
>> +	MD_RECOVERY_FROZEN,
>> +	/* waiting for pers->start() to finish */
>> +	MD_RECOVERY_WAIT,
>> +	/* interrupted because io-error */
>> +	MD_RECOVERY_ERROR,
>> +
>> +	/* flags determines sync action */
>> +
>> +	/* if just this flag is set, action is resync. */
>> +	MD_RECOVERY_SYNC,
>> +	/*
>> +	 * paired with MD_RECOVERY_SYNC, if MD_RECOVERY_CHECK is not set,
>> +	 * action is repair, means user requested resync.
>> +	 */
>> +	MD_RECOVERY_REQUESTED,
>>   	/*
>> -	 * If neither SYNC or RESHAPE are set, then it is a recovery.
>> +	 * paired with MD_RECOVERY_SYNC and MD_RECOVERY_REQUESTED, action is
>> +	 * check.
>>   	 */
>> -	MD_RECOVERY_RUNNING,	/* a thread is running, or about to be
>> started */
>> -	MD_RECOVERY_SYNC,	/* actually doing a resync, not a recovery
>> */
>> -	MD_RECOVERY_RECOVER,	/* doing recovery, or need to try it. */
>> -	MD_RECOVERY_INTR,	/* resync needs to be aborted for some
>> reason */
>> -	MD_RECOVERY_DONE,	/* thread is done and is waiting to be
>> reaped */
>> -	MD_RECOVERY_NEEDED,	/* we might need to start a
>> resync/recover */
>> -	MD_RECOVERY_REQUESTED,	/* user-space has requested a sync
>> (used with SYNC) */
>> -	MD_RECOVERY_CHECK,	/* user-space request for check-only, no
>> repair */
>> -	MD_RECOVERY_RESHAPE,	/* A reshape is happening */
>> -	MD_RECOVERY_FROZEN,	/* User request to abort, and not
>> restart, any action */
>> -	MD_RECOVERY_ERROR,	/* sync-action interrupted because
>> io-error */
>> -	MD_RECOVERY_WAIT,	/* waiting for pers->start() to finish */
>> -	MD_RESYNCING_REMOTE,	/* remote node is running resync thread
>> */
>> +	MD_RECOVERY_CHECK,
>> +	/* recovery, or need to try it */
>> +	MD_RECOVERY_RECOVER,
>> +	/* reshape */
>> +	MD_RECOVERY_RESHAPE,
>> +	/* remote node is running resync thread */
>> +	MD_RESYNCING_REMOTE,
>>   };
>>   
>>   enum md_ro_state {
> 
> I don't know if it is better readable but I know that Kernel coding style comes
> with different approach. I used it for enum mddev_flags in md.h please take a
> look.

1) There are two kinds of flags here, for sync thread running status and
for sync action, I think it's better to distinguish them.
2) The names are confusing, for sync thread status I prefer
SYNC_THREAD_xxx and for sync action I prefer SYNC_ACTION_xxx, I plan to
remove the flags to sync action and replace them with enum type in patch
2.

> 
> Also, I get used to comment above, not below enum values but I don't have strong
> justification here.

I do comment above here.

Thanks,
Kuai

> 
> Thanks,
> Mariusz
> 
> 
> .
>