[PATCH] md/raid0: add config parameters to specify zone layout

[PATCH] md/raid0: add config parameters to specify zone layout

[Jason Baron]

Let's add some CONFIG_* options to directly configure the raid0 layout
if you know in advance how your raid0 array was created. This can be
simpler than having to manage module or kernel command-line parameters.

If the raid0 array was created by a pre-3.14 kernel, use
RAID0_ORIG_LAYOUT. If the raid0 array was created by a 3.14 or newer
kernel then select RAID0_ALT_MULTIZONE_LAYOUT. Otherwise, the default
setting is RAID0_LAYOUT_NONE, in which case the current behavior of
needing to specify a module parameter raid0.default_layout=1|2 is
preserved.

Cc: Guoqing Jiang <guoqing.jiang@cloud.ionos.com>
Cc: NeilBrown <neilb@suse.de>
Cc: Song Liu <songliubraving@fb.com>
Signed-off-by: Jason Baron <jbaron@akamai.com>
---
 drivers/md/Kconfig | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/md/raid0.c |  7 +++++++
 2 files changed, 62 insertions(+)

diff --git a/drivers/md/Kconfig b/drivers/md/Kconfig
index d6d5ab2..c0c6d82 100644
--- a/drivers/md/Kconfig
+++ b/drivers/md/Kconfig
@@ -79,6 +79,61 @@ config MD_RAID0
 
 	  If unsure, say Y.
 
+choice
+	prompt "RAID0 Layout"
+	default RAID0_LAYOUT_NONE
+	depends on MD_RAID0
+	help
+	  A change was made in Linux 3.14 that unintentinally changed the
+	  the layout for RAID0. This can result in data corruption if a pre-3.14
+	  and a 3.14 or later kernel both wrote to the array. However, if the
+	  devices in the array are all of the same size then the layout would
+	  have been unaffected by this change, and there is no risk of data
+	  corruption from this issue.
+
+	  Unfortunately, the layout can not be determined by the kernel. If the
+	  array has only been written to by a 3.14 or later kernel its safe to
+	  set RAID0_ALT_MULTIZONE_LAYOUT. If its only been written to by a
+	  pre-3.14 kernel its safe to select RAID0_ORIG_LAYOUT. If its been
+	  written by both then select RAID0_LAYOUT_NONE, which will not
+	  configure the array. The array can then be examined for corruption.
+
+	  For new arrays you may choose either layout version. Neither version
+	  is inherently better than the other.
+
+	  Alternatively, these parameters can also be specified via the module
+	  parameter raid0.default_layout=<N>. N=2 selects the 'new' or multizone
+	  layout, while N=1 selects the 'old' layout or original layout. If
+	  unset the array will not be configured.
+
+	  The layout can also be written directly to the raid0 array via the
+	  mdadm command, which can be auto-detected by the kernel. See:
+	  <https://www.kernel.org/doc/html/latest/admin-guide/md.html#multi-zone-raid0-layout-migration>
+
+config RAID0_ORIG_LAYOUT
+	bool "raid0 layout for arrays only written to by a pre-3.14 kernel"
+	help
+	  If the raid0 array was only created and written to by a pre-3.14 kernel.
+
+config RAID0_ALT_MULTIZONE_LAYOUT
+	bool "raid0 layout for arrays only written to be a 3.14 or newer kernel"
+	help
+	  If the raid0 array was only created and written to by a 3.14 or later
+	  kernel.
+
+config RAID0_LAYOUT_NONE
+	bool "raid0 layout must be specified via a module parameter"
+	help
+	  If a raid0 array was written to by both a pre-3.14 and a 3.14 or
+	  later kernel, you may have data corruption. This option will not
+	  auto configure the array and thus you can examine the array offline
+	  to determine the best way to proceed. With RAID0_LAYOUT_NONE
+	  set, the choice for raid0 layout can be set via a module parameter
+	  raid0.default_layout=<N>. Or the layout can be written directly
+	  to the raid0 array via the mdadm command.
+
+endchoice
+
 config MD_RAID1
 	tristate "RAID-1 (mirroring) mode"
 	depends on BLK_DEV_MD
diff --git a/drivers/md/raid0.c b/drivers/md/raid0.c
index 322386f..576eaa6 100644
--- a/drivers/md/raid0.c
+++ b/drivers/md/raid0.c
@@ -19,7 +19,14 @@
 #include "raid0.h"
 #include "raid5.h"
 
