[PATCH] xen-blkfront: use bitmap_set() and bitmap_clear()

[PATCH] xen-blkfront: use bitmap_set() and bitmap_clear()

[Akinobu Mita]

Use bitmap_set and bitmap_clear rather than modifying individual bits
in a memory region.

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: xen-devel@lists.xensource.com
Cc: virtualization@lists.linux-foundation.org
---
 drivers/block/xen-blkfront.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 2f22874..619868d 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -43,6 +43,7 @@
 #include <linux/slab.h>
 #include <linux/mutex.h>
 #include <linux/scatterlist.h>
+#include <linux/bitmap.h>
 
 #include <xen/xen.h>
 #include <xen/xenbus.h>
@@ -177,8 +178,7 @@ static int xlbd_reserve_minors(unsigned int minor, unsigned int nr)
 
 	spin_lock(&minor_lock);
 	if (find_next_bit(minors, end, minor) >= end) {
-		for (; minor < end; ++minor)
-			__set_bit(minor, minors);
+		bitmap_set(minors, minor, nr);
 		rc = 0;
 	} else
 		rc = -EBUSY;
@@ -193,8 +193,7 @@ static void xlbd_release_minors(unsigned int minor, unsigned int nr)
 
 	BUG_ON(end > nr_minors);
 	spin_lock(&minor_lock);
-	for (; minor < end; ++minor)
-		__clear_bit(minor, minors);
+	bitmap_clear(minors,  minor, nr);
 	spin_unlock(&minor_lock);
 }
 
-- 
1.7.4.4

Re: [PATCH] xen-blkfront: use bitmap_set() and bitmap_clear()

[Akinobu Mita]

I used Jeremy Fitzhardinge's old email address...

2012/1/21 Akinobu Mita <akinobu.mita@gmail.com>:
> Use bitmap_set and bitmap_clear rather than modifying individual bits
> in a memory region.
>
> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> Cc: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>

Cc: Jeremy Fitzhardinge <jeremy@goop.org>

> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: xen-devel@lists.xensource.com
> Cc: virtualization@lists.linux-foundation.org
> ---
>  drivers/block/xen-blkfront.c |    7 +++----
>  1 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
> index 2f22874..619868d 100644
> --- a/drivers/block/xen-blkfront.c
> +++ b/drivers/block/xen-blkfront.c
> @@ -43,6 +43,7 @@
>  #include <linux/slab.h>
>  #include <linux/mutex.h>
>  #include <linux/scatterlist.h>
> +#include <linux/bitmap.h>
>
>  #include <xen/xen.h>
>  #include <xen/xenbus.h>
> @@ -177,8 +178,7 @@ static int xlbd_reserve_minors(unsigned int minor, unsigned int nr)
>
>        spin_lock(&minor_lock);
>        if (find_next_bit(minors, end, minor) >= end) {
> -               for (; minor < end; ++minor)
> -                       __set_bit(minor, minors);
> +               bitmap_set(minors, minor, nr);
>                rc = 0;
>        } else
>                rc = -EBUSY;
> @@ -193,8 +193,7 @@ static void xlbd_release_minors(unsigned int minor, unsigned int nr)
>
>        BUG_ON(end > nr_minors);
>        spin_lock(&minor_lock);
> -       for (; minor < end; ++minor)
> -               __clear_bit(minor, minors);
> +       bitmap_clear(minors,  minor, nr);
>        spin_unlock(&minor_lock);
>  }
>
> --
> 1.7.4.4
>

Re: [PATCH] xen-blkfront: use bitmap_set() and bitmap_clear()

[Konrad Rzeszutek Wilk]

