[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <9a9c8ea4-8e17-4e7e-95fe-7b51441a228c@efficios.com>
Date: Tue, 2 Jul 2024 10:36:03 -0400
From: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
To: Steven Rostedt <rostedt@...dmis.org>, "Dmitry V. Levin" <ldv@...ace.io>
Cc: Vincent Donnefort <vdonnefort@...gle.com>, mhiramat@...nel.org,
kernel-team@...roid.com, rdunlap@...radead.org, rppt@...nel.org,
david@...hat.com, linux-trace-kernel@...r.kernel.org, linux-mm@...ck.org,
linux-api@...r.kernel.org, linux-kernel@...r.kernel.org,
Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH v23 3/5] tracing: Allow user-space mapping of the
ring-buffer
On 2024-06-30 08:40, Steven Rostedt wrote:
> On Sun, 30 Jun 2024 13:53:23 +0300
> "Dmitry V. Levin" <ldv@...ace.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.
You need to reserve an unused ioctl Code and Seq# range within:
Documentation/userspace-api/ioctl/ioctl-number.rst
Otherwise this duplicate will confuse all system call instrumentation
tooling.
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com
Powered by blists - more mailing lists