2007-12-03 15:49:43

by Daniel Walker

[permalink] [raw]
Subject: [PATCH] isapnp driver semaphore to mutex

Changed the isapnp semaphore to a mutex.

Signed-Off-By: Daniel Walker <[email protected]>

---
drivers/pnp/interface.c | 11 ++++++-----
drivers/pnp/manager.c | 19 ++++++++++---------
2 files changed, 16 insertions(+), 14 deletions(-)

Index: linux-2.6.23/drivers/pnp/interface.c
===================================================================
--- linux-2.6.23.orig/drivers/pnp/interface.c
+++ linux-2.6.23/drivers/pnp/interface.c
@@ -13,6 +13,7 @@
#include <linux/stat.h>
#include <linux/ctype.h>
#include <linux/slab.h>
+#include <linux/mutex.h>
#include <asm/uaccess.h>

#include "base.h"
@@ -315,7 +316,7 @@ static ssize_t pnp_show_current_resource
return ret;
}

-extern struct semaphore pnp_res_mutex;
+extern struct mutex pnp_res_mutex;

static ssize_t
pnp_set_current_resources(struct device *dmdev, struct device_attribute *attr,
@@ -361,10 +362,10 @@ pnp_set_current_resources(struct device
goto done;
}
if (!strnicmp(buf, "get", 3)) {
- down(&pnp_res_mutex);
+ mutex_lock(&pnp_res_mutex);
if (pnp_can_read(dev))
dev->protocol->get(dev, &dev->res);
- up(&pnp_res_mutex);
+ mutex_unlock(&pnp_res_mutex);
goto done;
}
if (!strnicmp(buf, "set", 3)) {
@@ -373,7 +374,7 @@ pnp_set_current_resources(struct device
goto done;
buf += 3;
pnp_init_resource_table(&dev->res);
- down(&pnp_res_mutex);
+ mutex_lock(&pnp_res_mutex);
while (1) {
while (isspace(*buf))
++buf;
@@ -455,7 +456,7 @@ pnp_set_current_resources(struct device
}
break;
}
- up(&pnp_res_mutex);
+ mutex_unlock(&pnp_res_mutex);
goto done;
}

Index: linux-2.6.23/drivers/pnp/manager.c
===================================================================
--- linux-2.6.23.orig/drivers/pnp/manager.c
+++ linux-2.6.23/drivers/pnp/manager.c
@@ -12,9 +12,10 @@
#include <linux/pnp.h>
#include <linux/slab.h>
#include <linux/bitmap.h>
+#include <linux/mutex.h>
#include "base.h"

-DECLARE_MUTEX(pnp_res_mutex);
+DEFINE_MUTEX(pnp_res_mutex);

static int pnp_assign_port(struct pnp_dev *dev, struct pnp_port *rule, int idx)
{
@@ -297,7 +298,7 @@ static int pnp_assign_resources(struct p
if (!pnp_can_configure(dev))
return -ENODEV;

- down(&pnp_res_mutex);
+ mutex_lock(&pnp_res_mutex);
pnp_clean_resource_table(&dev->res); /* start with a fresh slate */
if (dev->independent) {
port = dev->independent->port;
@@ -366,12 +367,12 @@ static int pnp_assign_resources(struct p
} else if (dev->dependent)
goto fail;

- up(&pnp_res_mutex);
+ mutex_unlock(&pnp_res_mutex);
return 1;

fail:
pnp_clean_resource_table(&dev->res);
- up(&pnp_res_mutex);
+ mutex_unlock(&pnp_res_mutex);
return 0;
}

@@ -396,7 +397,7 @@ int pnp_manual_config_dev(struct pnp_dev
return -ENOMEM;
*bak = dev->res;

- down(&pnp_res_mutex);
+ mutex_lock(&pnp_res_mutex);
dev->res = *res;
if (!(mode & PNP_CONFIG_FORCE)) {
for (i = 0; i < PNP_MAX_PORT; i++) {
@@ -416,14 +417,14 @@ int pnp_manual_config_dev(struct pnp_dev
goto fail;
}
}
- up(&pnp_res_mutex);
+ mutex_unlock(&pnp_res_mutex);

kfree(bak);
return 0;

fail:
dev->res = *bak;
- up(&pnp_res_mutex);
+ mutex_unlock(&pnp_res_mutex);
kfree(bak);
return -EINVAL;
}
@@ -547,9 +548,9 @@ int pnp_disable_dev(struct pnp_dev *dev)
dev->active = 0;

/* release the resources so that other devices can use them */
- down(&pnp_res_mutex);
+ mutex_lock(&pnp_res_mutex);
pnp_clean_resource_table(&dev->res);
- up(&pnp_res_mutex);
+ mutex_unlock(&pnp_res_mutex);

return 1;
}
--

--


2007-12-03 15:58:51

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] isapnp driver semaphore to mutex


* Daniel Walker <[email protected]> wrote:

> Changed the isapnp semaphore to a mutex.

cool - i'll give it a whirl.

small immaterial nit:

> Signed-Off-By: Daniel Walker <[email protected]>

it's Signed-off-by (note the capitalization). Having this consistent
makes scripting around patches a tiny bit easier.

Ingo

2007-12-03 16:03:48

by Daniel Walker

[permalink] [raw]
Subject: Re: [PATCH] isapnp driver semaphore to mutex

On Mon, 2007-12-03 at 16:57 +0100, Ingo Molnar wrote:
> * Daniel Walker <[email protected]> wrote:
>
> > Changed the isapnp semaphore to a mutex.
>
> cool - i'll give it a whirl.
>
> small immaterial nit:
>
> > Signed-Off-By: Daniel Walker <[email protected]>
>
> it's Signed-off-by (note the capitalization). Having this consistent
> makes scripting around patches a tiny bit easier.

Yeah, I know about it but sometimes still slips by..

Daniel

2007-12-03 16:48:48

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] isapnp driver semaphore to mutex


* Daniel Walker <[email protected]> wrote:

> On Mon, 2007-12-03 at 16:57 +0100, Ingo Molnar wrote:
> > * Daniel Walker <[email protected]> wrote:
> >
> > > Changed the isapnp semaphore to a mutex.
> >
> > cool - i'll give it a whirl.
> >
> > small immaterial nit:
> >
> > > Signed-Off-By: Daniel Walker <[email protected]>
> >
> > it's Signed-off-by (note the capitalization). Having this consistent
> > makes scripting around patches a tiny bit easier.
>
> Yeah, I know about it but sometimes still slips by..

it's worth automating these repetative bits. I use small templates when
creating patches. Also, it's worth running patches through
scripts/checkpatch.pl - it has a check for exactly this typo. [ Not a
big issue at all, i just mentioned it in case you get a taste for
sending more sem2mutex patches :-) ]

Ingo

2007-12-03 18:46:29

by Daniel Walker

[permalink] [raw]
Subject: Re: [PATCH] isapnp driver semaphore to mutex

On Mon, 2007-12-03 at 17:47 +0100, Ingo Molnar wrote:
> * Daniel Walker <[email protected]> wrote:
>
> > On Mon, 2007-12-03 at 16:57 +0100, Ingo Molnar wrote:
> > > * Daniel Walker <[email protected]> wrote:
> > >
> > > > Changed the isapnp semaphore to a mutex.
> > >
> > > cool - i'll give it a whirl.
> > >
> > > small immaterial nit:
> > >
> > > > Signed-Off-By: Daniel Walker <[email protected]>
> > >
> > > it's Signed-off-by (note the capitalization). Having this consistent
> > > makes scripting around patches a tiny bit easier.
> >
> > Yeah, I know about it but sometimes still slips by..
>
> it's worth automating these repetative bits. I use small templates when
> creating patches. Also, it's worth running patches through
> scripts/checkpatch.pl - it has a check for exactly this typo. [ Not a
> big issue at all, i just mentioned it in case you get a taste for
> sending more sem2mutex patches :-) ]
>

