| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <ac1b4cf8ef40b8f723abad5c5421e3dc9ad310c0.camel@kernel.org>
Date: Tue, 15 Apr 2025 08:54:50 -0400
From: Jeff Layton <jlayton@...nel.org>
To: Andrew Lunn <andrew@...n.ch>
Cc: Andrew Morton <akpm@...ux-foundation.org>, "David S. Miller"
<davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski
<kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Simon Horman
<horms@...nel.org>, Qasim Ijaz <qasdev00@...il.com>, Nathan Chancellor
<nathan@...nel.org>, linux-kernel@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCH 2/4] ref_tracker: add ability to register a file in
debugfs for a ref_tracker_dir
On Tue, 2025-04-15 at 06:23 -0400, Jeff Layton wrote:
> On Tue, 2025-04-15 at 01:08 +0200, Andrew Lunn wrote:
> > On Mon, Apr 14, 2025 at 10:45:47AM -0400, Jeff Layton wrote:
> > > Currently, there is no convenient way to see the info that the
> > > ref_tracking infrastructure collects. Add a new function that other
> > > subsystems can optionally call to update the name field in the
> > > ref_tracker_dir and register a corresponding seq_file for it in the
> > > top-level ref_tracker directory.
> > >
> > > Also, alter the pr_ostream infrastructure to allow the caller to specify
> > > a seq_file to which the output should go instead of printing to an
> > > arbitrary buffer or the kernel's ring buffer.
> >
> > When i see an Also, or And, or a list in a commit message, i always
> > think, should this be multiple patches?
> >
>
> Sure. I actually had this part in a separate patch earlier, but I don't
> usually like adding functions with no callers and this patch was pretty
> small. I can break it up though.
>
> > > struct ostream {
> > > char *buf;
> > > + struct seq_file *seq;
> > > int size, used;
> > > };
> > >
> > > @@ -73,7 +83,9 @@ struct ostream {
> > > ({ \
> > > struct ostream *_s = (stream); \
> > > \
> > > - if (!_s->buf) { \
> > > + if (_s->seq) { \
> > > + seq_printf(_s->seq, fmt, ##args); \
> > > + } else if (!_s->buf) { \
> > > pr_err(fmt, ##args); \
> > > } else { \
> > > int ret, len = _s->size - _s->used; \
> >
> > The pr_ostream() macro is getting pretty convoluted. It currently
> > supports two user cases:
> >
> > struct ostream os = {}; which means just use pr_err().
> >
> > And os.buf points to an allocated buffer and the output should be
> > dumped there.
> >
> > You are about to add a third.
> >
> > Is it about time this got split up into three helper functions, and
> > you pass one to __ref_tracker_dir_pr_ostream()? Your choice.
> >
>
> Maybe? It doesn't seem worth it for this, but I'll take a look.
>
> >
I took a crack at this and it's trickier than it looks. We have to pass
a variadic function pointer (which is fine), but that means that they
can't use handy macros like pr_err. We have to call functions that can
take a va_list (which is also fine).
The part I'm having trouble with is incorporating the pr_fmt(). I've
attached the patch I have on top of the current series. It doesn't
compile for me. If I remove the pr_fmt() calls and just pass in the
format string, it does compile.
What am I doing wrong here?
-------------------------8<---------------------------------
[PATCH] ref_tracker: have callers pass output function to pr_ostream
Signed-off-by: Jeff Layton <jlayton@...nel.org>
---
lib/ref_tracker.c | 44 ++++++++++++++++++++++++++++++++------------
1 file changed, 32 insertions(+), 12 deletions(-)
diff --git a/lib/ref_tracker.c b/lib/ref_tracker.c
index 4cc49cc21f5b..e131fdc51838 100644
--- a/lib/ref_tracker.c
+++ b/lib/ref_tracker.c
@@ -77,21 +77,41 @@ struct ostream {
char *buf;
struct seq_file *seq;
int size, used;
+ void (*func)(struct ostream *stream, char *fmt, ...);
};
+#define ref_tracker_log(fmt, args) vprintk_emit(0, LOGLEVEL_ERR, NULL,
pr_fmt(fmt), args)
+
+static void pr_ostream_log(struct ostream *stream, char *fmt, ...)
+{
+ va_list args;
+
+ ref_tracker_log(fmt, args);
+}
+
+#define ref_tracker_vsnprintf(buf, size, fmt, args) vsnprintf(buf,
size, pr_fmt(fmt), args)
+
+static void pr_ostream_buf(struct ostream *stream, char *fmt, ...)
+{
+ int ret, len = stream->size - stream->used;
+ va_list args;
+
+ ret = ref_tracker_vsnprintf(stream->buf + stream->used, len,
fmt, args);
+ stream->used += min(ret, len);
+}
+
+static void pr_ostream_seq(struct ostream *stream, char *fmt, ...)
+{
+ va_list args;
+
+ seq_vprintf(stream->seq, fmt, args);
+}
+
#define pr_ostream(stream, fmt, args...) \
({ \
struct ostream *_s = (stream); \
\
- if (_s->seq) { \
- seq_printf(_s->seq, fmt, ##args); \
- } else if (!_s->buf) { \
- pr_err(fmt, ##args); \
- } else { \
- int ret, len = _s->size - _s->used; \
- ret = snprintf(_s->buf + _s->used, len, pr_fmt(fmt),
##args); \
- _s->used += min(ret, len); \
- } \
+ _s->func(_s, fmt, ##args); \
})
static void
@@ -138,7 +158,7 @@ __ref_tracker_dir_pr_ostream(struct ref_tracker_dir
*dir,
void ref_tracker_dir_print_locked(struct ref_tracker_dir *dir,
unsigned int display_limit)
{
- struct ostream os = {};
+ struct ostream os = { .func = pr_ostream_log };
__ref_tracker_dir_pr_ostream(dir, display_limit, &os);
}
@@ -157,7 +177,7 @@ EXPORT_SYMBOL(ref_tracker_dir_print);
int ref_tracker_dir_snprint(struct ref_tracker_dir *dir, char *buf,
size_t size)
{
- struct ostream os = { .buf = buf, .size = size };
+ struct ostream os = { .buf = buf, .size = size, .func =
pr_ostream_buf };
unsigned long flags;
spin_lock_irqsave(&dir->lock, flags);
@@ -294,7 +314,7 @@ EXPORT_SYMBOL_GPL(ref_tracker_free);
static int ref_tracker_dir_seq_print(struct ref_tracker_dir *dir,
struct seq_file *seq)
{
- struct ostream os = { .seq = seq };
+ struct ostream os = { .seq = seq, .func = pr_ostream_seq };
unsigned long flags;
spin_lock_irqsave(&dir->lock, flags);
--
2.49.0
Powered by blists - more mailing lists