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

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

[Shaohua Li]

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);
 	} else if (mddev->pers == NULL) {
 		/* Insist of good event counter while assembling, except for
 		 * spares (which don't need an event count) */
@@ -1653,8 +1657,6 @@ 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);
 			rdev->raid_disk = 0;
 			break;
 		default:
@@ -1674,8 +1676,6 @@ static int super_1_validate(struct mddev *mddev, struct md_rdev *rdev)
 			set_bit(WriteMostly, &rdev->flags);
 		if (le32_to_cpu(sb->feature_map) & MD_FEATURE_REPLACEMENT)
 			set_bit(Replacement, &rdev->flags);
-		if (le32_to_cpu(sb->feature_map) & MD_FEATURE_JOURNAL)
-			set_bit(MD_HAS_JOURNAL, &mddev->flags);
 	} else /* MULTIPATH are always insync */
 		set_bit(In_sync, &rdev->flags);
 
diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
index 31e0fad..16cf876 100644
--- a/drivers/md/raid5-cache.c
+++ b/drivers/md/raid5-cache.c
@@ -1242,6 +1242,7 @@ int r5l_init_log(struct r5conf *conf, struct md_rdev *rdev)
 		goto error;
 
 	rcu_assign_pointer(conf->log, log);
+	set_bit(MD_HAS_JOURNAL, &conf->mddev->flags);
 	return 0;
 
 error:
-- 
2.4.6

[PATCH 2/3] MD: add journal with array suspended

[Shaohua Li]

Hot add journal disk in recovery thread context brings a lot of trouble
as IO could be running. Unlike spare disk hot add, adding journal disk
with array suspended makes more sense and implmentation is much easier.

Signed-off-by: Shaohua Li <shli@fb.com>
---
 drivers/md/md.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index e896320..dad9a25 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -2464,15 +2464,20 @@ static int add_bound_rdev(struct md_rdev *rdev)
 {
 	struct mddev *mddev = rdev->mddev;
 	int err = 0;
+	bool add_journal = test_bit(Journal, &rdev->flags);
 
-	if (!mddev->pers->hot_remove_disk) {
+	if (!mddev->pers->hot_remove_disk || add_journal) {
 		/* If there is hot_add_disk but no hot_remove_disk
 		 * then added disks for geometry changes,
 		 * and should be added immediately.
 		 */
 		super_types[mddev->major_version].
 			validate_super(mddev, rdev);
+		if (add_journal)
+			mddev_suspend(mddev);
 		err = mddev->pers->hot_add_disk(mddev, rdev);
+		if (add_journal)
+			mddev_resume(mddev);
 		if (err) {
 			unbind_rdev_from_array(rdev);
 			export_rdev(rdev);
-- 
2.4.6

[PATCH 3/3] raid5-cache: handle journal hotadd in quiesce

[Shaohua Li]

Handle journal hotadd in quiesce to avoid creating duplicated threads.

Signed-off-by: Shaohua Li <shli@fb.com>
---
 drivers/md/raid5-cache.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
index 16cf876..4e36eec 100644
--- a/drivers/md/raid5-cache.c
+++ b/drivers/md/raid5-cache.c
@@ -834,6 +834,13 @@ void r5l_quiesce(struct r5conf *conf, int state)
 		return;
 	if (state == 0) {
 		log->in_teardown = 0;
+		/*
+		 * This is a special case for hotadd. In suspend, the array has
+		 * no journal. In resume, journal is initialized as well as the
+		 * reclaim thread.
+		 */
+		if (log->reclaim_thread)
+			return;
 		log->reclaim_thread = md_register_thread(r5l_reclaim_thread,
 					log->rdev->mddev, "reclaim");
 	} else if (state == 1) {
-- 
2.4.6

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

[NeilBrown]

[-- 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 --]