Re: [patch 2/3] Latencytop instrumentations part 1

2008-01-18 Thread Arjan van de Ven

Frank Ch. Eigler wrote:

Hi -

On Fri, Jan 18, 2008 at 02:33:34PM -0800, Arjan van de Ven wrote:

[...]

Can you suggest of some reason why all this instrumentation could
not be in the form of standard markers (perhaps conditionally
compiled out if necessary)?

sure. Every instrumentation you see is of the nested kind (since the lowest 
level
of nesting is already automatic via wchan).
If markers can provide me the following semantics, I'd be MORE than happy to 
use markers:
[...]
If markers can provide that semantics ... you sold me.


Further to what acme said, markers are semantics-free.  Callback
functions that implement your entry & exit semantics can be attached
at run time, at your pleasure.  (So can systemtap probes, for that
matter.)  The main difference would be that these callback functions
would have manage the per-thread LIFO data structures themselves,
instead of allocating backpointers on the kernel stack.  (Bonus marks
for not modifying task_struct. :-)


modifying task struct to have storage space is no big deal...
--
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: [patch 2/3] Latencytop instrumentations part 1

2008-01-18 Thread Frank Ch. Eigler
Hi -

On Fri, Jan 18, 2008 at 02:33:34PM -0800, Arjan van de Ven wrote:
> [...]
> > Can you suggest of some reason why all this instrumentation could
> > not be in the form of standard markers (perhaps conditionally
> > compiled out if necessary)?
> 
> sure. Every instrumentation you see is of the nested kind (since the lowest 
> level
> of nesting is already automatic via wchan).
> If markers can provide me the following semantics, I'd be MORE than happy to 
> use markers:
> [...]
> If markers can provide that semantics ... you sold me.

Further to what acme said, markers are semantics-free.  Callback
functions that implement your entry & exit semantics can be attached
at run time, at your pleasure.  (So can systemtap probes, for that
matter.)  The main difference would be that these callback functions
would have manage the per-thread LIFO data structures themselves,
instead of allocating backpointers on the kernel stack.  (Bonus marks
for not modifying task_struct. :-)

- FChE
--
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: [patch 2/3] Latencytop instrumentations part 1

2008-01-18 Thread Arnaldo Carvalho de Melo
Em Fri, Jan 18, 2008 at 02:33:34PM -0800, Arjan van de Ven escreveu:
> On Fri, 18 Jan 2008 17:26:09 -0500
> [EMAIL PROTECTED] (Frank Ch. Eigler) wrote:
> 
> > Arjan van de Ven <[EMAIL PROTECTED]> writes:
> > 
> > > This patch contains the first set of instrumentations for
> > > latencytop; each instrumentation needs to be evaluated for
> > > usefulness before this can go into mainline; posting here just to
> > > show how the infrastructure can be used
> > > [...]
> > 
> > Can you suggest of some reason why all this instrumentation could
> > not be in the form of standard markers (perhaps conditionally
> > compiled out if necessary)?
> > 
> 
> sure. Every instrumentation you see is of the nested kind (since the lowest 
> level
> of nesting is already automatic via wchan).
> 
> If markers can provide me the following semantics, I'd be MORE than happy to 
> use markers:
> 
> If the code path is like this
> 
> 
> set_latency_reason("Reading file");
> 
> 
>   set_latency_reason("Allocating memory");
>   kmalloc()  <--- blocks for 100 msec
>   restore_latency_reason()
> 
> restore_latency_reason();
> 
> via several layers of functions, the requirement is that the 100 msec is 
> accounted 
> to "reading file" and not "Allocating memory". But if some other codepath 
> hits the allocating memory function,
> without an outer "set_latency_reason", it should be accounted to "Allocating 
> memory".
> 
> If markers can provide that semantics ... you sold me.

My limited understanding of markers is that you can attach something to
them that later work out any semantics you may want to associate with
them.

So I guess that as soon as the outermost marker is reached you can
just bump a counter and when the (several) inner markers are reached and
the counter is not zero you just bump it and go on with life, leaving
the latency measurement being done to the "owner" (outermost) marker.

Frank, am I talking bullshit?

- Arnaldo
--
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: [patch 2/3] Latencytop instrumentations part 1

2008-01-18 Thread Arjan van de Ven
On Fri, 18 Jan 2008 17:26:09 -0500
[EMAIL PROTECTED] (Frank Ch. Eigler) wrote:

> Arjan van de Ven <[EMAIL PROTECTED]> writes:
> 
> > This patch contains the first set of instrumentations for
> > latencytop; each instrumentation needs to be evaluated for
> > usefulness before this can go into mainline; posting here just to
> > show how the infrastructure can be used
> > [...]
> 
> Can you suggest of some reason why all this instrumentation could
> not be in the form of standard markers (perhaps conditionally
> compiled out if necessary)?
> 

sure. Every instrumentation you see is of the nested kind (since the lowest 
level
of nesting is already automatic via wchan).

If markers can provide me the following semantics, I'd be MORE than happy to 
use markers:

If the code path is like this


set_latency_reason("Reading file");


  set_latency_reason("Allocating memory");
  kmalloc()  <--- blocks for 100 msec
  restore_latency_reason()

restore_latency_reason();

via several layers of functions, the requirement is that the 100 msec is 
accounted 
to "reading file" and not "Allocating memory". But if some other codepath hits 
the allocating memory function,
without an outer "set_latency_reason", it should be accounted to "Allocating 
memory".

If markers can provide that semantics ... you sold me.


-- 
If you want to reach me at my work email, use [EMAIL PROTECTED]
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org
--
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: [patch 2/3] Latencytop instrumentations part 1

2008-01-18 Thread Frank Ch. Eigler
Arjan van de Ven <[EMAIL PROTECTED]> writes:

