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