+#if defined(CONFIG_RAID0_ORIG_LAYOUT)
+static int default_layout = RAID0_ORIG_LAYOUT;
+#elif defined(CONFIG_RAID0_ALT_MULTIZONE_LAYOUT)
+static int default_layout = RAID0_ALT_MULTIZONE_LAYOUT;
+#else
 static int default_layout = 0;
+#endif
+
 module_param(default_layout, int, 0644);
 
 #define UNSUPPORTED_MDDEV_FLAGS		\
-- 
2.7.4

Re: [PATCH] md/raid0: add config parameters to specify zone layout

[Randy Dunlap]

On 3/26/20 8:28 AM, Jason Baron wrote:
> Let's add some CONFIG_* options to directly configure the raid0 layout
> if you know in advance how your raid0 array was created. This can be
> simpler than having to manage module or kernel command-line parameters.
> 
> If the raid0 array was created by a pre-3.14 kernel, use
> RAID0_ORIG_LAYOUT. If the raid0 array was created by a 3.14 or newer
> kernel then select RAID0_ALT_MULTIZONE_LAYOUT. Otherwise, the default
> setting is RAID0_LAYOUT_NONE, in which case the current behavior of
> needing to specify a module parameter raid0.default_layout=1|2 is
> preserved.
> 
> Cc: Guoqing Jiang <guoqing.jiang@cloud.ionos.com>
> Cc: NeilBrown <neilb@suse.de>
> Cc: Song Liu <songliubraving@fb.com>
> Signed-off-by: Jason Baron <jbaron@akamai.com>
> ---
>  drivers/md/Kconfig | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/md/raid0.c |  7 +++++++
>  2 files changed, 62 insertions(+)
> 
> diff --git a/drivers/md/Kconfig b/drivers/md/Kconfig
> index d6d5ab2..c0c6d82 100644
> --- a/drivers/md/Kconfig
> +++ b/drivers/md/Kconfig
> @@ -79,6 +79,61 @@ config MD_RAID0
>  
>  	  If unsure, say Y.
>  
> +choice
> +	prompt "RAID0 Layout"
> +	default RAID0_LAYOUT_NONE
> +	depends on MD_RAID0
> +	help
> +	  A change was made in Linux 3.14 that unintentinally changed the

	                                       unintentionally

> +	  the layout for RAID0. This can result in data corruption if a pre-3.14
> +	  and a 3.14 or later kernel both wrote to the array. However, if the
> +	  devices in the array are all of the same size then the layout would
> +	  have been unaffected by this change, and there is no risk of data
> +	  corruption from this issue.
> +
> +	  Unfortunately, the layout can not be determined by the kernel. If the
> +	  array has only been written to by a 3.14 or later kernel its safe to

	                                                           it's

> +	  set RAID0_ALT_MULTIZONE_LAYOUT. If its only been written to by a

	                                     it has

> +	  pre-3.14 kernel its safe to select RAID0_ORIG_LAYOUT. If its been

	                  it's                                  If it has been

> +	  written by both then select RAID0_LAYOUT_NONE, which will not
> +	  configure the array. The array can then be examined for corruption.
> +
> +	  For new arrays you may choose either layout version. Neither version
> +	  is inherently better than the other.
> +
> +	  Alternatively, these parameters can also be specified via the module
> +	  parameter raid0.default_layout=<N>. N=2 selects the 'new' or multizone
> +	  layout, while N=1 selects the 'old' layout or original layout. If
> +	  unset the array will not be configured.
> +
> +	  The layout can also be written directly to the raid0 array via the
> +	  mdadm command, which can be auto-detected by the kernel. See:
> +	  <https://www.kernel.org/doc/html/latest/admin-guide/md.html#multi-zone-raid0-layout-migration>
> +
> +config RAID0_ORIG_LAYOUT
> +	bool "raid0 layout for arrays only written to by a pre-3.14 kernel"
> +	help
> +	  If the raid0 array was only created and written to by a pre-3.14 kernel.
> +
> +config RAID0_ALT_MULTIZONE_LAYOUT
> +	bool "raid0 layout for arrays only written to be a 3.14 or newer kernel"

	                                              by

