Re: [PATCH 7/8] md: skip resync for raid array with journal

[Neil Brown <neilb@suse.de>]

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

Shaohua Li <shli@fb.com> writes:

> If a raid array has journal, the journal can guarantee the consistency,
> we can skip resync after a unclean shutdown. The exception is raid
> creation or user initiated resync, which we still do a raid resync.
>
> Signed-off-by: Shaohua Li <shli@fb.com>
> ---
>  drivers/md/md.c | 4 ++++
>  drivers/md/md.h | 1 +
>  2 files changed, 5 insertions(+)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index b3f9eed..95824fb 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -1669,6 +1669,8 @@ static int super_1_validate(struct mddev *mddev, struct md_rdev *rdev)
>  			}
>  			set_bit(Journal, &rdev->flags);
>  			rdev->journal_tail = le64_to_cpu(sb->journal_tail);
> +			if (mddev->recovery_cp == MaxSector)
> +				set_bit(MD_JOURNAL_CLEAN, &mddev->flags);
>  			break;
>  		default:
>  			rdev->saved_raid_disk = role;
> @@ -1711,6 +1713,8 @@ static void super_1_sync(struct mddev *mddev, struct md_rdev *rdev)
>  	sb->events = cpu_to_le64(mddev->events);
>  	if (mddev->in_sync)
>  		sb->resync_offset = cpu_to_le64(mddev->recovery_cp);
> +	else if (test_bit(MD_JOURNAL_CLEAN, &mddev->flags))
> +		sb->resync_offset = cpu_to_le64(MaxSector);
>  	else
>  		sb->resync_offset = cpu_to_le64(0);
>  
> diff --git a/drivers/md/md.h b/drivers/md/md.h
> index 226f4ba..0288a0b 100644
> --- a/drivers/md/md.h
> +++ b/drivers/md/md.h
> @@ -236,6 +236,7 @@ struct mddev {
>  #define MD_STILL_CLOSED	4	/* If set, then array has not been opened since
>  				 * md_ioctl checked on it.
>  				 */
> +#define MD_JOURNAL_CLEAN 5	/* A raid with journal is already clean */
>  
>  	int				suspended;
>  	atomic_t			active_io;
> -- 
> 1.8.1

This looks right as far as it goes, but I don't think it goes far
enough.
The particular scenario that bothers me is if the array is started
without the journal being present.
I cannot see anything to prevent that - is there?

In that case we need to assume that the array is not in-sync,
and we need to clear MD_FEATURE_JOURNAL so if it gets stopped and then
assembled again with the stale journal doesn't get used.

One unfortunate side effect of that is that you couldn't stop the array
cleanly (leaving the journal effectively empty) and then restart with no
journal and no resync.  Is that a problem I wonder?

I'm not sure what the best solution is here, but we need a clear
understanding of what happens if you try to assemble an array without
the journal where previously it had one, and I don't think the current
code gets it right.

Thanks,
NeilBrown

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