Skip to contentSkip to conversation

stopmachine semaphore to mutex

PATCH In review Daniel Walker 5 participants 22 messages Dec 6, 2007 – Dec 8, 2007 1 message not in the archive Char and misc drivers

lore
Share & tools
lore
#0 lore raw

PATCH 1/3 stopmachine semaphore to mutex

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

---
 kernel/stop_machine.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
kernel/stop_machine.c +3 -3
Index: 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;
 }
 

A message that is not in the archive (sent off-list or not yet received): [email protected]

#2 lore raw

PATCH 2/3 Amiga serial driver: port_write_mutex fixup

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

---
 drivers/char/ser_a2232.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/char/ser_a2232.c +1 -1
Index: 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);
#3 lore raw

PATCH 3/3 printer port driver: semaphore to mutex

In reply to a message not in the archive

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

---
 drivers/char/lp.c  |   11 ++++++-----
 include/linux/lp.h |    2 +-
 2 files changed, 7 insertions(+), 6 deletions(-)
drivers/char/lp.c +6 -5
Index: 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;
 	}
 
include/linux/lp.h +1 -1
Index: 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;
#5 lore raw

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

Quoted 11 lines · Ingo Molnar
Quoted 4 lines · Daniel Walker

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

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

#6 lore raw

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

Daniel,

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

Remy

2007/12/6, Daniel Walker <[email protected]>:

Quoted 23 lines · Ingo Molnar
Quoted 11 lines · Daniel Walker
Quoted 5 lines

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

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

#7 lore raw

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

Quoted 3 lines · Remy Bohmer

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

#8 lore raw

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

Hello Daniel,

Quoted 5 lines

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

#9 lore raw

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

In reply to Daniel Walker

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

Quoted 13 lines · Ingo Molnar
Quoted 11 lines · Daniel Walker
Quoted 5 lines

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

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

Quoted 6 lines

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

Signature · 10 lines
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                  `-
#10 lore raw

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

Hello Matthias,

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

Remy

2007/12/6, Matthias Kaehlcke <[email protected]>:

Quoted 37 lines

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

Quoted 13 lines · Ingo Molnar
Quoted 11 lines · Daniel Walker
Quoted 5 lines

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

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

Quoted 6 lines

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 `-

#11 lore raw

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

Quoted 4 lines · Remy Bohmer

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

#12 lore raw

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

In reply to Remy Bohmer

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

Quoted 2 lines

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.

Quoted 34 lines

2007/12/6, Matthias Kaehlcke <[email protected]>:

Quoted 33 lines

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

Quoted 13 lines · Ingo Molnar
Quoted 11 lines · Daniel Walker
Quoted 5 lines

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

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

Quoted 6 lines

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 `-

Signature · 12 lines
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                  `-
#13 lore raw

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

In reply to Daniel Walker
Quoted 4 lines · Daniel Walker
Quoted 2 lines

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.

Quoted 4 lines

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

#14 lore raw

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

In reply to Daniel Walker
Quoted 12 lines · Daniel Walker
Quoted 10 lines · Ingo Molnar
Quoted 4 lines · Daniel Walker

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

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

Quoted 7 lines

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?

Signature · 1 line
Kevin Winchester
#15 lore raw

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

Quoted 21 lines · Kevin Winchester
Quoted 12 lines · Daniel Walker
Quoted 10 lines · Ingo Molnar
Quoted 4 lines · Daniel Walker

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

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

Quoted 7 lines

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) ..

Quoted 8 lines

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

#16 lore raw

Possible locking issue in viotape.c

Quoted 11 lines · Daniel Walker

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.

Signature · 1 line
Kevin Winchester
#17 lore raw

Re: Possible locking issue in viotape.c

Quoted 24 lines · Kevin Winchester
Quoted 11 lines · Daniel Walker

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:

Quoted 8 lines
	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

#18 lore raw

Re: Possible locking issue in viotape.c

Quoted 36 lines · Daniel Walker
Quoted 22 lines · Kevin Winchester
Quoted 10 lines · Daniel Walker

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:

Quoted 8 lines
	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 ..

Quoted 7 lines

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?

Signature · 1 line
Kevin Winchester
#19 lore raw

Re: Possible locking issue in viotape.c

Quoted 45 lines · Kevin Winchester
Quoted 36 lines · Daniel Walker
Quoted 22 lines · Kevin Winchester
Quoted 10 lines · Daniel Walker

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:

Quoted 8 lines
	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 ..

Quoted 7 lines

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

Quoted 5 lines

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

#20 lore raw

Re: Possible locking issue in viotape.c

Quoted 11 lines · Daniel Walker
Quoted 5 lines

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'.

Signature · 1 line
Kevin Winchester
#21 lore raw

Re: Possible locking issue in viotape.c

Quoted 9 lines · Kevin Winchester

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="[email protected]" --cc="[email protected]"'

That's how you add the recipients ..

Daniel