> +	help
> +	  If the raid0 array was only created and written to by a 3.14 or later
> +	  kernel.
> +
> +config RAID0_LAYOUT_NONE
> +	bool "raid0 layout must be specified via a module parameter"
> +	help
> +	  If a raid0 array was written to by both a pre-3.14 and a 3.14 or
> +	  later kernel, you may have data corruption. This option will not
> +	  auto configure the array and thus you can examine the array offline
> +	  to determine the best way to proceed. With RAID0_LAYOUT_NONE
> +	  set, the choice for raid0 layout can be set via a module parameter
> +	  raid0.default_layout=<N>. Or the layout can be written directly
> +	  to the raid0 array via the mdadm command.
> +
> +endchoice
> +
>  config MD_RAID1
>  	tristate "RAID-1 (mirroring) mode"
>  	depends on BLK_DEV_MD
> diff --git a/drivers/md/raid0.c b/drivers/md/raid0.c
> index 322386f..576eaa6 100644
> --- a/drivers/md/raid0.c
> +++ b/drivers/md/raid0.c
> @@ -19,7 +19,14 @@
>  #include "raid0.h"
>  #include "raid5.h"
>  
> +#if defined(CONFIG_RAID0_ORIG_LAYOUT)
> +static int default_layout = RAID0_ORIG_LAYOUT;
> +#elif defined(CONFIG_RAID0_ALT_MULTIZONE_LAYOUT)
> +static int default_layout = RAID0_ALT_MULTIZONE_LAYOUT;
> +#else
>  static int default_layout = 0;
> +#endif
> +
>  module_param(default_layout, int, 0644);
>  
>  #define UNSUPPORTED_MDDEV_FLAGS		\
> 


-- 
~Randy
Reported-by: Randy Dunlap <rdunlap@infradead.org>

[PATCH v2] md/raid0: add config parameters to specify zone layout

[Jason Baron]

Let's add some CONFIG_* options to directly configure the raid0 layout
if you know in advance how your raid0 array was created. This can be
simpler than having to manage module or kernel command-line parameters.

If the raid0 array was created by a pre-3.14 kernel, use
RAID0_ORIG_LAYOUT. If the raid0 array was created by a 3.14 or newer
kernel then select RAID0_ALT_MULTIZONE_LAYOUT. Otherwise, the default
setting is RAID0_LAYOUT_NONE, in which case the current behavior of
needing to specify a module parameter raid0.default_layout=1|2 is
preserved.

Cc: Guoqing Jiang <guoqing.jiang@cloud.ionos.com>
Cc: NeilBrown <neilb@suse.de>
Cc: Song Liu <songliubraving@fb.com>
Signed-off-by: Jason Baron <jbaron@akamai.com>
---
Changes in v2:
- Cleaned up text (Randy Dunlap)

 drivers/md/Kconfig | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/md/raid0.c |  7 +++++++
 2 files changed, 62 insertions(+)

diff --git a/drivers/md/Kconfig b/drivers/md/Kconfig
index d6d5ab2..cf6baa1 100644
--- a/drivers/md/Kconfig
+++ b/drivers/md/Kconfig
@@ -79,6 +79,61 @@ config MD_RAID0
 
 	  If unsure, say Y.
 
+choice
+	prompt "RAID0 Layout"
+	default RAID0_LAYOUT_NONE
+	depends on MD_RAID0
+	help
+	  A change was made in Linux 3.14 that unintentionally changed the
+	  the layout for RAID0. This can result in data corruption if a pre-3.14
+	  and a 3.14 or later kernel both wrote to the array. However, if the
+	  devices in the array are all of the same size then the layout would
+	  have been unaffected by this change, and there is no risk of data
+	  corruption from this issue.
+
+	  Unfortunately, the layout can not be determined by the kernel. If the
+	  array has only been written to by a 3.14 or later kernel it's safe to
+	  set RAID0_ALT_MULTIZONE_LAYOUT. If it has only been written to by a
+	  pre-3.14 kernel it's safe to select RAID0_ORIG_LAYOUT. If it has been
+	  written by both then select RAID0_LAYOUT_NONE, which will not
+	  configure the array. The array can then be examined for corruption.
+
+	  For new arrays you may choose either layout version. Neither version
+	  is inherently better than the other.
+
+	  Alternatively, these parameters can also be specified via the module
+	  parameter raid0.default_layout=<N>. N=2 selects the 'new' or multizone
+	  layout, while N=1 selects the 'old' layout or original layout. If
+	  unset the array will not be configured.
+
+	  The layout can also be written directly to the raid0 array via the
+	  mdadm command, which can be auto-detected by the kernel. See:
+	  <https://www.kernel.org/doc/html/latest/admin-guide/md.html#multi-zone-raid0-layout-migration>
+
+config RAID0_ORIG_LAYOUT
+	bool "raid0 layout for arrays only written to by a pre-3.14 kernel"
+	help
+	  If the raid0 array was only created and written to by a pre-3.14 kernel.
+
+config RAID0_ALT_MULTIZONE_LAYOUT
+	bool "raid0 layout for arrays only written to by a 3.14 or newer kernel"
+	help
+	  If the raid0 array was only created and written to by a 3.14 or later
+	  kernel.
+
+config RAID0_LAYOUT_NONE
+	bool "raid0 layout must be specified via a module parameter"
+	help
+	  If a raid0 array was written to by both a pre-3.14 and a 3.14 or
+	  later kernel, you may have data corruption. This option will not
+	  auto configure the array and thus you can examine the array offline
+	  to determine the best way to proceed. With RAID0_LAYOUT_NONE
+	  set, the choice for raid0 layout can be set via a module parameter
+	  raid0.default_layout=<N>. Or the layout can be written directly
+	  to the raid0 array via the mdadm command.
+
+endchoice
+
 config MD_RAID1
 	tristate "RAID-1 (mirroring) mode"
 	depends on BLK_DEV_MD