Speaking of automating.. I created a little .vimrc add-on which helps
doing sem2mutex type changes. Here's the chunk I added,

function Semtomutex( lo )
exe '%s/down(&'.a:lo.')/mutex_lock\(\&'.a:lo.'\)/g'
exe '%s/down_trylock(&'.a:lo.')/mutex_trylock\(\&'.a:lo.'\)/g'
exe '%s/up(&'.a:lo.')/mutex_unlock\(\&'.a:lo.'\)/g'
exe '%s/DECLARE_MUTEX('.a:lo.')/DEFINE_MUTEX\('.a:lo.'\)/g'
endfunction
map <C-\>o :call Semtomutex("<c-r><c-w>")<CR>


I just finished this about 30 seconds ago, and although I tested it once
I would guess it might have some bugs.. If you add it to your .vimrc ,
then you move your cursor over the lock name and hit Ctrl-\-o ,it will
convert all the calls for that lock name.. The list of calls it converts
isn't complete tho (down, down_trylock, and up) ..

I hope people still review the lock usage tho, since it's critical.

Daniel

2007-12-03 19:18:33

by Michal Schmidt

[permalink] [raw]
Subject: Re: [PATCH] isapnp driver semaphore to mutex

Dne Mon, 03 Dec 2007 10:35:01 -0800
Daniel Walker <[email protected]> napsal(a):

