Re: [PATCH 1/3] md: set MD_HAS_JOURNAL in correct places

[NeilBrown <neilb@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 1299 bytes --]

On Thu, Jan 07 2016, Shaohua Li wrote:

> Set MD_HAS_JOURNAL when a array is loaded or journal is initialized.
> This is to avoid the flags set too early in journal disk hotadd.
>
> Signed-off-by: Shaohua Li <shli@fb.com>
> ---
>  drivers/md/md.c          | 8 ++++----
>  drivers/md/raid5-cache.c | 1 +
>  2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index c0c3e6d..e896320 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -1607,6 +1607,10 @@ static int super_1_validate(struct mddev *mddev, struct md_rdev *rdev)
>  			mddev->new_chunk_sectors = mddev->chunk_sectors;
>  		}
>  
> +		if (mddev->recovery_cp == MaxSector)
> +			set_bit(MD_JOURNAL_CLEAN, &mddev->flags);
> +		if (le32_to_cpu(sb->feature_map) & MD_FEATURE_JOURNAL)
> +			set_bit(MD_HAS_JOURNAL, &mddev->flags);

This looks wrong.  I don't think it is safe to set MD_JOURNAL_CLEAR if
MD_HAS_JOURNAL is not set.
So I rearranged the code to:

		if (le32_to_cpu(sb->feature_map) & MD_FEATURE_JOURNAL) {
			set_bit(MD_HAS_JOURNAL, &mddev->flags);
			if (mddev->recovery_cp == MaxSector)
				set_bit(MD_JOURNAL_CLEAN, &mddev->flags);
		}

Please say if you disagree.

I've applied this and the other two patches - thanks.

NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]