Re: [patch 0/1] extending low-level markers

2007-08-02 Thread Noah Watkins
> > > 
> > 
> > The locking is internal to our framework and done on a per-type basis.
> > The start/end example i mentioned could represent an interval. When the
> > two points are wired up they both have their private data pointing at a
> > kmalloc'd interval structure internal to our framework, which contains
> > a lock.
> > 
> 
> Then this lock is taken in _start and unlocked in _end ? What happens if
> the _start and _end markers are enabled/disabled in a non-equilibrated
> way while the kernel code is being executed ? Would it deadlock the next
> time the lock is taken due to a missing unlock ?

The locks are aquired and released in each _start and _end marker, so
the equilibrium is not a issue.

A more sane example is the insertion of values into a histogram. Instead
of the instrumenation point logging the values and having the histogram
constructed during a post-process step, data structures implemented the
histogrm are associated with the instrumenation point. The lock protects
this structure in a more intuitive way than the interval example.

We have additional instrumenation points associated with the histogram
which for example trigger the logging of the histgoram when conditions
are met (conditions are implied by the placement of the marker).

In the interval example, they provide the data consistency, and
make no attempt to insure that the markers are used in a way that
makes sense (for example an interval in a preemptable re-entrant path).

So you can see that encoding the types of markers in the name of the
marker may become cumbersome as the amount of combinations grow.

> 
> > 
> > The cookie approach would accomplish the functionality, though it does
> > complicate the procedure of adding the instrumenation point. If the
> > start and the end events are not in the same function/file then
> > the linking of the cookie could be too instrusive.
> > 
> > In all of our cases the cookie would certainly be a void* pointing to
> > internal data structures. What is the objection to using the marker's
> > private data to point to the internal structures?
> > 
> 
> SMP, preemption, all those things while requires locking around a static
> variable. And the problem of non atomicity of connexion/disconnexion of
> multiple probes vs unbalanced lock/unlock.

I think most of problems involving non atomicity of the
connection/disconnection of probes can be accomplished by good house
keeping and settings things up in phases which insure consistency. For
example wiring things up before enabling events. An issue such as
hitting a _end event before a _start event can easily be handled by
whatever framework the markers are attached to.

- Noah

> 
> > Thanks for helping out. I realize the situations I'm describing are
> > quite specialized to our particular situation. Many students here use
> > the framework for learning systems programming, and as such it evolves
> > often.
> > 
> 
> No problem. I think some interesting things came come out of this.
> 
> Mathieu
> 
> > - Noah
> > 
> > > 
> > > > > The current approach is to use the marker name as a way to specify
> > > > > markers "group". If we go with a "flavor" enumeration instead, we 
> > > > > would
> > > > > have to add an enumeration of every markers users in marker.h, which I
> > > > > am a bit reluctant to do.
> > > > 
> > > > This would have to be my biggest complaint with the 'flavor' concept as
> > > > well. However, if all points in main-line always used no concept of
> > > > flavor (or essentially the default flavor) then users wishing to use the
> > > > flavor enumeration out of main-line development could do so?
> > > > 
> > > 
> > > Hrm, the ideal thing would be to agree on an instrumentation set for a
> > > given subsystem/driver and to get the said instrumentation integrated in
> > > mainline. That would sound like a better way to stop reinventing the
> > > wheel forever. And actually, if there are some features in your tracer
> > > that you would like to add into a mainline tracer, I'll be glad to
> > > discuss those with you. A lot of people out there are facing the same
> > > issues as you are anyway :)
> > > 
> > > Ideally, a marker should express what the kernel code is doing and what
> > > information we want to extract from it more than being tied to one
> > > particular consumer (probe).
> > > 
> > > > Hope this helps portray more of what we are trying to do.
> > > > 
> > > 
> > > It sure helps, thanks :)
> > > 
> > > Mathieu
> > > 
> > > > Noah
> > > 
> > > -- 
> > > Mathieu Desnoyers
> > > Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
> > > OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 
> > > 9A68
> 
> -- 
> Mathieu Desnoyers
> Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
> OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTE

Re: [patch 0/1] extending low-level markers

2007-08-02 Thread Noah Watkins
> Hrm, what is wrong with :
> 
> trace_mark(ds_myevent, "%d %zu %p", arg1, arg2, arg3);
> 
> then ?
> 
> You could then attach your probe to all markers staring with a ds_
> prefix. (we should keep a list of the used prefixes somewhere) You could
> match with format strings to figure out which callback should be
> connected to which marker (if you don't plan to have your callback
> parsing the format string dynamically).

