Re: [PATCH 1/1] drivers/s390/cio/qdio_main.c: replace shift loop by ilog2

2014-06-03 Thread Sebastian Ott 
On Mon, 2 Jun 2014, Fabian Frederick wrote:
> On Mon, 2 Jun 2014 16:41:48 +0200 (CEST)
> Sebastian Ott  wrote:
> 
> > Hello Fabian,
> > 
> > On Tue, 20 May 2014, Fabian Frederick wrote:
> > > 
> > > This is untested.
> > > 
> > > Cc: Sebastian Ott 
> > > Cc: Andrew Morton 
> > > Signed-off-by: Fabian Frederick 
> > > ---
> > >  drivers/s390/cio/qdio_main.c | 5 ++---
> > >  1 file changed, 2 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/s390/cio/qdio_main.c b/drivers/s390/cio/qdio_main.c
> > > index 77466c4..8bf9ec1 100644
> > > --- a/drivers/s390/cio/qdio_main.c
> > > +++ b/drivers/s390/cio/qdio_main.c
> > > @@ -411,15 +411,14 @@ static inline void qdio_stop_polling(struct qdio_q 
> > > *q)
> > > 
> > >  static inline void account_sbals(struct qdio_q *q, int count)
> > >  {
> > > - int pos = 0;
> > > + int pos;
> > > 
> > >   q->q_stats.nr_sbal_total += count;
> > >   if (count == QDIO_MAX_BUFFERS_MASK) {
> > >   q->q_stats.nr_sbals[7]++;
> > >   return;
> > >   }
> > > - while (count >>= 1)
> > > - pos++;
> > > + pos = ilog2(count);
> > >   q->q_stats.nr_sbals[pos]++;
> > >  }
> > > 
> > > -- 
> > > 1.8.4.5
> > > 
> > > 
> > 
> > Could you please resend the patch with a better description plus
> > the change Joe suggested.
> > 
> > Thanks,
> > Sebastian
> 
> Hello Sebastian,
>Conclusion of that patch/thread was that callers guarantee count to be > 0

Correct.

> Joe suggested to have unsigned count. Is it what you're talking about ?

Yes.

Regards,
Sebastian

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] drivers/s390/cio/qdio_main.c: replace shift loop by ilog2

2014-06-03 Thread Sebastian Ott 
On Mon, 2 Jun 2014, Fabian Frederick wrote:
 On Mon, 2 Jun 2014 16:41:48 +0200 (CEST)
 Sebastian Ott seb...@linux.vnet.ibm.com wrote:
 
  Hello Fabian,
  
  On Tue, 20 May 2014, Fabian Frederick wrote:
   
   This is untested.
   
   Cc: Sebastian Ott seb...@linux.vnet.ibm.com
   Cc: Andrew Morton a...@linux-foundation.org
   Signed-off-by: Fabian Frederick f...@skynet.be
   ---
drivers/s390/cio/qdio_main.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
   
   diff --git a/drivers/s390/cio/qdio_main.c b/drivers/s390/cio/qdio_main.c
   index 77466c4..8bf9ec1 100644
   --- a/drivers/s390/cio/qdio_main.c
   +++ b/drivers/s390/cio/qdio_main.c
   @@ -411,15 +411,14 @@ static inline void qdio_stop_polling(struct qdio_q 
   *q)
   
static inline void account_sbals(struct qdio_q *q, int count)
{
   - int pos = 0;
   + int pos;
   
 q-q_stats.nr_sbal_total += count;
 if (count == QDIO_MAX_BUFFERS_MASK) {
 q-q_stats.nr_sbals[7]++;
 return;
 }
   - while (count = 1)
   - pos++;
   + pos = ilog2(count);
 q-q_stats.nr_sbals[pos]++;
}
   
   -- 
   1.8.4.5
   
   
  
  Could you please resend the patch with a better description plus
  the change Joe suggested.
  
  Thanks,
  Sebastian
 
 Hello Sebastian,
Conclusion of that patch/thread was that callers guarantee count to be  0

Correct.

 Joe suggested to have unsigned count. Is it what you're talking about ?

Yes.

Regards,
Sebastian

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] drivers/s390/cio/qdio_main.c: replace shift loop by ilog2

