[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZLVH6t25HD+HhCka@smile.fi.intel.com>
Date: Mon, 17 Jul 2023 16:53:46 +0300
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: Alexander Potapenko <glider@...gle.com>
Cc: catalin.marinas@....com, will@...nel.org, pcc@...gle.com,
andreyknvl@...il.com, linux@...musvillemoes.dk,
yury.norov@...il.com, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, eugenis@...gle.com,
syednwaris@...il.com, william.gray@...aro.org
Subject: Re: [PATCH v3 5/5] arm64: mte: add compression support to mteswap.c
On Mon, Jul 17, 2023 at 01:37:08PM +0200, Alexander Potapenko wrote:
> Define the internal mteswap.h interface:
> - _mte_alloc_and_save_tags()
> - _mte_free_saved_tags()
> - _mte_restore_tags()
>
> , that encapsulates saving tags for a struct page (together with memory
> allocation), restoring tags, and deleting the storage allocated for them.
>
> These functions accept opaque pointers, which may point to 128-byte
> tag buffers, as well as smaller buffers containing compressed tags, or
> have compressed tags stored directly in them.
>
> The existing code from mteswap.c operating with uncompressed tags is split
> away into mteswap_nocomp.c, and the newly introduced mteswap_comp.c
> provides compression with the EA0 algorithm. The latter implementation
> is picked if CONFIG_ARM64_MTE_COMP=y.
>
> Soon after booting Android, tag compression saves ~2.5x memory previously
> spent by mteswap.c on tag allocations. With the growing uptime, the
> savings reach 20x and even more.
...
> +#ifndef ARCH_ARM64_MM_MTESWAP_H_
> +#define ARCH_ARM64_MM_MTESWAP_H_
> +#include <linux/mm_types.h>
But you actually don't use that.
struct page;
forward declaration is enough.
> +void *_mte_alloc_and_save_tags(struct page *page);
> +void _mte_free_saved_tags(void *tags);
> +void _mte_restore_tags(void *tags, struct page *page);
> +
> +#endif // ARCH_ARM64_MM_MTESWAP_H_
...
> +void _mte_free_saved_tags(void *storage)
> +{
> + unsigned long handle = xa_to_value(storage);
> + int size;
> +
> + if (!handle)
> + return;
Perhaps
unsigned long handle;
handle = xa_to_value(storage);
if (!handle)
return;
> + size = ea0_storage_size(handle);
> + ea0_release_handle(handle);
> +}
> +void _mte_restore_tags(void *tags, struct page *page)
> +{
As per above.
> + if (try_page_mte_tagging(page)) {
> + if (!ea0_decompress(handle, tags_decomp))
> + return;
> + mte_restore_page_tags(page_address(page), tags_decomp);
> + set_page_mte_tagged(page);
> + }
I think you may drop an indentation level by
if (!try_page_mte_tagging(page))
return;
> +}
...
> +void _mte_restore_tags(void *tags, struct page *page)
> +{
> + if (try_page_mte_tagging(page)) {
> + mte_restore_page_tags(page_address(page), tags);
> + set_page_mte_tagged(page);
> + }
Ditto.
> +}
--
With Best Regards,
Andy Shevchenko
Powered by blists - more mailing lists