This has been what I considered the only way to accomplish the 'type'
feature without adding another field to the struct, and is just fine,
especially if prefixes are listed some where, or not. I was just
interested in the possibility of adding types.

> 
> 
> > The biggest problem we are facing now is with a set of points which
> > cooperate together in the sense that one references the others (or some
> > other connection topology). For example:
> > 
> > ds_point_start(NAME, params)
> > ds_point_end(NAME, params)
> > 
> > In this situation ds_point_start collects some data, stashes it in its
> > private data area,  and ds_point_end references the corresponding
> > ds_point_start when it fires. The two points are wired up by our
> > framework (using the marker's private data) and the NAME, in order to
> > avoid lookups at logging time.
> > 
> 
> I wonder how you do your locking there ? I guess you are writing data
> that *should* reside on the stack into the private data, which is
> somewhat static. A solution to this that would be both not too intrusive
> for kernel code and fast enough would be to keep a buffer of such
> saved/read data, indexed with a cookie that would be passed from _start
> to _end. The only issue is that this cookie would have to be placed on
> the kernel stack, its space being reserved even when markers are
> disabled.
> 

The locking is internal to our framework and done on a per-type basis.
The start/end example i mentioned could represent an interval. When the
two points are wired up they both have their private data pointing at a
kmalloc'd interval structure internal to our framework, which contains
a lock.

> 
> > So, in this case the names are the same which logically links them, but
> > their functionality is different. The difference in functionality could
> > again be encoded by the 'type' of point.
> > 
> 
> if you differentiate them with names like:
> ds_point_start and ds_point_end
> 
> and pass the cookie as write parameter to _start and read parameter to
> _end:
> 
> void somefct()
> {
>   long cookie;
> 
>   trace_mark(ds_point_start, "%p %d %zu %p", &cookie, arg1, arg2, arg3);
> 
>   ...
> 
>   trace_mark(ds_point_end, "%lu %d %zu %p", cookie, arg1, arg2, arg3);
> }
> It should do the job ?
> (note: if mutual exclusion access to a static variable is insured by
> proper locking in the kernel, then you don't need to do such trick)

The cookie approach would accomplish the functionality, though it does
complicate the procedure of adding the instrumenation point. If the
start and the end events are not in the same function/file then
the linking of the cookie could be too instrusive.

In all of our cases the cookie would certainly be a void* pointing to
internal data structures. What is the objection to using the marker's
private data to point to the internal structures?

Thanks for helping out. I realize the situations I'm describing are
quite specialized to our particular situation. Many students here use
the framework for learning systems programming, and as such it evolves
often.

- Noah

> 
> > > The current approach is to use the marker name as a way to specify
> > > markers "group". If we go with a "flavor" enumeration instead, we would
> > > have to add an enumeration of every markers users in marker.h, which I
> > > am a bit reluctant to do.
> > 
> > This would have to be my biggest complaint with the 'flavor' concept as
> > well. However, if all points in main-line always used no concept of
> > flavor (or essentially the default flavor) then users wishing to use the
> > flavor enumeration out of main-line development could do so?
> > 
> 
> Hrm, the ideal thing would be to agree on an instrumentation set for a
> given subsystem/driver and to get the said instrumentation integrated in
> mainline. That would sound like a better way to stop reinventing the
> wheel forever. And actually, if there are some features in your tracer
> that you would like to add into a mainline tracer, I'll be glad to
> discuss those with you. A lot of people out there are facing the same
> issues as you are anyway :)
> 
> Ideally, a marker should express what the kernel code is doing and what
> information we want to extract from it more than being tied to one
> particular consumer (probe).
> 
> > Hope this helps portray more of what we are trying to do.
> > 
> 
> It sure helps, thanks :)
> 
> Mathieu
> 
> > Noah
> 
> -- 
> Mathieu Desnoyers
> Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
> OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3

Re: [patch 0/1] extending low-level markers

2007-08-02 Thread Noah Watkins
On 02/08/07 12:44 -0400, Mathieu Desnoyers wrote:
> * [EMAIL PROTECTED] ([EMAIL PROTECTED]) wrote:
> > Mathieu
> > 
> > I have been working with your Kernel Markers infrastructure now for some
> > time and have run into an extendability issue.
> 
> Hi Noah,
> 
> Can you tell us a little bit more about what you are doing with the
> markers ? I guess it could be useful to know so we can get a sense of
> how it fits in the big picture.

