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]
Date:   Fri, 14 Apr 2023 15:47:13 +0200
From:   Hannes Reinecke <hare@...e.de>
To:     Pankaj Raghav <p.raghav@...sung.com>, brauner@...nel.org,
        willy@...radead.org, viro@...iv.linux.org.uk,
        akpm@...ux-foundation.org
Cc:     linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
        mcgrof@...nel.org, gost.dev@...sung.com
Subject: Re: [RFC 0/4] convert create_page_buffers to create_folio_buffers

On 4/14/23 13:08, Pankaj Raghav wrote:
> One of the first kernel panic we hit when we try to increase the
> block size > 4k is inside create_page_buffers()[1]. Even though buffer.c
> function do not support large folios (folios > PAGE_SIZE) at the moment,
> these changes are required when we want to remove that constraint.
> 
> Willy had already mentioned that he wanted to convert create_page_buffers to
> create_folio_buffers but didn't get to it yet, so I decided to take a
> shot.
> 
> No functional changes introduced.
> 
> OI:
> - I don't like the fact that I had to introduce
>    folio_create_empty_buffers() as create_empty_buffers() is used in
>    many parts of the kernel. Should I do a big bang change as a part of
>    this series where we convert create_empty_buffers to take a folio and
>    change the callers to pass a folio instead of a page?
> 
> - I split the series into 4 commits for clarity. I could squash them
>    into one patch if needed.
> 
> [1] https://lore.kernel.org/all/ZBnGc4WbBOlnRUgd@casper.infradead.org/
> 
> Pankaj Raghav (4):
>    fs/buffer: add set_bh_folio helper
>    buffer: add alloc_folio_buffers() helper
>    fs/buffer: add folio_create_empty_buffers helper
>    fs/buffer: convert create_page_buffers to create_folio_buffers
> 
>   fs/buffer.c                 | 131 +++++++++++++++++++++++++++++++++---
>   include/linux/buffer_head.h |   4 ++
>   2 files changed, 125 insertions(+), 10 deletions(-)
> 
Funnily enough, I've been tinkering along the same lines, and ended up 
with pretty similar patches.
I've had to use two additional patches to get my modified 'brd' driver 
off the ground with logical blocksize of 16k:
- mm/filemap: allocate folios according to the blocksize
   (will be sending the patch separately)
- Modify read_folio() to use the correct order:

@@ -2333,13 +2395,15 @@ int block_read_full_folio(struct folio *folio, 
get_block_t *get_block)
         if (IS_ENABLED(CONFIG_FS_VERITY) && IS_VERITY(inode))
                 limit = inode->i_sb->s_maxbytes;

-       VM_BUG_ON_FOLIO(folio_test_large(folio), folio);
-
         head = create_folio_buffers(folio, inode, 0);
         blocksize = head->b_size;
         bbits = block_size_bits(blocksize);

-       iblock = (sector_t)folio->index << (PAGE_SHIFT - bbits);
+       if (WARN_ON(PAGE_SHIFT < bbits)) {
+               iblock = (sector_t)folio->index >> (bbits - PAGE_SHIFT);
+       } else {
+               iblock = (sector_t)folio->index << (PAGE_SHIFT - bbits);
+       }
         lblock = (limit+blocksize-1) >> bbits;
         bh = head;
         nr = 0;


With that (and my modified brd driver) I've been able to set the logical 
blocksize to 16k for brd and have it happily loaded.
Haven't tested the write path yet, though; there's surely quite some 
work to be done.

BTW; I've got another patch replacing 'writepage' with 'write_folio'
(and the corresponding argument update). Is that a direction you want to go?

Cheers,

Hannes

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