On Thu 2020-09-24 11:17:56, Sergey Senozhatsky wrote:
> On (20/09/23 15:56), Petr Mladek wrote:
> [..]
> > /*
> > * To reduce unnecessarily reopening, first check if the descriptor
> > - * state and caller ID are correct.
> > + * state and caller infromation are correct.
> > */
> > - d_state = desc_read(desc_ring, id, &desc, NULL, &cid);
> > - if (d_state != desc_committed || cid != caller_id)
> > + d_state = desc_read(desc_ring, id, &desc, NULL, &cal);
> > + if (d_state != desc_committed ||
> > + cal.pid != caller->pid ||
> > + cal.cpu_ctx != caller->cpu_ctx) {
>
> You probably might want to factor out ctx check into a static
> inline helper. Since you use this check in several places, and
> we may check more context fields in the future.
Makes sense.
> [..]
> > +/* Information about the process and context that adds the message */
> > +struct printk_caller {
> > + pid_t pid; /* thread id */
> > + u32 cpu_ctx; /* processor id and interrupt context */
> > +};
>
> A question. Suppose we have a task which does
>
> CPU0
>
> pr_err(...);
>
> preempt_disable();
> pr_err(...);
> preempt_enable();
>
> pr_err(...);
>
> rcu_read_lock();
> pr_info(...);
> rcu_read_unlock();
>
> Should we distinguish those as 3 different contexts?
>
> - normal printk
> - printk under disabled preemption (affects scheduling)
> - printk under RCU read side lock (affects RCU grace periods)
Good question. Well, these contexts could not get detected in
PREEMPT_NONE or PREEMPT_VOLUNTARY. Also I wonder where it would
stop. There are several RCU flavors.
I would not distinguish them unless there is a real demand.
Best Regards,
Petr