We have been maintaining an in-house instrumentation framework. The
instrumenation points we use differ from Kernel Markers in that the desired
callback is attached at compile time. Other than this difference, they
are implemented in much the same way. The biggest difference is that we
have been implicitly encoding the 'type' of instrumenation point by
assigning a particular callback. For example one type of instrumenation
point in our framework has the form:

ds_event(name, int, size_t, void *);

Thus, a ds_event has a definite set of arguments, and some name, which
is compatible with a trace_mark.

We would like to be able to construct a ds_event on top of the low-level
kernel markers and be able to differentiate between a standard trace_mark in
in the markers section, and a trace_mark which actually represents a
ds_event, or another event type.

Granted we could simply match on compatible argument formats, its only
that a separation between types would be nice in order to not touch
markers which are of not interest to us, hence this notion of type.
Our framework would have blindly attached some callback to all
points which had matching argument formats, perhaps many that were not
inserted using ds_event (or some other api built on top of markers).

The biggest problem we are facing now is with a set of points which
cooperate together in the sense that one references the others (or some
other connection topology). For example:

ds_point_start(NAME, params)
ds_point_end(NAME, params)

In this situation ds_point_start collects some data, stashes it in its
private data area,  and ds_point_end references the corresponding
ds_point_start when it fires. The two points are wired up by our
framework (using the marker's private data) and the NAME, in order to
avoid lookups at logging time.

So, in this case the names are the same which logically links them, but
their functionality is different. The difference in functionality could
again be encoded by the 'type' of point.

> The current approach is to use the marker name as a way to specify
> markers "group". If we go with a "flavor" enumeration instead, we would
> have to add an enumeration of every markers users in marker.h, which I
> am a bit reluctant to do.

This would have to be my biggest complaint with the 'flavor' concept as
well. However, if all points in main-line always used no concept of
flavor (or essentially the default flavor) then users wishing to use the
flavor enumeration out of main-line development could do so?

Hope this helps portray more of what we are trying to do.

Noah
-
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/


Re: 2.6.23-rc1 -- build failure

2007-07-25 Thread Noah Watkins

Thanks this fixed it. My LKML search didn't find this patch.
-noah

On 7/25/07, Len Brown <[EMAIL PROTECTED]> wrote:

On Wednesday 25 July 2007 12:11, Noah Watkins wrote:
> Latest git tree gives following build errors (config attached):
>
> make -C /yggnfs/nwatkins/kernels/git-linux-2.6/linux-2.6 
O=/yggnfs/nwatkins/kernels/git-linux-2.6/build
>   Using /yggnfs/nwatkins/kernels/git-linux-2.6/linux-2.6 as source for kernel
>   GEN /yggnfs/nwatkins/kernels/git-linux-2.6/build/Makefile
>   CHK include/linux/version.h
>   CHK include/linux/utsrelease.h
>   CALL
/yggnfs/nwatkins/kernels/git-linux-2.6/linux-2.6/scripts/checksyscalls.sh
>   CHK include/linux/compile.h
>   CC  arch/i386/kernel/acpi/cstate.o
> In file included from 
/yggnfs/nwatkins/kernels/git-linux-2.6/linux-2.6/arch/i386/kernel/acpi/cstate.c:16:
> /yggnfs/nwatkins/kernels/git-linux-2.6/linux-2.6/include/acpi/processor.h:90: 
error: expected specifier-qualifier-list before 'acpi_integer'
> 
/yggnfs/nwatkins/kernels/git-linux-2.6/linux-2.6/include/acpi/processor.h:108: 
error: expected specifier-qualifier-list before 'acpi_integer'
> 
/yggnfs/nwatkins/kernels/git-linux-2.6/linux-2.6/include/acpi/processor.h:131: 
error: expected specifier-qualifier-list before 'acpi_integer'
> 
/yggnfs/nwatkins/kernels/git-linux-2.6/linux-2.6/include/acpi/processor.h:149: 
error: expected specifier-qualifier-list before 'acpi_integer'
> 
/yggnfs/nwatkins/kernels/git-linux-2.6/linux-2.6/include/acpi/processor.h:205: 
error: expected specifier-qualifier-list before 'acpi_handle'
> 
/yggnfs/nwatkins/kernels/git-linux-2.6/linux-2.6/include/acpi/processor.h:319: 
warning: 'struct acpi_device' declared inside parameter list
> 
/yggnfs/nwatkins/kernels/git-linux-2.6/linux-2.6/include/acpi/processor.h:319: 
warning: its scope is only this definition or declaration, which is probably not 
what you want
> 
/yggnfs/nwatkins/kernels/git-linux-2.6/linux-2.6/include/acpi/processor.h:322: 
warning: 'struct acpi_device' declared inside parameter list
> make[4]: *** [arch/i386/kernel/acpi/cstate.o] Error 1
> make[3]: *** [arch/i386/kernel/acpi] Error 2
> make[2]: *** [arch/i386/kernel] Error 2
> make[1]: *** [_all] Error 2
> make: *** [all] Error 2

try this:

From [EMAIL PROTECTED] Mon Jul 23 20:50:44 2007


-
Subject: X86_POWERNOW_K8_ACPI must depend on ACPI
From: Adrian Bunk <[EMAIL PROTECTED]>

This patch fixes the following compile error introduced by
commit e8666b2718fdb5bf0ea7c3126f7e292bbbf2946b and reported
by Alexey Dobriyan:

<--  snip  -->

   CC  arch/i386/kernel/acpi/cstate.o
In file included from arch/i386/kernel/acpi/cstate.c:17:
include/acpi/processor.h:88: error: expected specifier-qualifier-list before 
'acpi_integer'

<--  snip  -->

If you select something you must ensure that the dependencies of what
you are selecting are fulfilled.

Signed-off-by: Adrian Bunk <[EMAIL PROTECTED]>
Cc: Alexey Dobriyan <[EMAIL PROTECTED]>
Cc: Joshua Hoblitt <[EMAIL PROTECTED]>
Cc: Dave Jones <[EMAIL PROTECTED]>
Cc: Michal Piotrowski <[EMAIL PROTECTED]>
Cc: Len Brown <[EMAIL PROTECTED]>
Cc: Andrew Morton <[EMAIL PROTECTED]>
Signed-off-by: Andrew Morton <[EMAIL PROTECTED]>
---

 arch/i386/kernel/cpu/cpufreq/Kconfig |2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)

