linux-kernel - Re: [RFC PATCH v1 00/57] Boot-time page size selection for arm64

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: <059721e2-5dd8-47da-b7f8-d6423f912216@arm.com>
Date: Mon, 21 Oct 2024 12:47:21 +0100
From: Ryan Roberts <ryan.roberts@....com>
To: Petr Tesarik <ptesarik@...e.com>, Michael Kelley <mhklinux@...look.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
 Anshuman Khandual <anshuman.khandual@....com>,
 Ard Biesheuvel <ardb@...nel.org>, Catalin Marinas <catalin.marinas@....com>,
 David Hildenbrand <david@...hat.com>, Greg Marsden
 <greg.marsden@...cle.com>, Ivan Ivanov <ivan.ivanov@...e.com>,
 Kalesh Singh <kaleshsingh@...gle.com>, Marc Zyngier <maz@...nel.org>,
 Mark Rutland <mark.rutland@....com>, Matthias Brugger <mbrugger@...e.com>,
 Miroslav Benes <mbenes@...e.cz>, Will Deacon <will@...nel.org>,
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
 linux-mm@...ck.org
Subject: Re: [RFC PATCH v1 00/57] Boot-time page size selection for arm64

On 18/10/2024 15:41, Petr Tesarik wrote:
> On Fri, 18 Oct 2024 14:56:00 +0200
> Petr Tesarik <ptesarik@...e.com> wrote:
> 
>> On Thu, 17 Oct 2024 13:32:43 +0100
>> Ryan Roberts <ryan.roberts@....com> wrote:
>>
>>> On 17/10/2024 13:27, Petr Tesarik wrote:  
>>>> On Mon, 14 Oct 2024 11:55:11 +0100
>>>> Ryan Roberts <ryan.roberts@....com> wrote:
>>>>     
>>>>> [...]
>>>>> The series is arranged as follows:
>>>>>
>>>>>   - patch 1:	   Add macros required for converting non-arch code to support
>>>>>   		   boot-time page size selection
>>>>>   - patches 2-36:  Remove PAGE_SIZE compile-time constant assumption from all
>>>>>   		   non-arch code    
>>>>
>>>> I have just tried to recompile the openSUSE kernel with these patches
>>>> applied, and I'm running into this:
>>>>
>>>>   CC      arch/arm64/hyperv/hv_core.o
>>>> In file included from ../arch/arm64/hyperv/hv_core.c:14:0:
>>>> ../include/linux/hyperv.h:158:5: error: variably modified ‘reserved2’ at file scope
>>>>   u8 reserved2[PAGE_SIZE - 68];
>>>>      ^~~~~~~~~
>>>>
>>>> It looks like one more place which needs a patch, right?    
>>>
>>> As mentioned in the cover letter, so far I've only converted enough to get the
>>> defconfig *image* building (i.e. no modules). If you are compiling a different
>>> config or compiling the modules for defconfig, you will likely run into these
>>> types of issues.
>>>
>>> That said, I do have some patches to fix Hyper-V, which Michael Kelley was kind
>>> enough to send me.
>>>
>>> I understand that Suse might be able to help with wider performance testing - if
>>> that's the reason you are trying to compile, you could send me your config and
>>> I'll start working on fixing up other drivers?  
>>
>> You're right, performance testing is my goal.
>>
>> Heh, the openSUSE master config is cranked up to max. ;-) That would be
>> a lot of work, and we don't need all those options for running our test
>> suite. Let me disable the conflicting options instead.
>> [...]
>> I'll see if I can do something about btrfs. Then I can try to boot the
>> kernel...
> 
> FWIW the kernel builds and _boots_ after applying this patch:

Amazing - thanks for doing this!

