Re: [PATCH] btrfs: Disable BTRFS on platforms having 256K pages
> On Jun 10, 2021, at 1:23 AM, Christophe Leroy > wrote: > > With a config having PAGE_SIZE set to 256K, BTRFS build fails > with the following message > > include/linux/compiler_types.h:326:38: error: call to > '__compiletime_assert_791' declared with attribute error: BUILD_BUG_ON > failed: (BTRFS_MAX_COMPRESSED % PAGE_SIZE) != 0 > > BTRFS_MAX_COMPRESSED being 128K, BTRFS cannot support platforms with > 256K pages at the time being. > > There are two platforms that can select 256K pages: > - hexagon > - powerpc > > Disable BTRFS when 256K page size is selected. > We’ll have other subpage blocksize concerns with 256K pages, but this BTRFS_MAX_COMPRESSED #define is arbitrary. It’s just trying to have an upper bound on the amount of memory we’ll need to uncompress a single page’s worth of random reads. We could change it to max(PAGE_SIZE, 128K) or just bump to 256K. -chris
Re: [PATCH] btrfs: Disable BTRFS on platforms having 256K pages
> On Jun 10, 2021, at 12:20 PM, David Sterba wrote: > > On Thu, Jun 10, 2021 at 04:50:09PM +0200, Christophe Leroy wrote: >> >> >> Le 10/06/2021 à 15:54, Chris Mason a écrit : >>> >>>> On Jun 10, 2021, at 1:23 AM, Christophe Leroy >>>> wrote: >>>> >>>> With a config having PAGE_SIZE set to 256K, BTRFS build fails >>>> with the following message >>>> >>>> include/linux/compiler_types.h:326:38: error: call to >>>> '__compiletime_assert_791' declared with attribute error: BUILD_BUG_ON >>>> failed: (BTRFS_MAX_COMPRESSED % PAGE_SIZE) != 0 >>>> >>>> BTRFS_MAX_COMPRESSED being 128K, BTRFS cannot support platforms with >>>> 256K pages at the time being. >>>> >>>> There are two platforms that can select 256K pages: >>>> - hexagon >>>> - powerpc >>>> >>>> Disable BTRFS when 256K page size is selected. >>>> >>> >>> We’ll have other subpage blocksize concerns with 256K pages, but this >>> BTRFS_MAX_COMPRESSED #define is arbitrary. It’s just trying to have an >>> upper bound on the amount of memory we’ll need to uncompress a single >>> page’s worth of random reads. >>> >>> We could change it to max(PAGE_SIZE, 128K) or just bump to 256K. >>> >> >> But if 256K is problematic in other ways, is it worth bumping >> BTRFS_MAX_COMPRESSED to 256K ? >> >> David, in below mail, said that 256K support would require deaper changes. >> So disabling BTRFS >> support seems the easiest solution for the time being, at least for Stable >> (I forgot the Fixes: tag >> and the CC: to stable). >> >> On powerpc, 256k pages is a corner case, it requires customised binutils, so >> I don't think disabling >> BTRFS is a issue there. For hexagon I don't know. > > That it blew up due to the max compressed size is a coincidence. We > could have explicit BUILD_BUG_ONs for page size or other constraints > derived from the page size like INLINE_EXTENT_BUFFER_PAGES. > Right, the constraint is bigger and more complex than BTRFS_MAX_COMPRESSED. > And there's no such thing like "just bump BTRFS_MAX_COMPRESSED to 256K". > The constant is part of on-disk format for lzo and otherwise changing it > would impact performance so this would need proper evaluation. Sorry, how is it baked into LZO? It definitely will have performance implications, I agree there. -chris
Re: [PATCH] btrfs: Disable BTRFS on platforms having 256K pages
> On Jun 11, 2021, at 9:21 AM, David Sterba wrote:
>
> On Fri, Jun 11, 2021 at 12:58:58PM +0000, Chris Mason wrote:
>>> On Jun 10, 2021, at 12:20 PM, David Sterba wrote:
>>> On Thu, Jun 10, 2021 at 04:50:09PM +0200, Christophe Leroy wrote:
>>>> Le 10/06/2021 à 15:54, Chris Mason a écrit :
>>>>>> On Jun 10, 2021, at 1:23 AM, Christophe Leroy
>>>>>> wrote:
>>> And there's no such thing like "just bump BTRFS_MAX_COMPRESSED to 256K".
>>> The constant is part of on-disk format for lzo and otherwise changing it
>>> would impact performance so this would need proper evaluation.
>>
>> Sorry, how is it baked into LZO? It definitely will have performance
>> implications, I agree there.
>
> lzo_decompress_bio:
>
> 309 /*
> 310 * Compressed data header check.
> 311 *
> 312 * The real compressed size can't exceed the maximum extent
> length, and
> 313 * all pages should be used (whole unused page with just the
> segment
> 314 * header is not possible). If this happens it means the
> compressed
> 315 * extent is corrupted.
> 316 */
> 317 if (tot_len > min_t(size_t, BTRFS_MAX_COMPRESSED, srclen) ||
> 318 tot_len < srclen - PAGE_SIZE) {
> 319 ret = -EUCLEAN;
> 320 goto done;
> 321 }
Ah I see, so going back to an old LZO kernel will get upset. Ok, fair enough.
So if we want to bump this for other reasons, we’ll need to make an LZO max
size to maintain compatibility.
-chris