diff -puN 
arch/i386/kernel/cpu/cpufreq/Kconfig~x86_powernow_k8_acpi-must-depend-on-acpi 
arch/i386/kernel/cpu/cpufreq/Kconfig
--- 
a/arch/i386/kernel/cpu/cpufreq/Kconfig~x86_powernow_k8_acpi-must-depend-on-acpi
+++ a/arch/i386/kernel/cpu/cpufreq/Kconfig
@@ -92,7 +92,7 @@ config X86_POWERNOW_K8
 config X86_POWERNOW_K8_ACPI
bool "ACPI Support"
select ACPI_PROCESSOR
-   depends on X86_POWERNOW_K8
+   depends on ACPI && X86_POWERNOW_K8
default y
help
  This provides access to the K8s Processor Performance States via ACPI.
_
-
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/


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


[PATCH -rt] whitespace cleanup for 2.6.20-rc5-rt7

2007-01-20 Thread Noah Watkins
fixes trailing whitespace and spaces before tab indents in 2.6.20-rc5-rt7 as
reported with: git-apply --whitespace=error-all

---

 arch/arm/mach-omap1/time.c|2 +-
 arch/i386/kernel/entry.S  |2 +-
 arch/i386/kernel/reboot.c |2 +-
 arch/ia64/Kconfig |4 ++--
 arch/ia64/kernel/fsys.S   |2 +-
 arch/ia64/kernel/time.c   |4 ++--
 arch/x86_64/kernel/entry.S|2 +-
 arch/x86_64/kernel/hpet.c |2 +-
 arch/x86_64/kernel/reboot.c   |2 +-
 arch/x86_64/kernel/smpboot.c  |8 
 arch/x86_64/kernel/tsc.c  |8 
 arch/x86_64/kernel/vsyscall.c |2 +-
 drivers/char/lpptest.c|2 +-
 include/asm-arm/atomic.h  |2 +-
 include/asm-generic/vmlinux.lds.h |4 ++--
 include/linux/irq.h   |6 +++---
 include/linux/mutex.h |2 +-
 kernel/fork.c |6 +++---
 kernel/futex.c|4 ++--
 kernel/hrtimer.c  |2 +-
 kernel/latency_trace.c|6 +++---
 kernel/spinlock.c |2 +-
 kernel/workqueue.c|2 +-
 mm/migrate.c  |2 +-
 mm/slab.c |   14 +++---
 25 files changed, 47 insertions(+), 47 deletions(-)

