lkml.org 
[lkml]   [2020]   [May]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH v6 5/6] Optionally flush L1D on context switch
Date
Balbir Singh <sblbir@amazon.com> writes:
>
> + if (prev_mm & LAST_USER_MM_L1D_FLUSH)
> + arch_l1d_flush(0); /* Just flush, don't populate the TLB */

Bah. I fundamentally hate tail comments. They are just disturbing the
reading flow. Aside of that, this states the WHAT but not the WHY. And
if you add that explanation then you need more than 20 characters and
end up with

if (prev_mm & LAST_USER_MM_L1D_FLUSH) {
/*
* Proper comment explaining why this is flushing
* without prepopulating the TLB.
*/
arch_l1d_flush(0);
}

anyway. And even for a short comment which fits after the function call
it's way better to have:

if (prev_mm & LAST_USER_MM_L1D_FLUSH) {
/* Short explanation */
arch_l1d_flush(0);
}

Hmm?

> + /*
> + * Leave last_user_mm_spec at LAST_USER_MM_IBPB, we don't
> + * want to set LAST_USER_MM_L1D_FLUSH and force a flush before
> + * we've allocated the flush pages.

Ah here is the comment. I still like the explicit define for the (re)
init.

\
 
 \ /
  Last update: 2020-05-13 17:05    [W:0.219 / U:0.724 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site