All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michael Ellerman <mpe@ellerman.id.au>
To: Linus Torvalds <torvalds@linux-foundation.org>,
	"Sudip Mukherjee (Codethink)" <sudipm.mukherjee@gmail.com>
Cc: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>,
	linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
	"Huang Rui" <ray.huang@amd.com>,
	"Christian Koenig" <christian.koenig@amd.com>
Subject: Re: mainline build failure due to 322458c2bb1a ("drm/ttm: Reduce the number of used allocation orders for TTM pages")
Date: Thu, 27 Apr 2023 08:49:11 +1000	[thread overview]
Message-ID: <871qk6z2c8.fsf@mail.concordia> (raw)
In-Reply-To: <CAHk-=winmePW-RJdPEE031U=7z92aAv5TAnTU0bR74uEZOMb3Q@mail.gmail.com>

Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Wed, Apr 26, 2023 at 10:44 AM Sudip Mukherjee (Codethink)
> <sudipm.mukherjee@gmail.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.
>
> 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.
..
>
> 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.

Yeah, it allows us to build a single kernel that can choose at runtime
whether it uses the HPT or Radix MMU. The page table geometry is
different between the MMUs because they support a different sized huge
page for THP.

cheers

WARNING: multiple messages have this Message-ID (diff)
From: Michael Ellerman <mpe@ellerman.id.au>
To: Linus Torvalds <torvalds@linux-foundation.org>,
	"Sudip Mukherjee (Codethink)" <sudipm.mukherjee@gmail.com>
Cc: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>,
	"Christian Koenig" <christian.koenig@amd.com>,
	"Huang Rui" <ray.huang@amd.com>,
	"David Airlie" <airlied@gmail.com>,
	"Daniel Vetter" <daniel@ffwll.ch>,
	dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
Subject: Re: mainline build failure due to 322458c2bb1a ("drm/ttm: Reduce the number of used allocation orders for TTM pages")
Date: Thu, 27 Apr 2023 08:49:11 +1000	[thread overview]
Message-ID: <871qk6z2c8.fsf@mail.concordia> (raw)
In-Reply-To: <CAHk-=winmePW-RJdPEE031U=7z92aAv5TAnTU0bR74uEZOMb3Q@mail.gmail.com>

Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Wed, Apr 26, 2023 at 10:44 AM Sudip Mukherjee (Codethink)
> <sudipm.mukherjee@gmail.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.
>
> 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.
..
>
> 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.

Yeah, it allows us to build a single kernel that can choose at runtime
whether it uses the HPT or Radix MMU. The page table geometry is
different between the MMUs because they support a different sized huge
page for THP.

cheers

  parent reply	other threads:[~2023-04-26 22:49 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-26 17:44 mainline build failure due to 322458c2bb1a ("drm/ttm: Reduce the number of used allocation orders for TTM pages") Sudip Mukherjee (Codethink)
2023-04-26 17:44 ` Sudip Mukherjee (Codethink)
2023-04-26 18:30 ` Linus Torvalds
2023-04-26 18:30   ` Linus Torvalds
2023-04-26 19:02   ` Christian König
2023-04-26 19:02     ` Christian König
2023-04-26 22:49   ` Michael Ellerman [this message]
2023-04-26 22:49     ` Michael Ellerman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=871qk6z2c8.fsf@mail.concordia \
    --to=mpe@ellerman.id.au \
    --cc=christian.koenig@amd.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ray.huang@amd.com \
    --cc=sudipm.mukherjee@gmail.com \
    --cc=thomas.hellstrom@linux.intel.com \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.