diff --git a/drivers/md/raid0.c b/drivers/md/raid0.c
index 322386f..576eaa6 100644
--- a/drivers/md/raid0.c
+++ b/drivers/md/raid0.c
@@ -19,7 +19,14 @@
 #include "raid0.h"
 #include "raid5.h"
 
+#if defined(CONFIG_RAID0_ORIG_LAYOUT)
+static int default_layout = RAID0_ORIG_LAYOUT;
+#elif defined(CONFIG_RAID0_ALT_MULTIZONE_LAYOUT)
+static int default_layout = RAID0_ALT_MULTIZONE_LAYOUT;
+#else
 static int default_layout = 0;
+#endif
+
 module_param(default_layout, int, 0644);
 
 #define UNSUPPORTED_MDDEV_FLAGS		\
-- 
2.7.4

Re: [PATCH v2] md/raid0: add config parameters to specify zone layout

[Song Liu]

> On Mar 30, 2020, at 9:00 AM, Jason Baron <jbaron@akamai.com> wrote:
> 
> Let's add some CONFIG_* options to directly configure the raid0 layout
> if you know in advance how your raid0 array was created. This can be
> simpler than having to manage module or kernel command-line parameters.
> 
> If the raid0 array was created by a pre-3.14 kernel, use
> RAID0_ORIG_LAYOUT. If the raid0 array was created by a 3.14 or newer
> kernel then select RAID0_ALT_MULTIZONE_LAYOUT. Otherwise, the default
> setting is RAID0_LAYOUT_NONE, in which case the current behavior of
> needing to specify a module parameter raid0.default_layout=1|2 is
> preserved.
> 
> Cc: Guoqing Jiang <guoqing.jiang@cloud.ionos.com>
> Cc: NeilBrown <neilb@suse.de>
> Cc: Song Liu <songliubraving@fb.com>
> Signed-off-by: Jason Baron <jbaron@akamai.com>

This patch looks good. However, I am not sure whether the user will 
recompile the kernel for a different default value. Do you have real
world use case for this?

Thanks,
Song

Re: [PATCH] md/raid0: add config parameters to specify zone layout

[Coly Li]

On 2020/3/26 23:28, Jason Baron wrote:
> Let's add some CONFIG_* options to directly configure the raid0 layout
> if you know in advance how your raid0 array was created. This can be
> simpler than having to manage module or kernel command-line parameters.
> 

Hi Jason,

If the people who compiling the kernel is not the end users, the
communication gap has potential risk to make users to use a different
layout for existing raid0 array after a kernel upgrade.

If this patch goes into upstream, it is very probably such risky
situation may happen.

The purpose of adding default_layout is to let *end user* to be aware of
they layout when they use difference sizes component disks to assemble
the raid0 array, and make decision which layout algorithm should be
used. Such situation cannot be decided in kernel compiling time.

> If the raid0 array was created by a pre-3.14 kernel, use
> RAID0_ORIG_LAYOUT. If the raid0 array was created by a 3.14 or newer
> kernel then select RAID0_ALT_MULTIZONE_LAYOUT. Otherwise, the default
> setting is RAID0_LAYOUT_NONE, in which case the current behavior of
> needing to specify a module parameter raid0.default_layout=1|2 is
> preserved.
> 

The difficulty is for a given md raid0 array, there is no clue whether
it is built on pre-3.14 kernel or not. If the kernel is not configured
and built by end user, we have risk that wrong algorithm will be applied
to unmatched layout.

Thanks.

Coly Li


