linux-kernel - Re: mainline build failure due to 322458c2bb1a ("drm/ttm: Reduce the number of used allocation orders for TTM pages")

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: <066e88f0-366f-f9fc-e0a8-ba60d7777e28@amd.com>
Date:   Wed, 26 Apr 2023 21:02:57 +0200
From:   Christian König <christian.koenig@....com>
To:     Linus Torvalds <torvalds@...ux-foundation.org>,
        "Sudip Mukherjee (Codethink)" <sudipm.mukherjee@...il.com>,
        Michael Ellerman <mpe@...erman.id.au>
Cc:     Thomas Hellström 
        <thomas.hellstrom@...ux.intel.com>, Huang Rui <ray.huang@....com>,
        David Airlie <airlied@...il.com>,
        Daniel Vetter <daniel@...ll.ch>,
        dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: mainline build failure due to 322458c2bb1a ("drm/ttm: Reduce the
 number of used allocation orders for TTM pages")

Am 26.04.23 um 20:30 schrieb Linus Torvalds:
> On Wed, Apr 26, 2023 at 10:44 AM Sudip Mukherjee (Codethink)
> <sudipm.mukherjee@...il.com> wrote:
>> drivers/gpu/drm/ttm/ttm_pool.c:73:29: error: variably modified 'global_write_combined' at file scope
>>     73 | static struct ttm_pool_type global_write_combined[TTM_DIM_ORDER];
>>        |                             ^~~~~~~~~~~~~~~~~~~~~
> Ugh.

Revert of this is patch already on the way to you.

>
> This is because we have
>
>    #define TTM_DIM_ORDER (__TTM_DIM_ORDER <= MAX_ORDER ?
> __TTM_DIM_ORDER : MAX_ORDER)
>
> which looks perfectly fine as a constant ("pick the smaller of
> MAX_ORDER and __TTM_DIM_ORDER").
>
> But:
>
>    #define __TTM_DIM_ORDER (TTM_MAX_ORDER + 1)
>    #define TTM_MAX_ORDER (PMD_SHIFT - PAGE_SHIFT)
>
> which still _looks_ fine, but  on 64-bit powerpc, we then have
>
>     #define PTE_INDEX_SIZE  __pte_index_size
>
> so that __TTM_DIM_ORDER constant isn't actually a constant at all.
>
> I would suggest that the DRM code just make those fixed-size arrays
> use "MAX_ORDER", because even though it might be a bit bigger than
> necessary, that's still not a very big constant (ie typically it's
> "11", but it could be up to 64).
>
> It's a bit sad how that macro that _looks_ like a constant (and is one
> pretty much everywhere else) isn't actually constant on powerpc, but
> looking around it looks fairly unavoidable.
>
> Maybe the DRM code could try to avoid using things like PMD_SHIFT entirely?

Yeah we already try very hard to do so.

I quite often have to remind people that the CPU architecture doesn't 
affect how many bits the internal address space of a GPU has and they 
should always assume u64 for those instead of unsigned long.

But I have to agree that it's a bit unexpected that a constant 
everywhere else is not constant on this particular architecture.

Christian.


>
>                  Linus