Andrew,
The first patch in this series is a replacement patch for the prempt-fix
patch I sent earlier this morning. There was a missing paren in that
previous version.
Adding this to 2.6.9-rc2-mm1 (just after lockmeter-2.patch) fixes the
preempt compile problem on ia64, at least. However, I then started
getting a missing symbol for generic_raw_read_trylock. Hence the second
patch of this series.
(It is the nature of this lockmeter fix for the COOL bits that whenever
a change is made to kernel/spinlock.c, you need to make a correspoding
change to the kernel/lockmeter.c code that parallels spinlock.c. I
don't see any good way around this.)
Finally, Zwane has suggested making in_lock_functions() an inline.
I sent him and you a simple patch to do that, and if that is applied,
you then need the third patch of this series.
All three of these patches are intended to follow lockmeter-2.patch in
the series.
I compiled lockmeter on/off, preempt on/off (all 4) kernels on ia64.
I'm working on doing that for i386, but my i386 machines are much slower
than an 8-way Altix for doing compiles. :-) I'll test some of these
as well.
Best Regards,
Ray
[email protected]
--
Best Regards,
Ray
-----------------------------------------------
Ray Bryant [email protected]
The box said: "Requires Windows 98 or better",
so I installed Linux.
-----------------------------------------------
Fix to lockmeter.c for inline in_lock_functions().
(Only required if in_lock_functions() is modified to be inline.)
Signed-off-by: Ray Bryant <[email protected]>
Index: linux-2.6.9-rc2-mm1/kernel/lockmeter.c
===================================================================
--- linux-2.6.9-rc2-mm1.orig/kernel/lockmeter.c 2004-09-16 12:29:08.000000000 -0700
+++ linux-2.6.9-rc2-mm1/kernel/lockmeter.c 2004-09-16 12:53:57.000000000 -0700
@@ -1510,13 +1510,3 @@
return 0;
}
EXPORT_SYMBOL(_spin_trylock_bh);
-
-int in_lock_functions(unsigned long addr)
-{
- /* Linker adds these: start and end of __lockfunc functions */
- extern char __lock_text_start[], __lock_text_end[];
-
- return addr >= (unsigned long)__lock_text_start
- && addr < (unsigned long)__lock_text_end;
-}
-EXPORT_SYMBOL(in_lock_functions);
--
Best Regards,
Ray
-----------------------------------------------
Ray Bryant [email protected]
The box said: "Requires Windows 98 or better",
so I installed Linux.
-----------------------------------------------
Updated lockmeter-preempt-fix.patch.
Signed-off-by: Ray Bryant <[email protected]>
Index: linux-2.6.9-rc2-mm1/kernel/lockmeter.c
===================================================================
--- linux-2.6.9-rc2-mm1.orig/kernel/lockmeter.c 2004-09-16 11:03:05.000000000 -0700
+++ linux-2.6.9-rc2-mm1/kernel/lockmeter.c 2004-09-16 12:03:20.000000000 -0700
@@ -1236,18 +1236,58 @@
EXPORT_SYMBOL(_write_trylock);
#if defined(CONFIG_SMP) && defined(CONFIG_PREEMPT)
+/*
+ * This could be a long-held lock. If another CPU holds it for a long time,
+ * and that CPU is not asked to reschedule then *this* CPU will spin on the
+ * lock for a long time, even if *this* CPU is asked to reschedule.
+ *
+ * So what we do here, in the slow (contended) path is to spin on the lock by
+ * hand while permitting preemption.
+ *
+ * Called inside preempt_disable().
+ */
+static inline void __preempt_spin_lock(spinlock_t *lock, void *caller_pc)
+{
+ if (preempt_count() > 1) {
+ _metered_spin_lock(lock, caller_pc);
+ return;
+ }
+
+ do {
+ preempt_enable();
+ while (spin_is_locked(lock))
+ cpu_relax();
+ preempt_disable();
+ } while (!_metered_spin_trylock(lock, caller_pc));
+}
+
void __lockfunc _spin_lock(spinlock_t *lock)
{
preempt_disable();
if (unlikely(!_metered_spin_trylock(lock, __builtin_return_address(0))))
- __preempt_spin_lock(lock);
+ __preempt_spin_lock(lock, __builtin_return_address(0));
+}
+
+static inline void __preempt_write_lock(rwlock_t *lock, void *caller_pc)
+{
+ if (preempt_count() > 1) {
+ _metered_write_lock(lock, caller_pc);
+ return;
+ }
+
+ do {
+ preempt_enable();
+ while (rwlock_is_locked(lock))
+ cpu_relax();
+ preempt_disable();
+ } while (!_metered_write_trylock(lock,caller_pc));
}
void __lockfunc _write_lock(rwlock_t *lock)
{
preempt_disable();
if (unlikely(!_metered_write_trylock(lock, __builtin_return_address(0))))
- __preempt_write_lock(lock);
+ __preempt_write_lock(lock, __builtin_return_address(0));
}
#else
void __lockfunc _spin_lock(spinlock_t *lock)
@@ -1458,3 +1498,13 @@
return 0;
}
EXPORT_SYMBOL(_spin_trylock_bh);
+
+int in_lock_functions(unsigned long addr)
+{
+ /* Linker adds these: start and end of __lockfunc functions */
+ extern char __lock_text_start[], __lock_text_end[];
+
+ return addr >= (unsigned long)__lock_text_start
+ && addr < (unsigned long)__lock_text_end;
+}
+EXPORT_SYMBOL(in_lock_functions);
--
Best Regards,
Ray
-----------------------------------------------
Ray Bryant [email protected]
The box said: "Requires Windows 98 or better",
so I installed Linux.
-----------------------------------------------
On Thu, 16 Sep 2004, Ray Bryant wrote:
> Update lockmeter.c with generic_raw_read_trylock fix.
>
> + * Generic declaration of the raw read_trylock() function,
> + * architectures are supposed to optimize this:
> + */
> +int __lockfunc generic_raw_read_trylock(rwlock_t *lock)
> +{
> + _metered_read_lock(lock, __builtin_return_address(0));
> + return 1;
> +}
What's really going on here? I'm slightly confused by the
_metered_read_lock usage.
Thanks,
Zwane
Zwane Mwaikambo wrote:
> On Thu, 16 Sep 2004, Ray Bryant wrote:
>
>
>>Update lockmeter.c with generic_raw_read_trylock fix.
>>
>>+ * Generic declaration of the raw read_trylock() function,
>>+ * architectures are supposed to optimize this:
>>+ */
>>+int __lockfunc generic_raw_read_trylock(rwlock_t *lock)
>>+{
>>+ _metered_read_lock(lock, __builtin_return_address(0));
>>+ return 1;
>>+}
>
>
> What's really going on here? I'm slightly confused by the
> _metered_read_lock usage.
>
> Thanks,
> Zwane
>
>
Yeah, I think overly patterned matched on this one. I just
grabbed the code from spinlock.c and do what I normally do to
make it lockmetered, but in this case since its a _raw_ routine, I
should have left it alone. It just needs to be duplicated in
lockmeter.c, not lockmeter'd.
So that middle line there should be:
_raw_read_lock(lock);
instead. I'll get this straightend out with Andrew shortly.
--
Best Regards,
Ray
-----------------------------------------------
Ray Bryant
512-453-9679 (work) 512-507-7807 (cell)
[email protected] [email protected]
The box said: "Requires Windows 98 or better",
so I installed Linux.
-----------------------------------------------
Update lockmeter.c with generic_raw_read_trylock fix.
Signed-off-by: Ray Bryant <[email protected]>
Index: linux-2.6.9-rc2-mm1/kernel/lockmeter.c
===================================================================
--- linux-2.6.9-rc2-mm1.orig/kernel/lockmeter.c 2004-09-16 12:03:20.000000000 -0700
+++ linux-2.6.9-rc2-mm1/kernel/lockmeter.c 2004-09-16 12:29:08.000000000 -0700
@@ -1213,6 +1213,18 @@
* except for the fact tht calls to _raw_ routines are replaced by
* corresponding calls to the _metered_ routines
*/
+
+/*
+ * Generic declaration of the raw read_trylock() function,
+ * architectures are supposed to optimize this:
+ */
+int __lockfunc generic_raw_read_trylock(rwlock_t *lock)
+{
+ _metered_read_lock(lock, __builtin_return_address(0));
+ return 1;
+}
+EXPORT_SYMBOL(generic_raw_read_trylock);
+
int __lockfunc _spin_trylock(spinlock_t *lock)
{
preempt_disable();
--
Best Regards,
Ray
-----------------------------------------------
Ray Bryant [email protected]
The box said: "Requires Windows 98 or better",
so I installed Linux.
-----------------------------------------------
On Thu, Sep 16, 2004 at 04:03:44PM -0700, Ray Bryant wrote:
> Andrew,
>
> The first patch in this series is a replacement patch for the prempt-fix
> patch I sent earlier this morning. There was a missing paren in that
> previous version.
Any chance you could stop lockmeter patching around in fs/proc/proc_misc.c?
procfs files can be created easily from individual drivers.
Christoph Hellwig wrote:
> On Thu, Sep 16, 2004 at 04:03:44PM -0700, Ray Bryant wrote:
>
>>Andrew,
>>
>>The first patch in this series is a replacement patch for the prempt-fix
>>patch I sent earlier this morning. There was a missing paren in that
>>previous version.
>
>
> Any chance you could stop lockmeter patching around in fs/proc/proc_misc.c?
> procfs files can be created easily from individual drivers.
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
Hi Christoph,
Yes, I'll take a look at it as soon as I get done chasing the COOL bits changes.
Is there a specific example you can point me at as to how others have done this?
Thanks,
--
Best Regards,
Ray
-----------------------------------------------
Ray Bryant
512-453-9679 (work) 512-507-7807 (cell)
[email protected] [email protected]
The box said: "Requires Windows 98 or better",
so I installed Linux.
-----------------------------------------------
> Yes, I'll take a look at it as soon as I get done chasing the COOL bits changes.
>
> Is there a specific example you can point me at as to how others have done this?
Just grep for create_proc_entry - lots of drivers are using this. In fact I wonder
whether a miscdevice wouldn't be the better choice for lockmeter, but given that'd
need userland changes let's stay away from that for now.
> Andrew,
>
> The first patch in this series is a replacement patch for the prempt-fix
> patch I sent earlier this morning. There was a missing paren in that
> previous version.
>
> Adding this to 2.6.9-rc2-mm1 (just after lockmeter-2.patch) fixes the
> preempt compile problem on ia64, at least. However, I then started
> getting a missing symbol for generic_raw_read_trylock. Hence the second
> patch of this series.
Hi Ray
I need the following to compile on x86-64, works fine otherwise.
--- mm/kernel/lockmeter.c.orig 2004-09-19 12:49:52.000000000 +0200
+++ mm/kernel/lockmeter.c 2004-09-19 12:50:25.000000000 +0200
@@ -1510,3 +1510,13 @@ int __lockfunc _spin_trylock_bh(spinlock
return 0;
}
EXPORT_SYMBOL(_spin_trylock_bh);
+
+int in_lock_functions(unsigned long addr)
+{
+ /* Linker adds these: start and end of __lockfunc functions */
+ extern char __lock_text_start[], __lock_text_end[];
+
+ return addr >= (unsigned long)__lock_text_start
+ && addr < (unsigned long)__lock_text_end;
+}
+EXPORT_SYMBOL(in_lock_functions);
--- mm/include/asm-x86_64/spinlock.h.orig 2004-09-19 12:31:32.000000000 +0200
+++ mm/include/asm-x86_64/spinlock.h 2004-09-19 13:04:56.117109248 +0200
@@ -232,16 +232,6 @@ static inline void _raw_write_lock(rwloc
#define _raw_read_unlock(rw) asm volatile("lock ; incl %0" :"=m" ((rw)->lock) : : "memory")
#define _raw_write_unlock(rw) asm volatile("lock ; addl $" RW_LOCK_BIAS_STR ",%0":"=m" ((rw)->lock) : : "memory")
-static inline int _raw_read_trylock(rwlock_t *lock)
-{
- atomic_t *count = (atomic_t *)lock;
- atomic_dec(count);
- if (atomic_read(count) < RW_LOCK_BIAS)
- return 1;
- atomic_inc(count);
- return 0;
-}
-
static inline int _raw_write_trylock(rwlock_t *lock)
{
atomic_t *count = (atomic_t *)lock;
@@ -262,6 +252,16 @@ static inline int _raw_read_trylock(rwlo
atomic_inc(count);
return 0;
}
+#else
+static inline int _raw_read_trylock(rwlock_t *lock)
+{
+ atomic_t *count = (atomic_t *)lock;
+ atomic_dec(count);
+ if (atomic_read(count) < RW_LOCK_BIAS)
+ return 1;
+ atomic_inc(count);
+ return 0;
+}
#endif
#if defined(CONFIG_LOCKMETER) && defined(CONFIG_HAVE_DEC_LOCK)
Alexander Nyberg wrote:
>>Andrew,
>>
>>The first patch in this series is a replacement patch for the prempt-fix
>>patch I sent earlier this morning. There was a missing paren in that
>>previous version.
>>
>>Adding this to 2.6.9-rc2-mm1 (just after lockmeter-2.patch) fixes the
>>preempt compile problem on ia64, at least. However, I then started
>>getting a missing symbol for generic_raw_read_trylock. Hence the second
>>patch of this series.
>
>
> Hi Ray
>
> I need the following to compile on x86-64, works fine otherwise.
>
>
> --- mm/kernel/lockmeter.c.orig 2004-09-19 12:49:52.000000000 +0200
> +++ mm/kernel/lockmeter.c 2004-09-19 12:50:25.000000000 +0200
> @@ -1510,3 +1510,13 @@ int __lockfunc _spin_trylock_bh(spinlock
> return 0;
> }
> EXPORT_SYMBOL(_spin_trylock_bh);
> +
> +int in_lock_functions(unsigned long addr)
> +{
> + /* Linker adds these: start and end of __lockfunc functions */
> + extern char __lock_text_start[], __lock_text_end[];
> +
> + return addr >= (unsigned long)__lock_text_start
> + && addr < (unsigned long)__lock_text_end;
> +}
> +EXPORT_SYMBOL(in_lock_functions);
> --- mm/include/asm-x86_64/spinlock.h.orig 2004-09-19 12:31:32.000000000 +0200
> +++ mm/include/asm-x86_64/spinlock.h 2004-09-19 13:04:56.117109248 +0200
> @@ -232,16 +232,6 @@ static inline void _raw_write_lock(rwloc
> #define _raw_read_unlock(rw) asm volatile("lock ; incl %0" :"=m" ((rw)->lock) : : "memory")
> #define _raw_write_unlock(rw) asm volatile("lock ; addl $" RW_LOCK_BIAS_STR ",%0":"=m" ((rw)->lock) : : "memory")
>
> -static inline int _raw_read_trylock(rwlock_t *lock)
> -{
> - atomic_t *count = (atomic_t *)lock;
> - atomic_dec(count);
> - if (atomic_read(count) < RW_LOCK_BIAS)
> - return 1;
> - atomic_inc(count);
> - return 0;
> -}
> -
> static inline int _raw_write_trylock(rwlock_t *lock)
> {
> atomic_t *count = (atomic_t *)lock;
> @@ -262,6 +252,16 @@ static inline int _raw_read_trylock(rwlo
> atomic_inc(count);
> return 0;
> }
> +#else
> +static inline int _raw_read_trylock(rwlock_t *lock)
> +{
> + atomic_t *count = (atomic_t *)lock;
> + atomic_dec(count);
> + if (atomic_read(count) < RW_LOCK_BIAS)
> + return 1;
> + atomic_inc(count);
> + return 0;
> +}
> #endif
>
> #if defined(CONFIG_LOCKMETER) && defined(CONFIG_HAVE_DEC_LOCK)
>
>
>
Hi Alex,
There were a flurry of spinlock and corresponding lockmeter changes on
Thursday and Friday. One of the patches was to make in_lock_functions()
an inline, so that solves the first problem you found above. A second
one that Andrew found was the duplicate definition of _raw_read_trylock(),
which is what I think the second part of your patch is.
I'm currently working on testing a lockmeter rollup patch. I'll add the
x86_64 _raw_read_trylock() fix unless Andrew has already got that covered.
Thanks,
--
Best Regards,
Ray
-----------------------------------------------
Ray Bryant
512-453-9679 (work) 512-507-7807 (cell)
[email protected] [email protected]
The box said: "Requires Windows 98 or better",
so I installed Linux.
-----------------------------------------------