lkml.org 
[lkml]   [2009]   [Apr]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 0/4] workqueue_tracepoint: Add worklet tracepoints for worklet lifecycle tracing
    On 04/26, Andrew Morton wrote:
    >
    > Most workqueue work lately has come from Oleg. I'm unaware that he has
    > expressed an interest in this feature? Oleg, would it have been useful
    > in any of the work you've done?

    Well. Probably not. But I don't think this matters. Other people (and
    not only kernel developers) can find this useful.

    I _think_ that if you are going to hack workqueue.c itself, it is
    more easy to just add some printks, may be I am wrong. But, probably
    tracepoints can help to write/debug, say, device drivers. Or admins
    can use debugfs to see whats going on, or to provide more info for
    the bugreports.

    I try to avoid "do we need this feauture" discussions as much as
    possible. Because I never know. My usage of kernel is very, very
    limited, I never do something "interesting" on my machine. This reminds
    me the discussion about the ability to trace /sbin/init. Some developers
    were unhappy with the trivial patch I sent. They said it is trivial
    to change your kernel if you need this. But no, it was not trivial
    to me when I was admin. So, I just can't judge.

    > > And the thing is, the workqueue code has been pretty problematic
    > > lately - with lockups and other regressions.

    Hmm. Perhaps I missed some bug-reports... But I don't remember any
    recent problems with workueues except the "usual" bugs like "flush
    shares the lock with work->func".


    As for the patches, I can't review them now. They are on top of
    some other changes which I didn't see (or perhaps I lost the patches
    I was cc'ed? sorry in this case).

    But at least the change in workqueue.c looks very simple, and do
    not complicate the code for readers. And, if we add any tracing
    to workqueues, then it is very natural to add entry/exit handlers.


    I must admit, I don't really understand why trace_workqueue.c uses
    cwq->thread as a "primary key". I have the feeling we can simplify
    this code if we pass "struct workqueue_struct *" instead, but I am
    not sure.

    In particular, trace_workqueue_flush(cwq->thread) looks a bit strange
    to me. I can't imagine how it can be useful per-thread and without
    trace_workqueue_flush_end() or something. I mean, if we need to trace
    flushes, then imho it makes much more sense to add flush_start/flush_end
    handlers into flush_workqueue().

    Oleg.



    \
     
     \ /
      Last update: 2009-04-27 17:19    [W:0.024 / U:0.344 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site