> Speaking of automating.. I created a little .vimrc add-on which helps
> doing sem2mutex type changes. Here's the chunk I added,
>
> function Semtomutex( lo )
> exe '%s/down(&'.a:lo.')/mutex_lock\(\&'.a:lo.'\)/g'
> exe '%s/down_trylock(&'.a:lo.')/mutex_trylock\(\&'.a:lo.'\)/g'

>From the comment above mutex_trylock():
* NOTE: this function follows the spin_trylock() convention, so
* it is negated to the down_trylock() return values! Be careful
* about this when converting semaphore users to mutexes.

Michal

2007-12-03 19:32:07

by Daniel Walker

[permalink] [raw]
Subject: Re: [PATCH] isapnp driver semaphore to mutex

On Mon, 2007-12-03 at 20:11 +0100, Michal Schmidt wrote:
> Dne Mon, 03 Dec 2007 10:35:01 -0800
> Daniel Walker <[email protected]> napsal(a):
>
> > Speaking of automating.. I created a little .vimrc add-on which helps
> > doing sem2mutex type changes. Here's the chunk I added,
> >
> > function Semtomutex( lo )
> > exe '%s/down(&'.a:lo.')/mutex_lock\(\&'.a:lo.'\)/g'
> > exe '%s/down_trylock(&'.a:lo.')/mutex_trylock\(\&'.a:lo.'\)/g'
>
> From the comment above mutex_trylock():
> * NOTE: this function follows the spin_trylock() convention, so
> * it is negated to the down_trylock() return values! Be careful
> * about this when converting semaphore users to mutexes.
>
> Michal

Thanks, I didn't notice this .. I've only run into one tho..

Daniel

2007-12-03 19:36:58

by Daniel Walker

[permalink] [raw]
Subject: Re: [PATCH] isapnp driver semaphore to mutex

On Mon, 2007-12-03 at 17:47 +0100, Ingo Molnar wrote:
> * Daniel Walker <[email protected]> wrote:
>
> > On Mon, 2007-12-03 at 16:57 +0100, Ingo Molnar wrote:
> > > * Daniel Walker <[email protected]> wrote:
> > >
> > > > Changed the isapnp semaphore to a mutex.
> > >
> > > cool - i'll give it a whirl.
> > >
> > > small immaterial nit:
> > >
> > > > Signed-Off-By: Daniel Walker <[email protected]>
> > >
> > > it's Signed-off-by (note the capitalization). Having this consistent
> > > makes scripting around patches a tiny bit easier.
> >
> > Yeah, I know about it but sometimes still slips by..
>
> it's worth automating these repetative bits. I use small templates when
> creating patches. Also, it's worth running patches through
> scripts/checkpatch.pl - it has a check for exactly this typo. [ Not a
> big issue at all, i just mentioned it in case you get a taste for
> sending more sem2mutex patches :-) ]


These are the patches, and files I've touched so far in case anyone is
working on this .. These are all to be submitted.

unix98-semaphore-to-mutex.patch
linux-2.6.23/drivers/char/tty_io.c
spi-semaphore-to-mutex.patch
linux-2.6.23/drivers/spi/spi.c
au1000-semaphore-to-mutex.patch
linux-2.6.23/drivers/pcmcia/au1000_generic.c
soc_common-semaphore-to-mutex.patch
linux-2.6.23/drivers/pcmcia/soc_common.c
linux-2.6.23/drivers/pcmcia/soc_common.h
qla2xxx-semaphore-to-mutex.patch
linux-2.6.23/drivers/scsi/qla2xxx/qla_os.c
bcm43xx-semaphore-to-mutex.patch
linux-2.6.23/drivers/net/wireless/bcm43xx/bcm43xx_debugfs.c
adb_handler_sem-to-mutex.patch
linux-2.6.23/drivers/macintosh/adb.c

Daniel