| Message ID | 20221020045903.19950-1-kinga.tanska@intel.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | super-intel: make freesize not required for chunk size migration | expand |
> 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 >
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
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
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);
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(-)