2014-06-02 Thread Fabian Frederick 
On Mon, 2 Jun 2014 16:41:48 +0200 (CEST)
Sebastian Ott  wrote:

> Hello Fabian,
> 
> On Tue, 20 May 2014, Fabian Frederick wrote:
> > 
> > This is untested.
> > 
> > Cc: Sebastian Ott 
> > Cc: Andrew Morton 
> > Signed-off-by: Fabian Frederick 
> > ---
> >  drivers/s390/cio/qdio_main.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/s390/cio/qdio_main.c b/drivers/s390/cio/qdio_main.c
> > index 77466c4..8bf9ec1 100644
> > --- a/drivers/s390/cio/qdio_main.c
> > +++ b/drivers/s390/cio/qdio_main.c
> > @@ -411,15 +411,14 @@ static inline void qdio_stop_polling(struct qdio_q *q)
> > 
> >  static inline void account_sbals(struct qdio_q *q, int count)
> >  {
> > -   int pos = 0;
> > +   int pos;
> > 
> > q->q_stats.nr_sbal_total += count;
> > if (count == QDIO_MAX_BUFFERS_MASK) {
> > q->q_stats.nr_sbals[7]++;
> > return;
> > }
> > -   while (count >>= 1)
> > -   pos++;
> > +   pos = ilog2(count);
> > q->q_stats.nr_sbals[pos]++;
> >  }
> > 
> > -- 
> > 1.8.4.5
> > 
> > 
> 
> Could you please resend the patch with a better description plus
> the change Joe suggested.
> 
> Thanks,
> Sebastian

Hello Sebastian,
   Conclusion of that patch/thread was that callers guarantee count to be > 0

Joe suggested to have unsigned count. Is it what you're talking about ?

Regards,
Fabian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] drivers/s390/cio/qdio_main.c: replace shift loop by ilog2

2014-06-02 Thread Sebastian Ott 
Hello Fabian,

On Tue, 20 May 2014, Fabian Frederick wrote:
> 
> This is untested.
> 
> Cc: Sebastian Ott 
> Cc: Andrew Morton 
> Signed-off-by: Fabian Frederick 
> ---
>  drivers/s390/cio/qdio_main.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/s390/cio/qdio_main.c b/drivers/s390/cio/qdio_main.c
> index 77466c4..8bf9ec1 100644
> --- a/drivers/s390/cio/qdio_main.c
> +++ b/drivers/s390/cio/qdio_main.c
> @@ -411,15 +411,14 @@ static inline void qdio_stop_polling(struct qdio_q *q)
> 
>  static inline void account_sbals(struct qdio_q *q, int count)
>  {
> - int pos = 0;
> + int pos;
> 
>   q->q_stats.nr_sbal_total += count;
>   if (count == QDIO_MAX_BUFFERS_MASK) {
>   q->q_stats.nr_sbals[7]++;
>   return;
>   }
> - while (count >>= 1)
> - pos++;
> + pos = ilog2(count);
>   q->q_stats.nr_sbals[pos]++;
>  }
> 
> -- 
> 1.8.4.5
> 
> 

Could you please resend the patch with a better description plus
the change Joe suggested.

Thanks,
Sebastian

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] drivers/s390/cio/qdio_main.c: replace shift loop by ilog2

2014-06-02 Thread Sebastian Ott 
Hello Fabian,

On Tue, 20 May 2014, Fabian Frederick wrote:
 
 This is untested.
 
 Cc: Sebastian Ott seb...@linux.vnet.ibm.com
 Cc: Andrew Morton a...@linux-foundation.org
 Signed-off-by: Fabian Frederick f...@skynet.be
 ---
  drivers/s390/cio/qdio_main.c | 5 ++---
  1 file changed, 2 insertions(+), 3 deletions(-)
 
 diff --git a/drivers/s390/cio/qdio_main.c b/drivers/s390/cio/qdio_main.c
 index 77466c4..8bf9ec1 100644
 --- a/drivers/s390/cio/qdio_main.c
 +++ b/drivers/s390/cio/qdio_main.c
 @@ -411,15 +411,14 @@ static inline void qdio_stop_polling(struct qdio_q *q)
 
  static inline void account_sbals(struct qdio_q *q, int count)
  {
 - int pos = 0;
 + int pos;
 
   q-q_stats.nr_sbal_total += count;
   if (count == QDIO_MAX_BUFFERS_MASK) {
   q-q_stats.nr_sbals[7]++;
   return;
   }
 - while (count = 1)
 - pos++;
 + pos = ilog2(count);
   q-q_stats.nr_sbals[pos]++;
  }
 
 -- 
 1.8.4.5
 
 

