mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Zhi Wang <zhiw@nvidia.com>
To: "Thomas Weißschuh" <linux@weissschuh.net>
Cc: "Alexandre Courbot" <acourbot@nvidia.com>,
	"Jason Baron" <jbaron@akamai.com>,
	"Jim Cromie" <jim.cromie@gmail.com>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Boqun Feng" <boqun@kernel.org>, "Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <lossin@kernel.org>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Danilo Krummrich" <dakr@kernel.org>,
	"Daniel Almeida" <daniel.almeida@collabora.com>,
	"Tamir Duberstein" <tamird@kernel.org>,
	"Onur Özkan" <work@onurozkan.dev>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"Will Deacon" <will@kernel.org>,
	"Peter Zijlstra" <peterz@infradead.org>,
	"Mark Rutland" <mark.rutland@arm.com>,
	"Lyude Paul" <lyude@redhat.com>,
	"Yury Norov" <yury.norov@gmail.com>,
	"Luis Chamberlain" <mcgrof@kernel.org>,
	"Petr Pavlu" <petr.pavlu@suse.com>,
	"Daniel Gomez" <da.gomez@kernel.org>,
	"Sami Tolvanen" <samitolvanen@google.com>,
	"Aaron Tomlin" <atomlin@atomlin.com>,
	"Brendan Higgins" <brendan.higgins@linux.dev>,
	"David Gow" <david@davidgow.net>,
	"Rae Moar" <raemoar63@gmail.com>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org,
	linux-modules@vger.kernel.org, linux-kselftest@vger.kernel.org,
	kunit-dev@googlegroups.com, driver-core@lists.linux.dev
Subject: Re: [PATCH RFC 00/11] rust: add support for dynamic debug
Date: Wed, 21 Oct 2026 12:58:49 +0300	[thread overview]
Message-ID: <20261021125849.19939717@inno-dell> (raw)
In-Reply-To: <ffe50a28-72d7-4b88-b7db-b7a590aacbc4@t-8ch.de>

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

On Tue, 20 Oct 2026 23:36:56 +0200
Thomas Weißschuh <linux@weissschuh.net> wrote:

> On 2026-10-20 21:25:42+0300, Zhi Wang wrote:
> > On Tue, 20 Oct 2026 19:30:33 +0200
> > Thomas Weißschuh <linux@weissschuh.net> wrote:
> > 
> > > +To: Zhi Wang
> > > 
> > 
> > Thanks Alex for bridge the threads together!
> > 
> > Hey Thomas,
> > 
> > Thanks for the comparison! As a user of this feature, I'm happy to
> > combine the two series and see this making progress.
> > 
> > The Bounded types, bitfield tests, and module_path!() fallback look
> > like useful additions. One clarification: with CONFIG_JUMP_LABEL,
> > our disabled path already has no FFI call.
> > static_branch_unlikely!() is sufficient for our initially disabled
> > callsites, including enabling them at runtime.
> 
> Ack. Although adding jump label support as a later step would make the
> patches smaller and easier to review.
> 
> > For the combined series, I'd like to retain these behaviors:
> > 
> > - Debug logging is independent of debug_assertions, starts disabled,
> >   and is controlled through dynamic debug. IMHO, fewer distro
> > kernels are compiled with CONFIG_RUST_DEBUG_ASSERTIONS=y nowadays.
> > (At least, Arch and Fedora are compiled with N)
> 
> Ack. It would still be useful to also support default-enabled
> callsites, like the C version does. What about '--cfg DEBUG'?
> 

Agree.

> > - Disabled pr_debug!()/dev_dbg!() calls do not evaluate their
> >   arguments. IMHO, it seems the current dev_dbg!() in your patch
> >   evaluates the params before checking the switch on/off.
> > (pr_debug()! looks fine though)
> 
> As far as I understand, fmt!() itself does not yet evaluate the
> expressions passed as arguments. It only typechecks and
> creates the precompiled Arguments<'_> structure. The parameters'
> Display traits are only executed when the Arguments<'_> is written
> somewhere. IMO the typechecking should be done even if the code is
> going to be compiled out. Similar to no_printk() on the C side.
> 

I encountered this problem by chance while developing my patches and
testing it on nova-core (unexpected evaluation of a register operation
while dynamic debug is off). IMHO, it seems constructing Arguments
evaluates the expressions immediately even if the resulting arguments
are never formatted.

I made a test on top of your WIP branch, the example is:

