linux-kernel - Re: [PATCH v2] mm/highmem: make kmap cache coloring aware

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20140723141721.d6a58555f124a7024d010067@linux-foundation.org>
Date:	Wed, 23 Jul 2014 14:17:21 -0700
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Max Filippov <jcmvbkbc@...il.com>
Cc:	linux-mm@...ck.org, linux-arch@...r.kernel.org,
	linux-mips@...ux-mips.org, linux-xtensa@...ux-xtensa.org,
	linux-kernel@...r.kernel.org,
	Leonid Yegoshin <Leonid.Yegoshin@...tec.com>
Subject: Re: [PATCH v2] mm/highmem: make kmap cache coloring aware

On Thu, 17 Jul 2014 21:03:18 +0400 Max Filippov <jcmvbkbc@...il.com> wrote:

> From: Leonid Yegoshin <Leonid.Yegoshin@...tec.com>
> 
> Provide hooks that allow architectures with aliasing cache to align
> mapping address of high pages according to their color. Such architectures
> may enforce similar coloring of low- and high-memory page mappings and
> reuse existing cache management functions to support highmem.
> 
> ...
>
> --- a/mm/highmem.c
> +++ b/mm/highmem.c
> @@ -44,6 +44,14 @@ DEFINE_PER_CPU(int, __kmap_atomic_idx);
>   */
>  #ifdef CONFIG_HIGHMEM
>  
> +#ifndef ARCH_PKMAP_COLORING
> +#define set_pkmap_color(pg, cl)		/* */
> +#define get_last_pkmap_nr(p, cl)	(p)
> +#define get_next_pkmap_nr(p, cl)	(((p) + 1) & LAST_PKMAP_MASK)
> +#define is_no_more_pkmaps(p, cl)	(!(p))
> +#define get_next_pkmap_counter(c, cl)	((c) - 1)
> +#endif

This is the old-school way of doing things.  The new Linus-approved way is

#ifndef set_pkmap_color
#define set_pkmap_color ...
#define get_last_pkmap_nr ...
#endif

so we don't need to add yet another symbol and to avoid typos, etc.

Secondly, please identify which per-arch header file is responsible for
defining these symbols.  Document that here and make sure that
mm/highmem.c is directly including that file.  Otherwise we end up with
different architectures using different header files and it's all a big
mess.

Thirdly, macros are nasty things.  It would be nicer to do

#ifndef set_pkmap_color
static inline void set_pkmap_color(...)
{
	...
}
#define set_pkmap_color set_pkmap_color

...

#endif

Fourthly, please document these proposed interfaces with code comments.

Fifthly, it would be very useful to publish the performance testing
results for at least one architecture so that we can determine the
patchset's desirability.  And perhaps to motivate other architectures
to implement this.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/