> 
>  fs/btrfs/compression.h  |    2 +-
>  fs/btrfs/defrag.c       |    2 +-
>  fs/btrfs/extent_io.h    |    2 +-
>  fs/btrfs/scrub.c        |    2 +-
>  include/linux/raid/pq.h |    4 ++--
>  lib/raid6/algos.c       |    2 +-
>  6 files changed, 7 insertions(+), 7 deletions(-)
> 
> --- a/fs/btrfs/compression.h
> +++ b/fs/btrfs/compression.h
> @@ -33,7 +33,7 @@ struct btrfs_bio;
>  /* Maximum length of compressed data stored on disk */
>  #define BTRFS_MAX_COMPRESSED		(SZ_128K)
>  #define BTRFS_MAX_COMPRESSED_PAGES	(BTRFS_MAX_COMPRESSED / PAGE_SIZE)
> -static_assert((BTRFS_MAX_COMPRESSED % PAGE_SIZE) == 0);
> +static_assert((BTRFS_MAX_COMPRESSED % PAGE_SIZE_MAX) == 0);
>  
>  /* Maximum size of data before compression */
>  #define BTRFS_MAX_UNCOMPRESSED		(SZ_128K)
> --- a/fs/btrfs/defrag.c
> +++ b/fs/btrfs/defrag.c
> @@ -1144,7 +1144,7 @@ next:
>  }
>  
>  #define CLUSTER_SIZE	(SZ_256K)
> -static_assert(PAGE_ALIGNED(CLUSTER_SIZE));
> +static_assert(IS_ALIGNED(CLUSTER_SIZE, PAGE_SIZE_MAX));
>  
>  /*
>   * Defrag one contiguous target range.
> --- a/fs/btrfs/extent_io.h
> +++ b/fs/btrfs/extent_io.h
> @@ -89,7 +89,7 @@ enum {
>  int __init extent_buffer_init_cachep(void);
>  void __cold extent_buffer_free_cachep(void);
>  
> -#define INLINE_EXTENT_BUFFER_PAGES     (BTRFS_MAX_METADATA_BLOCKSIZE / PAGE_SIZE)
> +#define INLINE_EXTENT_BUFFER_PAGES     (BTRFS_MAX_METADATA_BLOCKSIZE / PAGE_SIZE_MIN)

While this works, I'm not sure if you would want to have 2 separate macros; 1
for worst-case static allocation, and 1 for dynamic allocation and iterating. I
could imagine if you allocate PAGE_SIZE_MAX pages into the worst case number of
slots that would increase memory. I'm not familiar with the code so don't know
if this is a problem in practice. Certainly what you have done is much simpler
if acceptable.

>  struct extent_buffer {
>  	u64 start;
>  	u32 len;
> --- a/fs/btrfs/scrub.c
> +++ b/fs/btrfs/scrub.c
> @@ -100,7 +100,7 @@ enum scrub_stripe_flags {
>  	SCRUB_STRIPE_FLAG_NO_REPORT,
>  };
>  
> -#define SCRUB_STRIPE_PAGES		(BTRFS_STRIPE_LEN / PAGE_SIZE)
> +#define SCRUB_STRIPE_PAGES		(BTRFS_STRIPE_LEN / PAGE_SIZE_MIN)

Same comment.

Thanks,
Ryan

>  
>  /*
>   * Represent one contiguous range with a length of BTRFS_STRIPE_LEN.
> --- a/include/linux/raid/pq.h
> +++ b/include/linux/raid/pq.h
> @@ -12,7 +12,7 @@
>  
>  #include <linux/blkdev.h>
>  
> -extern const char raid6_empty_zero_page[PAGE_SIZE];
> +extern const char raid6_empty_zero_page[PAGE_SIZE_MAX];
>  
>  #else /* ! __KERNEL__ */
>  /* Used for testing in user space */
> @@ -39,7 +39,7 @@ typedef uint64_t u64;
>  #ifndef PAGE_SHIFT
>  # define PAGE_SHIFT 12
>  #endif
> -extern const char raid6_empty_zero_page[PAGE_SIZE];
> +extern const char raid6_empty_zero_page[PAGE_SIZE_MAX];
>  
>  #define __init
>  #define __exit
> --- a/lib/raid6/algos.c
> +++ b/lib/raid6/algos.c
> @@ -19,7 +19,7 @@
>  #include <linux/module.h>
>  #include <linux/gfp.h>
>  /* In .bss so it's zeroed */
> -const char raid6_empty_zero_page[PAGE_SIZE] __attribute__((aligned(256)));
> +const char raid6_empty_zero_page[PAGE_SIZE_MAX] __attribute__((aligned(256)));
>  EXPORT_SYMBOL(raid6_empty_zero_page);
>  #endif
>  
> 
> Petr T