> This patch contains the first set of instrumentations for latencytop;
> each instrumentation needs to be evaluated for usefulness before this
> can go into mainline; posting here just to show how the infrastructure
> can be used
> [...]

Can you suggest of some reason why all this instrumentation could
not be in the form of standard markers (perhaps conditionally
compiled out if necessary)?

- FChE
--
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 2/3] Latencytop instrumentations part 1

2008-01-18 Thread Arjan van de Ven
This patch contains the first set of instrumentations for latencytop;
each instrumentation needs to be evaluated for usefulness before this
can go into mainline; posting here just to show how the infrastructure
can be used

---
 arch/x86/mm/fault_64.c|4 
 block/ll_rw_blk.c |3 +++
 drivers/scsi/hosts.c  |1 -
 drivers/scsi/scsi_error.c |6 +-
 drivers/scsi/scsi_lib.c   |4 
 drivers/scsi/sd.c |5 +
 drivers/scsi/sr.c |   12 +++-
 fs/buffer.c   |   22 +-
 fs/eventpoll.c|5 +
 fs/ext3/inode.c   |   30 --
 fs/inode.c|   13 +
 fs/jbd/checkpoint.c   |6 ++
 fs/jbd/commit.c   |5 +
 fs/jbd/journal.c  |5 +
 fs/jbd/transaction.c  |   13 +
 fs/pipe.c |5 +
 fs/select.c   |3 +++
 fs/sync.c |   19 +--
 kernel/fork.c |4 
 kernel/futex.c|8 +++-
 kernel/mutex.c|   13 +
 kernel/rwsem.c|   12 +++-
 mm/filemap.c  |6 ++
 mm/slub.c |7 ++-
 24 files changed, 200 insertions(+), 11 deletions(-)

Index: linux-2.6.24-rc7/arch/x86/mm/fault_64.c
===
--- linux-2.6.24-rc7.orig/arch/x86/mm/fault_64.c
+++ linux-2.6.24-rc7/arch/x86/mm/fault_64.c
@@ -303,6 +303,7 @@ asmlinkage void __kprobes do_page_fault(
int write, fault;
unsigned long flags;
siginfo_t info;
+   struct latency_entry reason;
 
/*
 * We can fault from pretty much anywhere, with unknown IRQ state.
@@ -441,7 +442,10 @@ good_area:
 * make sure we exit gracefully rather than endlessly redo
 * the fault.
 */
+
+   set_latency_reason("page fault", &reason);
fault = handle_mm_fault(mm, vma, address, write);
+   restore_latency_reason(&reason);
if (unlikely(fault & VM_FAULT_ERROR)) {
if (fault & VM_FAULT_OOM)
goto out_of_memory;
Index: linux-2.6.24-rc7/block/ll_rw_blk.c
===
--- linux-2.6.24-rc7.orig/block/ll_rw_blk.c
+++ linux-2.6.24-rc7/block/ll_rw_blk.c
@@ -2190,7 +2190,9 @@ static struct request *get_request_wait(
 {
const int rw = rw_flags & 0x01;
struct request *rq;
+   struct latency_entry reason;
 
+   set_latency_reason("Waiting for a block layer request", &reason);
rq = get_request(q, rw_flags, bio, GFP_NOIO);
while (!rq) {
DEFINE_WAIT(wait);
@@ -2224,6 +2226,7 @@ static struct request *get_request_wait(
finish_wait(&rl->wait[rw], &wait);
}
 
+   restore_latency_reason(&reason);
return rq;
 }
 
Index: linux-2.6.24-rc7/drivers/scsi/scsi_error.c
===
--- linux-2.6.24-rc7.orig/drivers/scsi/scsi_error.c
+++ linux-2.6.24-rc7/drivers/scsi/scsi_error.c
@@ -228,11 +227,16 @@ void scsi_times_out(struct scsi_cmnd *sc
 int scsi_block_when_processing_errors(struct scsi_device *sdev)
 {
int online;
+   struct latency_entry reason;
+
+   set_latency_reason("SCSI block due to error recovery", &reason);
 
wait_event(sdev->host->host_wait, !scsi_host_in_recovery(sdev->host));
 
online = scsi_device_online(sdev);
 
+   restore_latency_reason(&reason);
+
SCSI_LOG_ERROR_RECOVERY(5, printk("%s: rtn: %d\n", __FUNCTION__,
  online));
 
Index: linux-2.6.24-rc7/drivers/scsi/scsi_lib.c
===
--- linux-2.6.24-rc7.orig/drivers/scsi/scsi_lib.c
+++ linux-2.6.24-rc7/drivers/scsi/scsi_lib.c
@@ -183,6 +183,9 @@ int scsi_execute(struct scsi_device *sde
struct request *req;
int write = (data_direction == DMA_TO_DEVICE);
int ret = DRIVER_ERROR << 24;
+   struct latency_entry reason;
+
+   set_latency_reason("SCSI layer command execute", &reason);
 
req = blk_get_request(sdev->request_queue, write, __GFP_WAIT);
 
@@ -208,6 +211,7 @@ int scsi_execute(struct scsi_device *sde
  out:
blk_put_request(req);
 
+   restore_latency_reason(&reason);
return ret;
 }
 EXPORT_SYMBOL(scsi_execute);
Index: linux-2.6.24-rc7/drivers/scsi/sd.c
===
--- linux-2.6.24-rc7.orig/drivers/scsi/sd.c
+++ linux-2.6.24-rc7/drivers/scsi/sd.c
@@ -47,6 +47,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include 
@@ -796,10 +797,13 @@ static int sd_sync_cache(struct scsi_dis
int retries, res;
struct scsi_device *sdp = sdkp->device;
struct scsi_sense_hdr sshdr;
+   struct latency_entry re