Re: [PATCH 3/3] profile: simplify /proc/profile read layout

[Bradley Morgan]

On 21 October 2026 02:27:36 BST, Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
wrote:
>On Tue, 20 Oct 2026 17:52:03 -0700 Andrew Morton
><akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
>
>> read_profile() is needlessly difficult to understand.  It copies the
>> four-byte sampling-step header one byte at a time, then manufactures
>> a pointer before prof_buffer so a file offset can be used as a
>> counter-array offset.  Its signed byte counter, unsigned long file
>> offset and repeated size calculations further obscure where each
>> read starts and how many bytes it transfers.
>> 
>> Treat the file as two consecutive regions: a four-byte header and
>> the profiling counters.  Use size_t for byte positions and lengths,
>> copy the bounded header in one operation, and address the counters
>> relative to the start of prof_buffer.  Brief comments explain each
>> region.
>> 
>> Keep the word-sized shift and explicit narrowing required by Avi
>> Weiss's "profiling: fix sampling step on 64-bit big-endian systems".
>> Assign the header value separately and explain why the 32-bit ABI
>> requires a narrowing conversion.
>> 
>> Preserve the partial-read and fault semantics established by [1/2],
>> including short copies within either region.  No userspace-visible
>> behavior change is intended.
>
>Me:0, Sashiko:1
>
>revised version, I'll resend in a few days.  I tested 32-bit now.

Good luck if your using AI, they tend to include small regressions, maybe
send them the sashiko prompt in memories so they put a pass through that?
If you have Gemini 4 argon it will be better.


>
>
>From: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
>Subject: profile: simplify /proc/profile read layout
>Date: Tue, 20 Oct 2026 23:58:33 +0000
>
>read_profile() is needlessly difficult to understand.  It copies the
>four-byte sampling-step header one byte at a time, then manufactures
>a pointer before prof_buffer so a file offset can be used as a
>counter-array offset.  Its signed byte counter, unsigned long file
>offset and repeated size calculations further obscure where each
>read starts and how many bytes it transfers.
>
>Treat the file as two consecutive regions: a four-byte header and
>the profiling counters.  Use size_t for byte positions and lengths,
>copy the bounded header in one operation, and address the counters
>relative to the start of prof_buffer.  Retain the full loff_t value
>for EOF and negative-offset checks before narrowing it to size_t.
>Brief comments explain each region.
>
>Keep the word-sized shift and explicit narrowing required by Avi
>Weiss's "profiling: fix sampling step on 64-bit big-endian systems".
>Assign the header value separately and explain why the 32-bit ABI
>requires a narrowing conversion.
>
>Preserve the partial-read and fault semantics established by [1/2],
>including short copies within either region.  The full-width offset
>check also prevents 32-bit pread64() offsets from wrapping into earlier
>profile data.  No other userspace-visible
>behavior change is intended.
>
>Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
>---
>Tested the combined three-patch series on x86-64 and i686 QEMU/KVM
>guests with CONFIG_PROFILING=y and profile=12.
>profile-read-test.c: PASS (0 failures) on both architectures.
>Covered short and unaligned reads, header boundaries, EOF, partial
>user-copy faults, return values, file positions and retry behavior.
>The 64-bit big-endian sampling-step fix was not runtime tested.
>No separate per-patch or unpatched-versus-patched run was reported.
>
> kernel/profile.c |   61 ++++++++++++++++++++++++++++-----------------
> 1 file changed, 39 insertions(+), 22 deletions(-)
>
>--- a/kernel/profile.c~profile-simplify-proc-profile-read-layout
>+++ a/kernel/profile.c
>@@ -153,35 +153,52 @@ void profile_tick(int type)
> static ssize_t
> read_profile(struct file *file, char __user *buf, size_t count, loff_t
> *ppos)
> {
>-	unsigned long p = *ppos;
>-	ssize_t read;
>-	char *pnt;
>-	unsigned int sample_step = (unsigned int)(1UL << prof_shift);
>-	size_t not_copied;
>+	const size_t header_size = sizeof(unsigned int);
>+	const size_t file_size = (prof_len + 1) * sizeof(unsigned int);
>+	unsigned int sample_step;
>+	loff_t offset = *ppos;
>+	size_t pos;
>+	size_t done = 0;
>+	size_t n, not_copied;
> 
>-	if (p >= (prof_len+1)*sizeof(unsigned int))
>+	/* Keep the shift word-sized before narrowing to the 32-bit ABI. */
>+	sample_step = (unsigned int)(1UL << prof_shift);
>+
>+	if (offset < 0)
>+		return -EINVAL;
>+	if (offset >= file_size)
> 		return 0;
>-	if (count > (prof_len+1)*sizeof(unsigned int) - p)
>-		count = (prof_len+1)*sizeof(unsigned int) - p;
>-	read = 0;
>+	pos = offset;
>+	count = min(count, file_size - pos);
> 
>-	while (p < sizeof(unsigned int) && count > 0) {
>-		if (put_user(*((char *)(&sample_step)+p), buf))
>-			goto out;
>-		buf++; p++; count--; read++;
>+	/* Copy the four-byte sampling resolution. */
>+	if (pos < header_size) {
>+		n = min(count, header_size - pos);
>+		not_copied = copy_to_user(buf, (u8 *)&sample_step + pos, n);
>+		done += n - not_copied;
>+		if (not_copied)
>+			goto fault;
>+		buf += n;
>+		pos += n;
>+		count -= n;
> 	}
>+
>+	/* Copy raw counters using offsets relative to the counter array. */
> 	if (count) {
>-		pnt = (char *)prof_buffer + p - sizeof(atomic_t);
>-		not_copied = copy_to_user(buf, pnt, count);
>-		read += count - not_copied;
>+		not_copied = copy_to_user(buf,
>+					(u8 *)prof_buffer + pos - header_size,
>+					count);
>+		done += count - not_copied;
> 		if (not_copied)
>-			goto out;
>+			goto fault;
> 	}
>-	*ppos += read;
>-	return read;
>-out:
>-	*ppos += read;
>-	return read ? read : -EFAULT;
>+	*ppos += done;
>+	return done;
>+fault:
>+	*ppos += done;
>+	if (!done)
>+		return -EFAULT;
>+	return done;
> }
> 
> /* default is to not implement this call */
>_
>

--- Thanks!
"I'm not a very positive person" - Linus torvalds