[PATCH] spin_lock_unlocked cleanups

[PATCH] spin_lock_unlocked cleanups

[roel]

Replace some SPIN_LOCK_UNLOCKED with DEFINE_SPINLOCK

Signed-off-by: Roel Kluin <12o3l@tiscali.nl>
---
diff --git a/arch/mips/pci/ops-pmcmsp.c b/arch/mips/pci/ops-pmcmsp.c
index 09fa007..059eade 100644
--- a/arch/mips/pci/ops-pmcmsp.c
+++ b/arch/mips/pci/ops-pmcmsp.c
@@ -206,7 +206,7 @@ static void pci_proc_init(void)
 }
 #endif /* CONFIG_PROC_FS && PCI_COUNTERS */
 
-spinlock_t bpci_lock = SPIN_LOCK_UNLOCKED;
+DEFINE_SPINLOCK(bpci_lock);
 
 /*****************************************************************************
  *
diff --git a/arch/powerpc/mm/slice.c b/arch/powerpc/mm/slice.c
index d5fd390..cd2766e 100644
--- a/arch/powerpc/mm/slice.c
+++ b/arch/powerpc/mm/slice.c
@@ -34,7 +34,7 @@
 #include <asm/mmu.h>
 #include <asm/spu.h>
 
-static spinlock_t slice_convert_lock = SPIN_LOCK_UNLOCKED;
+static DEFINE_SPINLOCK(slice_convert_lock);
 
 
 #ifdef DEBUG
diff --git a/drivers/char/watchdog/bfin_wdt.c b/drivers/char/watchdog/bfin_wdt.c
index 309d279..31dc7a6 100644
--- a/drivers/char/watchdog/bfin_wdt.c
+++ b/drivers/char/watchdog/bfin_wdt.c
@@ -71,7 +71,7 @@ static int nowayout = WATCHDOG_NOWAYOUT;
 static struct watchdog_info bfin_wdt_info;
 static unsigned long open_check;
 static char expect_close;
-static spinlock_t bfin_wdt_spinlock = SPIN_LOCK_UNLOCKED;
+static DEFINE_SPINLOCK(bfin_wdt_spinlock);
 
 /**
  *	bfin_wdt_keepalive - Keep the Userspace Watchdog Alive
diff --git a/drivers/ieee1394/ieee1394_core.c b/drivers/ieee1394/ieee1394_core.c
index 98fd985..36c747b 100644
--- a/drivers/ieee1394/ieee1394_core.c
+++ b/drivers/ieee1394/ieee1394_core.c
@@ -488,7 +488,7 @@ void hpsb_selfid_complete(struct hpsb_host *host, int phyid, int isroot)
 	highlevel_host_reset(host);
 }
 
-static spinlock_t pending_packets_lock = SPIN_LOCK_UNLOCKED;
+static DEFINE_SPINLOCK(pending_packets_lock);
 
 /**
  * hpsb_packet_sent - notify core of sending a packet
diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index 83e76b3..94fd78f 100644
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -15,9 +15,9 @@
 #include "sysfs.h"
 
 DEFINE_MUTEX(sysfs_mutex);
-spinlock_t sysfs_assoc_lock = SPIN_LOCK_UNLOCKED;
+DEFINE_SPINLOCK(sysfs_assoc_lock);
 
-static spinlock_t sysfs_ino_lock = SPIN_LOCK_UNLOCKED;
+static DEFINE_SPINLOCK(sysfs_ino_lock);
 static DEFINE_IDA(sysfs_ino_ida);
 
 /**

Re: [PATCH] spin_lock_unlocked cleanups

[Thomas Gleixner]

On Thu, 2007-09-27 at 23:36 +0200, roel wrote:
> Replace some SPIN_LOCK_UNLOCKED with DEFINE_SPINLOCK
> 
> Signed-off-by: Roel Kluin <12o3l@tiscali.nl>

Acked-by: Thomas Gleixner <tglx@linutronix.de>

Andy, Randy,

can we please add this to checkpatch.pl ? 

> -spinlock_t bpci_lock = SPIN_LOCK_UNLOCKED;
> +DEFINE_SPINLOCK(bpci_lock);

This code was introduced in June 2007, almost two years after the first
big DEFINE_SPINLOCK cleanup. Sigh.

Thanks,

	tglx

Re: [PATCH] spin_lock_unlocked cleanups

[Andrew Morton]

On Fri, 28 Sep 2007 10:17:30 +0200 Thomas Gleixner <tglx@linutronix.de> wrote:

> can we please add this to checkpatch.pl ? 
> 
> > -spinlock_t bpci_lock = SPIN_LOCK_UNLOCKED;
> > +DEFINE_SPINLOCK(bpci_lock);

That check is already in checkpatch.  Problem is that hardly anyone
runs the thing.

I think we're ready to wire checkpatch up to a email robot which monitors
the mailing lists and sends people nastygrams.  I bet that'll be popular ;)

(I'd love it if it could detect wordwrapped and tab-expanded patches, too. 
You wouldn't _believe_...)

Re: [PATCH] spin_lock_unlocked cleanups

[Ingo Molnar]

* Andrew Morton <akpm@linux-foundation.org> wrote:

> On Fri, 28 Sep 2007 10:17:30 +0200 Thomas Gleixner <tglx@linutronix.de> wrote:
> 
> > can we please add this to checkpatch.pl ? 
> > 
> > > -spinlock_t bpci_lock = SPIN_LOCK_UNLOCKED;
> > > +DEFINE_SPINLOCK(bpci_lock);
> 
> That check is already in checkpatch.  Problem is that hardly anyone 
> runs the thing.

i automatically run it for every patch i submit or push out via git.

> I think we're ready to wire checkpatch up to a email robot which 
> monitors the mailing lists and sends people nastygrams.  I bet that'll 
> be popular ;)

heh ;-) It could be automated for patches that are sent out with a 
Signed-off-by [or a Reviewed-by] line. If you send a SoB patch that is 
broken, prepare to get a nastygram. (Initially i'd suggest the nastygram 
to Cc: to a different email list, not lkml.)

	Ingo

Re: [PATCH] spin_lock_unlocked cleanups

[Thomas Gleixner]

On Fri, 2007-09-28 at 01:26 -0700, Andrew Morton wrote:
> On Fri, 28 Sep 2007 10:17:30 +0200 Thomas Gleixner <tglx@linutronix.de> wrote:
> 
> > can we please add this to checkpatch.pl ? 
> > 
> > > -spinlock_t bpci_lock = SPIN_LOCK_UNLOCKED;
> > > +DEFINE_SPINLOCK(bpci_lock);
> 
> That check is already in checkpatch.  Problem is that hardly anyone
> runs the thing.

Sigh, I forgot that perl is write only. :)

> I think we're ready to wire checkpatch up to a email robot which monitors
> the mailing lists and sends people nastygrams.  I bet that'll be popular ;)

We should wire it up to git-commit as well. A lot of that comes in via
git subsystems.

> (I'd love it if it could detect wordwrapped and tab-expanded patches, too. 
> You wouldn't _believe_...)

I know ...

	tglx

Re: [PATCH] spin_lock_unlocked cleanups

[Andy Whitcroft]

On Fri, Sep 28, 2007 at 01:26:56AM -0700, Andrew Morton wrote:
> On Fri, 28 Sep 2007 10:17:30 +0200 Thomas Gleixner <tglx@linutronix.de> wrote:
> 
> > can we please add this to checkpatch.pl ? 
> > 
> > > -spinlock_t bpci_lock = SPIN_LOCK_UNLOCKED;
> > > +DEFINE_SPINLOCK(bpci_lock);
> 
> That check is already in checkpatch.  Problem is that hardly anyone
> runs the thing.
> 
> I think we're ready to wire checkpatch up to a email robot which monitors
> the mailing lists and sends people nastygrams.  I bet that'll be popular ;)

That shouldn't be too hard.  checkpatch has been subscribed since birth
but short circuiting the replies to me only.

I guess the main question is whether to reply-all or reply just to the
sender when commenting on patches.  Perhaps for the sanity of the rest
of the world, just the sender makes most sense.

> (I'd love it if it could detect wordwrapped and tab-expanded patches, too. 
> You wouldn't _believe_...)

It should pick up both of these, the word-wrapping is already there as
we detect lines within patch segments which don't start '[ +-]', the
tab-expanded should be picked up as every line would be "don't use
spaces use tabs for indent".

-apw

Re: [PATCH] spin_lock_unlocked cleanups

[Andrew Morton]

On Fri, 28 Sep 2007 10:30:37 +0200 Ingo Molnar <mingo@elte.hu> wrote:

> 
> * Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> > On Fri, 28 Sep 2007 10:17:30 +0200 Thomas Gleixner <tglx@linutronix.de> wrote:
> > 
> > > can we please add this to checkpatch.pl ? 
> > > 
> > > > -spinlock_t bpci_lock = SPIN_LOCK_UNLOCKED;
> > > > +DEFINE_SPINLOCK(bpci_lock);
> > 
> > That check is already in checkpatch.  Problem is that hardly anyone 
> > runs the thing.
> 
> i automatically run it for every patch i submit or push out via git.

you're hardly anyone ;)

> > I think we're ready to wire checkpatch up to a email robot which 
> > monitors the mailing lists and sends people nastygrams.  I bet that'll 
> > be popular ;)
> 
> heh ;-) It could be automated for patches that are sent out with a 
> Signed-off-by [or a Reviewed-by] line. If you send a SoB patch that is 
> broken, prepare to get a nastygram. (Initially i'd suggest the nastygram 
> to Cc: to a different email list, not lkml.)

I was thinking it would reply to the sender only.

I have this vision of dragging my sorry butt to the keyboard in the morning
to be greeted by the usual shower of tab-replaced, space-stuffed
wordwrappery, except now each one is followed ten minutes later by a fixed up
version.

One can dream.

Re: [PATCH] spin_lock_unlocked cleanups

[Andy Whitcroft]

On Fri, Sep 28, 2007 at 10:32:38AM +0200, Thomas Gleixner wrote:
> On Fri, 2007-09-28 at 01:26 -0700, Andrew Morton wrote:
> > On Fri, 28 Sep 2007 10:17:30 +0200 Thomas Gleixner <tglx@linutronix.de> wrote:
> > 
> > > can we please add this to checkpatch.pl ? 
> > > 
> > > > -spinlock_t bpci_lock = SPIN_LOCK_UNLOCKED;
> > > > +DEFINE_SPINLOCK(bpci_lock);
> > 
> > That check is already in checkpatch.  Problem is that hardly anyone
> > runs the thing.
> 
> Sigh, I forgot that perl is write only. :)
> 
> > I think we're ready to wire checkpatch up to a email robot which monitors
> > the mailing lists and sends people nastygrams.  I bet that'll be popular ;)
> 
> We should wire it up to git-commit as well. A lot of that comes in via
> git subsystems.

The problem with git-commit is who's repo to add the hook to.  I did
attempt to do this by picking up each of linus' main releases and then
using the git blame engine to attribute each "failure" to a particular
commit.  The plan then would be to send a nasty-gram to the committer
about violations there-in.

I'll try and find some time to get this bit polished and at least
emailing me.

-apw

Re: [PATCH] spin_lock_unlocked cleanups

[Andrew Morton]

On Fri, 28 Sep 2007 09:53:47 +0100 Andy Whitcroft <apw@shadowen.org> wrote:

> On Fri, Sep 28, 2007 at 01:26:56AM -0700, Andrew Morton wrote:
> > On Fri, 28 Sep 2007 10:17:30 +0200 Thomas Gleixner <tglx@linutronix.de> wrote:
> > 
> > > can we please add this to checkpatch.pl ? 
> > > 
> > > > -spinlock_t bpci_lock = SPIN_LOCK_UNLOCKED;
> > > > +DEFINE_SPINLOCK(bpci_lock);
> > 
> > That check is already in checkpatch.  Problem is that hardly anyone
> > runs the thing.
> > 
> > I think we're ready to wire checkpatch up to a email robot which monitors
> > the mailing lists and sends people nastygrams.  I bet that'll be popular ;)
> 
> That shouldn't be too hard.  checkpatch has been subscribed since birth
> but short circuiting the replies to me only.
> 
> I guess the main question is whether to reply-all or reply just to the
> sender when commenting on patches.  Perhaps for the sanity of the rest
> of the world, just the sender makes most sense.

For sure.

> > (I'd love it if it could detect wordwrapped and tab-expanded patches, too. 
> > You wouldn't _believe_...)
> 
> It should pick up both of these, the word-wrapping is already there as
> we detect lines within patch segments which don't start '[ +-]', the
> tab-expanded should be picked up as every line would be "don't use
> spaces use tabs for indent".

OK.

Often patches are wordwrapped only in the header:

--- old/drivers/ata/libata-sff.c	2007-04-26 12:02:46.000000000 -0400
+++ linux/drivers/ata/libata-sff.c	2007-04-29 08:29:27.000000000 -0400
@@ -413,6 +413,24 @@
 	ap->ops->irq_on(ap);

comes through as

--- old/drivers/ata/libata-sff.c	2007-04-26
12:02:46.000000000 -0400
+++ linux/drivers/ata/libata-sff.c	2007-04-29
08:29:27.000000000 -0400
@@ -413,6 +413,24 @@
 	ap->ops->irq_on(ap);

and the rest of the patch is good.

<tests it>

Yup, fooled you ;)

Re: [PATCH] spin_lock_unlocked cleanups

[Thomas Gleixner]

On Fri, 2007-09-28 at 09:56 +0100, Andy Whitcroft wrote:
> > > I think we're ready to wire checkpatch up to a email robot which monitors
> > > the mailing lists and sends people nastygrams.  I bet that'll be popular ;)
> > 
> > We should wire it up to git-commit as well. A lot of that comes in via
> > git subsystems.
> 
> The problem with git-commit is who's repo to add the hook to.  I did
> attempt to do this by picking up each of linus' main releases and then
> using the git blame engine to attribute each "failure" to a particular
> commit.  The plan then would be to send a nasty-gram to the committer
> about violations there-in.
> 
> I'll try and find some time to get this bit polished and at least
> emailing me.

The question is, whether we can convince the git developers to integrate
it. When a commit happens and checkpatch.pl is in scripts/, then run the
patch through it before doing the actual commit.

	tglx

Re: [PATCH] spin_lock_unlocked cleanups

[Mark Gross]

On Fri, Sep 28, 2007 at 01:26:56AM -0700, Andrew Morton wrote:
> On Fri, 28 Sep 2007 10:17:30 +0200 Thomas Gleixner <tglx@linutronix.de> wrote:
> 
> > can we please add this to checkpatch.pl ? 
> > 
> > > -spinlock_t bpci_lock = SPIN_LOCK_UNLOCKED;
> > > +DEFINE_SPINLOCK(bpci_lock);
> 
> That check is already in checkpatch.  Problem is that hardly anyone
> runs the thing.
> 
> I think we're ready to wire checkpatch up to a email robot which monitors
> the mailing lists and sends people nastygrams.  I bet that'll be popular ;)

One could make check patch create a signature hashing a check patch key
and the patch one could put in the post like a signed-off-by: thing.

checkpatch-sig : 2f818bcf0c2333a461affc4a170814f23adf2e08

this puts the burden on the sender to run the thing.

--mgross

Re: [PATCH] spin_lock_unlocked cleanups

[Sean]

On Fri, 28 Sep 2007 11:06:09 +0200
Thomas Gleixner <tglx@linutronix.de> wrote:

> > The problem with git-commit is who's repo to add the hook to.  I did
> > attempt to do this by picking up each of linus' main releases and then
> > using the git blame engine to attribute each "failure" to a particular
> > commit.  The plan then would be to send a nasty-gram to the committer
> > about violations there-in.

Wouldn't it be easier to pass each commit through checkpatch and
email the committer if there is a problem?  Each commit can be viewed
as a standalone patch afterall; what does blame add?

> The question is, whether we can convince the git developers to integrate
> it. When a commit happens and checkpatch.pl is in scripts/, then run the
> patch through it before doing the actual commit.

Definitely the way to go.  I'm pretty sure the Git guys would agree to
distribute checkpatch.pl along with the existing pre-commit hook.  So
at least enabling checkpatch would be trivial for those convinced to
use it.

Sean