> Cc: Guoqing Jiang <guoqing.jiang@cloud.ionos.com>
> Cc: NeilBrown <neilb@suse.de>
> Cc: Song Liu <songliubraving@fb.com>
> Signed-off-by: Jason Baron <jbaron@akamai.com>
> ---
>  drivers/md/Kconfig | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/md/raid0.c |  7 +++++++
>  2 files changed, 62 insertions(+)
> 
> diff --git a/drivers/md/Kconfig b/drivers/md/Kconfig
> index d6d5ab2..c0c6d82 100644
> --- a/drivers/md/Kconfig
> +++ b/drivers/md/Kconfig
> @@ -79,6 +79,61 @@ config MD_RAID0
>  
>  	  If unsure, say Y.
>  
> +choice
> +	prompt "RAID0 Layout"
> +	default RAID0_LAYOUT_NONE
> +	depends on MD_RAID0
> +	help
> +	  A change was made in Linux 3.14 that unintentinally changed the
> +	  the layout for RAID0. This can result in data corruption if a pre-3.14
> +	  and a 3.14 or later kernel both wrote to the array. However, if the
> +	  devices in the array are all of the same size then the layout would
> +	  have been unaffected by this change, and there is no risk of data
> +	  corruption from this issue.
> +
> +	  Unfortunately, the layout can not be determined by the kernel. If the
> +	  array has only been written to by a 3.14 or later kernel its safe to
> +	  set RAID0_ALT_MULTIZONE_LAYOUT. If its only been written to by a
> +	  pre-3.14 kernel its safe to select RAID0_ORIG_LAYOUT. If its been
> +	  written by both then select RAID0_LAYOUT_NONE, which will not
> +	  configure the array. The array can then be examined for corruption.
> +
> +	  For new arrays you may choose either layout version. Neither version
> +	  is inherently better than the other.
> +
> +	  Alternatively, these parameters can also be specified via the module
> +	  parameter raid0.default_layout=<N>. N=2 selects the 'new' or multizone
> +	  layout, while N=1 selects the 'old' layout or original layout. If
> +	  unset the array will not be configured.
> +
> +	  The layout can also be written directly to the raid0 array via the
> +	  mdadm command, which can be auto-detected by the kernel. See:
> +	  <https://www.kernel.org/doc/html/latest/admin-guide/md.html#multi-zone-raid0-layout-migration>
> +
> +config RAID0_ORIG_LAYOUT
> +	bool "raid0 layout for arrays only written to by a pre-3.14 kernel"
> +	help
> +	  If the raid0 array was only created and written to by a pre-3.14 kernel.
> +
> +config RAID0_ALT_MULTIZONE_LAYOUT
> +	bool "raid0 layout for arrays only written to be a 3.14 or newer kernel"
> +	help
> +	  If the raid0 array was only created and written to by a 3.14 or later
> +	  kernel.
> +
> +config RAID0_LAYOUT_NONE
> +	bool "raid0 layout must be specified via a module parameter"
> +	help
> +	  If a raid0 array was written to by both a pre-3.14 and a 3.14 or
> +	  later kernel, you may have data corruption. This option will not
> +	  auto configure the array and thus you can examine the array offline
> +	  to determine the best way to proceed. With RAID0_LAYOUT_NONE
> +	  set, the choice for raid0 layout can be set via a module parameter
> +	  raid0.default_layout=<N>. Or the layout can be written directly
> +	  to the raid0 array via the mdadm command.
> +
> +endchoice
> +
>  config MD_RAID1
>  	tristate "RAID-1 (mirroring) mode"
>  	depends on BLK_DEV_MD
> diff --git a/drivers/md/raid0.c b/drivers/md/raid0.c
> index 322386f..576eaa6 100644
> --- a/drivers/md/raid0.c
> +++ b/drivers/md/raid0.c
> @@ -19,7 +19,14 @@
>  #include "raid0.h"
>  #include "raid5.h"
>  
> +#if defined(CONFIG_RAID0_ORIG_LAYOUT)
> +static int default_layout = RAID0_ORIG_LAYOUT;
> +#elif defined(CONFIG_RAID0_ALT_MULTIZONE_LAYOUT)
> +static int default_layout = RAID0_ALT_MULTIZONE_LAYOUT;
> +#else
>  static int default_layout = 0;
> +#endif
> +
>  module_param(default_layout, int, 0644);
>  
>  #define UNSUPPORTED_MDDEV_FLAGS		\
> 

Re: [PATCH v2] md/raid0: add config parameters to specify zone layout

[Jason Baron]

