C does implicit casts from enum to integer types, right? So always using
u32 here would not impose any extra typing on the user, or am I missing
something subtle here?
No, that's right. I had just been thinking of the documentation /
convenience issue. I figured with u32 people might tend to
On Sun, 2009-12-13 at 16:25 -0800, Roland McGrath wrote:
I'm sorry for the delay. I'm picking up with responding to the parts of
your review that I did not include in my first reply. I appreciate very
much the discussion you've had with Oleg about the issues that I did not
address myself. I
On 12/13, Roland McGrath wrote:
Its not entirely clear why we can check pending_attach outside of the
utrace-lock and not be racy.
I think it can be racy sometimes but that does not matter.
Maybe Oleg can verify my logic here. If it's right, he can
add some comments to make it more
On 12/14, Peter Zijlstra wrote:
Quite gross this.. can't we key off the
tracehoook_report_clone_complete() and use a wakeup there?
Yes, we intended to clean this up at some point later. But doing that
is just a distraction and complication right now so we put it off. For
this
On 12/14, Oleg Nesterov wrote:
IOW, we must ensure that if ever clear TIF_NOTIFY_RESUME we must not
miss -pending_attach, correct? and for this case we have mb() after
clear_thread_flag(). Perhaps instead we should add mb__after_clear_bit()
into arch/ hooks, but this needs a lot of
On 12/14, Oleg Nesterov wrote:
IOW, we must ensure that if ever clear TIF_NOTIFY_RESUME we must not
miss -pending_attach, correct? and for this case we have mb() after
clear_thread_flag(). Perhaps instead we should add mb__after_clear_bit()
into arch/ hooks, but this needs a lot of
Yes, I think this is correct. It is fine to miss -pending_attach == T,
and in any case the new attacher can come right after the check, even
if it was checked under utrace-lock.
Right.
It is important that the tracee can't miss, say, UTRACE_REPORT request
(as you already explained), and
On 12/14, Roland McGrath wrote:
In the stopped cases, there are lots of locks and barriers and things
after resuming. (Oleg?)
Every time the tracee resumes after TASK_TRACED it uses utrace-lock
to synchronize with utrace_control/etc, it must see any changes.
And TASK_STOPPED?
All that seems to do is call -release() and kmem_cache_free()s the
utrace_engine thing, why can't that be done with utrace-lock held?
Calling -release with a lock held is clearly insane, sorry. It is true
that any engine-writer who does anything like utrace_* calls inside their
release
I'm sorry for the delay. I'm picking up with responding to the parts of
your review that I did not include in my first reply. I appreciate very
much the discussion you've had with Oleg about the issues that I did not
address myself. I look forward to your replies to my comments in that
first
On Tue, 2009-12-08 at 16:04 +0100, Oleg Nesterov wrote:
The
problem is, this code was developed out-of-tree. That is why we would
like to merge it asap, then do other changes which could be easily
reviewed.
Now, do you really mean we should throw out the working code, rewrite
it avoiding
On 12/08, Peter Zijlstra wrote:
On Tue, 2009-12-08 at 16:04 +0100, Oleg Nesterov wrote:
Well, this is subjective, but I don't agree that
get_task_struct(task);
task-utrace_flags = flags;
spin_unlock(utrace-lock);
put_task_struct(task);
looks
On 12/08, Peter Zijlstra wrote:
On Tue, 2009-12-08 at 16:04 +0100, Oleg Nesterov wrote:
The
problem is, this code was developed out-of-tree. That is why we would
like to merge it asap, then do other changes which could be easily
reviewed.
Now, do you really mean we should throw out
On Tue, 2009-12-08 at 17:31 +0100, Oleg Nesterov wrote:
If you take a task ref you can write the much saner:
utrace_control()
{
...
spin_lock(utrace-lock);
...
if (reset)
utrace_reset(utrace);
spin_unlock(utrace-lock);
}
No, get_task_struct() in
On Tue, 2009-12-01 at 23:08 +0100, Oleg Nesterov wrote:
@@ -560,6 +625,20 @@ static inline void tracehook_report_deat
int signal, void *death_cookie,
int group_dead)
{
+ /*
+ * This barrier ensures that
On 12/01, Peter Zijlstra wrote:
+static inline __must_check int utrace_control_pid(
+ struct pid *pid, struct utrace_engine *engine,
+ enum utrace_resume_action action)
+{
+ /*
+* We don't bother with rcu_read_lock() here to protect the
+* task_struct pointer, because
On Tue, 2009-11-24 at 21:02 +0100, Oleg Nesterov wrote:
+ sect2 id=reaptitleFinal callbacks/title
+ para
+The functionreport_reap/function callback is always the final event
+in the life cycle of a traced thread. Tracing engines can use this as
+the trigger to clean up their
On 12/01, Peter Zijlstra wrote:
On Tue, 2009-11-24 at 21:02 +0100, Oleg Nesterov wrote:
+ para
+There is nothing a kernel module can do to keep a structnamestruct
+task_struct/structname alive outside of
+functionrcu_read_lock/function.
Sure there is, get_task_struct()
Could we just drop the tracehook layer if this finally merged
and call the low level functions directly?
We can certainly do appropriate streamlining cleanups later, by all
means. The original purpose of the tracehook layer is simply this:
A person hacking on core kernel code or arch code
This above text seems inconsistent. First you say report_reap() is the
final callback and should be used for cleanup, then you say
report_death() is the penultimate callback but might not always happen
and people would normally use that as cleanup.
If we cannot rely on report_death() then I
This is subjective, but personally I disagree. Contrary, imho it
is good that tracehook hides the (simple) details. I do not understand
why the reader of, say, do_fork() should see the contents of
tracehook_report_clone_complete(). This will complicate the understanding.
Someone who has to
* Oleg Nesterov o...@redhat.com wrote:
Much better. But in this case please note that most of tracehooks just
do:
if (unlikely(task_utrace_flags(current) SOME_EVENT))
utrace_report_some_event();
I really don't understand why we shouldn't have
Oleg Nesterov o...@redhat.com writes:
From: Roland McGrath rol...@redhat.com
This adds the utrace facility, a new modular interface in the kernel
for implementing user thread tracing and debugging. This fits on top
of the tracehook_* layer, so the new code is well-isolated.
Could we just
On 11/24, Andi Kleen wrote:
Oleg Nesterov o...@redhat.com writes:
From: Roland McGrath rol...@redhat.com
This adds the utrace facility, a new modular interface in the kernel
for implementing user thread tracing and debugging. This fits on top
of the tracehook_* layer, so the new code
On Tue, Nov 24, 2009 at 09:41:52PM +0100, Oleg Nesterov wrote:
On 11/24, Andi Kleen wrote:
Oleg Nesterov o...@redhat.com writes:
From: Roland McGrath rol...@redhat.com
This adds the utrace facility, a new modular interface in the kernel
for implementing user thread tracing and
Hi -
On Tue, Nov 24, 2009 at 10:26:19PM +0100, Andi Kleen wrote:
[...]
For example. tracehook_report_syscall_entry() has a lot of callers
in arch/, each callsite should be changed to do
if ((task_utrace_flags(current) UTRACE_EVENT(SYSCALL_ENTRY))
26 matches
Mail list logo
