On Fri, 17 Jan 2025 03:02:40 -0800 Breno Leitao wrote:

quoted

Looks like previously all the data was on the stack, now we have a mix.  

Not sure I followed. The data ({userdata,extradata}_complete) was always
inside nt field, which belongs to target_list.

I mean the buffer we use for formatting. Today it's this:

	static char buf[MAX_PRINT_CHUNK]; /* protected by target_list_lock */
	int header_len, msgbody_len;
	const char *msgbody;

right? I missed that "static" actually so it's not on the stack, 
it's in the .bss section.

quoted

Maybe we can pack all the bits of state into a struct for easier
passing around, but still put it on the stack?  

It depends on what state you need here. We can certainly pass runtime
(aka sysdata in this patchset) data in the stack, but doing the same for
userdata would require extra computation in runtime. In other words, the
userdata_complete and length are calculated at configfs update time
today, and only read during runtime, and there is no connection between
configfs and runtime (write_ext_msg()) except through the stack.


On the other side, if we want to have extradata_complete in the stack, I
still think that userdata will need to be in the stack, and create a
buffer in runtime's frame and copy userdata + sysdata at run time, doing
an extra copy. 

Trying to put this in code, this is what I thought:

	/* Copy to the stack (buf) the userdata string + sysdata */
	static void append_runtime_sysdata(struct netconsole_target *nt, char *buf) {
		if (!(nt->sysdata_fields & CPU_NR))
			return;

		return scnprintf(buf,  MAX_EXTRADATA_ENTRY_LEN * MAX_EXTRADATA_ITEMS,
				  "%s cpu=%u\n", nt->userdata_complete, raw_smp_processor_id());
	}

	/* Move complete string in the stack and send from there */
	static void send_ext_msg_udp(struct netconsole_target *nt, const char *msg,
				     int msg_len) {
		...
	#ifdef CONFIG_NETCONSOLE_DYNAMIC
		struct char buf[MAX_EXTRADATA_ENTRY_LEN * MAX_EXTRADATA_ITEMS];
		extradata_len = append_runtime_sysdata(nt, buf);
	#endif

		send_msg_{no}_fragmentation(nt, msg, buf, extradata_len, release_len)
		...
	}

My thinking was to handle it like the release.
Print it at the send_msg_no_fragmentation() stage directly 
into the static buffer. Does that get hairy coding-wise?