mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Petr Mladek <pmladek@suse.com>, Dave Airlie <airlied@gmail.com>,
	Arnd Bergmann <arnd@arndb.de>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: arm32 build warnings in workqueue.c
Date: Thu, 3 Aug 2023 12:23:46 -1000	[thread overview]
Message-ID: <ZMwo8rZrRilARZBr@slm.duckdns.org> (raw)
In-Reply-To: <CAHk-=wgGmV6x1brPoROABozj99hrKo031qq8F8K9Mjx2FC6nfw@mail.gmail.com>

Hello, Linus.

On Mon, Jul 31, 2023 at 12:22:06PM -0700, Linus Torvalds wrote:
> We do have that
> 
>    scripts/gdb/linux/constants.py.in
> 
> thing. Which seems to be the logical place do deal with this all.
> 
> That's where other - and arguably much more fundamental - kernel
> #define's are dealt with.
> 
> Now, looking at the particular constants that are listed, I get the
> feeling that the people who have done that script may be mostly
> interested in filesystems, but I don't see why it wouldn't be
> appropriate for the workstruct stuff too...

It can be used but there are some drawbacks:

* It requires maintaining external code, which usually doesn't scale that
  well, especially over extended period of time.

* It requires pre-processing header files and then evaluating the resulting
  C expressions. It's not a showstopper and some tools can already do so
  but, for example, to use this with drgn, we'd have to generate this into
  python expressions using something and then source it. Maybe we can
  prepackage the pre-processed results but things do get more complicated
  and likely more error-prone.

* This likely will cover some of the constants but there are also many which
  aren't readily visible in public header files. It is possible to try to
  preprocess .c files too but then now each machine has to have full source
  deployed and so on.

So, not that it's impossible but if you contrast this to having the
constants available in the debug info:

* There's only one source of truth. No external maintenance is needed.

* The tools already rely heavily on having debug info available for type
  information and the symbols. As needing debug info is pretty common, most
  distros and prod environments provide ready access to them, whether kernel
  or applications.
  
* Having constants in debug info doesn't add any complexity or point of
  possible mistakes. As long as the binary matches the debug info, which is
  a property that tools and environments watch and maintain closely and easy
  to notice when broken, you know that you're using the same numers as the
  running kernel, which is a huge relief.

The downside of using enums is that C's enum is not great in terms of type
predictability. I personally would much prefer specifying the types
explicitly, but, as long as the rules are stable, I think it's something we
can live with. While a bit annoying, the benefits of using them outweighs
the annoyances in general.

It's just so silly that constants of all the stuff we need for monitoring
and debugging are the one thing that we can't provide through debug info and
requires all the special, potentially fragile, workarounds.

Thanks.

-- 
tejun

      reply	other threads:[~2023-08-03 22:23 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-23  3:57 Dave Airlie
2023-06-23 18:24 ` Linus Torvalds
2023-06-23 18:52   ` Linus Torvalds
2023-06-23 19:16     ` Nick Desaulniers
2023-06-23 19:33       ` Linus Torvalds
2023-06-23 19:21     ` Arnd Bergmann
2023-06-23 19:42       ` Linus Torvalds
2023-06-23 19:47   ` Tejun Heo
2023-06-23 22:15     ` Linus Torvalds
2023-06-23 22:22       ` Linus Torvalds
2023-06-24  1:23       ` Tejun Heo
2023-07-31  8:20     ` Petr Mladek
2023-07-31 18:40       ` Linus Torvalds
2023-07-31 19:14         ` Tejun Heo
2023-07-31 19:22           ` Linus Torvalds
2023-08-03 22:23             ` Tejun Heo [this message]

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=ZMwo8rZrRilARZBr@slm.duckdns.org \
    --to=tj@kernel.org \
    --cc=airlied@gmail.com \
    --cc=arnd@arndb.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pmladek@suse.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®