pr_debug!("dyndbg repro pr: {}\n", read_status(&reads, &formats));
dev_dbg!(fdev, dyndbg repro dev: {}\n", read_status(&reads, &formats));

reads and formats are two counters.

With dynamic debug disabled, both macros produce:

dyndbg repro pr: args=1 fmt=0
dyndbg repro dev: args=1 fmt=0

which shows the params are evaluated unexpectedly.

My solution was moving the fmt!() into dynamic_func_call()!

I attached the testing code as well. Feel free to try it.

> > - The dynamic-debug prefix flags remain supported. (I use the C
> > version of dynamic debug to support e.g. +m, +l and _t)
> 
> Absolutely, this is a bug in my v1. It is fixed in v2.
> 
> > Could you share your pending encapsulation/Opaque and static-branch
> > wip patches? We can then work out how they fit together, incorporate
> > Alice's review feedback, and preserve attribution for Andrew's work.
> 
> Here:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/thomas.weissschuh/linux.git/log/?h=b4/rust-dyndbg
> 
> > Thanks,
> > Zhi.
> > 
> > > Hi Alexandre,
> > > 
> > > On 2026-10-20 22:14:12+0900, Alexandre Courbot wrote:
> > > > On Sun Oct 11, 2026 at 9:52 PM JST, Thomas Weißschuh wrote:  
> > > > > Add the Rust infrastructure to integrate with the dynamic
> > > > > debug infrastructure.
> > > > >
> > > > > Static keys are not used for now, as the full bindings are
> > > > > missing for those. A follow-up series will add that.
> > > > > Also custom classes are not supported yet, but could be added
> > > > > on top.
> > > > >
> > > > > Patches 1-4: Generic Rust preparation patches.
> > > > > Patches 5-7: C dynamic debug preparation patches.
> > > > > Patches 8: Rust Dynamic debug infrastructure.
> > > > > Patches 9-11: Rust Dynamic debug enablement.  
> > > > 
> > > > Were you aware of this series [1]?  
> > > 
> > > I was was told about the original work from Andrew back at
> > > Kangrejos. However I was unable to find it on LKML and it was not
> > > clear how far along that series has come along. The posting of
> > > Zhi's v3 only happened after I started on my variant, and I did
> > > not see it until you mentioned it. So my development is
> > > completely independent from Andrew's and Zhi's.
> > > 
> > > > There seems to be quite a bit of overlap.  
> > > 
> > > Indeed. I see pros for both serieses:
> > > 
> > > Mine:
> > > * No FFI in the fastpath.
> > > * No need for RUST_MODNAME.
> > > * Use of Bounded.
> > > * Tests for the flag encoding.
> > > * modname!() as function name.
> > >   (As discussed with Benno and Gary and LPC)
> > > * Better encapsulation of 'struct Descriptor' (partially not yet
> > > posted)
> > > * Use of Opaque<> to wrap bindings::_ddebug (not yet posted).
> > > * Use of !cv() (not yet posted).
> > > 
> > > Andrew/Zhi:
> > > * Better docs.
> > > * Static branch support.
> > >   * static_branch_likely!() support is missing however.
> > >   * I also have follow-up patches to enable
> > > static_branch_likely!().
> > > * host doctest support.
> > > 
> > > I am perfectly fine with merging both serieses.
> > > 
> > > 
> > > Thomas
> > > 
> > > > [1]
> > > > https://lore.kernel.org/all/20261001145940.1077801-1-zhiw@nvidia.com/
> > > >  
> > 
> 


[-- Attachment #2: 0001-kernel-repro.patch --]
[-- Type: text/x-patch, Size: 1763 bytes --]

diff --git a/samples/rust/rust_driver_faux.rs b/samples/rust/rust_driver_faux.rs
index de131fce491a..6db3124ed3a9 100644
--- a/samples/rust/rust_driver_faux.rs
+++ b/samples/rust/rust_driver_faux.rs
@@ -2,8 +2,10 @@
 
 //! Rust faux device sample.
 
+use core::cell::Cell;
 use kernel::{
     faux,
+    fmt,
     prelude::*,
     Module, //
 };
@@ -20,6 +22,20 @@ struct SampleModule {
     _reg: faux::Registration,
 }
 
+struct Status<'a>(&'a Cell<usize>);
+
+impl fmt::Display for Status<'_> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        self.0.set(self.0.get() + 1);
+        f.write_str("ready")
+    }
+}
+
+fn read_status<'a>(reads: &Cell<usize>, formats: &'a Cell<usize>) -> Status<'a> {
+    reads.set(reads.get() + 1);
+    Status(formats)
+}
+
 impl Module for SampleModule {
     fn init(_module: &'static ThisModule) -> Result<Self> {
         pr_info!("Initialising Rust Faux Device Sample\n");
@@ -30,6 +46,28 @@ fn init(_module: &'static ThisModule) -> Result<Self> {
         dev_info!(fdev, "Hello from faux device!\n");
         dev_dbg!(fdev, "foobar\n");
 
+        let reads = Cell::new(0);
+        let formats = Cell::new(0);
+        pr_debug!("dyndbg repro pr: {}\n", read_status(&reads, &formats));
+        pr_info!(
+            "dyndbg repro pr: args={} fmt={}\n",
+            reads.get(),
+            formats.get()
+        );
+
+        reads.set(0);
+        formats.set(0);
+        dev_dbg!(
+            fdev,
+            "dyndbg repro dev: {}\n",
+            read_status(&reads, &formats)
+        );
+        pr_info!(
+            "dyndbg repro dev: args={} fmt={}\n",
+            reads.get(),
+            formats.get()
+        );
+
         Ok(Self { _reg: reg })
     }
 }
-- 
2.53.0


      reply	other threads:[~2026-10-21  9:59 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-11 12:52 Thomas Weißschuh
2026-10-11 12:52 ` [PATCH RFC 01/11] rust: print: use CStr for __LOG_PREFIX Thomas Weißschuh
2026-10-11 13:01   ` Gary Guo
2026-10-11 14:43     ` Thomas Weißschuh
2026-10-11 12:52 ` [PATCH RFC 02/11] rust: sync: atomic: Add Atomic<u{8,16}> Thomas Weißschuh
2026-10-11 13:04   ` Gary Guo
2026-10-11 12:52 ` [PATCH RFC 03/11] rust: num: Add Bounded::USABLE_BITS Thomas Weißschuh
2026-10-11 12:52 ` [PATCH RFC 04/11] samples: rust_print: add debug messages Thomas Weißschuh
2026-10-11 13:53   ` Miguel Ojeda
2026-10-11 14:48     ` Thomas Weißschuh
2026-10-11 12:52 ` [PATCH RFC 05/11] dyndbg: use u32 for struct _ddebug bitfields Thomas Weißschuh
2026-10-11 12:52 ` [PATCH RFC 06/11] dyndbg: make _ddebug::flags its own field Thomas Weißschuh
2026-10-11 13:23   ` Gary Guo
2026-10-11 14:47     ` Thomas Weißschuh
2026-10-11 15:44       ` Gary Guo
2026-10-11 12:52 ` [PATCH RFC 07/11] dyndbg: use READ_ONCE()/WRITE_ONCE() to access descriptor flags Thomas Weißschuh
2026-10-11 13:16   ` Gary Guo
2026-10-11 12:52 ` [PATCH RFC 08/11] rust: add support for dynamic debug Thomas Weißschuh
2026-10-11 12:52 ` [PATCH RFC 09/11] rust: print: enable dynamic debug for pr_debug!() Thomas Weißschuh
2026-10-11 12:52 ` [PATCH RFC 10/11] rust: device: remove logging methods Thomas Weißschuh
2026-10-11 12:52 ` [PATCH RFC 11/11] rust: device: enable dynamic debug for dev_dbg!() Thomas Weißschuh
2026-10-20 13:14 ` [PATCH RFC 00/11] rust: add support for dynamic debug Alexandre Courbot
2026-10-20 17:30   ` Thomas Weißschuh
2026-10-20 18:25     ` Zhi Wang
2026-10-20 21:36       ` Thomas Weißschuh
2026-10-21  9:58         ` Zhi Wang [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=20261021125849.19939717@inno-dell \
    --to=zhiw@nvidia.com \
    --cc=a.hindborg@kernel.org \
    --cc=acourbot@nvidia.com \
    --cc=akpm@linux-foundation.org \
    --cc=aliceryhl@google.com \
    --cc=atomlin@atomlin.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun@kernel.org \
    --cc=brendan.higgins@linux.dev \
    --cc=da.gomez@kernel.org \
    --cc=dakr@kernel.org \
    --cc=daniel.almeida@collabora.com \
    --cc=david@davidgow.net \
    --cc=driver-core@lists.linux.dev \
    --cc=gary@garyguo.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=jbaron@akamai.com \
    --cc=jim.cromie@gmail.com \
    --cc=kunit-dev@googlegroups.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-modules@vger.kernel.org \
    --cc=linux@weissschuh.net \
    --cc=lossin@kernel.org \
    --cc=lyude@redhat.com \
    --cc=mark.rutland@arm.com \
    --cc=mcgrof@kernel.org \
    --cc=ojeda@kernel.org \
    --cc=peterz@infradead.org \
    --cc=petr.pavlu@suse.com \
    --cc=raemoar63@gmail.com \
    --cc=rafael@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=samitolvanen@google.com \
    --cc=tamird@kernel.org \
    --cc=tmgross@umich.edu \
    --cc=will@kernel.org \
    --cc=work@onurozkan.dev \
    --cc=yury.norov@gmail.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

all inboxes | Powered by JetHome®