[PATCH 1/3] stopmachine semaphore to mutex

[PATCH 1/3] stopmachine semaphore to mutex

[Daniel Walker]

[-- Attachment #1: stopmachine_semaphore_to_mutex.patch --]
[-- Type: text/plain, Size: 1213 bytes --]

It's called stopmachine_mutex now, but it's a semaphore. So make it
a "struct mutex" .

Signed-off-by: Daniel Walker <dwalker@mvista.com>

---
 kernel/stop_machine.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Index: linux-2.6.23/kernel/stop_machine.c
===================================================================
--- linux-2.6.23.orig/kernel/stop_machine.c
+++ linux-2.6.23/kernel/stop_machine.c
@@ -29,7 +29,7 @@ enum stopmachine_state {
 static enum stopmachine_state stopmachine_state;
 static unsigned int stopmachine_num_threads;
 static atomic_t stopmachine_thread_ack;
-static DECLARE_MUTEX(stopmachine_mutex);
+static DEFINE_MUTEX(stopmachine_mutex);
 
 static int stopmachine(void *cpu)
 {
@@ -177,7 +177,7 @@ struct task_struct *__stop_machine_run(i
 	smdata.data = data;
 	init_completion(&smdata.done);
 
-	down(&stopmachine_mutex);
+	mutex_lock(&stopmachine_mutex);
 
 	/* If they don't care which CPU fn runs on, bind to any online one. */
 	if (cpu == NR_CPUS)
@@ -193,7 +193,7 @@ struct task_struct *__stop_machine_run(i
 		wake_up_process(p);
 		wait_for_completion(&smdata.done);
 	}
-	up(&stopmachine_mutex);
+	mutex_unlock(&stopmachine_mutex);
 	return p;
 }
 

-- 

[PATCH 2/3] Amiga serial driver: port_write_mutex fixup

[Daniel Walker]

[-- Attachment #1: ser_a2232-semaphore-fixes.patch --]
[-- Type: text/plain, Size: 826 bytes --]

The port_write_mutex was converted from a semaphore to a mutex, 
but there was still this ifdef'd init_MUTEX reference remaining.

Signed-off-by: Daniel Walker <dwalker@mvista.com>

---
 drivers/char/ser_a2232.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6.23/drivers/char/ser_a2232.c
===================================================================
--- linux-2.6.23.orig/drivers/char/ser_a2232.c
+++ linux-2.6.23/drivers/char/ser_a2232.c
@@ -653,7 +653,7 @@ static void a2232_init_portstructs(void)
 		port->gs.closing_wait = 30 * HZ;
 		port->gs.rd = &a2232_real_driver;
 #ifdef NEW_WRITE_LOCKING
-		init_MUTEX(&(port->gs.port_write_mutex));
+		mutex_init(&(port->gs.port_write_mutex));
 #endif
 		init_waitqueue_head(&port->gs.open_wait);
 		init_waitqueue_head(&port->gs.close_wait);

-- 

[PATCH 3/3] printer port driver: semaphore to mutex

[Daniel Walker]

[-- Attachment #1: lp-semaphore-to-mutex.patch --]
[-- Type: text/plain, Size: 2401 bytes --]

The port_mutex is actually a semaphore, so easily converted to
a struct mutex.

Signed-off-by: Daniel Walker <dwalker@mvista.com>

---
 drivers/char/lp.c  |   11 ++++++-----
 include/linux/lp.h |    2 +-
 2 files changed, 7 insertions(+), 6 deletions(-)

Index: linux-2.6.23/drivers/char/lp.c
===================================================================
--- linux-2.6.23.orig/drivers/char/lp.c
+++ linux-2.6.23/drivers/char/lp.c
@@ -126,6 +126,7 @@
 #include <linux/device.h>
 #include <linux/wait.h>
 #include <linux/jiffies.h>
+#include <linux/mutex.h>
 
 #include <linux/parport.h>
 #undef LP_STATS
@@ -312,7 +313,7 @@ static ssize_t lp_write(struct file * fi
 	if (copy_size > LP_BUFFER_SIZE)
 		copy_size = LP_BUFFER_SIZE;
 
-	if (down_interruptible (&lp_table[minor].port_mutex))
+	if (mutex_lock_interruptible(&lp_table[minor].port_mutex))
 		return -EINTR;
 
 	if (copy_from_user (kbuf, buf, copy_size)) {
@@ -399,7 +400,7 @@ static ssize_t lp_write(struct file * fi
 		lp_release_parport (&lp_table[minor]);
 	}
 out_unlock:
-	up (&lp_table[minor].port_mutex);
+	mutex_unlock(&lp_table[minor].port_mutex);
 
  	return retv;
 }
@@ -421,7 +422,7 @@ static ssize_t lp_read(struct file * fil
 	if (count > LP_BUFFER_SIZE)
 		count = LP_BUFFER_SIZE;
 
-	if (down_interruptible (&lp_table[minor].port_mutex))
+	if (mutex_lock_interruptible(&lp_table[minor].port_mutex))
 		return -EINTR;
 
 	lp_claim_parport_or_block (&lp_table[minor]);
@@ -479,7 +480,7 @@ static ssize_t lp_read(struct file * fil
 	if (retval > 0 && copy_to_user (buf, kbuf, retval))
 		retval = -EFAULT;
 
-	up (&lp_table[minor].port_mutex);
+	mutex_unlock(&lp_table[minor].port_mutex);
 
 	return retval;
 }
@@ -888,7 +889,7 @@ static int __init lp_init (void)
 		lp_table[i].last_error = 0;
 		init_waitqueue_head (&lp_table[i].waitq);
 		init_waitqueue_head (&lp_table[i].dataq);
-		init_MUTEX (&lp_table[i].port_mutex);
+		mutex_init(&lp_table[i].port_mutex);
 		lp_table[i].timeout = 10 * HZ;
 	}
 
Index: linux-2.6.23/include/linux/lp.h
===================================================================
--- linux-2.6.23.orig/include/linux/lp.h
+++ linux-2.6.23/include/linux/lp.h
@@ -145,7 +145,7 @@ struct lp_struct {
 #endif
 	wait_queue_head_t waitq;
 	unsigned int last_error;
-	struct semaphore port_mutex;
+	struct mutex port_mutex;
 	wait_queue_head_t dataq;
 	long timeout;
 	unsigned int best_mode;

-- 

Re: [PATCH 1/3] stopmachine semaphore to mutex

[Ingo Molnar]

* Daniel Walker <dwalker@mvista.com> wrote:

> It's called stopmachine_mutex now, but it's a semaphore. So make it a 
> "struct mutex" .
> 
> Signed-off-by: Daniel Walker <dwalker@mvista.com>

Acked-by: Ingo Molnar <mingo@elte.hu>

	Ingo

Re: [PATCH 3/3] printer port driver: semaphore to mutex

[Ingo Molnar]

* Daniel Walker <dwalker@mvista.com> wrote:

> The port_mutex is actually a semaphore, so easily converted to a 
> struct mutex.
> 
> Signed-off-by: Daniel Walker <dwalker@mvista.com>

Acked-by: Ingo Molnar <mingo@elte.hu>

cool. How far away are we from being able to remove all the semaphore 
code? :-)

	Ingo

Re: [PATCH 3/3] printer port driver: semaphore to mutex

[Daniel Walker]

On Thu, 2007-12-06 at 11:23 +0100, Ingo Molnar wrote:
> * Daniel Walker <dwalker@mvista.com> wrote:
> 
> > The port_mutex is actually a semaphore, so easily converted to a 
> > struct mutex.
> > 
> > Signed-off-by: Daniel Walker <dwalker@mvista.com>
> 
> Acked-by: Ingo Molnar <mingo@elte.hu>
> 
> cool. How far away are we from being able to remove all the semaphore 
> code? :-)

I wish my 7 patches made a dent, but it's hasn't done much. ;(

I would guess at least a week just to mop up the relatively easy ones..
I've got 12 in my queue, and there still ~50 hopefully trivial ones
still to be looked at.. Then another ~30 more difficult ones (that use
init_MUTEX_LOCKED, or sema_init with 0 instead of 1) ..

Daniel

Re: [PATCH 3/3] printer port driver: semaphore to mutex

[Remy Bohmer]

Daniel,

FYI: I am working on the conversion of the 2 sem->mutex in kernel/printk.c

Remy

2007/12/6, Daniel Walker <dwalker@mvista.com>:
> On Thu, 2007-12-06 at 11:23 +0100, Ingo Molnar wrote:
> > * Daniel Walker <dwalker@mvista.com> wrote:
> >
> > > The port_mutex is actually a semaphore, so easily converted to a
> > > struct mutex.
> > >
> > > Signed-off-by: Daniel Walker <dwalker@mvista.com>
> >
> > Acked-by: Ingo Molnar <mingo@elte.hu>
> >
> > cool. How far away are we from being able to remove all the semaphore
> > code? :-)
>
> I wish my 7 patches made a dent, but it's hasn't done much. ;(
>
> I would guess at least a week just to mop up the relatively easy ones..
> I've got 12 in my queue, and there still ~50 hopefully trivial ones
> still to be looked at.. Then another ~30 more difficult ones (that use
> init_MUTEX_LOCKED, or sema_init with 0 instead of 1) ..
>
> Daniel
>
>

Re: [PATCH 3/3] printer port driver: semaphore to mutex

[Matthias Kaehlcke]

El Thu, Dec 06, 2007 at 08:34:06AM -0800 Daniel Walker ha dit:

> On Thu, 2007-12-06 at 11:23 +0100, Ingo Molnar wrote:
> > * Daniel Walker <dwalker@mvista.com> wrote:
> > 
> > > The port_mutex is actually a semaphore, so easily converted to a 
> > > struct mutex.
> > > 
> > > Signed-off-by: Daniel Walker <dwalker@mvista.com>
> > 
> > Acked-by: Ingo Molnar <mingo@elte.hu>
> > 
> > cool. How far away are we from being able to remove all the semaphore 
> > code? :-)
> 
> I wish my 7 patches made a dent, but it's hasn't done much. ;(
> 
> I would guess at least a week just to mop up the relatively easy ones..
> I've got 12 in my queue, and there still ~50 hopefully trivial ones
> still to be looked at.. Then another ~30 more difficult ones (that use
> init_MUTEX_LOCKED, or sema_init with 0 instead of 1) ..

I've also more patches of this type on my list, though i work in a
slower pace

-- 
Matthias Kaehlcke
Linux Application Developer
Barcelona

             You must have a plan. If you don't have a plan,
               you'll become part of somebody else's plan
                                                                 .''`.
    using free software / Debian GNU/Linux | http://debian.org  : :'  :
                                                                `. `'`
gpg --keyserver pgp.mit.edu --recv-keys 47D8E5D4                  `-

Re: [PATCH 3/3] printer port driver: semaphore to mutex

[Remy Bohmer]

Hello Matthias,

Which do you have exactly on your list? (good to know, it prevents
double work...)

Remy

2007/12/6, Matthias Kaehlcke <matthias.kaehlcke@gmail.com>:
> El Thu, Dec 06, 2007 at 08:34:06AM -0800 Daniel Walker ha dit:
>
> > On Thu, 2007-12-06 at 11:23 +0100, Ingo Molnar wrote:
> > > * Daniel Walker <dwalker@mvista.com> wrote:
> > >
> > > > The port_mutex is actually a semaphore, so easily converted to a
> > > > struct mutex.
> > > >
> > > > Signed-off-by: Daniel Walker <dwalker@mvista.com>
> > >
> > > Acked-by: Ingo Molnar <mingo@elte.hu>
> > >
> > > cool. How far away are we from being able to remove all the semaphore
> > > code? :-)
> >
> > I wish my 7 patches made a dent, but it's hasn't done much. ;(
> >
> > I would guess at least a week just to mop up the relatively easy ones..
> > I've got 12 in my queue, and there still ~50 hopefully trivial ones
> > still to be looked at.. Then another ~30 more difficult ones (that use
> > init_MUTEX_LOCKED, or sema_init with 0 instead of 1) ..
>
> I've also more patches of this type on my list, though i work in a
> slower pace
>
> --
> Matthias Kaehlcke
> Linux Application Developer
> Barcelona
>
>              You must have a plan. If you don't have a plan,
>                you'll become part of somebody else's plan
>                                                                  .''`.
>     using free software / Debian GNU/Linux | http://debian.org  : :'  :
>                                                                 `. `'`
> gpg --keyserver pgp.mit.edu --recv-keys 47D8E5D4                  `-
>

Re: [PATCH 3/3] printer port driver: semaphore to mutex

[Daniel Walker]

On Thu, 2007-12-06 at 21:01 +0100, Remy Bohmer wrote:
> Daniel,
> 
> FYI: I am working on the conversion of the 2 sem->mutex in kernel/printk.c

I looked at the console_sem , but i was going to leave that as last..

The problem with the console_sem is that it can get locked from
interrupt context, which is discouraged with mutex types.. I think it
will be complicated to convert..

Daniel

Re: [PATCH 3/3] printer port driver: semaphore to mutex

[Daniel Walker]

On Thu, 2007-12-06 at 21:12 +0100, Remy Bohmer wrote:
> Hello Matthias,
> 
> Which do you have exactly on your list? (good to know, it prevents
> double work...)

Here's my list so far (in no particular order).. Most aren't tested
fully yet.. I'll try to post them someplace eventually (later today
maybe)

plip-sem-to-complete.patch
	linux-2.6.23/drivers/net/plip.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
therm_pm72-semaphore-to-mutex.patch
	linux-2.6.23/drivers/macintosh/therm_pm72.c
mbcs-semaphore-to-mutex.patch
	linux-2.6.23/drivers/char/mbcs.c
	linux-2.6.23/drivers/char/mbcs.h
ldusb-semaphore-to-mutex.patch
	linux-2.6.23/drivers/usb/misc/ldusb.c
u132-hdc-semaphore-to-mutex.patch
	linux-2.6.23/drivers/usb/host/u132-hcd.c
watchdog-wdt_pci.patch
	linux-2.6.23/drivers/watchdog/wdt_pci.c
mcheck_semaphore-to-mutex.patch
	linux-2.6.23/arch/x86/kernel/cpu/mcheck/mce_64.c
doc-kernel-locking-convert-semaphore-reference.patch
	linux-2.6.23/Documentation/DocBook/kernel-locking.tmpl
net-9p-trans_virtio-semaphore-to-mutex.patch
	linux-2.6.23/net/9p/trans_virtio.c
sound-s3c24xx-semaphore-to-mutex.patch
	linux-2.6.23/sound/soc/s3c24xx/s3c2443-ac97.c
usb-microtek-semaphore-to-mutex.patch
	linux-2.6.23/drivers/usb/image/microtek.c
	linux-2.6.23/drivers/usb/image/microtek.h
video-sa1100fb-semaphore-to-mutex.patch
	linux-2.6.23/drivers/video/sa1100fb.c
	linux-2.6.23/drivers/video/sa1100fb.h
video-pxafb-semaphore-to-mutex.patch
	linux-2.6.23/drivers/video/pxafb.c
	linux-2.6.23/drivers/video/pxafb.h
macintosh-windfarm_smu_sat-semaphore-to-mutex.patch
	linux-2.6.23/drivers/macintosh/windfarm_smu_sat.c
macintosh-therm_windtunnel-semaphore-to-mutex.patch
	linux-2.6.23/drivers/macintosh/therm_windtunnel.c
macintosh-mediabay-semaphore-to-mutex.patch
	linux-2.6.23/drivers/macintosh/mediabay.c
md-suspend_lock-semaphore-to-mutex.patch
	linux-2.6.23/drivers/md/dm.c
s390-cio-semaphore-to-mutex.patch
	linux-2.6.23/drivers/s390/cio/qdio.c
	linux-2.6.23/drivers/s390/cio/qdio.h
message-fusion-mpt-semaphore-to-mutex.patch
	linux-2.6.23/drivers/message/fusion/mptbase.c
	linux-2.6.23/drivers/message/fusion/mptbase.h
	linux-2.6.23/drivers/message/fusion/mptscsih.c
net-wan-cosa-semaphore-to-mutex.patch
	linux-2.6.23/drivers/net/wan/cosa.c
usbvision-video-error-patch-unlock-fix.patch
	linux-2.6.23/drivers/media/video/usbvision/usbvision-video.c
usbvision-video-semaphore-to-mutex.patch
	linux-2.6.23/drivers/media/video/usbvision/usbvision-video.c
	linux-2.6.23/drivers/media/video/usbvision/usbvision.h
ps3-vuart-locking-fix.patch
	linux-2.6.23/drivers/ps3/ps3-vuart.c
ps3-vuart-semaphore-to-mutex.patch
	linux-2.6.23/drivers/ps3/ps3-vuart.c
usbvision-video-remove-CtrlUrbLock.patch
	linux-2.6.23/drivers/media/video/usbvision/usbvision-core.c
	linux-2.6.23/drivers/media/video/usbvision/usbvision-video.c
	linux-2.6.23/drivers/media/video/usbvision/usbvision.h

Re: [PATCH 3/3] printer port driver: semaphore to mutex

[Remy Bohmer]

Hello Daniel,

> I looked at the console_sem , but i was going to leave that as last..
>
> The problem with the console_sem is that it can get locked from
> interrupt context, which is discouraged with mutex types.. I think it
> will be complicated to convert..

At first it looked simple, but after I sent my mail, I saw also the
interrupt issue...
But nevertheless, I am going to look at it anyway... (I think not today)

Further, I looked also at include/linux/device.h where in the 'struct
device', a semaphore is listed that is used at several places as
mutex. I was wondering what was wise here:
* replacing the semaphore with a mutex at once, and replace it
everywhere in the kernel also, or
* just add a new mutex to the struct, and change the the semaphore to
a mutex per driver, and eventually remove the semaphore from the
struct.

The first option creates the biggest patch, the 2nd leaves the risk
for never removing the semaphore from the struct device...

What is your opinion?


BTW: I also will look at the dvb_frontend.c semaphore->mutex.
I also want to look at the mutex in the PS3 code, but I cannot test it
before I receive my new PS3 ;-)

Remy

Re: [PATCH 3/3] printer port driver: semaphore to mutex

[Ingo Molnar]

* Daniel Walker <dwalker@mvista.com> wrote:

> > cool. How far away are we from being able to remove all the 
> > semaphore code? :-)
> 
> I wish my 7 patches made a dent, but it's hasn't done much. ;(

it's a beginning.

> I would guess at least a week just to mop up the relatively easy 
> ones.. I've got 12 in my queue, and there still ~50 hopefully trivial 
> ones still to be looked at.. Then another ~30 more difficult ones 
> (that use init_MUTEX_LOCKED, or sema_init with 0 instead of 1) ..

With BKL removal being projected to happen between 5 and 10 years from 
now, i think you are moving at lightning's pace :-)

	Ingo

Re: [PATCH 3/3] printer port driver: semaphore to mutex

[Kevin Winchester]

Daniel Walker wrote:
> On Thu, 2007-12-06 at 11:23 +0100, Ingo Molnar wrote:
>> * Daniel Walker <dwalker@mvista.com> wrote:
>>
>>> The port_mutex is actually a semaphore, so easily converted to a 
>>> struct mutex.
>>>
>>> Signed-off-by: Daniel Walker <dwalker@mvista.com>
>> Acked-by: Ingo Molnar <mingo@elte.hu>
>>
>> cool. How far away are we from being able to remove all the semaphore 
>> code? :-)
> 
> I wish my 7 patches made a dent, but it's hasn't done much. ;(
> 
> I would guess at least a week just to mop up the relatively easy ones..
> I've got 12 in my queue, and there still ~50 hopefully trivial ones
> still to be looked at.. Then another ~30 more difficult ones (that use
> init_MUTEX_LOCKED, or sema_init with 0 instead of 1) ..
> 

I didn't realise there were so many of these patch sets still to go.  I
could probably help out with some of them.  Is there somewhere we could
keep track of which ones are left to do, and who is handling them?  If
it would be helpful, I could go through all of the semaphore uses in the
tree and try to figure out which (if any) are true counting semaphores
that cannot be converted, and then I could post/send the list of
convertible cases.  Would that be helpful, or has it already been done
somewhere else?

-- 
Kevin Winchester

Re: [PATCH 3/3] printer port driver: semaphore to mutex

[Daniel Walker]

On Thu, 2007-12-06 at 19:30 -0400, Kevin Winchester wrote:
> Daniel Walker wrote:
> > On Thu, 2007-12-06 at 11:23 +0100, Ingo Molnar wrote:
> >> * Daniel Walker <dwalker@mvista.com> wrote:
> >>
> >>> The port_mutex is actually a semaphore, so easily converted to a 
> >>> struct mutex.
> >>>
> >>> Signed-off-by: Daniel Walker <dwalker@mvista.com>
> >> Acked-by: Ingo Molnar <mingo@elte.hu>
> >>
> >> cool. How far away are we from being able to remove all the semaphore 
> >> code? :-)
> > 
> > I wish my 7 patches made a dent, but it's hasn't done much. ;(
> > 
> > I would guess at least a week just to mop up the relatively easy ones..
> > I've got 12 in my queue, and there still ~50 hopefully trivial ones
> > still to be looked at.. Then another ~30 more difficult ones (that use
> > init_MUTEX_LOCKED, or sema_init with 0 instead of 1) ..
> > 
> 
> I didn't realise there were so many of these patch sets still to go.  I
> could probably help out with some of them.  Is there somewhere we could
> keep track of which ones are left to do, and who is handling them?  If
> it would be helpful, I could go through all of the semaphore uses in the
> tree and try to figure out which (if any) are true counting semaphores
> that cannot be converted, and then I could post/send the list of
> convertible cases.  Would that be helpful, or has it already been done
> somewhere else?

I've posted all the ones I've done so far ..

ftp://source.mvista.com/pub/dwalker/sem2mutex-2.6.24-rc4/

Feel free to review or test them.. I've found it pretty easy to simply
grep for certain class of semaphore usage, check if it's conforming to
the mutex requirements, then convert it or not.. Checking them is
getting to be a habit, so I don't think a list would help me.. However,
someone else might be able to use it..

Daniel

Possible locking issue in viotape.c

[Kevin Winchester]

Daniel Walker wrote:
> 
> I've posted all the ones I've done so far ..
> 
> ftp://source.mvista.com/pub/dwalker/sem2mutex-2.6.24-rc4/
> 
> Feel free to review or test them.. I've found it pretty easy to simply
> grep for certain class of semaphore usage, check if it's conforming to
> the mutex requirements, then convert it or not.. Checking them is
> getting to be a habit, so I don't think a list would help me.. However,
> someone else might be able to use it..
> 

Thanks, that helps me not duplicate anything.  One of the first ones I
was looking at (before your post) was viotape.c, which is in your patch
set.  However, looking at the uses of the semaphore, I see that on line
409-410 the following code:

        if (noblock)
                return count;

which seems to ignore the fact that the semaphore has been downed (not
to mention the dma buffer and op struct allocations.  I think it should be:

	if (noblock)
		ret = count;
		goto free_dma;

instead.  Do you want to make sure I'm right about that and fold it into
your patch?  Or have you already submitted your patch (or should it be
in a separate patch?  Alternatively, I can submit the patch if you don't
want to bother with it.

-- 
Kevin Winchester

Re: Possible locking issue in viotape.c

[Daniel Walker]

On Thu, 2007-12-06 at 21:29 -0400, Kevin Winchester wrote:
> Daniel Walker wrote:
> > 
> > I've posted all the ones I've done so far ..
> > 
> > ftp://source.mvista.com/pub/dwalker/sem2mutex-2.6.24-rc4/
> > 
> > Feel free to review or test them.. I've found it pretty easy to simply
> > grep for certain class of semaphore usage, check if it's conforming to
> > the mutex requirements, then convert it or not.. Checking them is
> > getting to be a habit, so I don't think a list would help me.. However,
> > someone else might be able to use it..
> > 
> 
> Thanks, that helps me not duplicate anything.  One of the first ones I
> was looking at (before your post) was viotape.c, which is in your patch
> set.  However, looking at the uses of the semaphore, I see that on line
> 409-410 the following code:
> 
>         if (noblock)
>                 return count;
> 
> which seems to ignore the fact that the semaphore has been downed (not
> to mention the dma buffer and op struct allocations.  I think it should be:
> 
> 	if (noblock)
> 		ret = count;
> 		goto free_dma;
> 
> instead.  Do you want to make sure I'm right about that and fold it into
> your patch?  Or have you already submitted your patch (or should it be
> in a separate patch?  Alternatively, I can submit the patch if you don't
> want to bother with it.

viotape was one of the first I started converting, but later I noticed
the same thing you found above. I have it commented out of my series for
that reason ..

I think this noblock path is actually doing what the author intended..
There are a few stray up() calls related to event handling and ioctls ,
and I think those are used to release the semaphore..

Daniel

Re: [PATCH 3/3] printer port driver: semaphore to mutex

[Matthias Kaehlcke]

El Thu, Dec 06, 2007 at 09:12:05PM +0100 Remy Bohmer ha dit:

> Which do you have exactly on your list? (good to know, it prevents
> double work...)

it isn't really an elaborated list, until now i greped for certain
semaphore usages, had a look at the code and converted it if
necessary. at the moment i'm having a look at usb drivers.

> 2007/12/6, Matthias Kaehlcke <matthias.kaehlcke@gmail.com>:
> > El Thu, Dec 06, 2007 at 08:34:06AM -0800 Daniel Walker ha dit:
> >
> > > On Thu, 2007-12-06 at 11:23 +0100, Ingo Molnar wrote:
> > > > * Daniel Walker <dwalker@mvista.com> wrote:
> > > >
> > > > > The port_mutex is actually a semaphore, so easily converted to a
> > > > > struct mutex.
> > > > >
> > > > > Signed-off-by: Daniel Walker <dwalker@mvista.com>
> > > >
> > > > Acked-by: Ingo Molnar <mingo@elte.hu>
> > > >
> > > > cool. How far away are we from being able to remove all the semaphore
> > > > code? :-)
> > >
> > > I wish my 7 patches made a dent, but it's hasn't done much. ;(
> > >
> > > I would guess at least a week just to mop up the relatively easy ones..
> > > I've got 12 in my queue, and there still ~50 hopefully trivial ones
> > > still to be looked at.. Then another ~30 more difficult ones (that use
> > > init_MUTEX_LOCKED, or sema_init with 0 instead of 1) ..
> >
> > I've also more patches of this type on my list, though i work in a
> > slower pace
> >
> >
> >              You must have a plan. If you don't have a plan,
> >                you'll become part of somebody else's plan
> >                                                                  .''`.
> >     using free software / Debian GNU/Linux | http://debian.org  : :'  :
> >                                                                 `. `'`
> > gpg --keyserver pgp.mit.edu --recv-keys 47D8E5D4                  `-
> >

-- 
Matthias Kaehlcke
Linux System Developer
Barcelona

                Don't walk behind me, I may not lead
             Don't walk in front of me, I may not follow
                Just walk beside me and be my friend
                          (Albert Camus)
                                                                 .''`.
    using free software / Debian GNU/Linux | http://debian.org  : :'  :
                                                                `. `'`
gpg --keyserver pgp.mit.edu --recv-keys 47D8E5D4                  `-

Re: Possible locking issue in viotape.c

[Kevin Winchester]

Daniel Walker wrote:
> On Thu, 2007-12-06 at 21:29 -0400, Kevin Winchester wrote:
>> Daniel Walker wrote:
>>> I've posted all the ones I've done so far ..
>>>
>>> ftp://source.mvista.com/pub/dwalker/sem2mutex-2.6.24-rc4/
>>>
>>> Feel free to review or test them.. I've found it pretty easy to simply
>>> grep for certain class of semaphore usage, check if it's conforming to
>>> the mutex requirements, then convert it or not.. Checking them is
>>> getting to be a habit, so I don't think a list would help me.. However,
>>> someone else might be able to use it..
>>>
>> Thanks, that helps me not duplicate anything.  One of the first ones I
>> was looking at (before your post) was viotape.c, which is in your patch
>> set.  However, looking at the uses of the semaphore, I see that on line
>> 409-410 the following code:
>>
>>         if (noblock)
>>                 return count;
>>
>> which seems to ignore the fact that the semaphore has been downed (not
>> to mention the dma buffer and op struct allocations.  I think it should be:
>>
>> 	if (noblock)
>> 		ret = count;
>> 		goto free_dma;
>>
>> instead.  Do you want to make sure I'm right about that and fold it into
>> your patch?  Or have you already submitted your patch (or should it be
>> in a separate patch?  Alternatively, I can submit the patch if you don't
>> want to bother with it.
> 
> viotape was one of the first I started converting, but later I noticed
> the same thing you found above. I have it commented out of my series for
> that reason ..
> 
> I think this noblock path is actually doing what the author intended..
> There are a few stray up() calls related to event handling and ioctls ,
> and I think those are used to release the semaphore..
> 
> Daniel
> 
> 

Yes, you are right, I hadn't finished looking at all of the up() calls
when I came to my conclusion.  I will try to convert a few that are not
on your list, but I would like to know how you are generating your
patches into those files with the diffstat and recipient list.  Is that
a feature of some git command?

-- 
Kevin Winchester

Re: Possible locking issue in viotape.c

[Daniel Walker]

On Sat, 2007-12-08 at 14:17 -0400, Kevin Winchester wrote:
> Daniel Walker wrote:
> > On Thu, 2007-12-06 at 21:29 -0400, Kevin Winchester wrote:
> >> Daniel Walker wrote:
> >>> I've posted all the ones I've done so far ..
> >>>
> >>> ftp://source.mvista.com/pub/dwalker/sem2mutex-2.6.24-rc4/
> >>>
> >>> Feel free to review or test them.. I've found it pretty easy to simply
> >>> grep for certain class of semaphore usage, check if it's conforming to
> >>> the mutex requirements, then convert it or not.. Checking them is
> >>> getting to be a habit, so I don't think a list would help me.. However,
> >>> someone else might be able to use it..
> >>>
> >> Thanks, that helps me not duplicate anything.  One of the first ones I
> >> was looking at (before your post) was viotape.c, which is in your patch
> >> set.  However, looking at the uses of the semaphore, I see that on line
> >> 409-410 the following code:
> >>
> >>         if (noblock)
> >>                 return count;
> >>
> >> which seems to ignore the fact that the semaphore has been downed (not
> >> to mention the dma buffer and op struct allocations.  I think it should be:
> >>
> >> 	if (noblock)
> >> 		ret = count;
> >> 		goto free_dma;
> >>
> >> instead.  Do you want to make sure I'm right about that and fold it into
> >> your patch?  Or have you already submitted your patch (or should it be
> >> in a separate patch?  Alternatively, I can submit the patch if you don't
> >> want to bother with it.
> > 
> > viotape was one of the first I started converting, but later I noticed
> > the same thing you found above. I have it commented out of my series for
> > that reason ..
> > 
> > I think this noblock path is actually doing what the author intended..
> > There are a few stray up() calls related to event handling and ioctls ,
> > and I think those are used to release the semaphore..
> > 
> > Daniel
> > 
> > 
> 
> Yes, you are right, I hadn't finished looking at all of the up() calls
> when I came to my conclusion.  I will try to convert a few that are not
> on your list, but I would like to know how you are generating your
> patches into those files with the diffstat and recipient list.  Is that
> a feature of some git command?

Git may have a similar feature, but I'm using a tool call Quilt. It's
used for managing a list of patches, and it can automatically add a
diffstat to the patch header (part of the quilt refresh command). I also
use it for emailing a list of patches.

Daniel

Re: Possible locking issue in viotape.c

[Kevin Winchester]

Daniel Walker wrote:
>> Yes, you are right, I hadn't finished looking at all of the up() calls
>> when I came to my conclusion.  I will try to convert a few that are not
>> on your list, but I would like to know how you are generating your
>> patches into those files with the diffstat and recipient list.  Is that
>> a feature of some git command?
> 
> Git may have a similar feature, but I'm using a tool call Quilt. It's
> used for managing a list of patches, and it can automatically add a
> diffstat to the patch header (part of the quilt refresh command). I also
> use it for emailing a list of patches.
> 

Yes, I've used quilt for working with mm patches in the past, but I'm
not too familiar with the mail features.  For example, how do you get
the recipient list and Signed-off-by in the patch file?  Do you just
edit it by hand?  Or is there some guide to quilt I should be reading?
I looked at the guide in /usr/share/doc/quilt/quilt.pdf, but it didn't
have anything about email.  /usr/share/doc/quilt/README.MAIL has the
details of all of the mail options, but doesn't give a nice example of
what to do with the patch file and how to call 'quilt mail'.

-- 
Kevin Winchester

Re: Possible locking issue in viotape.c

[Daniel Walker]

On Sat, 2007-12-08 at 15:19 -0400, Kevin Winchester wrote:

> 
> Yes, I've used quilt for working with mm patches in the past, but I'm
> not too familiar with the mail features.  For example, how do you get
> the recipient list and Signed-off-by in the patch file?  Do you just
> edit it by hand?  Or is there some guide to quilt I should be reading?
> I looked at the guide in /usr/share/doc/quilt/quilt.pdf, but it didn't
> have anything about email.  /usr/share/doc/quilt/README.MAIL has the
> details of all of the mail options, but doesn't give a nice example of
> what to do with the patch file and how to call 'quilt mail'.

The patch description and Signed-off-by, is added with the command
"quilt header -e" which opens and editor.. I usually use something like
this to mail out patches 

'quilt mail --send --to="akpm@linux-foundation.org" --cc="linux-kernel@vger.kernel.org"'

That's how you add the recipients ..

Daniel