Could you please resend the patch with a better description plus
the change Joe suggested.

Thanks,
Sebastian

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] drivers/s390/cio/qdio_main.c: replace shift loop by ilog2

2014-06-02 Thread Fabian Frederick 
On Mon, 2 Jun 2014 16:41:48 +0200 (CEST)
Sebastian Ott seb...@linux.vnet.ibm.com wrote:

 Hello Fabian,
 
 On Tue, 20 May 2014, Fabian Frederick wrote:
  
  This is untested.
  
  Cc: Sebastian Ott seb...@linux.vnet.ibm.com
  Cc: Andrew Morton a...@linux-foundation.org
  Signed-off-by: Fabian Frederick f...@skynet.be
  ---
   drivers/s390/cio/qdio_main.c | 5 ++---
   1 file changed, 2 insertions(+), 3 deletions(-)
  
  diff --git a/drivers/s390/cio/qdio_main.c b/drivers/s390/cio/qdio_main.c
  index 77466c4..8bf9ec1 100644
  --- a/drivers/s390/cio/qdio_main.c
  +++ b/drivers/s390/cio/qdio_main.c
  @@ -411,15 +411,14 @@ static inline void qdio_stop_polling(struct qdio_q *q)
  
   static inline void account_sbals(struct qdio_q *q, int count)
   {
  -   int pos = 0;
  +   int pos;
  
  q-q_stats.nr_sbal_total += count;
  if (count == QDIO_MAX_BUFFERS_MASK) {
  q-q_stats.nr_sbals[7]++;
  return;
  }
  -   while (count = 1)
  -   pos++;
  +   pos = ilog2(count);
  q-q_stats.nr_sbals[pos]++;
   }
  
  -- 
  1.8.4.5
  
  
 
 Could you please resend the patch with a better description plus
 the change Joe suggested.
 
 Thanks,
 Sebastian

Hello Sebastian,
   Conclusion of that patch/thread was that callers guarantee count to be  0

Joe suggested to have unsigned count. Is it what you're talking about ?

Regards,
Fabian
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] drivers/s390/cio/qdio_main.c: replace shift loop by ilog2

2014-05-22 Thread Ursula Braun 
yes, the callers guarantee that count > 0 here.

Regards, Ursula Braun

On Thu, 2014-05-22 at 12:14 +0200, Ursula Braun1 wrote:
> From:Sebastian Ott  
> To:Joe Perches , Ursula
> Braun1/Germany/IBM@IBMDE, 
> Cc:Fabian Frederick , linux-kernel
> , akpm  
> Date:21/05/2014 14:32 
> Subject:    Re: [PATCH 1/1] drivers/s390/cio/qdio_main.c: replace
> shift loop by ilog2 
> 
> __
> 
> 
> 
> On Tue, 20 May 2014, Joe Perches wrote:
> > On Tue, 2014-05-20 at 18:37 +0200, Fabian Frederick wrote:
> > > This is untested.
> > []
> > > diff --git a/drivers/s390/cio/qdio_main.c
> b/drivers/s390/cio/qdio_main.c
> > []
> > > @@ -411,15 +411,14 @@ static inline void qdio_stop_polling(struct
> qdio_q *q)
> > >  
> > >  static inline void account_sbals(struct qdio_q *q, int count)
> > >  {
> > > - int pos = 0;
> > > + int pos;
> > >  
> > >   q->q_stats.nr_sbal_total += count;
> > >   if (count == QDIO_MAX_BUFFERS_MASK) {
> > >q->q_stats.nr_sbals[7]++;
> > >return;
> > >   }
> > > - while (count >>= 1)
> > > -  pos++;
> > > + pos = ilog2(count);
> > 
> > What guarantees count > 0 here?
> 
> We would already be screwed when called with count < 0. Ursula, do the
> callers assure that count is positive?
> 
> Regards,
> Sebastian
> > 
> > count may be better unsigned.
> > 
> > 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] drivers/s390/cio/qdio_main.c: replace shift loop by ilog2

