Thread (11 messages) 11 messages, 4 authors, 2007-02-12

Re: [PATCH] drivers/isdn/gigaset: new M101 driver

From: Andrew Morton <akpm@linux-foundation.org>
Date: 2007-02-05 04:58:40
Also in: lkml

On Mon, 05 Feb 2007 02:42:09 +0100 Tilman Schmidt [off-list ref] wrote:
Am 04.02.2007 02:56 schrieb Andrew Morton:
quoted
On Sun, 04 Feb 2007 02:32:41 +0100 Tilman Schmidt [off-list ref] wrote:
quoted
quoted
quoted
+	spin_lock_irqsave(&cs->cmdlock, flags);
+	cb = cs->cmdbuf;
+	spin_unlock_irqrestore(&cs->cmdlock, flags);
It is doubtful if the locking here does anything useful.
It assures atomicity when reading the cs->cmdbuf pointer.
I think it's bogus.  If the quantity being copied here is more than 32-bits
then yes, a lock is appropriate.  But if it's a single word then it's
unlikely that the locking does anything useful.  Or there might be a bug
here.
It's a pointer. Are reads and writes of pointer sized objects
guaranteed to be atomic on every platform?
Yup - we make the same assumption about longs in various places.

It's a bit strange to read a pointer which can be changing at the
same time.  Because the local copy will no longer represent the
thing which it was just copied from.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help