Re: [PATCH net-next v4 1/5] printk: Add execution context (task name/CPU) to printk_info

[John Ogness]

On 2026-01-23, Breno Leitao <leitao@xxxxxxxxxx> wrote:
> diff --git a/kernel/printk/nbcon.c b/kernel/printk/nbcon.c
> index 32fc12e536752..391a58be0c5b3 100644
> --- a/kernel/printk/nbcon.c
> +++ b/kernel/printk/nbcon.c
> @@ -946,6 +946,19 @@ void nbcon_reacquire_nobuf(struct nbcon_write_context *wctxt)
>  }
>  EXPORT_SYMBOL_GPL(nbcon_reacquire_nobuf);
>  
> +#ifdef CONFIG_PRINTK_EXECUTION_CTX
> +static void wctxt_load_execution_ctx(struct nbcon_write_context *wctxt,
> +				     struct printk_message *pmsg)
> +{
> +	wctxt->cpu = pmsg->cpu;
> +	wctxt->pid = pmsg->pid;
> +	memcpy(wctxt->comm, pmsg->comm, TASK_COMM_LEN);

Perhaps using sizeof() instead?

	memcpy(wctxt->comm, pmsg->comm, sizeof(wctxt->comm));

And adding a static assert that the sizes match?

	static_assert(sizeof(wctxt->comm) == sizeof(pmsg->comm));

> +}

[...]

> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 1d765ad242b82..7daaa27705339 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2131,12 +2131,37 @@ static inline void printk_delay(int level)
>  	}
>  }
>  
> +#define CALLER_ID_MASK 0x80000000
> +
>  static inline u32 printk_caller_id(void)
>  {
>  	return in_task() ? task_pid_nr(current) :
> -		0x80000000 + smp_processor_id();
> +		CALLER_ID_MASK + smp_processor_id();
> +}
> +
> +#ifdef CONFIG_PRINTK_EXECUTION_CTX
> +/* Store the opposite info than caller_id. */
> +static u32 printk_caller_id2(void)
> +{
> +	return !in_task() ? task_pid_nr(current) :
> +		CALLER_ID_MASK + smp_processor_id();
>  }
>  
> +static pid_t printk_info_get_pid(const struct printk_info *info)
> +{
> +	u32 caller_id = info->caller_id;
> +	u32 caller_id2 = info->caller_id2;
> +
> +	return caller_id & CALLER_ID_MASK ? caller_id2 : caller_id;
> +}
> +
> +static int printk_info_get_cpu(const struct printk_info *info)
> +{
> +	return ((info->caller_id & CALLER_ID_MASK ?
> +		 info->caller_id : info->caller_id2) & ~CALLER_ID_MASK);
> +}

It is a bit odd that printk_info_get_pid() uses local variables and
printk_info_get_cpu() does not. I could understand if things evolve that
way over time, but it is odd to use the two different styles from the
beginning.

I would prefer the local variable variant. But mostly I would prefer
that they are the same style.

> +#endif
> +
>  /**
>   * printk_parse_prefix - Parse level and control flags.
>   *
> @@ -2213,6 +2238,27 @@ static u16 printk_sprint(char *text, u16 size, int facility,
>  	return text_len;
>  }
>  
> +#ifdef CONFIG_PRINTK_EXECUTION_CTX
> +static void printk_store_execution_ctx(struct printk_info *info)
> +{
> +	info->caller_id2 = printk_caller_id2();
> +	get_task_comm(info->comm, current);
> +}
> +
> +static void pmsg_load_execution_ctx(struct printk_message *pmsg,
> +				    const struct printk_info *info)
> +{
> +	pmsg->cpu = printk_info_get_cpu(info);
> +	pmsg->pid = printk_info_get_pid(info);
> +	memcpy(pmsg->comm, info->comm, TASK_COMM_LEN);

Here I also suggest using sizeof() and static_assert():

	memcpy(pmsg->comm, info->comm, sizeof(pmsg->comm));
	static_assert(sizeof(pmsg->comm) == sizeof(info->comm));

John Ogness