[PATCH 00/23] tracehook - Kernel

This is a discussion on [PATCH 00/23] tracehook - Kernel ; > It's a strange time to be sending this. I've never managed to understand how the timing of your cycles works exactly, and I really just work on the "send it when you can" principle. To be honest all I ...

+ Reply to Thread
Page 2 of 2 FirstFirst 1 2
Results 21 to 38 of 38

Thread: [PATCH 00/23] tracehook

  1. Re: [PATCH 00/23] tracehook

    > It's a strange time to be sending this.

    I've never managed to understand how the timing of your cycles works
    exactly, and I really just work on the "send it when you can" principle.
    To be honest all I really know is that stuff is going in now, and that
    I think this stuff of mine is ready to go.

    > That being said, the impact on existing code is fairly modest and we
    > could perhaps slip it into 2.6.27-rc1. I haven't looked at it yet.


    I really hope this can be merged in soon (for 2.6.27). I do think its
    impact is low. The sooner this goes in, the sooner arch folks can round
    out all their bits and the more time folks like me can spend on actually
    making something new and interesting happen in this area.

    I am happy to do whatever merging/rebasing work helps that happen the
    soonest. If there is a different GIT tree to branch a rebase from,
    that is easy for me to do.


    Thanks,
    Roland

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. Re: [PATCH 00/23] tracehook

    On Thu, 17 Jul 2008 01:11:21 -0700 (PDT) Roland McGrath wrote:

    > > It's a strange time to be sending this.

    >
    > I've never managed to understand how the timing of your cycles works
    > exactly, and I really just work on the "send it when you can" principle.


    It's pretty simple - it's just piplining. During the -rc's we
    accumulate stuff for the next release in linux-next and in -mm. During
    the 2.6.x->2.6.x+1-rc1 merge window we transfer that material into
    mainline then the cycle starts again.

    So new code which turns up during the merge window is a problem. It
    should be reviewed, integrated on top of the already-pending work and
    should get some testing in linux-next. But during the patch-merging
    chaos nobody has the time or a suitable tree upon which to be doing
    that.

    > To be honest all I really know is that stuff is going in now, and that
    > I think this stuff of mine is ready to go.


    Yes, but all the stuff which is going in now has been reviewed and has
    had testing in linux-next and/or -mm. (In theory).

    > > That being said, the impact on existing code is fairly modest and we
    > > could perhaps slip it into 2.6.27-rc1. I haven't looked at it yet.

    >
    > I really hope this can be merged in soon (for 2.6.27). I do think its
    > impact is low. The sooner this goes in, the sooner arch folks can round
    > out all their bits and the more time folks like me can spend on actually
    > making something new and interesting happen in this area.
    >
    > I am happy to do whatever merging/rebasing work helps that happen the
    > soonest. If there is a different GIT tree to branch a rebase from,
    > that is easy for me to do.
    >


    erk.

    Well I got it all merged up relatively easily but I don't know what it
    does and I haven't looked at it yet.

    It will take some urgent effort from reviewers and, it appears, arch
    maintainers to be able to get this over the line. Even then it wom't
    have had any linux-next testing. This should all have started weeks
    ago!

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. Re: [PATCH 00/23] tracehook

    On Thu, 17 Jul 2008 00:25:41 -0700 (PDT) Roland McGrath wrote:

    > This patch series introduces the "tracehook" interface layer of inlines
    > in .


    Looks sane to me from a quick scan.

    A ~200 byte increase in i386 allnoconfig .text is liveable with. But
    nothing defines CONFIG_HAVE_ARCH_TRACEHOOK yet. What effect will that
    have?

    I don't like the name! We have ftrace and we have static tracepoints
    and we have dynamic tracepoints and we have linux trace toolkit and
    whatever is in kernel/trace/trace.c etc, etc. Now this work comes
    along with _userspace_ tracing capabilities and it goes and calls it,
    of all things, "trace"!

    Things would be much less confusing were we to do s/trace/xyzzy/g on
    the whole patchset.


    Apart from that, I think the other big issue with this patchset is that
    it doesn't do anything yet. It's effectively a blank cheque. There's
    not a lot of point in merging all this work unless we also merge
    something which uses it (is this correct?). And afacit the thing which
    will use it is utrace and utrace hasn't been sighted for a year or more
    and it has met objections.

    If we merge this and then utrace crashes on a rocky shore then there
    was no (or little) point in having merged this.

    Or am I wrong about that? Does it have sufficient standalone value to
    justify a standalone merge (yet alone to justify such a late merge)?
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. Re: [PATCH 01/23] tracehook: add linux/tracehook.h

    On Thu, Jul 17, 2008 at 12:27:55AM -0700, Roland McGrath wrote:
    > The aim is to formalize and consolidate all the places that the core
    > kernel code and the arch code now ties into the ptrace implementation.
    >
    > These patches mostly don't cause any functional change. They just
    > move the details of ptrace logic out of core code into tracehook.h
    > inlines, where they are mostly compiled away to the same as before.


    > All that changes is that everything is thoroughly documented


    This is fine.

    > and any future reworking of ptrace, or addition of something new,
    > would not have to touch core code all over, just change the tracehook.h
    > inlines.


    And this is suprising wish given one can't predict how exactly those
    "future reworking" will look like.

    > The new linux/ptrace.h inlines are used by the following patches in the
    > new tracehook_*() inlines. Using these helpers for the ptrace event
    > stops makes it simple to change or disable the old ptrace implementation
    > of these stops conditionally later.


    Call them "utrace_*" from the start?

    > --- a/include/linux/ptrace.h
    > +++ b/include/linux/ptrace.h
    > @@ -121,6 +121,39 @@ static inline void ptrace_unlink(struct task_struct *child)
    > int generic_ptrace_peekdata(struct task_struct *tsk, long addr, long data);
    > int generic_ptrace_pokedata(struct task_struct *tsk, long addr, long data);
    >
    > +/**
    > + * task_ptrace - return %PT_* flags that apply to a task
    > + * @task: pointer to &task_struct in question
    > + *
    > + * Returns the %PT_* flags that apply to @task.
    > + */
    > +static inline int task_ptrace(struct task_struct *task)
    > +{
    > + return task->ptrace;
    > +}


    Pointless 1:1 wrapper unless you're going to #ifdef ->ptrace later.

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  5. Re: [PATCH 01/23] tracehook: add linux/tracehook.h

    On Thu, 2008-07-17 at 12:48 +0400, Alexey Dobriyan wrote:
    > On Thu, Jul 17, 2008 at 12:27:55AM -0700, Roland McGrath wrote:
    > > The aim is to formalize and consolidate all the places that the core
    > > kernel code and the arch code now ties into the ptrace implementation.
    > >
    > > These patches mostly don't cause any functional change. They just
    > > move the details of ptrace logic out of core code into tracehook.h
    > > inlines, where they are mostly compiled away to the same as before.

    >
    > > All that changes is that everything is thoroughly documented

    >
    > This is fine.
    >
    > > and any future reworking of ptrace, or addition of something new,
    > > would not have to touch core code all over, just change the tracehook.h
    > > inlines.

    >
    > And this is suprising wish given one can't predict how exactly those
    > "future reworking" will look like.
    >
    > > The new linux/ptrace.h inlines are used by the following patches in the
    > > new tracehook_*() inlines. Using these helpers for the ptrace event
    > > stops makes it simple to change or disable the old ptrace implementation
    > > of these stops conditionally later.

    >
    > Call them "utrace_*" from the start?


    Ah, maybe justified, because I don't expect any other re-implementation
    of the same after utrace is finished, but -- there's still the old
    ptrace implementation, so _not_ mentioning utrace seems a bit better to
    me.

    > > --- a/include/linux/ptrace.h
    > > +++ b/include/linux/ptrace.h
    > > @@ -121,6 +121,39 @@ static inline void ptrace_unlink(struct task_struct *child)
    > > int generic_ptrace_peekdata(struct task_struct *tsk, long addr, long data);
    > > int generic_ptrace_pokedata(struct task_struct *tsk, long addr, long data);
    > >
    > > +/**
    > > + * task_ptrace - return %PT_* flags that apply to a task
    > > + * @task: pointer to &task_struct in question
    > > + *
    > > + * Returns the %PT_* flags that apply to @task.
    > > + */
    > > +static inline int task_ptrace(struct task_struct *task)
    > > +{
    > > + return task->ptrace;
    > > +}

    >
    > Pointless 1:1 wrapper unless you're going to #ifdef ->ptrace later.


    And that's exactly what's going to happen. Look at Roland's git.

    Petr


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  6. Re: [PATCH 01/23] tracehook: add linux/tracehook.h

    On Thu, Jul 17, 2008 at 12:48:18PM +0400, Alexey Dobriyan wrote:
    > > The new linux/ptrace.h inlines are used by the following patches in the
    > > new tracehook_*() inlines. Using these helpers for the ptrace event
    > > stops makes it simple to change or disable the old ptrace implementation
    > > of these stops conditionally later.

    >
    > Call them "utrace_*" from the start?


    Yes, please.

    > Pointless 1:1 wrapper unless you're going to #ifdef ->ptrace later.


    Even then I don't think it's a good idea. This one should only be
    touched in very very few places

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  7. Re: [PATCH 01/23] tracehook: add linux/tracehook.h

    On Thu, Jul 17, 2008 at 01:06:52PM +0200, Petr Tesarik wrote:
    > On Thu, 2008-07-17 at 12:48 +0400, Alexey Dobriyan wrote:
    > > On Thu, Jul 17, 2008 at 12:27:55AM -0700, Roland McGrath wrote:
    > > > The aim is to formalize and consolidate all the places that the core
    > > > kernel code and the arch code now ties into the ptrace implementation.
    > > >
    > > > These patches mostly don't cause any functional change. They just
    > > > move the details of ptrace logic out of core code into tracehook.h
    > > > inlines, where they are mostly compiled away to the same as before.

    > >
    > > > All that changes is that everything is thoroughly documented

    > >
    > > This is fine.
    > >
    > > > and any future reworking of ptrace, or addition of something new,
    > > > would not have to touch core code all over, just change the tracehook.h
    > > > inlines.

    > >
    > > And this is suprising wish given one can't predict how exactly those
    > > "future reworking" will look like.
    > >
    > > > The new linux/ptrace.h inlines are used by the following patches in the
    > > > new tracehook_*() inlines. Using these helpers for the ptrace event
    > > > stops makes it simple to change or disable the old ptrace implementation
    > > > of these stops conditionally later.

    > >
    > > Call them "utrace_*" from the start?

    >
    > Ah, maybe justified, because I don't expect any other re-implementation
    > of the same after utrace is finished, but -- there's still the old
    > ptrace implementation, so _not_ mentioning utrace seems a bit better to
    > me.


    ptrace(2) will start calling utrace_* hooks and functions.

    These tracehooks are generic and utrace is generic as well.

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  8. Re: [PATCH 23/23] /proc/PID/syscall

    On Thu, Jul 17, 2008 at 12:31:44AM -0700, Roland McGrath wrote:
    > This adds /proc/PID/syscall and /proc/PID/task/TID/syscall magic files.
    > These use task_current_syscall() to show the task's current system call
    > number and argument registers, stack pointer and PC.


    > --- a/fs/proc/base.c
    > +++ b/fs/proc/base.c
    > @@ -509,6 +509,26 @@ static int proc_pid_limits(struct task_struct *task, char *buffer)
    > return count;
    > }
    >
    > +#ifdef CONFIG_HAVE_ARCH_TRACEHOOK
    > +static int proc_pid_syscall(struct task_struct *task, char *buffer)
    > +{
    > + long nr;
    > + unsigned long args[6], sp, pc;
    > +
    > + if (task_current_syscall(task, &nr, args, 6, &sp, &pc))
    > + return sprintf(buffer, "running\n");
    > +
    > + if (nr < 0)
    > + return sprintf(buffer, "%ld 0x%lx 0x%lx\n", nr, sp, pc);
    > +
    > + return sprintf(buffer,
    > + "%ld 0x%lx 0x%lx 0x%lx 0x%lx 0x%lx 0x%lx 0x%lx 0x%lx\n",
    > + nr,
    > + args[0], args[1], args[2], args[3], args[4], args[5],
    > + sp, pc);
    > +}
    > +#endif /* CONFIG_HAVE_ARCH_TRACEHOOK */


    My gut feeling this code needs ptrace_may_access() checks.

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  9. Re: [PATCH 00/23] tracehook


    * Andrew Morton wrote:

    > On Thu, 17 Jul 2008 00:25:41 -0700 (PDT) Roland McGrath wrote:
    >
    > > This patch series introduces the "tracehook" interface layer of
    > > inlines in .

    >
    > Looks sane to me from a quick scan.


    same here.

    Reviewed-by: Ingo Molnar

    > A ~200 byte increase in i386 allnoconfig .text is liveable with. But
    > nothing defines CONFIG_HAVE_ARCH_TRACEHOOK yet. What effect will that
    > have?


    this is the second subtle step towards utrace and
    next-gen-instrumentation. (regset was the first, by far most risky step)

    > I don't like the name! We have ftrace and we have static tracepoints
    > and we have dynamic tracepoints and we have linux trace toolkit and
    > whatever is in kernel/trace/trace.c etc, etc. Now this work comes
    > along with _userspace_ tracing capabilities and it goes and calls it,
    > of all things, "trace"!


    > Apart from that, I think the other big issue with this patchset is
    > that it doesn't do anything yet. It's effectively a blank cheque.
    > There's not a lot of point in merging all this work unless we also
    > merge something which uses it (is this correct?). And afacit the
    > thing which will use it is utrace and utrace hasn't been sighted for a
    > year or more and it has met objections.
    >
    > If we merge this and then utrace crashes on a rocky shore then there
    > was no (or little) point in having merged this.
    >
    > Or am I wrong about that? Does it have sufficient standalone value to
    > justify a standalone merge (yet alone to justify such a late merge)?


    It has enough standalone value to me - it generally cleans up all things
    "abstract kernel events", collects them into a single entity and lets
    tracers interact with the kernel (not just passively observe its
    events). So it's good for next-gen debuggers too, etc.

    And task_current_syscall() avoids us hundreds of crappy hooks in every
    syscall handler and gives us in-kernel strace in essence. (it's not just
    useful to sample blocked threads - it could also be used by ftrace&co to
    sample the currently executing task) That alone makes it worth it IMO
    ;-)

    also in places it cleans up current special-case-for-ptrace code and
    makes it shorter - like in kernel/exit.c. Well thought out scheme and
    structure - as we've come to expect from Roland.

    Ingo
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  10. Re: [PATCH 00/23] tracehook

    From: Roland McGrath
    Date: Thu, 17 Jul 2008 00:25:41 -0700 (PDT)

    > I have done some arch work and will submit this to the arch maintainers
    > after the generic tracehook series settles in. For now, that work is
    > available in my GIT repositories, and in patch and mbox-of-patches form
    > at http://people.redhat.com/roland/utrace/2.6-current/


    Thanks for doing this work.

    I took a quick look at the sparc64 tracehook support and only spotted
    one issue.

    You'll need special handling for 32-bit compat tasks when copying the
    arguments in. You can't just use the full 64-bit values because they
    get 32-bit zero extended by the 32-bit syscall entry code, and this
    doesn't propagate into the pt_regs copies.

    See arch/sparc64/kernel/syscalls.S:linux_sparc_syscall32

    So at a minimum, when _TIF_32BIT is set, you'll need to "u32" cast
    the regs->u_regs[] values before assigning them into the unsigned long
    array slots.

    Actually, it's more complicated than that. We have to sign extend
    arguments in some cases, and this is performed by a class of stubs
    in arch/sparc64/kernel/sys32.S which then revector to the real
    system call handler.

    I don't know how you want to handle those cases. Should the tracehook
    user know that it's a 32-bit task, what system call it is, and therefore
    do the appropriate sign extensions? That doesn't sound right.

    So likely we have to properly sign extend those things here in the
    asm/syscall.h handlers.

    If you look at arch/sparc64/kernel/sys32.S it uses macros and then a
    list of cases that could be slightly modified such that they could be
    used to build argument sign extension tables. Simply change the
    explicit register name arguments to pure numbers (ie. %o0 becomes
    plain 0) and adjust the assembler macros accordingly.

    Then, in a C file somewhere, you set these macro defines differently
    then feed the list of instantiations through them and build the table.

    The other arch's with compat support will have this same exact issue.

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  11. Re: [PATCH 00/23] tracehook

    David Miller writes:
    >
    > The other arch's with compat support will have this same exact issue.


    On x86-64 the sign extension is in C code, so it would be somewhat more
    complicated to do a generic table or converter.

    I think the tracer will always need to know if it's tracing 32bit
    or 64bit though, because the interfaces are not always 100% compatible.
    e.g. I suspect any serious one will need to look at memory
    arguments for some cases and those generally differ.

    -Andi
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  12. Re: [PATCH 00/23] tracehook

    From: Andi Kleen
    Date: Fri, 18 Jul 2008 13:24:33 +0200

    > David Miller writes:
    > >
    > > The other arch's with compat support will have this same exact issue.

    >
    > On x86-64 the sign extension is in C code, so it would be somewhat more
    > complicated to do a generic table or converter.


    I was suggesting to use the list of syscalls in the sparc64 code
    in a generic spot, making a table indexed by __NR_* number
    that the tracehook syscall stuff can use.

    But yes, there are other issues such as datastructure layout.

    It all depends upon what Roland thinks those arg values obtained by
    the asm/syscall.h functions should mean.

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  13. Re: [PATCH 01/23] tracehook: add linux/tracehook.h

    On Thu, 2008-07-17 at 09:34 -0400, Christoph Hellwig wrote:
    > On Thu, Jul 17, 2008 at 12:48:18PM +0400, Alexey Dobriyan wrote:
    > > > The new linux/ptrace.h inlines are used by the following patches in the
    > > > new tracehook_*() inlines. Using these helpers for the ptrace event
    > > > stops makes it simple to change or disable the old ptrace implementation
    > > > of these stops conditionally later.

    > >
    > > Call them "utrace_*" from the start?

    >
    > Yes, please.
    >
    > > Pointless 1:1 wrapper unless you're going to #ifdef ->ptrace later.

    >
    > Even then I don't think it's a good idea. This one should only be
    > touched in very very few places


    So, do you suggest putting those #ifdef's in the middle of the function
    which uses it? Anyway, this patch should probably be posted later, i.e.
    when the other implementation is actually introduced.

    My $0.02.

    Petr


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  14. Re: [PATCH 23/23] /proc/PID/syscall

    > My gut feeling this code needs ptrace_may_access() checks.

    That would be fine with me. The nodes are readable only by user, which
    seems sufficient to me. But I certainly don't object to the stronger check
    there. I didn't put too much thought into /proc/pid/syscall, it's just
    there to demonstrate and test the internal task_current_syscall() call.


    Thanks,
    Roland
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  15. Re: [PATCH 02/23] tracehook: exec

    On Thu, Jul 17, 2008 at 12:28:20AM -0700, Roland McGrath wrote:
    > This moves all the ptrace hooks related to exec into tracehook.h inlines.
    >
    > This also lifts the calls for tracing out of the binfmt load_binary hooks
    > into search_binary_handler() after it calls into the binfmt module. This
    > change has no effect, since all the binfmt modules' load_binary functions
    > did the call at the end on success, and now search_binary_handler() does
    > it immediately after return if successful. We consolidate the repeated
    > code, and binfmt modules no longer need to import ptrace_notify().


    Care to first just consolidate the code from the binary handlers to
    exec.c and then restrucure it? Currently mainline doesn't even
    have ptrace_event yet so it's hard to verify the code is the same.
    It certainly isn't for binfmt_som although that's like just a bugfix
    that needs to be documented. Also the two new routines are too large
    to be inlined, and I would rather not rely on the guarantee that no
    other caller will pop up.

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  16. Re: [PATCH 00/23] tracehook

    Ingo said very well what I think the intrinsic value is. Indeed, I
    think it is of use even if utrace were never merged. Anyone who
    decides tomorrow that my crap is useless and whips up a different
    plan instead of utrace, will benefit from reading all the kerneldoc
    in tracehook.h.

    The practical value of the patch series right now is that it makes
    life easier for merging and patch juggling. The new utrace patches
    are indeed coming too, and I'm expecting some vigorous feedback.
    Different versions are likely to bounce around for a while.
    Meanwhile, there are some people trying to track latest+utrace via
    GIT or patching. The tracehook series makes it so that the utrace
    patches proper touch very little core code and few files, so merge
    and patch conflicts are rare. I can tell you from doing it a lot
    that there are frequently niggling conflicts to fix up in rebasing
    the tracehook patches after miscellaneous core kernel changes.
    Things requiring changes to tracehook.h are unlikely, but unrelated
    changes textually near the tracehook calls that foul automatic patch
    merging are common.

    I'll do another few days of polish on utrace first too, but one main
    reason I posted tracehook alone first was to see how much shrapnel I
    took in the face just for this, before getting to the substantive
    bits. If this much is accepted, that's enough to make it possible
    for utrace or another new thing to be added as a config option.

    The new utrace has internal differences from the first version, but
    the more essential point here is that it's entirely optional at
    config time. Whatever traits utrace has, it's only a new
    non-default config option marked EXPERIMENTAL. Once the tracehook
    series is accepted, then relative to that, no utrace patches will do
    anything at all if CONFIG_UTRACE is not chosen.

    Another value is for arch folks. I know at least a few arch
    maintainers are interested in adding this support. The generic
    tracehook series sets a base on which all the arch support can be
    finished up and ready to enable utrace or something different,
    whatever it is. If this base is in a canonical shared tree to be
    merged into, then any interested arch people can fully complete this
    work and push it to their arch's users. This lets people experiment
    on utrace (or a replacement) without also juggling arch patches.


    Thanks,
    Roland
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  17. Re: [PATCH 00/23] tracehook

    > I took a quick look at the sparc64 tracehook support and only spotted
    > one issue.


    Thanks for the feedback. The sparc64 (and ia64) code I have is meant
    only as a starting point, and there are some unfinished holes. I'd
    intended to point you (arch maintainers) at what I whipped up but
    leave it to you to decide on the correct arch bits and merge them
    eventually. (The x86 and powerpc patches are the only ones I've
    really used myself and would submit as claimed to be ready.)

    > You'll need special handling for 32-bit compat tasks when copying the
    > arguments in. You can't just use the full 64-bit values because they
    > get 32-bit zero extended by the 32-bit syscall entry code, and this
    > doesn't propagate into the pt_regs copies.


    This is true of powerpc too. I guess I'd figured that uses of the
    calls would know when it's 32-bit and truncate what they saw. Isn't
    that what happens now when a 64-bit ptrace caller uses the 64-bit
    getregs flavor on a target task that is actually 32-bit?

    But I don't disagree it might well be better for syscall_get_arguments
    to truncate values to 32 bits. I don't think there's a need for it to
    reflect sign-extended 64-bit values when it's a 32-bit task. Probably
    better not to, i.e. see "123 0xffffffff ..." in /proc/pid/syscall on a
    64-bit kernel for a 32-bit task as on 32-bit, instead of 32-bit apps
    that don't normally notice whether the kernel is 64 or 32 sometimes
    seeing "123 0xffffffffffffffff ...".

    > Actually, it's more complicated than that. We have to sign extend
    > arguments in some cases, and this is performed by a class of stubs
    > in arch/sparc64/kernel/sys32.S which then revector to the real
    > system call handler.


    It's odd that you need sign-extended arguments for all those calls
    when other machines don't.

    > I don't know how you want to handle those cases. Should the tracehook
    > user know that it's a 32-bit task, what system call it is, and therefore
    > do the appropriate sign extensions? That doesn't sound right.


    Which call are you talking about? For syscall_get_arguments, I think
    it's fine to get all values zero-extended from 32 bits (maybe even
    fine to get random high bits). The consumer of those values in the
    kernel code ought well enough to know whether it's a 32-bit or 64-bit
    task's system call--that changes what the call number and arguments
    mean, anyway.

    For syscall_set_arguments, it's a different question. In that case,
    we should expect that whatever original source of the value being
    poked in was, it just knows it's poking a 32-bit task and doesn't even
    consider that the kernel might be 64 bits. It should poke a 32-bit
    value and have the effect that poking that 32-bit value has on a
    32-bit kernel.

    Anyway, I don't see how this side of it is really an issue at all.
    syscall_set_arguments is only valid to use at the syscall entry
    tracing point. This is before the dispatch to those stubs doing the
    sign extension. (Or it could be used at a point entirely outside of
    syscall entry, like on the state just after -ERESTARTSYS handling
    to change the args before the user re-executes the syscall.)

    > The other arch's with compat support will have this same exact issue.


    They don't seem to.


    Thanks,
    Roland
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  18. Re: [PATCH 00/23] tracehook

    From: Roland McGrath
    Date: Mon, 21 Jul 2008 03:54:31 -0700 (PDT)

    > > The other arch's with compat support will have this same exact issue.

    >
    > They don't seem to.


    Look at S390 and PowerPC, they do the sign extension cases as well.

    PowerPC does it with C level stubs, whereas I believe S390 uses a
    similar assembler stub scheme as sparc64.

    But you are right, this is probably not an issue, and providing 32-bit
    truncated incoming args, and letting the tracer do whatever it likes
    for outgoing args, should work just fine.
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread
Page 2 of 2 FirstFirst 1 2