On 4/24/20 7:22 PM, Song Liu wrote:
> 
> 
>> On Mar 30, 2020, at 9:00 AM, Jason Baron <jbaron@akamai.com> wrote:
>>
>> Let's add some CONFIG_* options to directly configure the raid0 layout
>> if you know in advance how your raid0 array was created. This can be
>> simpler than having to manage module or kernel command-line parameters.
>>
>> If the raid0 array was created by a pre-3.14 kernel, use
>> RAID0_ORIG_LAYOUT. If the raid0 array was created by a 3.14 or newer
>> kernel then select RAID0_ALT_MULTIZONE_LAYOUT. Otherwise, the default
>> setting is RAID0_LAYOUT_NONE, in which case the current behavior of
>> needing to specify a module parameter raid0.default_layout=1|2 is
>> preserved.
>>
>> Cc: Guoqing Jiang <guoqing.jiang@cloud.ionos.com>
>> Cc: NeilBrown <neilb@suse.de>
>> Cc: Song Liu <songliubraving@fb.com>
>> Signed-off-by: Jason Baron <jbaron@akamai.com>
> 
> This patch looks good. However, I am not sure whether the user will 
> recompile the kernel for a different default value. Do you have real
> world use case for this?
> 
> Thanks,
> Song
> 

Hi Song,

Yes, we knew that all our raid0 arrays were created with >=3.14
kernels, and thus we wanted a way to specify the new layout without
needing to add to the command line or a module parameter. For us,
maintaining a CONFIG_* option is just simpler.

Thanks,

-Jason

Re: [PATCH] md/raid0: add config parameters to specify zone layout

[Jason Baron]

On 4/25/20 12:31 AM, Coly Li wrote:
> On 2020/3/26 23:28, Jason Baron wrote:
>> Let's add some CONFIG_* options to directly configure the raid0 layout
>> if you know in advance how your raid0 array was created. This can be
>> simpler than having to manage module or kernel command-line parameters.
>>
> 
> Hi Jason,
> 
> If the people who compiling the kernel is not the end users, the
> communication gap has potential risk to make users to use a different
> layout for existing raid0 array after a kernel upgrade.
> 
> If this patch goes into upstream, it is very probably such risky
> situation may happen.
> 
> The purpose of adding default_layout is to let *end user* to be aware of
> they layout when they use difference sizes component disks to assemble
> the raid0 array, and make decision which layout algorithm should be
> used. Such situation cannot be decided in kernel compiling time.

I agree that in general it may not be known at compile time. Thus,
I've left the default as RAID0_LAYOUT_NONE. However, there are
use-cases where it is known at compile-time which layout is needed.
In our use-case, we knew that we didn't have any pre-3.14 raid0
arrays. Thus, we can safely set RAID0_ALT_MULTIZONE_LAYOUT. So
this is a simpler configuration for us than setting module or command
line parameters.

Thanks,

-Jason

