Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 14 Apr 2023 18:01:16 -0700
From:   Luis Chamberlain <mcgrof@kernel.org>
To:     Hannes Reinecke <hare@suse.de>
Cc:     Pankaj Raghav <p.raghav@samsung.com>, brauner@kernel.org,
        willy@infradead.org, viro@zeniv.linux.org.uk,
        akpm@linux-foundation.org, linux-fsdevel@vger.kernel.org,
        linux-kernel@vger.kernel.org, gost.dev@samsung.com
Subject: Re: [RFC 0/4] convert create_page_buffers to create_folio_buffers
Message-ID: <ZDn3XPMA024t+C1x@bombadil.infradead.org>
References: <CGME20230414110825eucas1p1ed4d16627889ef8542dfa31b1183063d@eucas1p1.samsung.com>
 <20230414110821.21548-1-p.raghav@samsung.com>
 <1e68a118-d177-a218-5139-c8f13793dbbf@suse.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1e68a118-d177-a218-5139-c8f13793dbbf@suse.de>
Sender: Luis Chamberlain <mcgrof@infradead.org>
Precedence: bulk

On Fri, Apr 14, 2023 at 03:47:13PM +0200, Hannes Reinecke wrote:
> 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.

> 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;

At a quick glance I think both approaches (unless Hannes already did it)
seem to just miss that pesky static *arr[MAX_BUF_PER_PAGE], and so I
think we need to:

a) dynamically allocate those now
b) do a cursory review of the users of that and prepare them
   to grok buffer heads which are blocksize based rather than
   PAGE_SIZE based. So we just try to kill MAX_BUF_PER_PAGE.

Without a) I think buffers after PAGE_SIZE won't get submit_bh() or lock for
bs > PAGE_SIZE right now.

Luis