* [PATCH] imsm: fix: does not allow to use invalid chunk size
@ 2011-11-25 12:12 Przemyslaw Czarnowski
2011-12-06 0:56 ` NeilBrown
0 siblings, 1 reply; 4+ messages in thread
From: Przemyslaw Czarnowski @ 2011-11-25 12:12 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, dan.j.williams, marcin.labun, ed.ciechanowski
Only least significant bit of chunk size provided by user has been used
in test with OROM capabilities. This way user could pass value which is
not a power of 2.
Signed-off-by: Przemyslaw Czarnowski <przemyslaw.hawrylewicz.czarnowski@intel.com>
---
platform-intel.h | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/platform-intel.h b/platform-intel.h
index 6c094d7..99450ba 100644
--- a/platform-intel.h
+++ b/platform-intel.h
@@ -124,11 +124,13 @@ static inline int imsm_orom_has_raid5(const struct imsm_orom *orom)
static inline int imsm_orom_has_chunk(const struct imsm_orom *orom, int chunk)
{
int fs = ffs(chunk);
+ int orom_chunk_bit;
if (!fs)
return 0;
fs--; /* bit num to bit index */
- return !!(orom->sss & (1 << (fs - 1)));
+ orom_chunk_bit = (orom->sss & (1 << (fs - 1)));
+ return orom_chunk_bit && 1 << orom_chunk_bit == chunk;
}
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] imsm: fix: does not allow to use invalid chunk size
2011-11-25 12:12 [PATCH] imsm: fix: does not allow to use invalid chunk size Przemyslaw Czarnowski
@ 2011-12-06 0:56 ` NeilBrown
2011-12-06 15:49 ` Hawrylewicz Czarnowski, Przemyslaw
0 siblings, 1 reply; 4+ messages in thread
From: NeilBrown @ 2011-12-06 0:56 UTC (permalink / raw)
To: Przemyslaw Czarnowski
Cc: linux-raid, dan.j.williams, marcin.labun, ed.ciechanowski
[-- Attachment #1: Type: text/plain, Size: 1410 bytes --]
On Fri, 25 Nov 2011 13:12:04 +0100 Przemyslaw Czarnowski
<przemyslaw.hawrylewicz.czarnowski@intel.com> wrote:
> Only least significant bit of chunk size provided by user has been used
> in test with OROM capabilities. This way user could pass value which is
> not a power of 2.
>
> Signed-off-by: Przemyslaw Czarnowski <przemyslaw.hawrylewicz.czarnowski@intel.com>
> ---
> platform-intel.h | 4 +++-
> 1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/platform-intel.h b/platform-intel.h
> index 6c094d7..99450ba 100644
> --- a/platform-intel.h
> +++ b/platform-intel.h
> @@ -124,11 +124,13 @@ static inline int imsm_orom_has_raid5(const struct imsm_orom *orom)
> static inline int imsm_orom_has_chunk(const struct imsm_orom *orom, int chunk)
> {
> int fs = ffs(chunk);
> + int orom_chunk_bit;
>
> if (!fs)
> return 0;
> fs--; /* bit num to bit index */
> - return !!(orom->sss & (1 << (fs - 1)));
> + orom_chunk_bit = (orom->sss & (1 << (fs - 1)));
> + return orom_chunk_bit && 1 << orom_chunk_bit == chunk;
> }
>
>
applied, thanks.
However it feels a bit clumsy, though maybe that is just me.
I would use a separate explicit test for "is a power of 2".
ie.
if (chunk & (chunk-1))
return 0; /* not a power of 2 */
fs = ffs(chunk);
return !!(orom->sss & (1 << (fs-2)));
or something like that.
NeilBrown
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH] imsm: fix: does not allow to use invalid chunk size
2011-12-06 0:56 ` NeilBrown
@ 2011-12-06 15:49 ` Hawrylewicz Czarnowski, Przemyslaw
2011-12-07 1:16 ` NeilBrown
0 siblings, 1 reply; 4+ messages in thread
From: Hawrylewicz Czarnowski, Przemyslaw @ 2011-12-06 15:49 UTC (permalink / raw)
To: NeilBrown
Cc: linux-raid@vger.kernel.org, Williams, Dan J, Labun, Marcin,
Ciechanowski, Ed
> -----Original Message-----
> From: NeilBrown [mailto:neilb@suse.de]
> Sent: Tuesday, December 06, 2011 1:56 AM
> To: Hawrylewicz Czarnowski, Przemyslaw
> Cc: linux-raid@vger.kernel.org; Williams, Dan J; Labun, Marcin;
> Ciechanowski, Ed
> Subject: Re: [PATCH] imsm: fix: does not allow to use invalid chunk size
>
> On Fri, 25 Nov 2011 13:12:04 +0100 Przemyslaw Czarnowski
> <przemyslaw.hawrylewicz.czarnowski@intel.com> wrote:
>
> > Only least significant bit of chunk size provided by user has been
> > used in test with OROM capabilities. This way user could pass value
> > which is not a power of 2.
> >
> > Signed-off-by: Przemyslaw Czarnowski
> > <przemyslaw.hawrylewicz.czarnowski@intel.com>
> > ---
> > platform-intel.h | 4 +++-
> > 1 files changed, 3 insertions(+), 1 deletions(-)
> >
> > diff --git a/platform-intel.h b/platform-intel.h index
> > 6c094d7..99450ba 100644
> > --- a/platform-intel.h
> > +++ b/platform-intel.h
> > @@ -124,11 +124,13 @@ static inline int imsm_orom_has_raid5(const
> > struct imsm_orom *orom) static inline int imsm_orom_has_chunk(const
> > struct imsm_orom *orom, int chunk) {
> > int fs = ffs(chunk);
> > + int orom_chunk_bit;
> >
> > if (!fs)
> > return 0;
> > fs--; /* bit num to bit index */
> > - return !!(orom->sss & (1 << (fs - 1)));
> > + orom_chunk_bit = (orom->sss & (1 << (fs - 1)));
> > + return orom_chunk_bit && 1 << orom_chunk_bit == chunk;
> > }
> >
> >
>
> applied, thanks.
>
> However it feels a bit clumsy, though maybe that is just me.
> I would use a separate explicit test for "is a power of 2".
> ie.
>
> if (chunk & (chunk-1))
> return 0; /* not a power of 2 */
> fs = ffs(chunk);
> return !!(orom->sss & (1 << (fs-2)));
>
You're right,
Please get the one from the bottom of this email then... It is based on your input (~90%) so sign it up and ship:)
Moreover previous one is not the right one I wanted to send out...
--
Best Regards,
Przemyslaw Hawrylewicz-Czarnowski
> or something like that.
>
> NeilBrown
--
Subject: [PATCH] fix: imsm: validate strip size - tuned up
Neil's proposal seems more reasonable and shows what is really going on
here.
Signed-off-by: Przemyslaw Czarnowski <przemyslaw.hawrylewicz.czarnowski@intel.com>
---
platform-intel.h | 7 +++----
1 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/platform-intel.h b/platform-intel.h
index 99450ba..c997f1b 100644
--- a/platform-intel.h
+++ b/platform-intel.h
@@ -124,13 +124,12 @@ static inline int imsm_orom_has_raid5(const struct imsm_orom *orom)
static inline int imsm_orom_has_chunk(const struct imsm_orom *orom, int chunk)
{
int fs = ffs(chunk);
- int orom_chunk_bit;
-
if (!fs)
return 0;
fs--; /* bit num to bit index */
- orom_chunk_bit = (orom->sss & (1 << (fs - 1)));
- return orom_chunk_bit && 1 << orom_chunk_bit == chunk;
+ if (chunk & (chunk-1))
+ return 0; /* not a power of 2 */
+ return !!(orom->sss & (1 << (fs - 1)));
}
--
1.7.7
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] imsm: fix: does not allow to use invalid chunk size
2011-12-06 15:49 ` Hawrylewicz Czarnowski, Przemyslaw
@ 2011-12-07 1:16 ` NeilBrown
0 siblings, 0 replies; 4+ messages in thread
From: NeilBrown @ 2011-12-07 1:16 UTC (permalink / raw)
To: Hawrylewicz Czarnowski, Przemyslaw
Cc: linux-raid@vger.kernel.org, Williams, Dan J, Labun, Marcin,
Ciechanowski, Ed
[-- Attachment #1: Type: text/plain, Size: 2256 bytes --]
On Tue, 6 Dec 2011 15:49:48 +0000 "Hawrylewicz Czarnowski, Przemyslaw"
<przemyslaw.hawrylewicz.czarnowski@intel.com> wrote:
> > -----Original Message-----
> > From: NeilBrown [mailto:neilb@suse.de]
> > Sent: Tuesday, December 06, 2011 1:56 AM
> > To: Hawrylewicz Czarnowski, Przemyslaw
> > Cc: linux-raid@vger.kernel.org; Williams, Dan J; Labun, Marcin;
> > Ciechanowski, Ed
> > Subject: Re: [PATCH] imsm: fix: does not allow to use invalid chunk size
> >
> > On Fri, 25 Nov 2011 13:12:04 +0100 Przemyslaw Czarnowski
> > <przemyslaw.hawrylewicz.czarnowski@intel.com> wrote:
> >
> > > Only least significant bit of chunk size provided by user has been
> > > used in test with OROM capabilities. This way user could pass value
> > > which is not a power of 2.
> > >
> > > Signed-off-by: Przemyslaw Czarnowski
> > > <przemyslaw.hawrylewicz.czarnowski@intel.com>
> > > ---
> > > platform-intel.h | 4 +++-
> > > 1 files changed, 3 insertions(+), 1 deletions(-)
> > >
> > > diff --git a/platform-intel.h b/platform-intel.h index
> > > 6c094d7..99450ba 100644
> > > --- a/platform-intel.h
> > > +++ b/platform-intel.h
> > > @@ -124,11 +124,13 @@ static inline int imsm_orom_has_raid5(const
> > > struct imsm_orom *orom) static inline int imsm_orom_has_chunk(const
> > > struct imsm_orom *orom, int chunk) {
> > > int fs = ffs(chunk);
> > > + int orom_chunk_bit;
> > >
> > > if (!fs)
> > > return 0;
> > > fs--; /* bit num to bit index */
> > > - return !!(orom->sss & (1 << (fs - 1)));
> > > + orom_chunk_bit = (orom->sss & (1 << (fs - 1)));
> > > + return orom_chunk_bit && 1 << orom_chunk_bit == chunk;
> > > }
> > >
> > >
> >
> > applied, thanks.
> >
> > However it feels a bit clumsy, though maybe that is just me.
> > I would use a separate explicit test for "is a power of 2".
> > ie.
> >
> > if (chunk & (chunk-1))
> > return 0; /* not a power of 2 */
> > fs = ffs(chunk);
> > return !!(orom->sss & (1 << (fs-2)));
> >
> You're right,
>
> Please get the one from the bottom of this email then... It is based on your input (~90%) so sign it up and ship:)
> Moreover previous one is not the right one I wanted to send out...
Applied, thanks.
NeilBrown
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-12-07 1:16 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-11-25 12:12 [PATCH] imsm: fix: does not allow to use invalid chunk size Przemyslaw Czarnowski
2011-12-06 0:56 ` NeilBrown
2011-12-06 15:49 ` Hawrylewicz Czarnowski, Przemyslaw
2011-12-07 1:16 ` NeilBrown
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).