> 
>> If the raid0 array was created by a pre-3.14 kernel, use
>> RAID0_ORIG_LAYOUT. If the raid0 array was created by a 3.14 or newer
>> kernel then select RAID0_ALT_MULTIZONE_LAYOUT. Otherwise, the default
>> setting is RAID0_LAYOUT_NONE, in which case the current behavior of
>> needing to specify a module parameter raid0.default_layout=1|2 is
>> preserved.
>>
> 
> The difficulty is for a given md raid0 array, there is no clue whether
> it is built on pre-3.14 kernel or not. If the kernel is not configured
> and built by end user, we have risk that wrong algorithm will be applied
> to unmatched layout.
>> Thanks.
> 
> Coly Li
> 
> 
>> Cc: Guoqing Jiang <guoqing.jiang@cloud.ionos.com>
>> Cc: NeilBrown <neilb@suse.de>
>> Cc: Song Liu <songliubraving@fb.com>
>> Signed-off-by: Jason Baron <jbaron@akamai.com>
>> ---
>>  drivers/md/Kconfig | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  drivers/md/raid0.c |  7 +++++++
>>  2 files changed, 62 insertions(+)
>>
>> diff --git a/drivers/md/Kconfig b/drivers/md/Kconfig
>> index d6d5ab2..c0c6d82 100644
>> --- a/drivers/md/Kconfig
>> +++ b/drivers/md/Kconfig
>> @@ -79,6 +79,61 @@ config MD_RAID0
>>  
>>  	  If unsure, say Y.
>>  
>> +choice
>> +	prompt "RAID0 Layout"
>> +	default RAID0_LAYOUT_NONE
>> +	depends on MD_RAID0
>> +	help
>> +	  A change was made in Linux 3.14 that unintentinally changed the
>> +	  the layout for RAID0. This can result in data corruption if a pre-3.14
>> +	  and a 3.14 or later kernel both wrote to the array. However, if the
>> +	  devices in the array are all of the same size then the layout would
>> +	  have been unaffected by this change, and there is no risk of data
>> +	  corruption from this issue.
>> +
>> +	  Unfortunately, the layout can not be determined by the kernel. If the
>> +	  array has only been written to by a 3.14 or later kernel its safe to
>> +	  set RAID0_ALT_MULTIZONE_LAYOUT. If its only been written to by a
>> +	  pre-3.14 kernel its safe to select RAID0_ORIG_LAYOUT. If its been
>> +	  written by both then select RAID0_LAYOUT_NONE, which will not
>> +	  configure the array. The array can then be examined for corruption.
>> +
>> +	  For new arrays you may choose either layout version. Neither version
>> +	  is inherently better than the other.
>> +
>> +	  Alternatively, these parameters can also be specified via the module
>> +	  parameter raid0.default_layout=<N>. N=2 selects the 'new' or multizone
>> +	  layout, while N=1 selects the 'old' layout or original layout. If
>> +	  unset the array will not be configured.
>> +
>> +	  The layout can also be written directly to the raid0 array via the
>> +	  mdadm command, which can be auto-detected by the kernel. See:
>> +	  <https://www.kernel.org/doc/html/latest/admin-guide/md.html#multi-zone-raid0-layout-migration>
>> +
>> +config RAID0_ORIG_LAYOUT
>> +	bool "raid0 layout for arrays only written to by a pre-3.14 kernel"
>> +	help
>> +	  If the raid0 array was only created and written to by a pre-3.14 kernel.
>> +
>> +config RAID0_ALT_MULTIZONE_LAYOUT
>> +	bool "raid0 layout for arrays only written to be a 3.14 or newer kernel"
>> +	help
>> +	  If the raid0 array was only created and written to by a 3.14 or later
>> +	  kernel.
>> +
>> +config RAID0_LAYOUT_NONE
>> +	bool "raid0 layout must be specified via a module parameter"
>> +	help
>> +	  If a raid0 array was written to by both a pre-3.14 and a 3.14 or
>> +	  later kernel, you may have data corruption. This option will not
>> +	  auto configure the array and thus you can examine the array offline
>> +	  to determine the best way to proceed. With RAID0_LAYOUT_NONE
>> +	  set, the choice for raid0 layout can be set via a module parameter
>> +	  raid0.default_layout=<N>. Or the layout can be written directly
>> +	  to the raid0 array via the mdadm command.
>> +
>> +endchoice
>> +
>>  config MD_RAID1
>>  	tristate "RAID-1 (mirroring) mode"
>>  	depends on BLK_DEV_MD
>> diff --git a/drivers/md/raid0.c b/drivers/md/raid0.c
>> index 322386f..576eaa6 100644
>> --- a/drivers/md/raid0.c
>> +++ b/drivers/md/raid0.c
>> @@ -19,7 +19,14 @@
>>  #include "raid0.h"
>>  #include "raid5.h"
>>  
>> +#if defined(CONFIG_RAID0_ORIG_LAYOUT)
>> +static int default_layout = RAID0_ORIG_LAYOUT;
>> +#elif defined(CONFIG_RAID0_ALT_MULTIZONE_LAYOUT)
>> +static int default_layout = RAID0_ALT_MULTIZONE_LAYOUT;
>> +#else
>>  static int default_layout = 0;
>> +#endif
>> +
>>  module_param(default_layout, int, 0644);
>>  
>>  #define UNSUPPORTED_MDDEV_FLAGS		\
>>
> 

Re: [PATCH] md/raid0: add config parameters to specify zone layout

[Song Liu]

Hi Jason,

> On Apr 27, 2020, at 2:10 PM, Jason Baron <jbaron@akamai.com> wrote:
> 
> 
> 
> On 4/25/20 12:31 AM, Coly Li wrote:
>> On 2020/3/26 23:28, Jason Baron wrote:
>>> Let's add some CONFIG_* options to directly configure the raid0 layout
>>> if you know in advance how your raid0 array was created. This can be
>>> simpler than having to manage module or kernel command-line parameters.
>>> 
>> 
>> Hi Jason,
>> 
>> If the people who compiling the kernel is not the end users, the
>> communication gap has potential risk to make users to use a different
>> layout for existing raid0 array after a kernel upgrade.
>> 
>> If this patch goes into upstream, it is very probably such risky
>> situation may happen.
>> 
>> The purpose of adding default_layout is to let *end user* to be aware of
>> they layout when they use difference sizes component disks to assemble
>> the raid0 array, and make decision which layout algorithm should be
>> used. Such situation cannot be decided in kernel compiling time.
> 
> I agree that in general it may not be known at compile time. Thus,
> I've left the default as RAID0_LAYOUT_NONE. However, there are
> use-cases where it is known at compile-time which layout is needed.
> In our use-case, we knew that we didn't have any pre-3.14 raid0
> arrays. Thus, we can safely set RAID0_ALT_MULTIZONE_LAYOUT. So
> this is a simpler configuration for us than setting module or command
> line parameters.

