Make NOPed versions of __raw_read_lock and family available
under uniprocessor kernels.
Discovered when compiling a uniprocessor kernel with the
fusyn patch applied.
The standard kernel does not use __raw_read_lock etc
outside of spinlock.c, which may account for this bug
being undiscovered until now.
2.6.16-rc1-git4-jak/include/linux/spinlock_up.h | 20 ++++++++++----------
1 files changed, 10 insertions(+), 10 deletions(-)
diff -puNa include/linux/spinlock_up.h~define.__raw_read_lock.and.family.for.uniproc-nodebug.combo include/linux/spinlock_up.h
--- 2.6.16-rc1-git4/include/linux/spinlock_up.h~define.__raw_read_lock.and.family.for.uniproc-nodebug.combo 2006-01-24 12:57:15.000000000 -0500
+++ 2.6.16-rc1-git4-jak/include/linux/spinlock_up.h 2006-01-24 12:58:51.000000000 -0500
@@ -47,16 +47,6 @@ static inline void __raw_spin_unlock(raw
lock->slock = 1;
}
-/*
- * Read-write spinlocks. No debug version.
- */
-#define __raw_read_lock(lock) do { (void)(lock); } while (0)
-#define __raw_write_lock(lock) do { (void)(lock); } while (0)
-#define __raw_read_trylock(lock) ({ (void)(lock); 1; })
-#define __raw_write_trylock(lock) ({ (void)(lock); 1; })
-#define __raw_read_unlock(lock) do { (void)(lock); } while (0)
-#define __raw_write_unlock(lock) do { (void)(lock); } while (0)
-
#else /* DEBUG_SPINLOCK */
#define __raw_spin_is_locked(lock) ((void)(lock), 0)
/* for sched.c and kernel_lock.c: */
@@ -71,4 +61,14 @@ static inline void __raw_spin_unlock(raw
#define __raw_spin_unlock_wait(lock) \
do { cpu_relax(); } while (__raw_spin_is_locked(lock))
+/*
+ * Read-write spinlocks. Only non-debug versions available.
+ */
+#define __raw_read_lock(lock) do { (void)(lock); } while (0)
+#define __raw_write_lock(lock) do { (void)(lock); } while (0)
+#define __raw_read_trylock(lock) ({ (void)(lock); 1; })
+#define __raw_write_trylock(lock) ({ (void)(lock); 1; })
+#define __raw_read_unlock(lock) do { (void)(lock); } while (0)
+#define __raw_write_unlock(lock) do { (void)(lock); } while (0)
+
#endif /* __LINUX_SPINLOCK_UP_H */
_
On Tue, Jan 24, 2006 at 01:09:54PM -0500, Joe Korty wrote:
>
> Make NOPed versions of __raw_read_lock and family available
> under uniprocessor kernels.
>
> Discovered when compiling a uniprocessor kernel with the
> fusyn patch applied.
>
> The standard kernel does not use __raw_read_lock etc
> outside of spinlock.c, which may account for this bug
> being undiscovered until now.
No one should call these directly. Please fix your odd patch instead.
On Tue, Jan 24, 2006 at 06:17:12PM +0000, Christoph Hellwig wrote:
> On Tue, Jan 24, 2006 at 01:09:54PM -0500, Joe Korty wrote:
> >
> > Make NOPed versions of __raw_read_lock and family available
> > under uniprocessor kernels.
> >
> > Discovered when compiling a uniprocessor kernel with the
> > fusyn patch applied.
> >
> > The standard kernel does not use __raw_read_lock etc
> > outside of spinlock.c, which may account for this bug
> > being undiscovered until now.
>
> No one should call these directly. Please fix your odd patch instead.
Actually the patch calls the _raw version which is #defined to the __raw
version. So it is doing the correct thing.
Joe
On Tue, 2006-01-24 at 13:29 -0500, Joe Korty wrote:
> On Tue, Jan 24, 2006 at 06:17:12PM +0000, Christoph Hellwig wrote:
> > On Tue, Jan 24, 2006 at 01:09:54PM -0500, Joe Korty wrote:
> > >
> > > Make NOPed versions of __raw_read_lock and family available
> > > under uniprocessor kernels.
> > >
> > > Discovered when compiling a uniprocessor kernel with the
> > > fusyn patch applied.
> > >
> > > The standard kernel does not use __raw_read_lock etc
> > > outside of spinlock.c, which may account for this bug
> > > being undiscovered until now.
> >
> > No one should call these directly. Please fix your odd patch instead.
>
> Actually the patch calls the _raw version which is #defined to the __raw
> version. So it is doing the correct thing.
no it's not, it has no business calling the _raw version either.
On Tue, Jan 24, 2006 at 07:32:02PM +0100, Arjan van de Ven wrote:
> On Tue, 2006-01-24 at 13:29 -0500, Joe Korty wrote:
> > On Tue, Jan 24, 2006 at 06:17:12PM +0000, Christoph Hellwig wrote:
> > > On Tue, Jan 24, 2006 at 01:09:54PM -0500, Joe Korty wrote:
> > > >
> > > > Make NOPed versions of __raw_read_lock and family available
> > > > under uniprocessor kernels.
> > > >
> > > > Discovered when compiling a uniprocessor kernel with the
> > > > fusyn patch applied.
> > > >
> > > > The standard kernel does not use __raw_read_lock etc
> > > > outside of spinlock.c, which may account for this bug
> > > > being undiscovered until now.
> > >
> > > No one should call these directly. Please fix your odd patch instead.
> >
> > Actually the patch calls the _raw version which is #defined to the __raw
> > version. So it is doing the correct thing.
>
> no it's not, it has no business calling the _raw version either.
Nope.
1) The _raw_spin_lock family is used everywhere in the kernel. Why the
arbitrary special rule for _raw_read_lock?????? It makes no sense.
2) The _raw versions are intended to be used in places where it is
known that preemption is already disabled, so that the overhead of
re-disabling/enabling it can be avoided.
Joe
--
"All the revision in the world will not save a bad first draft, for the
architecture of the thing comes, or fails to come, in the first conception,
and revision only affects the detail and ornament. -- T.E. Lawrence
On Tue, 2006-01-24 at 13:38 -0500, Joe Korty wrote:
> On Tue, Jan 24, 2006 at 07:32:02PM +0100, Arjan van de Ven wrote:
> > On Tue, 2006-01-24 at 13:29 -0500, Joe Korty wrote:
> > > On Tue, Jan 24, 2006 at 06:17:12PM +0000, Christoph Hellwig wrote:
> > > > On Tue, Jan 24, 2006 at 01:09:54PM -0500, Joe Korty wrote:
> > > > >
> > > > > Make NOPed versions of __raw_read_lock and family available
> > > > > under uniprocessor kernels.
> > > > >
> > > > > Discovered when compiling a uniprocessor kernel with the
> > > > > fusyn patch applied.
> > > > >
> > > > > The standard kernel does not use __raw_read_lock etc
> > > > > outside of spinlock.c, which may account for this bug
> > > > > being undiscovered until now.
> > > >
> > > > No one should call these directly. Please fix your odd patch instead.
> > >
> > > Actually the patch calls the _raw version which is #defined to the __raw
> > > version. So it is doing the correct thing.
> >
> > no it's not, it has no business calling the _raw version either.
>
> Nope.
>
> 1) The _raw_spin_lock family is used everywhere in the kernel.
no it's not. It's used in a few very special architecture places, and in
the spinlock.c code, and in one place of the scheduler, which is
arguably special.
I don't know what kernel you're looking at.. but it's not a kernel.org
one.
> 2) The _raw versions are intended to be used in places where it is
> known that preemption is already disabled, so that the overhead of
> re-disabling/enabling it can be avoided.
that's not true either. If it was, then the name would have been
different.
On Tue, Jan 24, 2006 at 07:42:36PM +0100, Arjan van de Ven wrote:
> On Tue, 2006-01-24 at 13:38 -0500, Joe Korty wrote:
> > On Tue, Jan 24, 2006 at 07:32:02PM +0100, Arjan van de Ven wrote:
> > > On Tue, 2006-01-24 at 13:29 -0500, Joe Korty wrote:
> > > > On Tue, Jan 24, 2006 at 06:17:12PM +0000, Christoph Hellwig wrote:
> > > > > On Tue, Jan 24, 2006 at 01:09:54PM -0500, Joe Korty wrote:
> > > > > >
> > > > > > Make NOPed versions of __raw_read_lock and family available
> > > > > > under uniprocessor kernels.
> > > > > >
> > > > > > Discovered when compiling a uniprocessor kernel with the
> > > > > > fusyn patch applied.
> > > > > >
> > > > > > The standard kernel does not use __raw_read_lock etc
> > > > > > outside of spinlock.c, which may account for this bug
> > > > > > being undiscovered until now.
> > > > >
> > > > > No one should call these directly. Please fix your odd patch instead.
> > > >
> > > > Actually the patch calls the _raw version which is #defined to the __raw
> > > > version. So it is doing the correct thing.
> > >
> > > no it's not, it has no business calling the _raw version either.
> >
> > Nope.
> >
> > 1) The _raw_spin_lock family is used everywhere in the kernel.
>
> no it's not. It's used in a few very special architecture places, and in
> the spinlock.c code, and in one place of the scheduler, which is
> arguably special.
>
> I don't know what kernel you're looking at.. but it's not a kernel.org
> one.
>
>
> > 2) The _raw versions are intended to be used in places where it is
> > known that preemption is already disabled, so that the overhead of
> > re-disabling/enabling it can be avoided.
>
> that's not true either. If it was, then the name would have been
> different.
I'll leave it to Ingo to decide. After all, the NOPed versions are in the
tree already and have been for some time. They just are under the wrong
#ifdef, so it seems like it was his intent to provide it, but failed to
do so in a way that actually enabled them.
(and '_raw' is a perfect prefix for the core spinlock services).
Joe
* Arjan van de Ven <[email protected]> wrote:
> > 2) The _raw versions are intended to be used in places where it is
> > known that preemption is already disabled, so that the overhead of
> > re-disabling/enabling it can be avoided.
>
> that's not true either. If it was, then the name would have been
> different.
indeed. The __raw versions are _not_ to be used by anything else but the
generic spinlock code. (There's one other stray use by PARISC, but that
is justified.)
[ the -rt tree makes use of 'raw_' spinlocks, but they are totally
different things. ]
Ingo
* Joe Korty <[email protected]> wrote:
> Make NOPed versions of __raw_read_lock and family available under
> uniprocessor kernels.
NACK. The __raw ops are only used by the spinlock implementation, and
only on SMP.
Ingo