On Sat, Jan 21, 2012 at 12:15:26AM +0900, Akinobu Mita wrote:
> Use bitmap_set and bitmap_clear rather than modifying individual bits
> in a memory region.
> 
> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> Cc: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: xen-devel@lists.xensource.com
> Cc: virtualization@lists.linux-foundation.org
> ---
>  drivers/block/xen-blkfront.c |    7 +++----
>  1 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
> index 2f22874..619868d 100644
> --- a/drivers/block/xen-blkfront.c
> +++ b/drivers/block/xen-blkfront.c
> @@ -43,6 +43,7 @@
>  #include <linux/slab.h>
>  #include <linux/mutex.h>
>  #include <linux/scatterlist.h>
> +#include <linux/bitmap.h>
>  
>  #include <xen/xen.h>
>  #include <xen/xenbus.h>
> @@ -177,8 +178,7 @@ static int xlbd_reserve_minors(unsigned int minor, unsigned int nr)
>  
>  	spin_lock(&minor_lock);
>  	if (find_next_bit(minors, end, minor) >= end) {
> -		for (; minor < end; ++minor)
> -			__set_bit(minor, minors);
> +		bitmap_set(minors, minor, nr);

Hm, I would have thought the last argument should have been 'end'?

Did you test this patch with a large amount of minors?

>  		rc = 0;
>  	} else
>  		rc = -EBUSY;
> @@ -193,8 +193,7 @@ static void xlbd_release_minors(unsigned int minor, unsigned int nr)
>  
>  	BUG_ON(end > nr_minors);
>  	spin_lock(&minor_lock);
> -	for (; minor < end; ++minor)
> -		__clear_bit(minor, minors);
> +	bitmap_clear(minors,  minor, nr);

Ditto.
>  	spin_unlock(&minor_lock);
>  }
>  
> -- 
> 1.7.4.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@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] xen-blkfront: use bitmap_set() and bitmap_clear()

[Akinobu Mita]

2012/1/21 Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>:
> On Sat, Jan 21, 2012 at 12:15:26AM +0900, Akinobu Mita wrote:
>> Use bitmap_set and bitmap_clear rather than modifying individual bits
>> in a memory region.
>>
>> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
>> Cc: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
>> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>> Cc: xen-devel@lists.xensource.com
>> Cc: virtualization@lists.linux-foundation.org
>> ---
>>  drivers/block/xen-blkfront.c |    7 +++----
>>  1 files changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
>> index 2f22874..619868d 100644
>> --- a/drivers/block/xen-blkfront.c
>> +++ b/drivers/block/xen-blkfront.c
>> @@ -43,6 +43,7 @@
>>  #include <linux/slab.h>
>>  #include <linux/mutex.h>
>>  #include <linux/scatterlist.h>
>> +#include <linux/bitmap.h>
>>
>>  #include <xen/xen.h>
>>  #include <xen/xenbus.h>
>> @@ -177,8 +178,7 @@ static int xlbd_reserve_minors(unsigned int minor, unsigned int nr)
>>
>>       spin_lock(&minor_lock);
>>       if (find_next_bit(minors, end, minor) >= end) {
>> -             for (; minor < end; ++minor)
>> -                     __set_bit(minor, minors);
>> +             bitmap_set(minors, minor, nr);
>
> Hm, I would have thought the last argument should have been 'end'?

'end' is the index of the last bit to clear.  But the last argument of
bitmap_clear() is the number of bits to clear.  So I think 'nr' is correct.

> Did you test this patch with a large amount of minors?

Sorry I didn't do runtime test.

>>               rc = 0;
>>       } else
>>               rc = -EBUSY;
>> @@ -193,8 +193,7 @@ static void xlbd_release_minors(unsigned int minor, unsigned int nr)
>>
>>       BUG_ON(end > nr_minors);
>>       spin_lock(&minor_lock);
>> -     for (; minor < end; ++minor)
>> -             __clear_bit(minor, minors);
>> +     bitmap_clear(minors,  minor, nr);
>
> Ditto.
>>       spin_unlock(&minor_lock);
>>  }
>>
>> --
>> 1.7.4.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@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] xen-blkfront: use bitmap_set() and bitmap_clear()

[Konrad Rzeszutek Wilk]