2014-05-22 Thread Ursula Braun 
yes, the callers guarantee that count  0 here.

Regards, Ursula Braun

On Thu, 2014-05-22 at 12:14 +0200, Ursula Braun1 wrote:
 From:Sebastian Ott seb...@linux.vnet.ibm.com 
 To:Joe Perches j...@perches.com, Ursula
 Braun1/Germany/IBM@IBMDE, 
 Cc:Fabian Frederick f...@skynet.be, linux-kernel
 linux-kernel@vger.kernel.org, akpm a...@linux-foundation.org 
 Date:21/05/2014 14:32 
 Subject:Re: [PATCH 1/1] drivers/s390/cio/qdio_main.c: replace
 shift loop by ilog2 
 
 __
 
 
 
 On Tue, 20 May 2014, Joe Perches wrote:
  On Tue, 2014-05-20 at 18:37 +0200, Fabian Frederick wrote:
   This is untested.
  []
   diff --git a/drivers/s390/cio/qdio_main.c
 b/drivers/s390/cio/qdio_main.c
  []
   @@ -411,15 +411,14 @@ static inline void qdio_stop_polling(struct
 qdio_q *q)

static inline void account_sbals(struct qdio_q *q, int count)
{
   - int pos = 0;
   + int pos;

 q-q_stats.nr_sbal_total += count;
 if (count == QDIO_MAX_BUFFERS_MASK) {
  q-q_stats.nr_sbals[7]++;
  return;
 }
   - while (count = 1)
   -  pos++;
   + pos = ilog2(count);
  
  What guarantees count  0 here?
 
 We would already be screwed when called with count  0. Ursula, do the
 callers assure that count is positive?
 
 Regards,
 Sebastian
  
  count may be better unsigned.
  
  


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] drivers/s390/cio/qdio_main.c: replace shift loop by ilog2

2014-05-21 Thread Sebastian Ott 
On Tue, 20 May 2014, Joe Perches wrote:
> On Tue, 2014-05-20 at 18:37 +0200, Fabian Frederick wrote:
> > This is untested.
> []
> > diff --git a/drivers/s390/cio/qdio_main.c b/drivers/s390/cio/qdio_main.c
> []
> > @@ -411,15 +411,14 @@ static inline void qdio_stop_polling(struct qdio_q *q)
> >  
> >  static inline void account_sbals(struct qdio_q *q, int count)
> >  {
> > -   int pos = 0;
> > +   int pos;
> >  
> > q->q_stats.nr_sbal_total += count;
> > if (count == QDIO_MAX_BUFFERS_MASK) {
> > q->q_stats.nr_sbals[7]++;
> > return;
> > }
> > -   while (count >>= 1)
> > -   pos++;
> > +   pos = ilog2(count);
> 
> What guarantees count > 0 here?

We would already be screwed when called with count < 0. Ursula, do the
callers assure that count is positive?

Regards,
Sebastian
> 
> count may be better unsigned.
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] drivers/s390/cio/qdio_main.c: replace shift loop by ilog2

2014-05-21 Thread Sebastian Ott 
On Tue, 20 May 2014, Joe Perches wrote:
 On Tue, 2014-05-20 at 18:37 +0200, Fabian Frederick wrote:
  This is untested.
 []
  diff --git a/drivers/s390/cio/qdio_main.c b/drivers/s390/cio/qdio_main.c
 []
  @@ -411,15 +411,14 @@ static inline void qdio_stop_polling(struct qdio_q *q)
   
   static inline void account_sbals(struct qdio_q *q, int count)
   {
  -   int pos = 0;
  +   int pos;
   
  q-q_stats.nr_sbal_total += count;
  if (count == QDIO_MAX_BUFFERS_MASK) {
  q-q_stats.nr_sbals[7]++;
  return;
  }
  -   while (count = 1)
  -   pos++;
  +   pos = ilog2(count);
 
 What guarantees count  0 here?

We would already be screwed when called with count  0. Ursula, do the
callers assure that count is positive?

Regards,
Sebastian
 
 count may be better unsigned.
 
 

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] drivers/s390/cio/qdio_main.c: replace shift loop by ilog2

