On Mon, Aug 23, 2021 at 05:00:14PM +0100, James Clark wrote:
[...]
I couldn't see how it can ever return -1, it seems like it would loop forever until it reads the correct value.
I use this chunk comment to address the function compat_auxtrace_mmap__write_tail():
+int __weak compat_auxtrace_mmap__write_tail(struct auxtrace_mmap *mm, u64 tail) +{
- struct perf_event_mmap_page *pc = mm->userpg;
- u64 mask = (u64)(UINT32_MAX) << 32;
- if (tail & mask)
return -1;- /* Ensure all reads are done before we write the tail out */
- smp_mb();
- WRITE_ONCE(pc->aux_tail, tail);
- return 0;
+}
Please let me know if this is okay or not? Otherwise, if you think the format can cause confusion, I'd like to split the comments into two sections, one section for reading AUX head and another is for writing AUX tail.
I see what you mean now, I think keeping it in one section is fine, I would just replace "When update the AUX tail and detects" with "When compat_auxtrace_mmap__write_tail() detects". If the function name is there then it's clear that it's not the return value of compat_auxtrace_mmap__read_head()
Sure, will update and send out the new patch.
Thanks for suggestion. Leo