Re: [PATCH 1/2] utrace core

[Roland McGrath <roland@redhat.com>]

> Again, embed struct utrace directly into task_struct. task_struct
> lifetime rules are way more tested than struct utrace ones.

The most consistent feedback I've seen to all new features is that they
mustn't add overhead when they're not being used.  So I never considered it
an option to bloat task_struct by ~120 bytes.  Of course much more than
that is entailed when a task is actually being traced somehow.  But the
presumption is that most tasks most of the time aren't, and that's what not
to bloat.

The revamp of the API after the first prototype made some of the internals
much simpler to implement, that had been very sticky in the old prototype
code.  But the allocation and freeing of struct utrace is an area I did not
fully revisit.  Buggy is buggy, and sure it needs to be tested and fixed.
I'm still inclined to look into making it right rather than punting.

> Add simple spinlock guarding all accesses (OK, I haven't looked very
> closely if it's possible)

I can't tell what you mean here.  Do you mean something different 
from struct utrace.lock?  If there were no pointer and its allocation
to synchronize, then what other lock would you be adding?

> INIT_RCU_HEAD is not needed, call_rcu() will overwrite rcu head unconditionally.

I see.  Thanks!  At some point, this was in the recommended example uses of
RCU.  This macro is still used in several places around the kernel.
Shouldn't they all be removed?  I wonder why it still exists in rcupdate.h.


Thanks,
Roland