Andrew Morton wrote:
> > This patch introduces %pT format specifier for printing task_struct->comm.
> > Currently %pT does not provide consistency. I'm planning to change to use
> > RCU
> > in the future. By using RCU, the comm name read from task_struct->comm will
> > be
> > guaranteed to be consistent.
>
> Not completely accurate - RCU won't protect code which accesses ->comm
> from interrupts. Printing current->comm from irq is quite daft, but I
> bet there's code that does it :(
>
> As long as the kernel doesn't crash or otherwise misbehave when this
> happens, I think we're OK.
>
> (And I guess there's also non-daft code which accesses current->comm
> from interrupt context: oops, panic, etc).
Excuse me, but are interrupts relevant?
I think that readers that do
rcu_read_lock();
use p->comm here
rcu_read_unlock();
will be protected from writers that wait freeing p->comm using
synchronize_rcu() or call_rcu().
Is synchronize_rcu() or call_rcu() insufficient for protecting readers that do
rcu_read_lock();
use p->comm here
rcu_read_unlock();
from interrupts?
> > --- a/lib/vsprintf.c
> > +++ b/lib/vsprintf.c
> > @@ -1179,6 +1179,21 @@ char *address_val(char *buf, char *end, const void
> > *addr,
> > return number(buf, end, num, spec);
> > }
> >
> > +static noinline_for_stack
>
> hm, does noinline_for_stack actually do anything useful here? I
> suspect it *increases* stack depth in the way comm_name() is used here.
>
I just added noinline_for_stack as with other functions does.
But indeed, stack used by name[] is only 16 bytes but stack used by function
arguments are larger than 16 bytes. We should remove noinline_for_stack ?
> > +char *comm_name(char *buf, char *end, struct task_struct *tsk,
> > + struct printf_spec spec, const char *fmt)
> > +{
> > + char name[TASK_COMM_LEN];
> > +
> > + /* Caller can pass NULL instead of current. */
> > + if (!tsk)
> > + tsk = current;
> > + /* Not using get_task_comm() in case I'm in IRQ context. */
> > + memcpy(name, tsk->comm, TASK_COMM_LEN);
> > + name[sizeof(name) - 1] = '\0';
>
> get_task_comm() uses strncpy()?
I think unconditionally copying 16 bytes using memcpy() is faster than
conditionally copying until '\0'.
>
> > + return string(buf, end, name, spec);
> > +}
> > +
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