diff --git a/arch/arm/mach-omap1/time.c b/arch/arm/mach-omap1/time.c
index 7baf0df..85f9f5e 100644
--- a/arch/arm/mach-omap1/time.c
+++ b/arch/arm/mach-omap1/time.c
@@ -105,7 +105,7 @@ static inline unsigned long omap_mpu_tim
 
 static inline void omap_mpu_set_autoreset(int nr)
 {
-   volatile omap_mpu_timer_regs_t* timer = omap_mpu_timer_base(nr);
+   volatile omap_mpu_timer_regs_t* timer = omap_mpu_timer_base(nr);
 
timer->cntl = timer->cntl | MPU_TIMER_AR;
 }
diff --git a/arch/i386/kernel/entry.S b/arch/i386/kernel/entry.S
index ce8a092..33d6b80 100644
--- a/arch/i386/kernel/entry.S
+++ b/arch/i386/kernel/entry.S
@@ -272,7 +272,7 @@ need_resched:
jz restore_nocheck
testl $IF_MASK,PT_EFLAGS(%esp)  # interrupts off (exception path) ?
jz restore_nocheck
-   DISABLE_INTERRUPTS(CLBR_ANY)
+   DISABLE_INTERRUPTS(CLBR_ANY)
 
call preempt_schedule_irq
jmp need_resched
diff --git a/arch/i386/kernel/reboot.c b/arch/i386/kernel/reboot.c
index 5f81e80..a3e7410 100644
--- a/arch/i386/kernel/reboot.c
+++ b/arch/i386/kernel/reboot.c
@@ -327,7 +327,7 @@ void machine_emergency_restart(void)
asm volatile (
"1: .byte 0x0f, 0x01, 0xc4  \n" /* vmxoff */
"2: \n"
-   ".section __ex_table,\"a\"  \n"
+   ".section __ex_table,\"a\"  \n"
"   .align 4\n"
"   .long   1b,2b   \n"
".previous  \n"
diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
index 333049c..a1a6bc3 100644
--- a/arch/ia64/Kconfig
+++ b/arch/ia64/Kconfig
@@ -300,7 +300,7 @@ config HIGH_RES_RESOLUTION
 choice
prompt "Clock source"
depends on HIGH_RES_TIMERS
-   default HIGH_RES_TIMER_ITC
+   default HIGH_RES_TIMER_ITC
help
  This option allows you to choose the hardware source in charge
  of generating high precision interruptions on your system.
@@ -308,7 +308,7 @@ choice
 
 
  ITC Interval Time Counter 1/CPU clock
- HPET High Precision Event Timer   ~ (XXX:have to check the spec)
+ HPET High Precision Event Timer   ~ (XXX:have to check the spec)
 
  The ITC timer is available on all the ia64 computers because
  it is integrated directly into the processor. However it may not
diff --git a/arch/ia64/kernel/fsys.S b/arch/ia64/kernel/fsys.S
index 7b94c34..2abf920 100644
--- a/arch/ia64/kernel/fsys.S
+++ b/arch/ia64/kernel/fsys.S
@@ -219,7 +219,7 @@ ENTRY(fsys_gettimeofday)
 (p6)br.cond.spnt.few .fail_einval  // deferred branch
;;
ld8 r30 = [r10] // clocksource->mmio_ptr
-   movl r19 = itc_lastcycle 
+   movl r19 = itc_lastcycle
add r23 = IA64_CLOCKSOURCE_SHIFT_OFFSET,r20
cmp.ne p6, p0 = 0, r2   // Fallback if work is scheduled
 (p6)br.cond.spnt.many fsys_fallback_syscall
diff --git a/arch/ia64/kernel/time.c b/arch/ia64/kernel/time.c
index 983ef26..4fc3670 100644
--- a/arch/ia64/kernel/time.c
+++ b/arch/ia64/kernel/time.c
@@ -262,10 +262,10 @@ ia64_init_itm (void)
ia64_cpu_local_tick();
 
if (!clocksource_itc_p) {
-   /* Sort out mult/shift values: */
+   /* Sort out mult/shift values: */
clocksource_itc.mult = 
clocksource_hz2mult(local_cpu_data->itc_freq,
   clocksource_itc.shift);
-   clocksource_reg

[PATCH] include linux/fs.h in linux/cdev.h for struct inode

2007-01-19 Thread Noah Watkins
---
 include/linux/cdev.h |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/include/linux/cdev.h b/include/linux/cdev.h
index f309b00..b53e2a0 100644
--- a/include/linux/cdev.h
+++ b/include/linux/cdev.h
@@ -5,6 +5,7 @@
 #include 
 #include 
 #include 
+#include 
 
 struct cdev {
struct kobject kobj;
-- 
1.4.4.1

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