Re: [PATCH v6 5/6] Optionally flush L1D on context switch

[Thomas Gleixner]

Balbir Singh <sblbir@xxxxxxxxxx> 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.