On (20/07/21 08:40), Linus Torvalds wrote: > On Tue, Jul 21, 2020 at 7:42 AM Sergey Senozhatsky > <sergey.senozhat...@gmail.com> wrote: > > > > OK, so basically, extending printk_caller_id() so that for IRQ/NMI > > we will have more info than just "0x80000000 + raw_smp_processor_id()". > > I think it's really preempt_count() that we want to have there. > > That has the softirq/hardirq/NMI information in it. > > So the "context" identifier of an incomplete write would be { task, > cpu, preempt_count() } of the writer. If a new KERN_CONT writer comes > in, it would have to match in that context to actually combine. > > You can probably play "hide the bits" tricks to not *ac tually* use > three words for it. The task structure in particular tends to be very > aligned, you could hide bits in the low bits there. The CPU number > would fit in there, for example.
The purpose of callerid has changed. We used to keep it _only_ in struct printk_log and never used it for anything but if (cont.owner == current && (lflags & LOG_CONT)) ... so it was easy to use task_struct pointers - we only cared if cont.owner matches current and never dereferenced the cont.owner pointer. But we do show caller id to users now (CONFIG_PRINTK_CALLER), so it makes sense to keep there something more or less human readable, it helps syzkaller/fuzzer people. That's why we keep PID in [30,0] bits of callerid if the printk was called in_task(), and CPU_ID otherwise. Replacing easy to understand PID/CPU_ID with some magic hex, e.g. hash(current) >> shift or hash(smp_processor_id()) >> shift, will make things less convenient, so I think it is reasonable to preserve the existing scheme. I like what John suggests: - if printk was called from in_task(), then we keep PID in [30,0] bits - otherwise we keep smp_processor_id() in [27,0] bits and in the upper bits we keep the IRQ/NMI/etc flags. It may make sense to "promote" u32 callerid to u64, just in case if someone sometime in the future will have boxes with more than several billion PIDs. -ss