Re: [PATCH 08/19] LoongArch: Add memory management

From: Arnd Bergmann <arnd@kernel.org>
To: Huacai Chen <chenhuacai@loongson.cn>
Cc: Andy Lutomirski <luto@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Peter Zijlstra <peterz@infradead.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	David Airlie <airlied@linux.ie>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	linux-arch <linux-arch@vger.kernel.org>,
	Xuefeng Li <lixuefeng@loongson.cn>,
	Huacai Chen <chenhuacai@gmail.com>,
	Jiaxun Yang <jiaxun.yang@flygoat.com>
Subject: Re: [PATCH 08/19] LoongArch: Add memory management
Date: Tue, 6 Jul 2021 12:57:42 +0200	[thread overview]
Message-ID: <CAK8P3a0EMO9AD157n6VtRR0VCj5tASgdQ5BHEEaX9Rn7LtG=WQ@mail.gmail.com> (raw)
In-Reply-To: <CAK8P3a2Hr7TbuO6LBVC5p5AghWc3j-NCSi8rwd3aq5o8g=Jk1w@mail.gmail.com>

On Tue, Jul 6, 2021 at 6:18 AM Huacai Chen <chenhuacai@loongson.cn> wrote:

> diff --git a/arch/loongarch/include/asm/kmalloc.h b/arch/loongarch/include/asm/kmalloc.h
> new file mode 100644
> index 000000000000..b318c41520d8
> --- /dev/null
> +++ b/arch/loongarch/include/asm/kmalloc.h
> @@ -0,0 +1,10 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2020-2021 Loongson Technology Corporation Limited
> + */
> +#ifndef __ASM_KMALLOC_H
> +#define __ASM_KMALLOC_H
> +
> +#define ARCH_DMA_MINALIGN L1_CACHE_BYTES
> +
> +#endif /* __ASM_KMALLOC_H */

You wrote elsewhere that DMA is cache-coherent, so this should not
be needed at all.

> diff --git a/arch/loongarch/include/asm/shmparam.h b/arch/loongarch/include/asm/shmparam.h
> new file mode 100644
> index 000000000000..f726ac537710
> --- /dev/null
> +++ b/arch/loongarch/include/asm/shmparam.h
> @@ -0,0 +1,12 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2020-2021 Loongson Technology Corporation Limited
> + */
> +#ifndef _ASM_SHMPARAM_H
> +#define _ASM_SHMPARAM_H
> +
> +#define __ARCH_FORCE_SHMLBA    1
> +
> +#define        SHMLBA  (4 * PAGE_SIZE)          /* attach addr a multiple of this */
> +
> +#endif /* _ASM_SHMPARAM_H */

I think this needs to be defined in a way that is independent of the configured
page size to minimize the differences between kernel configuration visible to
user space.

Maybe make it always 64KB?

> diff --git a/arch/loongarch/include/asm/sparsemem.h b/arch/loongarch/include/asm/sparsemem.h
> new file mode 100644
> index 000000000000..9b57dc69f523
> --- /dev/null
> +++ b/arch/loongarch/include/asm/sparsemem.h
> @@ -0,0 +1,21 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _LOONGARCH_SPARSEMEM_H
> +#define _LOONGARCH_SPARSEMEM_H
> +
> +#ifdef CONFIG_SPARSEMEM
> +
> +/*
> + * SECTION_SIZE_BITS           2^N: how big each section will be
> + * MAX_PHYSMEM_BITS            2^N: how much memory we can have in that space
> + */
> +#define SECTION_SIZE_BITS      29
> +#define MAX_PHYSMEM_BITS       48

Maybe add a comment to explain how you got to '29'?

> +
> +#ifdef CONFIG_PAGE_SIZE_4KB
> +#define PAGE_SHIFT      12
> +#endif
> +#ifdef CONFIG_PAGE_SIZE_16KB
> +#define PAGE_SHIFT      14
> +#endif
> +#ifdef CONFIG_PAGE_SIZE_64KB
> +#define PAGE_SHIFT      16
> +#endif

Shouldn't these be defined in some header?

        Arnd