2014-05-20 Thread Joe Perches 
On Tue, 2014-05-20 at 18:37 +0200, Fabian Frederick wrote:
> This is untested.
[]
> diff --git a/drivers/s390/cio/qdio_main.c b/drivers/s390/cio/qdio_main.c
[]
> @@ -411,15 +411,14 @@ static inline void qdio_stop_polling(struct qdio_q *q)
>  
>  static inline void account_sbals(struct qdio_q *q, int count)
>  {
> - int pos = 0;
> + int pos;
>  
>   q->q_stats.nr_sbal_total += count;
>   if (count == QDIO_MAX_BUFFERS_MASK) {
>   q->q_stats.nr_sbals[7]++;
>   return;
>   }
> - while (count >>= 1)
> - pos++;
> + pos = ilog2(count);

What guarantees count > 0 here?

count may be better unsigned.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 1/1] drivers/s390/cio/qdio_main.c: replace shift loop by ilog2

2014-05-20 Thread Fabian Frederick 

This is untested.

Cc: Sebastian Ott 
Cc: Andrew Morton 
Signed-off-by: Fabian Frederick 
---
 drivers/s390/cio/qdio_main.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/s390/cio/qdio_main.c b/drivers/s390/cio/qdio_main.c
index 77466c4..8bf9ec1 100644
--- a/drivers/s390/cio/qdio_main.c
+++ b/drivers/s390/cio/qdio_main.c
@@ -411,15 +411,14 @@ static inline void qdio_stop_polling(struct qdio_q *q)
 
 static inline void account_sbals(struct qdio_q *q, int count)
 {
-   int pos = 0;
+   int pos;
 
q->q_stats.nr_sbal_total += count;
if (count == QDIO_MAX_BUFFERS_MASK) {
q->q_stats.nr_sbals[7]++;
return;
}
-   while (count >>= 1)
-   pos++;
+   pos = ilog2(count);
q->q_stats.nr_sbals[pos]++;
 }
 
-- 
1.8.4.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 1/1] drivers/s390/cio/qdio_main.c: replace shift loop by ilog2

2014-05-20 Thread Fabian Frederick 

This is untested.

Cc: Sebastian Ott seb...@linux.vnet.ibm.com
Cc: Andrew Morton a...@linux-foundation.org
Signed-off-by: Fabian Frederick f...@skynet.be
---
 drivers/s390/cio/qdio_main.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/s390/cio/qdio_main.c b/drivers/s390/cio/qdio_main.c
index 77466c4..8bf9ec1 100644
--- a/drivers/s390/cio/qdio_main.c
+++ b/drivers/s390/cio/qdio_main.c
@@ -411,15 +411,14 @@ static inline void qdio_stop_polling(struct qdio_q *q)
 
 static inline void account_sbals(struct qdio_q *q, int count)
 {
-   int pos = 0;
+   int pos;
 
q-q_stats.nr_sbal_total += count;
if (count == QDIO_MAX_BUFFERS_MASK) {
q-q_stats.nr_sbals[7]++;
return;
}
-   while (count = 1)
-   pos++;
+   pos = ilog2(count);
q-q_stats.nr_sbals[pos]++;
 }
 
-- 
1.8.4.5
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] drivers/s390/cio/qdio_main.c: replace shift loop by ilog2

2014-05-20 Thread Joe Perches 
On Tue, 2014-05-20 at 18:37 +0200, Fabian Frederick wrote:
 This is untested.
[]
 diff --git a/drivers/s390/cio/qdio_main.c b/drivers/s390/cio/qdio_main.c
[]
 @@ -411,15 +411,14 @@ static inline void qdio_stop_polling(struct qdio_q *q)
  
  static inline void account_sbals(struct qdio_q *q, int count)
  {
 - int pos = 0;
 + int pos;
  
   q-q_stats.nr_sbal_total += count;
   if (count == QDIO_MAX_BUFFERS_MASK) {
   q-q_stats.nr_sbals[7]++;
   return;
   }
 - while (count = 1)
 - pos++;
 + pos = ilog2(count);

What guarantees count  0 here?

count may be better unsigned.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/