On Sat, Jan 21, 2012 at 12:41:56AM +0900, Akinobu Mita wrote:
> 2012/1/21 Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>:
> > On Sat, Jan 21, 2012 at 12:15:26AM +0900, Akinobu Mita wrote:
> >> Use bitmap_set and bitmap_clear rather than modifying individual bits
> >> in a memory region.
> >>
> >> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> >> Cc: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
> >> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> >> Cc: xen-devel@lists.xensource.com
> >> Cc: virtualization@lists.linux-foundation.org
> >> ---
> >>  drivers/block/xen-blkfront.c |    7 +++----
> >>  1 files changed, 3 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
> >> index 2f22874..619868d 100644
> >> --- a/drivers/block/xen-blkfront.c
> >> +++ b/drivers/block/xen-blkfront.c
> >> @@ -43,6 +43,7 @@
> >>  #include <linux/slab.h>
> >>  #include <linux/mutex.h>
> >>  #include <linux/scatterlist.h>
> >> +#include <linux/bitmap.h>
> >>
> >>  #include <xen/xen.h>
> >>  #include <xen/xenbus.h>
> >> @@ -177,8 +178,7 @@ static int xlbd_reserve_minors(unsigned int minor, unsigned int nr)
> >>
> >>       spin_lock(&minor_lock);
> >>       if (find_next_bit(minors, end, minor) >= end) {
> >> -             for (; minor < end; ++minor)
> >> -                     __set_bit(minor, minors);
> >> +             bitmap_set(minors, minor, nr);
> >
> > Hm, I would have thought the last argument should have been 'end'?
> 
> 'end' is the index of the last bit to clear.  But the last argument of
> bitmap_clear() is the number of bits to clear.  So I think 'nr' is correct.

Ah, I see it.
> 
> > Did you test this patch with a large amount of minors?
> 
> Sorry I didn't do runtime test.

Please do.

Re: [PATCH] xen-blkfront: use bitmap_set() and bitmap_clear()

[Andrew Morton]

On Fri, 20 Jan 2012 11:09:38 -0500
Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:

> > 
> > > Did you test this patch with a large amount of minors?
> > 
> > Sorry I didn't do runtime test.
> 
> Please do.

The poor guy probably doesn't know how to test it and surely it would
be quite a lot of work for him to do so.

Overall, it would be much more efficient if the tester of this code is
someone who is set up to easily apply the patch and test it.  ie: the
code maintainer(s).

Re: [PATCH] xen-blkfront: use bitmap_set() and bitmap_clear()

[Konrad Rzeszutek Wilk]

On Fri, Jan 20, 2012 at 03:11:49PM -0800, Andrew Morton wrote:
> On Fri, 20 Jan 2012 11:09:38 -0500
> Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> 
> > > 
> > > > Did you test this patch with a large amount of minors?
> > > 
> > > Sorry I didn't do runtime test.
> > 
> > Please do.
> 
> The poor guy probably doesn't know how to test it and surely it would
> be quite a lot of work for him to do so.
> 
> Overall, it would be much more efficient if the tester of this code is
> someone who is set up to easily apply the patch and test it.  ie: the
> code maintainer(s).

<grins>

This patch aside - Andrew how do you deal with a large amount of patches
and make sure they are tested? Is there a weekly Test Tuesday where you 
kick off your automated tests and dilligently look for any variations?

Or is it more of compile the kernel with X features and run it for a week
doing normal (and abnormal!) things to see if it falls over?

Thanks!

Re: [PATCH] xen-blkfront: use bitmap_set() and bitmap_clear()

[Andrew Morton]

On Fri, 20 Jan 2012 18:55:08 -0500
Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:

> On Fri, Jan 20, 2012 at 03:11:49PM -0800, Andrew Morton wrote:
> > On Fri, 20 Jan 2012 11:09:38 -0500
> > Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> > 
> > > > 
> > > > > Did you test this patch with a large amount of minors?
> > > > 
> > > > Sorry I didn't do runtime test.
> > > 
> > > Please do.
> > 
> > The poor guy probably doesn't know how to test it and surely it would
> > be quite a lot of work for him to do so.
> > 
> > Overall, it would be much more efficient if the tester of this code is
> > someone who is set up to easily apply the patch and test it.  ie: the
> > code maintainer(s).
> 
> <grins>
> 
> This patch aside - Andrew how do you deal with a large amount of patches
> and make sure they are tested?

Not very well :(

> Is there a weekly Test Tuesday where you 
> kick off your automated tests and dilligently look for any variations?
> Or is it more of compile the kernel with X features and run it for a week
> doing normal (and abnormal!) things to see if it falls over?

I build all the patches I have along with all of linux-next and boot
the thing, then use the resulting kernel for a few hours compilation
testing, then shove it all into linux-next where I naively hope that
someone else is testing things.

The coverage which is obtained this way is pretty poor.