mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: "Dmitry V. Levin" <ldv@strace.io>
Cc: Vincent Donnefort <vdonnefort@google.com>,
	mhiramat@kernel.org, mathieu.desnoyers@efficios.com,
	kernel-team@android.com, rdunlap@infradead.org, rppt@kernel.org,
	david@redhat.com, linux-trace-kernel@vger.kernel.org,
	linux-mm@kvack.org, linux-api@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: [PATCH v23 3/5] tracing: Allow user-space mapping of the ring-buffer
Date: Sun, 30 Jun 2024 08:40:53 -0400	[thread overview]
Message-ID: <20240630084053.0b506916@rorschach.local.home> (raw)
In-Reply-To: <20240630105322.GA17573@altlinux.org>

On Sun, 30 Jun 2024 13:53:23 +0300
"Dmitry V. Levin" <ldv@strace.io> wrote:

> On Fri, May 10, 2024 at 03:04:32PM +0100, Vincent Donnefort wrote:
> [...]
> > diff --git a/include/uapi/linux/trace_mmap.h b/include/uapi/linux/trace_mmap.h
> > index b682e9925539..bd1066754220 100644
> > --- a/include/uapi/linux/trace_mmap.h
> > +++ b/include/uapi/linux/trace_mmap.h
> > @@ -43,4 +43,6 @@ struct trace_buffer_meta {
> >  	__u64	Reserved2;
> >  };
> >  
> > +#define TRACE_MMAP_IOCTL_GET_READER		_IO('T', 0x1)
> > +  
> 
> I'm sorry but among all the numbers this one was probably the least
> fortunate choice because it collides with TCGETS on most of architectures.

Hmm, that is unfortunate.

> 
> For example, this is how strace output would look like when
> TRACE_MMAP_IOCTL_GET_READER support is added:
> 
> $ strace -e ioctl stty
> ioctl(0, TCGETS or TRACE_MMAP_IOCTL_GET_READER, {c_iflag=ICRNL|IXON, c_oflag=NL0|CR0|TAB0|BS0|VT0|FF0|OPOST|ONLCR, c_cflag=B38400|CS8|CREAD, c_lflag=ISIG|ICANON|ECHO|ECHOE|ECHOK|IEXTEN|ECHOCTL|ECHOKE, ...}) = 0
> 
> Even though ioctl numbers are inherently not unique, TCGETS is
> a very traditional one, so it would be great if you could change
> TRACE_MMAP_IOCTL_GET_READER to avoid this collision.
> 
> Given that _IO('T', 0x1) is _IOC(_IOC_NONE, 'T', 0x1, 0),
> something like _IOC(_IOC_NONE, 'T', 0x1, 0x1) should be OK.

Well, it may not be too late to update this as it hasn't been
officially released in 6.10 yet. It's still only in the -rc and the
library doesn't rely on this yet (I've been holding off until 6.10 was
officially released before releasing the library that uses it).

I can send a patch this week to update it. Or feel free to send a patch
yourself.

Thanks,

-- Steve

  reply	other threads:[~2024-06-30 12:40 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-10 14:04 [PATCH v23 0/5] Introducing trace buffer mapping by user-space Vincent Donnefort
2024-05-10 14:04 ` [PATCH v23 1/5] ring-buffer: Allocate sub-buffers with __GFP_COMP Vincent Donnefort
2024-05-10 14:04 ` [PATCH v23 2/5] ring-buffer: Introducing ring-buffer mapping functions Vincent Donnefort
2024-05-10 14:06   ` David Hildenbrand
2024-05-10 14:04 ` [PATCH v23 3/5] tracing: Allow user-space mapping of the ring-buffer Vincent Donnefort
2024-06-30 10:53   ` Dmitry V. Levin
2024-06-30 12:40     ` Steven Rostedt [this message]
2024-07-02 14:36       ` Mathieu Desnoyers
2024-07-02 15:18         ` Steven Rostedt
2024-07-02 15:32           ` Mathieu Desnoyers
2024-07-02 16:44             ` Dmitry V. Levin
2024-07-02 16:51             ` Steven Rostedt
2024-07-02 17:37               ` Mathieu Desnoyers
2024-07-03 22:26               ` Beau Belgrave
2024-07-02 17:33           ` Dmitry V. Levin
2024-05-10 14:04 ` [PATCH v23 4/5] Documentation: tracing: Add ring-buffer mapping Vincent Donnefort
2024-05-10 14:04 ` [PATCH v23 5/5] ring-buffer/selftest: Add ring-buffer mapping test Vincent Donnefort

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=20240630084053.0b506916@rorschach.local.home \
    --to=rostedt@goodmis.org \
    --cc=david@redhat.com \
    --cc=kernel-team@android.com \
    --cc=ldv@strace.io \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mhiramat@kernel.org \
    --cc=rdunlap@infradead.org \
    --cc=rppt@kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=vdonnefort@google.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®