All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Danilo Krummrich" <dakr@kernel.org>
To: "Gary Guo" <gary@garyguo.net>
Cc: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>,
	Alexandre Courbot <acourbot@nvidia.com>,
	Joel Fernandes <joelagnelf@nvidia.com>,
	nouveau@lists.freedesktop.org, rust-for-linux@vger.kernel.org,
	Miguel Ojeda <ojeda@kernel.org>,
	Alice Ryhl <aliceryhl@google.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>
Subject: Re: [PATCH 2/2] gpu: nova-core: add missing newlines to several print strings
Date: Thu, 08 Jan 2026 14:36:21 +0100	[thread overview]
Message-ID: <DFJ8XRR2T22T.3R4FQHF50B4P4@kernel.org> (raw)
In-Reply-To: <20260108125523.5c7810ae.gary@garyguo.net>

(Cc: Greg, Rafael)

On Thu Jan 8, 2026 at 1:55 PM CET, Gary Guo wrote:
> In very early days (before RfL is upstreamed), I had a prototype print
> macro that is designed like this:
>
> 	info!("foo"); // pr_info("foo\n");
> 	info!(target: dev, "foo"); // dev_info(dev, "foo\n");
> 	info!(target: dev, once, "foo"); // dev_info_once(dev, "foo\n");
> 	info!(target: dev, ratelimited, "foo"); // dev_info_ratelimited(dev, "foo\n");
>
> There's a trait that is implemented for anything that can be used as a
> printing target.
>
> I still think this is superior than just having our macro mimicking the C
> side (and as a result, having a lot of macros rather than just one for
> each level).

Why do you think this syntax is superior?

One disadvantage for maintainers and reviewers would be that it is less
convinient to grep for pr_*() vs dev_*(), which is something that people
regularly get wrong. I.e. it regularly happens that people use pr_*() where they
actually print in the context of a specific device.

> I think with Rust printing machinary, `pr_cont` is rarely useful, instead
> of calling `pr_info` followed by multiple `pr_cont`, you can just have a
> custom `Display` implementation and print it in one go. This is also free
> from racing against another print and have your lines broken into two
> parts.

This I fully agree with.

> I would be much in favour of vouching deleteing `pr_cont` entirely from
> Rust codebase and always have newlines automatically added.

I don't think it is a good idea to always add newlines. It might be fine if you
only do C code or only do Rust code, but if you are switching back and forth, it
is a horrible inconsistency for muscle memory.

I'm pretty sure that this would turn into a generator for trivial fixup patches
either removing or adding the trailing '\n'. :)

- Danilo

WARNING: multiple messages have this Message-ID (diff)
From: "Danilo Krummrich" <dakr@kernel.org>
To: "Gary Guo" <gary@garyguo.net>
Cc: "Miguel Ojeda" <miguel.ojeda.sandonis@gmail.com>,
	"John Hubbard" <jhubbard@nvidia.com>,
	"Timur Tabi" <ttabi@nvidia.com>,
	"Alexandre Courbot" <acourbot@nvidia.com>,
	"Joel Fernandes" <joelagnelf@nvidia.com>,
	<nouveau@lists.freedesktop.org>, <rust-for-linux@vger.kernel.org>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>
Subject: Re: [PATCH 2/2] gpu: nova-core: add missing newlines to several print strings
Date: Thu, 08 Jan 2026 14:36:21 +0100	[thread overview]
Message-ID: <DFJ8XRR2T22T.3R4FQHF50B4P4@kernel.org> (raw)
In-Reply-To: <20260108125523.5c7810ae.gary@garyguo.net>

(Cc: Greg, Rafael)

On Thu Jan 8, 2026 at 1:55 PM CET, Gary Guo wrote:
> In very early days (before RfL is upstreamed), I had a prototype print
> macro that is designed like this:
>
> 	info!("foo"); // pr_info("foo\n");
> 	info!(target: dev, "foo"); // dev_info(dev, "foo\n");
> 	info!(target: dev, once, "foo"); // dev_info_once(dev, "foo\n");
> 	info!(target: dev, ratelimited, "foo"); // dev_info_ratelimited(dev, "foo\n");
>
> There's a trait that is implemented for anything that can be used as a
> printing target.
>
> I still think this is superior than just having our macro mimicking the C
> side (and as a result, having a lot of macros rather than just one for
> each level).

Why do you think this syntax is superior?

One disadvantage for maintainers and reviewers would be that it is less
convinient to grep for pr_*() vs dev_*(), which is something that people
regularly get wrong. I.e. it regularly happens that people use pr_*() where they
actually print in the context of a specific device.

> I think with Rust printing machinary, `pr_cont` is rarely useful, instead
> of calling `pr_info` followed by multiple `pr_cont`, you can just have a
> custom `Display` implementation and print it in one go. This is also free
> from racing against another print and have your lines broken into two
> parts.

This I fully agree with.

> I would be much in favour of vouching deleteing `pr_cont` entirely from
> Rust codebase and always have newlines automatically added.

I don't think it is a good idea to always add newlines. It might be fine if you
only do C code or only do Rust code, but if you are switching back and forth, it
is a horrible inconsistency for muscle memory.

I'm pretty sure that this would turn into a generator for trivial fixup patches
either removing or adding the trailing '\n'. :)

- Danilo

  reply	other threads:[~2026-01-08 13:36 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-07  1:24 [PATCH 1/2] gpu: nova-core: check for overflow to DMATRFBASE1 Timur Tabi
2026-01-07  1:24 ` [PATCH 2/2] gpu: nova-core: add missing newlines to several print strings Timur Tabi
2026-01-07  2:10   ` John Hubbard
2026-01-07  2:21     ` Timur Tabi
2026-01-07  2:39       ` John Hubbard
2026-01-07 10:22         ` Miguel Ojeda
2026-01-07 10:22           ` Miguel Ojeda
2026-01-07  9:54     ` Miguel Ojeda
2026-01-07  9:54       ` Miguel Ojeda
2026-01-08 12:55       ` Gary Guo
2026-01-08 12:55         ` Gary Guo
2026-01-08 13:36         ` Danilo Krummrich [this message]
2026-01-08 13:36           ` Danilo Krummrich
2026-01-08 13:42           ` Alice Ryhl
2026-01-08 13:42             ` Alice Ryhl
2026-01-08 14:13           ` Gary Guo
2026-01-08 14:13             ` Gary Guo
2026-01-07  1:56 ` [PATCH 1/2] gpu: nova-core: check for overflow to DMATRFBASE1 John Hubbard
2026-01-07  2:04   ` Timur Tabi
2026-01-07  3:55 ` Joel Fernandes
2026-01-07  3:55   ` Joel Fernandes

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=DFJ8XRR2T22T.3R4FQHF50B4P4@kernel.org \
    --to=dakr@kernel.org \
    --cc=acourbot@nvidia.com \
    --cc=aliceryhl@google.com \
    --cc=gary@garyguo.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=joelagnelf@nvidia.com \
    --cc=miguel.ojeda.sandonis@gmail.com \
    --cc=nouveau@lists.freedesktop.org \
    --cc=ojeda@kernel.org \
    --cc=rafael@kernel.org \
    --cc=rust-for-linux@vger.kernel.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.