[PATCH] super-intel: make freesize not required for chunk size migration

[PATCH] super-intel: make freesize not required for chunk size migration

[Kinga Tanska]

Freesize is not required when chunk size migration is performed. Fix
return value when superblock is not set.

Signed-off-by: Kinga Tanska <kinga.tanska@intel.com>
---
 super-intel.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/super-intel.c b/super-intel.c
index 4d82af3d..37c59da5 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -7714,11 +7714,11 @@ static int validate_geometry_imsm(struct supertype *st, int level, int layout,
 		struct intel_super *super = st->sb;
 
 		/*
-		 * Autolayout mode, st->sb and freesize must be set.
+		 * Autolayout mode, st->sb must be set.
 		 */
-		if (!super || !freesize) {
-			pr_vrb("freesize and superblock must be set for autolayout, aborting\n");
-			return 1;
+		if (!super) {
+			pr_vrb("superblock must be set for autolayout, aborting\n");
+			return 0;
 		}
 
 		if (!validate_geometry_imsm_orom(st->sb, level, layout,
@@ -7726,7 +7726,7 @@ static int validate_geometry_imsm(struct supertype *st, int level, int layout,
 						 verbose))
 			return 0;
 
-		if (super->orom) {
+		if (super->orom && freesize) {
 			imsm_status_t rv;
 			int count = count_volumes(super->hba, super->orom->dpa,
 					      verbose);
-- 
2.26.2

Re: [PATCH] super-intel: make freesize not required for chunk size migration

[Coly Li]

> 2022年10月20日 12:59，Kinga Tanska <kinga.tanska@intel.com> 写道：
> 
> Freesize is not required when chunk size migration is performed. Fix
> return value when superblock is not set.

Hi Kinga,

Could you please provide a bit more information why freesize is unnecessary in this situation?

Thanks.

Coly Li


> 
> Signed-off-by: Kinga Tanska <kinga.tanska@intel.com>
> ---
> super-intel.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/super-intel.c b/super-intel.c
> index 4d82af3d..37c59da5 100644
> --- a/super-intel.c
> +++ b/super-intel.c
> @@ -7714,11 +7714,11 @@ static int validate_geometry_imsm(struct supertype *st, int level, int layout,
> 		struct intel_super *super = st->sb;
> 
> 		/*
> -		 * Autolayout mode, st->sb and freesize must be set.
> +		 * Autolayout mode, st->sb must be set.
> 		 */
> -		if (!super || !freesize) {
> -			pr_vrb("freesize and superblock must be set for autolayout, aborting\n");
> -			return 1;
> +		if (!super) {
> +			pr_vrb("superblock must be set for autolayout, aborting\n");
> +			return 0;
> 		}
> 
> 		if (!validate_geometry_imsm_orom(st->sb, level, layout,
> @@ -7726,7 +7726,7 @@ static int validate_geometry_imsm(struct supertype *st, int level, int layout,
> 						 verbose))
> 			return 0;
> 
> -		if (super->orom) {
> +		if (super->orom && freesize) {
> 			imsm_status_t rv;
> 			int count = count_volumes(super->hba, super->orom->dpa,
> 					      verbose);
> -- 
> 2.26.2
> 

Re: [PATCH] super-intel: make freesize not required for chunk size migration

[Kinga Tanska]

On Fri, 21 Oct 2022 14:49:16 +0800
Coly Li <colyli@suse.de> wrote:

> 
> 
> > 2022年10月20日 12:59，Kinga Tanska <kinga.tanska@intel.com> 写道：
> > 
> > Freesize is not required when chunk size migration is performed. Fix
> > return value when superblock is not set.
> 
> Hi Kinga,
> 
> Could you please provide a bit more information why freesize is
> unnecessary in this situation?
> 
> Thanks.
> 
> Coly Li
> 
> 
> > 
> > Signed-off-by: Kinga Tanska <kinga.tanska@intel.com>
> > ---
> > super-intel.c | 10 +++++-----
> > 1 file changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/super-intel.c b/super-intel.c
> > index 4d82af3d..37c59da5 100644
> > --- a/super-intel.c
> > +++ b/super-intel.c
> > @@ -7714,11 +7714,11 @@ static int validate_geometry_imsm(struct
> > supertype *st, int level, int layout, struct intel_super *super =
> > st->sb;
> > 
> > 		/*
> > -		 * Autolayout mode, st->sb and freesize must be
> > set.
> > +		 * Autolayout mode, st->sb must be set.
> > 		 */
> > -		if (!super || !freesize) {
> > -			pr_vrb("freesize and superblock must be
> > set for autolayout, aborting\n");
> > -			return 1;
> > +		if (!super) {
> > +			pr_vrb("superblock must be set for
> > autolayout, aborting\n");
> > +			return 0;
> > 		}
> > 
> > 		if (!validate_geometry_imsm_orom(st->sb, level,
> > layout, @@ -7726,7 +7726,7 @@ static int
> > validate_geometry_imsm(struct supertype *st, int level, int layout,
> > verbose)) return 0;
> > 
> > -		if (super->orom) {
> > +		if (super->orom && freesize) {
> > 			imsm_status_t rv;
> > 			int count = count_volumes(super->hba,
> > super->orom->dpa, verbose);
> > -- 
> > 2.26.2
> > 
> 

Hi,

freesize is needed to be set for migrations where size of RAID could be
changed - expand. It tells how many free space is determined
for members. In chunk size migration freesize is not needed to
be set, so we shouldn't check if pointer exists. I moved this
check to condition which contains size calculations, instead of
checking it always at the first step.
We've tested it internally with both, chunk size migration and expand
and freesize is checked only when is really needed now.

Regards,
Kinga Tanska

Re: [PATCH] super-intel: make freesize not required for chunk size migration

[Xiao Ni]

On Fri, Oct 21, 2022 at 6:14 PM Kinga Tanska
<kinga.tanska@linux.intel.com> wrote:
>
> On Fri, 21 Oct 2022 14:49:16 +0800
> Coly Li <colyli@suse.de> wrote:
>
> >
> >
> > > 2022年10月20日 12:59，Kinga Tanska <kinga.tanska@intel.com> 写道：
> > >
> > > Freesize is not required when chunk size migration is performed. Fix
> > > return value when superblock is not set.
> >
> > Hi Kinga,
> >
> > Could you please provide a bit more information why freesize is
> > unnecessary in this situation?
> >
> > Thanks.
> >
> > Coly Li
> >
> >
> > >
> > > Signed-off-by: Kinga Tanska <kinga.tanska@intel.com>
> > > ---
> > > super-intel.c | 10 +++++-----
> > > 1 file changed, 5 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/super-intel.c b/super-intel.c
> > > index 4d82af3d..37c59da5 100644
> > > --- a/super-intel.c
> > > +++ b/super-intel.c
> > > @@ -7714,11 +7714,11 @@ static int validate_geometry_imsm(struct
> > > supertype *st, int level, int layout, struct intel_super *super =
> > > st->sb;
> > >
> > >             /*
> > > -            * Autolayout mode, st->sb and freesize must be
> > > set.
> > > +            * Autolayout mode, st->sb must be set.
> > >              */
> > > -           if (!super || !freesize) {
> > > -                   pr_vrb("freesize and superblock must be
> > > set for autolayout, aborting\n");
> > > -                   return 1;
> > > +           if (!super) {
> > > +                   pr_vrb("superblock must be set for
> > > autolayout, aborting\n");
> > > +                   return 0;
> > >             }
> > >
> > >             if (!validate_geometry_imsm_orom(st->sb, level,
> > > layout, @@ -7726,7 +7726,7 @@ static int
> > > validate_geometry_imsm(struct supertype *st, int level, int layout,
> > > verbose)) return 0;
> > >
> > > -           if (super->orom) {
> > > +           if (super->orom && freesize) {
> > >                     imsm_status_t rv;
> > >                     int count = count_volumes(super->hba,
> > > super->orom->dpa, verbose);
> > > --
> > > 2.26.2
> > >
> >
>
> Hi,
>
> freesize is needed to be set for migrations where size of RAID could be
> changed - expand. It tells how many free space is determined
> for members. In chunk size migration freesize is not needed to
> be set, so we shouldn't check if pointer exists. I moved this
> check to condition which contains size calculations, instead of
> checking it always at the first step.
> We've tested it internally with both, chunk size migration and expand
> and freesize is checked only when is really needed now.
>
> Regards,
> Kinga Tanska
>

Hi Kinga

Could you send the v2 with the detailed explanation?

Regards
Xiao