I would echo Coly's concern that CONFIG_ option could make it risky. 
If the overhead of maintaining extra command line parameter, I would
recommend you carry a private patch for this change. For upstream, it
is better NOT to carry the default in CONFIG_.

Thanks,
Song

Re: [PATCH] md/raid0: add config parameters to specify zone layout

[John Stoffel]

>>>>> "Song" == Song Liu <songliubraving@fb.com> writes:

Song> Hi Jason,
>> On Apr 27, 2020, at 2:10 PM, Jason Baron <jbaron@akamai.com> wrote:
>> 
>> 
>> 
>> On 4/25/20 12:31 AM, Coly Li wrote:
>>> On 2020/3/26 23:28, Jason Baron wrote:
>>>> Let's add some CONFIG_* options to directly configure the raid0 layout
>>>> if you know in advance how your raid0 array was created. This can be
>>>> simpler than having to manage module or kernel command-line parameters.
>>>> 
>>> 
>>> Hi Jason,
>>> 
>>> If the people who compiling the kernel is not the end users, the
>>> communication gap has potential risk to make users to use a different
>>> layout for existing raid0 array after a kernel upgrade.
>>> 
>>> If this patch goes into upstream, it is very probably such risky
>>> situation may happen.
>>> 
>>> The purpose of adding default_layout is to let *end user* to be aware of
>>> they layout when they use difference sizes component disks to assemble
>>> the raid0 array, and make decision which layout algorithm should be
>>> used. Such situation cannot be decided in kernel compiling time.
>> 
>> I agree that in general it may not be known at compile time. Thus,
>> I've left the default as RAID0_LAYOUT_NONE. However, there are
>> use-cases where it is known at compile-time which layout is needed.
>> In our use-case, we knew that we didn't have any pre-3.14 raid0
>> arrays. Thus, we can safely set RAID0_ALT_MULTIZONE_LAYOUT. So
>> this is a simpler configuration for us than setting module or command
>> line parameters.

Song> I would echo Coly's concern that CONFIG_ option could make it risky. 
Song> If the overhead of maintaining extra command line parameter, I would
Song> recommend you carry a private patch for this change. For upstream, it
Song> is better NOT to carry the default in CONFIG_.

I agree as well.  Just because you have a known base, doesn't mean
that others wouldn't be hit with this problem.

John

Re: [PATCH] md/raid0: add config parameters to specify zone layout

[John Stoffel]

>>>>> "Song" == Song Liu <songliubraving@fb.com> writes:

Song> Hi Jason,
>> On Apr 27, 2020, at 2:10 PM, Jason Baron <jbaron@akamai.com> wrote:
>> 
>> 
>> 
>> On 4/25/20 12:31 AM, Coly Li wrote:
>>> On 2020/3/26 23:28, Jason Baron wrote:
>>>> Let's add some CONFIG_* options to directly configure the raid0 layout
>>>> if you know in advance how your raid0 array was created. This can be
>>>> simpler than having to manage module or kernel command-line parameters.
>>>> 
>>> 
>>> Hi Jason,
>>> 
>>> If the people who compiling the kernel is not the end users, the
>>> communication gap has potential risk to make users to use a different
>>> layout for existing raid0 array after a kernel upgrade.
>>> 
>>> If this patch goes into upstream, it is very probably such risky
>>> situation may happen.
>>> 
>>> The purpose of adding default_layout is to let *end user* to be aware of
>>> they layout when they use difference sizes component disks to assemble
>>> the raid0 array, and make decision which layout algorithm should be
>>> used. Such situation cannot be decided in kernel compiling time.
>> 
>> I agree that in general it may not be known at compile time. Thus,
>> I've left the default as RAID0_LAYOUT_NONE. However, there are
>> use-cases where it is known at compile-time which layout is needed.
>> In our use-case, we knew that we didn't have any pre-3.14 raid0
>> arrays. Thus, we can safely set RAID0_ALT_MULTIZONE_LAYOUT. So
>> this is a simpler configuration for us than setting module or command
>> line parameters.

Song> I would echo Coly's concern that CONFIG_ option could make it risky. 
Song> If the overhead of maintaining extra command line parameter, I would
Song> recommend you carry a private patch for this change. For upstream, it
Song> is better NOT to carry the default in CONFIG_.

I agree as well.  Just because you have a known base, doesn't mean
that others wouldn't be hit with this problem.

John