[PATCH 1/3] Return error if request_module fails and returns positive value

[PATCH 1/3] Return error if request_module fails and returns positive value

[Goldwyn Rodrigues]

request_module() can return 256 (process exited) in some cases,
which is not as specified in the documentation before the
request_module() definition. Convert the error to -ENOENT.

This fixes edb39c9deda8 ("Introduce md_cluster_operations to
handle cluster functions")

Signed-off-By: Goldwyn Rodrigues <rgoldwyn@suse.com>
---
 drivers/md/md.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index d429c30..7a7870f 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -7437,7 +7437,7 @@ int md_setup_cluster(struct mddev *mddev, int nodes)
 	err = request_module("md-cluster");
 	if (err) {
 		pr_err("md-cluster module not found.\n");
-		return err;
+		return -ENOENT;
 	}
 
 	spin_lock(&pers_lock);
-- 
2.1.4

[PATCH 2/3] Skip cluster setup in case of error while reading bitmap

[Goldwyn Rodrigues]

If the bitmap read fails, the error code set is -EINVAL. However,
we don't check for errors and go ahead with cluster_setup.
Skip the cluster setup in case of error.

Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
---
 drivers/md/bitmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/md/bitmap.c b/drivers/md/bitmap.c
index ed2346d..f23b8e4 100644
--- a/drivers/md/bitmap.c
+++ b/drivers/md/bitmap.c
@@ -671,7 +671,7 @@ out:
 	kunmap_atomic(sb);
 	/* Assiging chunksize is required for "re_read" */
 	bitmap->mddev->bitmap_info.chunksize = chunksize;
-	if (nodes && (bitmap->cluster_slot < 0)) {
+	if (err == 0 && nodes && (bitmap->cluster_slot < 0)) {
 		err = md_setup_cluster(bitmap->mddev, nodes);
 		if (err) {
 			pr_err("%s: Could not setup cluster service (%d)\n",
-- 
2.1.4

[PATCH 3/3] Skip cluster setup for dm-raid

[Goldwyn Rodrigues]

There is a bug that the bitmap superblock isn't initialised properly for
dm-raid, so a new field can have garbage in new fields.
(dm-raid does initialisation in the kernel - md initialised the
 superblock in mdadm).

This means that for dm-raid we cannot currently trust the new ->nodes
field. So:
 - use __GFP_ZERO to initialise the superblock properly for all new
    arrays
 - initialise all field in bitmap_info in bitmap_new_disk_sb
 - ignore ->nodes for dm arrays (yes, this is a hack)

References: https://bugzilla.kernel.org/show_bug.cgi?id=100491

Signed-off-By: Goldwyn Rodrigues <rgoldwyn@suse.com>
---
 drivers/md/bitmap.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/md/bitmap.c b/drivers/md/bitmap.c
index f23b8e4..7ff37e0 100644
--- a/drivers/md/bitmap.c
+++ b/drivers/md/bitmap.c
@@ -494,7 +494,7 @@ static int bitmap_new_disk_sb(struct bitmap *bitmap)
 	bitmap_super_t *sb;
 	unsigned long chunksize, daemon_sleep, write_behind;
 
-	bitmap->storage.sb_page = alloc_page(GFP_KERNEL);
+	bitmap->storage.sb_page = alloc_page(GFP_KERNEL | __GFP_ZERO);
 	if (bitmap->storage.sb_page == NULL)
 		return -ENOMEM;
 	bitmap->storage.sb_page->index = 0;
@@ -541,6 +541,7 @@ static int bitmap_new_disk_sb(struct bitmap *bitmap)
 	sb->state = cpu_to_le32(bitmap->flags);
 	bitmap->events_cleared = bitmap->mddev->events;
 	sb->events_cleared = cpu_to_le64(bitmap->mddev->events);
+	bitmap->mddev->bitmap_info.nodes = 0;
 
 	kunmap_atomic(sb);
 
@@ -611,8 +612,16 @@ re_read:
 	daemon_sleep = le32_to_cpu(sb->daemon_sleep) * HZ;
 	write_behind = le32_to_cpu(sb->write_behind);
 	sectors_reserved = le32_to_cpu(sb->sectors_reserved);
-	nodes = le32_to_cpu(sb->nodes);
-	strlcpy(bitmap->mddev->bitmap_info.cluster_name, sb->cluster_name, 64);
+	/* XXX: This is a hack to ensure that we don't use clustering
+	 *  in case:
+	 *	- dm-raid is in use and
+	 *	- the nodes written in bitmap_sb is erroneous.
+	 */
+	if (!bitmap->mddev->sync_super) {
+		nodes = le32_to_cpu(sb->nodes);
+		strlcpy(bitmap->mddev->bitmap_info.cluster_name,
+				sb->cluster_name, 64);
+	}
 
 	/* verify that the bitmap-specific fields are valid */
 	if (sb->magic != cpu_to_le32(BITMAP_MAGIC))
-- 
2.1.4

Re: [PATCH 3/3] Skip cluster setup for dm-raid

[NeilBrown]

On Wed, 22 Jul 2015 12:09:17 -0500 Goldwyn Rodrigues
<rgoldwyn@suse.com> wrote:

> There is a bug that the bitmap superblock isn't initialised properly for
> dm-raid, so a new field can have garbage in new fields.
> (dm-raid does initialisation in the kernel - md initialised the
>  superblock in mdadm).
> 
> This means that for dm-raid we cannot currently trust the new ->nodes
> field. So:
>  - use __GFP_ZERO to initialise the superblock properly for all new
>     arrays
>  - initialise all field in bitmap_info in bitmap_new_disk_sb
>  - ignore ->nodes for dm arrays (yes, this is a hack)
> 
> References: https://bugzilla.kernel.org/show_bug.cgi?id=100491
> 
> Signed-off-By: Goldwyn Rodrigues <rgoldwyn@suse.com>
> ---
>  drivers/md/bitmap.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/md/bitmap.c b/drivers/md/bitmap.c
> index f23b8e4..7ff37e0 100644
> --- a/drivers/md/bitmap.c
> +++ b/drivers/md/bitmap.c
> @@ -494,7 +494,7 @@ static int bitmap_new_disk_sb(struct bitmap *bitmap)
>  	bitmap_super_t *sb;
>  	unsigned long chunksize, daemon_sleep, write_behind;
>  
> -	bitmap->storage.sb_page = alloc_page(GFP_KERNEL);
> +	bitmap->storage.sb_page = alloc_page(GFP_KERNEL | __GFP_ZERO);
>  	if (bitmap->storage.sb_page == NULL)
>  		return -ENOMEM;
>  	bitmap->storage.sb_page->index = 0;
> @@ -541,6 +541,7 @@ static int bitmap_new_disk_sb(struct bitmap *bitmap)
>  	sb->state = cpu_to_le32(bitmap->flags);
>  	bitmap->events_cleared = bitmap->mddev->events;
>  	sb->events_cleared = cpu_to_le64(bitmap->mddev->events);
> +	bitmap->mddev->bitmap_info.nodes = 0;
>  
>  	kunmap_atomic(sb);
>  
> @@ -611,8 +612,16 @@ re_read:
>  	daemon_sleep = le32_to_cpu(sb->daemon_sleep) * HZ;
>  	write_behind = le32_to_cpu(sb->write_behind);
>  	sectors_reserved = le32_to_cpu(sb->sectors_reserved);
> -	nodes = le32_to_cpu(sb->nodes);
> -	strlcpy(bitmap->mddev->bitmap_info.cluster_name, sb->cluster_name, 64);
> +	/* XXX: This is a hack to ensure that we don't use clustering
> +	 *  in case:
> +	 *	- dm-raid is in use and
> +	 *	- the nodes written in bitmap_sb is erroneous.
> +	 */
> +	if (!bitmap->mddev->sync_super) {
> +		nodes = le32_to_cpu(sb->nodes);
> +		strlcpy(bitmap->mddev->bitmap_info.cluster_name,
> +				sb->cluster_name, 64);
> +	}
>  
>  	/* verify that the bitmap-specific fields are valid */
>  	if (sb->magic != cpu_to_le32(BITMAP_MAGIC))


Thanks for these.
I've applied them all and marked this one for -stable ... it should stop
the cluster code from being touched by mistake.

NeilBrown