Messages in this thread | | | From | Thomas Gleixner <> | Subject | Re: [PATCH v6 5/6] Optionally flush L1D on context switch | Date | Wed, 13 May 2020 17:04:55 +0200 |
| |
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.
| |