* Toshi Kani <[email protected]> wrote:
> Data corruption issues were observed in tests which initiated
> a system crash/reset while accessing BTT devices. This problem
> is reproducible.
>
> The BTT driver calls pmem_rw_bytes() to update data in pmem
> devices. This interface calls __copy_user_nocache(), which
> uses non-temporal stores so that the stores to pmem are
> persistent.
>
> __copy_user_nocache() uses non-temporal stores when a request
> size is 8 bytes or larger (and is aligned by 8 bytes). The
> BTT driver updates the BTT map table, which entry size is
> 4 bytes. Therefore, updates to the map table entries remain
> cached, and are not written to pmem after a crash.
>
> Change __copy_user_nocache() to use non-temporal store when
> a request size is 4 bytes. The change extends the current
> byte-copy path for a less-than-8-bytes request, and does not
> add any overhead to the regular path.
>
> Also add comments to the code, and clarify the cases that
> lead cache copy.
>
> Reported-and-tested-by: Micah Parrish <[email protected]>
> Reported-and-tested-by: Brian Boylston <[email protected]>
> Signed-off-by: Toshi Kani <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: H. Peter Anvin <[email protected]>
> Cc: Borislav Petkov <[email protected]>
> Cc: Dan Williams <[email protected]>
> Cc: Ross Zwisler <[email protected]>
> Cc: Vishal Verma <[email protected]>
> ---
> v2:
> - Add comments (Ingo Molnar).
> - Make this patch as an individual patch since v2 debug changes
> will not depend on this patch.
> ---
> arch/x86/lib/copy_user_64.S | 74
> +++++++++++++++++++++++++++++++++++--------
> 1 file changed, 61 insertions(+), 13 deletions(-)
>
> diff --git a/arch/x86/lib/copy_user_64.S b/arch/x86/lib/copy_user_64.S
> index 982ce34..1641327 100644
> --- a/arch/x86/lib/copy_user_64.S
> +++ b/arch/x86/lib/copy_user_64.S
> @@ -232,17 +232,30 @@ ENDPROC(copy_user_enhanced_fast_string)
>
> /*
> * copy_user_nocache - Uncached memory copy with exception handling
> - * This will force destination/source out of cache for more performance.
> + * This will force destination out of cache for more performance.
> + *
> + * Note: Cached memory copy is used when destination or size is not
> + * naturally aligned. That is:
> + * - Require 8-byte alignment when size is 8 bytes or larger.
> + * - Require 4-byte alignment when size is 4 bytes.
> */
> ENTRY(__copy_user_nocache)
> ASM_STAC
> +
> + /* If size is less than 8 bytes, goto 4-byte copy */
> cmpl $8,%edx
> - jb 20f /* less then 8 bytes, go to byte copy loop */
> + jb 20f
So another pet peeve of mine is to not use numeric labels, please use
descriptively named labels instead.
It would also be nice to split the patch into two parts: first one does all
these
readability improvements, the second one adds the nocache enhancement/fix.
Thanks,
Ingo
