linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: "Guenter Roeck" <linux@roeck-us.net>,
	"Matthew Auld" <matthew.auld@intel.com>,
	"Arunpravin Paneer Selvam" <Arunpravin.PaneerSelvam@amd.com>,
	"Christian König" <christian.koenig@amd.com>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: Linux 6.8-rc5
Date: Tue, 20 Feb 2024 11:57:23 -0800	[thread overview]
Message-ID: <CAHk-=wj6xj_cGmsQK7g=hSfRZZNo-njC+u_1v3dE8fPZtjCBOg@mail.gmail.com> (raw)
In-Reply-To: <538327ff-8d34-41d5-a9ae-1a334744f5ae@roeck-us.net>

[-- Attachment #1: Type: text/plain, Size: 2334 bytes --]

On Tue, 20 Feb 2024 at 11:09, Guenter Roeck <linux@roeck-us.net> wrote:
>
> Build results:
>         total: 155 pass: 151 fail: 4
> Failed builds:
>         csky:allmodconfig
>         openrisc:allmodconfig
>         parisc:allmodconfig
>         xtensa:allmodconfig
> Qemu test results:
>         total: 549 pass: 547 fail: 2

Grr, I was hoping things would improve, not go backwards.

> ERROR: modpost: "__umoddi3" [drivers/gpu/drm/tests/drm_buddy_test.ko] undefined!
> ERROR: modpost: "__moddi3" [drivers/gpu/drm/tests/drm_buddy_test.ko] undefined!
>
> Commit a64056bb5a32 ("drm/tests/drm_buddy: add alloc_contiguous test"):
>
> +       u64 mm_size, ps = SZ_4K, i, n_pages, total;
> ...
> +       n_pages = mm_size / ps;

WTF? This code isn't lovely, but the build error indicates a complete
lack of compiler optimizations.

As far as I can tell, 'ps' is never modified, so the compiler should
be able to just treat it as a constant.

And 'mm_size' is a constant too in this context. It's a local variable
assigned once, with a compile-time constant value.

So that

        n_pages = mm_size / ps;

should be a constant value too (and that value should be 48).

Now, the __moddi3() is a *bit* more reasonable, because I assume it comes from

                int slot = i % 3;

where 'i' is marked as u64 too. For no good reason (it goes from 0 to
47), but it too does show a certain lack of basic optimizations (but
now it's at least a slightly more *complex* optimization because it
depends on the whole value range propagation).

None of this should be 'u64'. Even if the compiler has been unusually stupid.

The 'total' variable could possibly be considered validly be u64,
although even that is very very questinable.

> This patch breaks the build on all 32-bit systems since it introduces an
> unhandled direct 64-bit divide operation.

It turns out that that commit is buggy for another reason, but it's
hidden by the fact that apparently KUNIT_ASSERT_FALSE_MSG() doesn't
check the format string.

It randomly uses '%d' or '%llu' to print out various variations of
'ps'. All garbage.

Yes, yes, drm_buddy_init() takes u64 arguments. That in itself is a
bit questionable, but whatever. It does *NOT* mean that the variables
that don't need it should then be u64.

Suggested untested patch attached.

                  Linus

[-- Attachment #2: patch.diff --]
[-- Type: text/x-patch, Size: 3295 bytes --]

 drivers/gpu/drm/tests/drm_buddy_test.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/tests/drm_buddy_test.c b/drivers/gpu/drm/tests/drm_buddy_test.c
index fee6bec757d1..3dbfa3078449 100644
--- a/drivers/gpu/drm/tests/drm_buddy_test.c
+++ b/drivers/gpu/drm/tests/drm_buddy_test.c
@@ -21,7 +21,8 @@ static inline u64 get_size(int order, u64 chunk_size)
 
 static void drm_test_buddy_alloc_contiguous(struct kunit *test)
 {
-	u64 mm_size, ps = SZ_4K, i, n_pages, total;
+	const unsigned long ps = SZ_4K, mm_size = 16 * 3 * SZ_4K;
+	unsigned long i, n_pages, total;
 	struct drm_buddy_block *block;
 	struct drm_buddy mm;
 	LIST_HEAD(left);
@@ -29,8 +30,6 @@ static void drm_test_buddy_alloc_contiguous(struct kunit *test)
 	LIST_HEAD(right);
 	LIST_HEAD(allocated);
 
-	mm_size = 16 * 3 * SZ_4K;
-
 	KUNIT_EXPECT_FALSE(test, drm_buddy_init(&mm, mm_size, ps));
 
 	/*
@@ -56,30 +55,30 @@ static void drm_test_buddy_alloc_contiguous(struct kunit *test)
 		KUNIT_ASSERT_FALSE_MSG(test,
 				       drm_buddy_alloc_blocks(&mm, 0, mm_size,
 							      ps, ps, list, 0),
-				       "buddy_alloc hit an error size=%d\n",
+				       "buddy_alloc hit an error size=%lu\n",
 				       ps);
 	} while (++i < n_pages);
 
 	KUNIT_ASSERT_TRUE_MSG(test, drm_buddy_alloc_blocks(&mm, 0, mm_size,
 							   3 * ps, ps, &allocated,
 							   DRM_BUDDY_CONTIGUOUS_ALLOCATION),
-			       "buddy_alloc didn't error size=%d\n", 3 * ps);
+			       "buddy_alloc didn't error size=%lu\n", 3 * ps);
 
 	drm_buddy_free_list(&mm, &middle);
 	KUNIT_ASSERT_TRUE_MSG(test, drm_buddy_alloc_blocks(&mm, 0, mm_size,
 							   3 * ps, ps, &allocated,
 							   DRM_BUDDY_CONTIGUOUS_ALLOCATION),
-			       "buddy_alloc didn't error size=%llu\n", 3 * ps);
+			       "buddy_alloc didn't error size=%lu\n", 3 * ps);
 	KUNIT_ASSERT_TRUE_MSG(test, drm_buddy_alloc_blocks(&mm, 0, mm_size,
 							   2 * ps, ps, &allocated,
 							   DRM_BUDDY_CONTIGUOUS_ALLOCATION),
-			       "buddy_alloc didn't error size=%llu\n", 2 * ps);
+			       "buddy_alloc didn't error size=%lu\n", 2 * ps);
 
 	drm_buddy_free_list(&mm, &right);
 	KUNIT_ASSERT_TRUE_MSG(test, drm_buddy_alloc_blocks(&mm, 0, mm_size,
 							   3 * ps, ps, &allocated,
 							   DRM_BUDDY_CONTIGUOUS_ALLOCATION),
-			       "buddy_alloc didn't error size=%llu\n", 3 * ps);
+			       "buddy_alloc didn't error size=%lu\n", 3 * ps);
 	/*
 	 * At this point we should have enough contiguous space for 2 blocks,
 	 * however they are never buddies (since we freed middle and right) so
@@ -88,13 +87,13 @@ static void drm_test_buddy_alloc_contiguous(struct kunit *test)
 	KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(&mm, 0, mm_size,
 							    2 * ps, ps, &allocated,
 							    DRM_BUDDY_CONTIGUOUS_ALLOCATION),
-			       "buddy_alloc hit an error size=%d\n", 2 * ps);
+			       "buddy_alloc hit an error size=%lu\n", 2 * ps);
 
 	drm_buddy_free_list(&mm, &left);
 	KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(&mm, 0, mm_size,
 							    3 * ps, ps, &allocated,
 							    DRM_BUDDY_CONTIGUOUS_ALLOCATION),
-			       "buddy_alloc hit an error size=%d\n", 3 * ps);
+			       "buddy_alloc hit an error size=%lu\n", 3 * ps);
 
 	total = 0;
 	list_for_each_entry(block, &allocated, link)

  reply	other threads:[~2024-02-20 19:57 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-18 21:13 Linux 6.8-rc5 Linus Torvalds
2024-02-19  8:07 ` Build regressions/improvements in v6.8-rc5 Geert Uytterhoeven
2024-02-19  9:54   ` Geert Uytterhoeven
2024-02-19  8:12 ` Geert Uytterhoeven
2024-02-20 19:08 ` Linux 6.8-rc5 Guenter Roeck
2024-02-20 19:57   ` Linus Torvalds [this message]
2024-02-20 20:16     ` Linus Torvalds
2024-02-20 23:07       ` Shuah Khan
2024-02-20 20:37     ` Linus Torvalds
2024-02-20 20:51     ` Geert Uytterhoeven
2024-02-20 21:48     ` Guenter Roeck
2024-02-20 22:02       ` Linus Torvalds

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='CAHk-=wj6xj_cGmsQK7g=hSfRZZNo-njC+u_1v3dE8fPZtjCBOg@mail.gmail.com' \
    --to=torvalds@linux-foundation.org \
    --cc=Arunpravin.PaneerSelvam@amd.com \
    --cc=christian.koenig@amd.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=matthew.auld@intel.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).