On Thu, 17 Jul 2014 21:03:18 +0400 Max Filippov <jcmvbkbc@xxxxxxxxx> wrote:
> From: Leonid Yegoshin <Leonid.Yegoshin@xxxxxxxxxx>
>
> 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, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>