* Re: [PATCHv8 bpf-next 3/9] uprobe: Add uretprobe syscall to speed up return probe
From: Nathan Chancellor @ 2024-06-14 17:48 UTC (permalink / raw)
To: Jiri Olsa
Cc: Steven Rostedt, Masami Hiramatsu, Oleg Nesterov,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
linux-kernel, linux-trace-kernel, linux-api, linux-man, x86, bpf,
Song Liu, Yonghong Song, John Fastabend, Peter Zijlstra,
Thomas Gleixner, Borislav Petkov (AMD), Ingo Molnar,
Andy Lutomirski, Edgecombe, Rick P, Deepak Gupta, Huacai Chen,
loongarch
In-Reply-To: <20240611112158.40795-4-jolsa@kernel.org>
Hi Jiri,
On Tue, Jun 11, 2024 at 01:21:52PM +0200, Jiri Olsa wrote:
> Adding uretprobe syscall instead of trap to speed up return probe.
...
> diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
> index 2c83ba776fc7..2816e65729ac 100644
> --- a/kernel/events/uprobes.c
> +++ b/kernel/events/uprobes.c
> @@ -1474,11 +1474,20 @@ static int xol_add_vma(struct mm_struct *mm, struct xol_area *area)
> return ret;
> }
>
> +void * __weak arch_uprobe_trampoline(unsigned long *psize)
> +{
> + static uprobe_opcode_t insn = UPROBE_SWBP_INSN;
This change as commit ff474a78cef5 ("uprobe: Add uretprobe syscall to
speed up return probe") in -next causes the following build error for
ARCH=loongarch allmodconfig:
In file included from include/linux/uprobes.h:49,
from include/linux/mm_types.h:16,
from include/linux/mmzone.h:22,
from include/linux/gfp.h:7,
from include/linux/xarray.h:16,
from include/linux/list_lru.h:14,
from include/linux/fs.h:13,
from include/linux/highmem.h:5,
from kernel/events/uprobes.c:13:
kernel/events/uprobes.c: In function 'arch_uprobe_trampoline':
arch/loongarch/include/asm/uprobes.h:12:33: error: initializer element is not constant
12 | #define UPROBE_SWBP_INSN larch_insn_gen_break(BRK_UPROBE_BP)
| ^~~~~~~~~~~~~~~~~~~~
kernel/events/uprobes.c:1479:39: note: in expansion of macro 'UPROBE_SWBP_INSN'
1479 | static uprobe_opcode_t insn = UPROBE_SWBP_INSN;
| ^~~~~~~~~~~~~~~~
> + *psize = UPROBE_SWBP_INSN_SIZE;
> + return &insn;
> +}
> +
> static struct xol_area *__create_xol_area(unsigned long vaddr)
> {
> struct mm_struct *mm = current->mm;
> - uprobe_opcode_t insn = UPROBE_SWBP_INSN;
> + unsigned long insns_size;
> struct xol_area *area;
> + void *insns;
>
> area = kmalloc(sizeof(*area), GFP_KERNEL);
> if (unlikely(!area))
> @@ -1502,7 +1511,8 @@ static struct xol_area *__create_xol_area(unsigned long vaddr)
> /* Reserve the 1st slot for get_trampoline_vaddr() */
> set_bit(0, area->bitmap);
> atomic_set(&area->slot_count, 1);
> - arch_uprobe_copy_ixol(area->pages[0], 0, &insn, UPROBE_SWBP_INSN_SIZE);
> + insns = arch_uprobe_trampoline(&insns_size);
> + arch_uprobe_copy_ixol(area->pages[0], 0, insns, insns_size);
>
> if (!xol_add_vma(mm, area))
> return area;
> @@ -1827,7 +1837,7 @@ void uprobe_copy_process(struct task_struct *t, unsigned long flags)
> *
> * Returns -1 in case the xol_area is not allocated.
> */
> -static unsigned long get_trampoline_vaddr(void)
> +unsigned long uprobe_get_trampoline_vaddr(void)
> {
> struct xol_area *area;
> unsigned long trampoline_vaddr = -1;
> @@ -1878,7 +1888,7 @@ static void prepare_uretprobe(struct uprobe *uprobe, struct pt_regs *regs)
> if (!ri)
> return;
>
> - trampoline_vaddr = get_trampoline_vaddr();
> + trampoline_vaddr = uprobe_get_trampoline_vaddr();
> orig_ret_vaddr = arch_uretprobe_hijack_return_addr(trampoline_vaddr, regs);
> if (orig_ret_vaddr == -1)
> goto fail;
> @@ -2123,7 +2133,7 @@ static struct return_instance *find_next_ret_chain(struct return_instance *ri)
> return ri;
> }
>
> -static void handle_trampoline(struct pt_regs *regs)
> +void uprobe_handle_trampoline(struct pt_regs *regs)
> {
> struct uprobe_task *utask;
> struct return_instance *ri, *next;
> @@ -2187,8 +2197,8 @@ static void handle_swbp(struct pt_regs *regs)
> int is_swbp;
>
> bp_vaddr = uprobe_get_swbp_addr(regs);
> - if (bp_vaddr == get_trampoline_vaddr())
> - return handle_trampoline(regs);
> + if (bp_vaddr == uprobe_get_trampoline_vaddr())
> + return uprobe_handle_trampoline(regs);
>
> uprobe = find_active_uprobe(bp_vaddr, &is_swbp);
> if (!uprobe) {
> --
> 2.45.1
>
Cheers,
Nathan
^ permalink raw reply
* Re: termios constants should be unsigned
From: Alejandro Colomar @ 2024-06-13 21:37 UTC (permalink / raw)
To: Paul Eggert
Cc: Zack Weinberg, Andrew Morton, Palmer Dabbelt, linux-api,
GNU libc development, 'linux-man'
In-Reply-To: <6ce7434a-56bd-4e95-80f1-b2857834b0d4@cs.ucla.edu>
[-- Attachment #1: Type: text/plain, Size: 1072 bytes --]
On Thu, Jun 13, 2024 at 02:12:20PM GMT, Paul Eggert wrote:
> Part of the issue here is that GCC and Clang often do a better job of
> warning when constants are signed, not unsigned. For example, suppose a
> program mistakenly packages termios flags along with three other bits into
> an 'unsigned long', with code like this:
>
> unsigned long
> tagged_pendin (unsigned tag)
> {
> return (PENDIN << 3) | tag;
> }
>
> Since PENDIN is 0x20000000 Clang and GCC by default warn about the mistake,
> as the signed integer overflow has undefined behavior. But if PENDIN were
> changed to 0x20000000U the behavior would be well-defined, there would be no
> warning even with -Wall -Wextra -Wsign-conversion, and the code would
> silently behave as if PENDIN were zero, which is not intended.
>
> This is another reason why appending "U" to PENDIN's value would have
> drawbacks as well as advantages.
Hmmmm, very interesting point! I'll have that in mind when doing
bitwise stuff with constants.
--
<https://www.alejandro-colomar.es/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: termios constants should be unsigned
From: Paul Eggert @ 2024-06-13 21:12 UTC (permalink / raw)
To: Zack Weinberg, Alejandro Colomar, Andrew Morton, Palmer Dabbelt,
linux-api, GNU libc development, 'linux-man'
In-Reply-To: <f6ee2bd3-c1b7-4769-a313-b62f42c450ca@app.fastmail.com>
On 6/13/24 05:32, Zack Weinberg wrote:
> there is still a need for
> caution around conversions that change signedness.
Yes, just as there is need for caution around any use of unsigned types.
Unfortunately in my experience Clang's (and even GCC's) warnings about
signedness conversion are more likely to cause harm than good, with this
thread being an example of the harm.
Part of the issue here is that GCC and Clang often do a better job of
warning when constants are signed, not unsigned. For example, suppose a
program mistakenly packages termios flags along with three other bits
into an 'unsigned long', with code like this:
unsigned long
tagged_pendin (unsigned tag)
{
return (PENDIN << 3) | tag;
}
Since PENDIN is 0x20000000 Clang and GCC by default warn about the
mistake, as the signed integer overflow has undefined behavior. But if
PENDIN were changed to 0x20000000U the behavior would be well-defined,
there would be no warning even with -Wall -Wextra -Wsign-conversion, and
the code would silently behave as if PENDIN were zero, which is not
intended.
This is another reason why appending "U" to PENDIN's value would have
drawbacks as well as advantages.
^ permalink raw reply
* Re: termios constants should be unsigned
From: Zack Weinberg @ 2024-06-13 12:32 UTC (permalink / raw)
To: Paul Eggert, Alejandro Colomar, Andrew Morton, Palmer Dabbelt,
linux-api, GNU libc development, 'linux-man'
In-Reply-To: <87af5e8f-0dcb-44a0-94de-757cad7d5ded@cs.ucla.edu>
On Wed, Jun 12, 2024, at 10:55 AM, Paul Eggert wrote:
> A lot of this stuff is pedanticism that dates back to the bad old days
> when the C standard allowed ones' complement and signed magnitude
> representations of signed integers. Although it can be amusing to
> worry about that possibility (I know I've done it) it's never been a
> practical worry, and even the motivation of pedanticism is going away
> now that C23 requires two's complement.
Unless C23 eliminated *all* the cases where an operation on unsigned
integers is well-defined but the same operation on signed integers is
undefined, and last I checked it had not, there is still a need for
caution around conversions that change signedness.
zw
^ permalink raw reply
* Re: termios constants should be unsigned
From: Alejandro Colomar @ 2024-06-12 22:22 UTC (permalink / raw)
To: enh
Cc: Paul Eggert, Andrew Morton, Palmer Dabbelt, linux-api, libc-alpha,
linux-man
In-Reply-To: <CAJgzZorzcAP5wNa-UCMyarmjgwVBveg0c0Dj36ByVEacnOHrnw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2293 bytes --]
Hi Elliott,
On Wed, Jun 12, 2024 at 05:54:43PM GMT, enh wrote:
> > BTW, that seems to be a bogus way to workaround this; the cast should
> > have been on the other side. I'd say whoever maintains that code should
> > probably fix that to use unsigned types.
>
> indeed. i've already sent out such a change :-)
>
> > These constants are meant to
> > be 'tcflag_t', so a cast should be to that type, or the type of the
> > other side of the comparison, but casting to 'int' just for silencing a
> > waring seems nuts.
>
> i suspect the reasoning was one of readability --- keeping the [short]
> constants legible at the cost of making the expression slightly
> longer.
>
> > This makes me wonder if breaking _those_ users could be a good thing...
>
> like Paul Eggert said somewhere else today --- only if we're finding
> real bugs. and so far we're not.
>
> it's like the warn_unused_result argument. a purist would argue that
> every function should have that annotation, because you should always
> check for errors, and if you're not already doing so, your code is
> already broken. whereas a pragmatist would argue that most people are
> just going to add the "shut up, compiler" cast (or disable the warning
> entirely) if their already-working code suddenly starts spamming
> warnings next time they build it.
>
> while my bar for that might not be as high as my bar for ABI breakage,
> my source compatibility bar is still pretty high. it would be almost
> unethical of me to make app developers do random busywork. i have to
> be pretty confident (as with, say, "you just passed an fd > 1024 to an
> fd_set function/macro and thus corrupted memory") that their code is
> _definitely_ wrong. (and even there, that's going to have to be a
> runtime check!)
Yeah, I can agree with that. I'm that kind of pedantic purist for my
own code, and it's painful that historic accidents like this one don't
allow me to be so in my own code. But I agree that fixing the entire
world when their code is braindamaged but works is asking too much.
I'll just disable that pedantic warning when I use termbits. :)
Btw, thanks for fixing that brain-damaged cast. ;-)
Have a lovely night!
Alex
--
<https://www.alejandro-colomar.es/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: termios constants should be unsigned
From: enh @ 2024-06-12 21:54 UTC (permalink / raw)
To: Alejandro Colomar
Cc: Paul Eggert, Andrew Morton, Palmer Dabbelt, linux-api, libc-alpha,
linux-man
In-Reply-To: <5rfohnr4rs3tkfs7y3f7rth36c67pvcwv4q52onrjohdjtpo7m@stvcsncq7z4f>
On Wed, Jun 12, 2024 at 3:01 PM Alejandro Colomar <alx@kernel.org> wrote:
>
> On Wed, Jun 12, 2024 at 01:47:03PM GMT, enh wrote:
> > hacked these changes into AOSP, and it did break one bit of existing
> > code that was already working around the sign differences --- this
> > warning was enabled but the code had a cast to make the _other_ side
> > of the comparison signed (rather than make this side of the comparison
> > unsigned).
>
> BTW, that seems to be a bogus way to workaround this; the cast should
> have been on the other side. I'd say whoever maintains that code should
> probably fix that to use unsigned types.
indeed. i've already sent out such a change :-)
> These constants are meant to
> be 'tcflag_t', so a cast should be to that type, or the type of the
> other side of the comparison, but casting to 'int' just for silencing a
> waring seems nuts.
i suspect the reasoning was one of readability --- keeping the [short]
constants legible at the cost of making the expression slightly
longer.
> This makes me wonder if breaking _those_ users could be a good thing...
like Paul Eggert said somewhere else today --- only if we're finding
real bugs. and so far we're not.
it's like the warn_unused_result argument. a purist would argue that
every function should have that annotation, because you should always
check for errors, and if you're not already doing so, your code is
already broken. whereas a pragmatist would argue that most people are
just going to add the "shut up, compiler" cast (or disable the warning
entirely) if their already-working code suddenly starts spamming
warnings next time they build it.
while my bar for that might not be as high as my bar for ABI breakage,
my source compatibility bar is still pretty high. it would be almost
unethical of me to make app developers do random busywork. i have to
be pretty confident (as with, say, "you just passed an fd > 1024 to an
fd_set function/macro and thus corrupted memory") that their code is
_definitely_ wrong. (and even there, that's going to have to be a
runtime check!)
> --
> <https://www.alejandro-colomar.es/>
^ permalink raw reply
* Re: termios constants should be unsigned
From: Alejandro Colomar @ 2024-06-12 19:01 UTC (permalink / raw)
To: enh
Cc: Paul Eggert, Andrew Morton, Palmer Dabbelt, linux-api, libc-alpha,
linux-man
In-Reply-To: <CAJgzZorNc3gNVbiibz+DibrMLxc2dQoOS5NtL+RQUkSD-GMYaA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 854 bytes --]
On Wed, Jun 12, 2024 at 01:47:03PM GMT, enh wrote:
> hacked these changes into AOSP, and it did break one bit of existing
> code that was already working around the sign differences --- this
> warning was enabled but the code had a cast to make the _other_ side
> of the comparison signed (rather than make this side of the comparison
> unsigned).
BTW, that seems to be a bogus way to workaround this; the cast should
have been on the other side. I'd say whoever maintains that code should
probably fix that to use unsigned types. These constants are meant to
be 'tcflag_t', so a cast should be to that type, or the type of the
other side of the comparison, but casting to 'int' just for silencing a
waring seems nuts.
This makes me wonder if breaking _those_ users could be a good thing...
--
<https://www.alejandro-colomar.es/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: termios constants should be unsigned
From: Alejandro Colomar @ 2024-06-12 18:55 UTC (permalink / raw)
To: enh
Cc: Paul Eggert, Andrew Morton, Palmer Dabbelt, linux-api, libc-alpha,
linux-man
In-Reply-To: <CAJgzZorNc3gNVbiibz+DibrMLxc2dQoOS5NtL+RQUkSD-GMYaA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1153 bytes --]
Hi Elliott, Paul,
On Wed, Jun 12, 2024 at 01:47:03PM GMT, enh wrote:
> well, any change like this is a potential source incompatibility ... i
> hacked these changes into AOSP, and it did break one bit of existing
> code that was already working around the sign differences --- this
> warning was enabled but the code had a cast to make the _other_ side
> of the comparison signed (rather than make this side of the comparison
> unsigned).
>
> Android's libc [bionic] uses the uapi headers directly, so we would be
> affected, but to be clear --- i'm fine with this if the consensus is
> to go this way.
Hmmm; I see. I guess for this already-existent case we can just live
with it. Then I'll just ask to consider using unsigned constants for
new stuff that is a bit pattern and not just a number.
Let's drop the patch.
Have a lovely day!
Alex
> (but, yeah, i'm with the "how about we fix the language and compiler
> rather than all the extant code?" sentiment from Paul Eggert.)
Hmmm, can agree to that. At least if we don't continue writing bad code
for the new one. :)
--
<https://www.alejandro-colomar.es/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: termios constants should be unsigned
From: Paul Eggert @ 2024-06-12 18:27 UTC (permalink / raw)
To: Alejandro Colomar
Cc: Andrew Morton, Palmer Dabbelt, linux-api, libc-alpha, linux-man
In-Reply-To: <mdidkojqnhvf5b22vh3c4b6ajmq5miuyr3ole26kx2qkmnbfh3@woy2ghe5eyve>
On 2024-06-12 09:28, Alejandro Colomar wrote:
> adding U is a net
> improvement, without downsides (or I can't see them)
Adding U can change the generated code and thus cause behavior change in
programs (admittedly not well-written ones). It could also create new
false positives in well-written programs.
If adding U fixed real bugs then of course we should do it. Here, though....
^ permalink raw reply
* Re: termios constants should be unsigned
From: enh @ 2024-06-12 17:47 UTC (permalink / raw)
To: Alejandro Colomar
Cc: Paul Eggert, Andrew Morton, Palmer Dabbelt, linux-api, libc-alpha,
linux-man
In-Reply-To: <mdidkojqnhvf5b22vh3c4b6ajmq5miuyr3ole26kx2qkmnbfh3@woy2ghe5eyve>
On Wed, Jun 12, 2024 at 12:29 PM Alejandro Colomar <alx@kernel.org> wrote:
>
> Hi Paul,
>
> On Wed, Jun 12, 2024 at 07:55:14AM GMT, Paul Eggert wrote:
> > On 2024-06-12 05:16, Alejandro Colomar wrote:
> > > tcgets.c:53:24:
> > > error: implicit conversion changes signedness: 'int' to 'tcflag_t' (aka
> > > 'unsigned int') [clang-diagnostic-sign-conversion,-warnings-as-errors]
> >
> > This is a bug in Clang not glibc, and if you're worried about it I suggest
> > sending a bug report to the Clang folks about the false positive.
> >
> > Even GCC's -Wsign-conversion, which is at least smart enough to not warn
> > about benign conversions like that, is too often so chatty that it's best
> > avoided.
> >
> > A lot of this stuff is pedanticism that dates back to the bad old days when
> > the C standard allowed ones' complement and signed magnitude representations
> > of signed integers. Although it can be amusing to worry about that
> > possibility (I know I've done it) it's never been a practical worry, and
> > even the motivation of pedanticism is going away now that C23 requires two's
> > complement.
>
> I know; I think I have -Weverything enabled in that run, which is known
> for its pedanticity. I usually disable it when it triggers a warning,
> since they are usually nonsense. But in this case, adding U is a net
> improvement, without downsides (or I can't see them).
well, any change like this is a potential source incompatibility ... i
hacked these changes into AOSP, and it did break one bit of existing
code that was already working around the sign differences --- this
warning was enabled but the code had a cast to make the _other_ side
of the comparison signed (rather than make this side of the comparison
unsigned).
Android's libc [bionic] uses the uapi headers directly, so we would be
affected, but to be clear --- i'm fine with this if the consensus is
to go this way.
(but, yeah, i'm with the "how about we fix the language and compiler
rather than all the extant code?" sentiment from Paul Eggert.)
> So, while the kernel and glibc are just fine with this implicit
> conversion, they would be equally fine and even better without the
> conversion. Not a bug, but rather a slight improvement.
>
> Have a lovely day!
> Alex
>
> --
> <https://www.alejandro-colomar.es/>
^ permalink raw reply
* Re: [PATCH v1 11/14] futex: Implement FUTEX2_NUMA
From: Peter Zijlstra @ 2024-06-12 17:44 UTC (permalink / raw)
To: Christoph Lameter (Ampere)
Cc: tglx, axboe, linux-kernel, mingo, dvhart, dave, andrealmeid,
Andrew Morton, urezki, hch, lstoakes, Arnd Bergmann, linux-api,
linux-mm, linux-arch, malteskarupke
In-Reply-To: <9dc04e4c-2adc-5084-4ea1-b200d82be29f@linux.com>
On Wed, Jun 12, 2024 at 10:23:00AM -0700, Christoph Lameter (Ampere) wrote:
> On Fri, 21 Jul 2023, Peter Zijlstra wrote:
>
> > Extend the futex2 interface to be numa aware.
>
> Sorry to be chiming in that late but it seems that this is useful to
> mitigate NUMA issues also for our platform.
I read this like: I tested it and it works for me. Is that a correct
reading of your statement?
If so, I'll look at bumping this on the priority list and I'll look at
the placement suggestion you had when I respin the patches.
Thanks!
^ permalink raw reply
* Re: [PATCH v1 11/14] futex: Implement FUTEX2_NUMA
From: Christoph Lameter (Ampere) @ 2024-06-12 17:23 UTC (permalink / raw)
To: Peter Zijlstra
Cc: tglx, axboe, linux-kernel, mingo, dvhart, dave, andrealmeid,
Andrew Morton, urezki, hch, lstoakes, Arnd Bergmann, linux-api,
linux-mm, linux-arch, malteskarupke
In-Reply-To: <20230721105744.434742902@infradead.org>
On Fri, 21 Jul 2023, Peter Zijlstra wrote:
> Extend the futex2 interface to be numa aware.
Sorry to be chiming in that late but it seems that this is useful to
mitigate NUMA issues also for our platform.
> When FUTEX2_NUMA is not set, the node is simply an extention of the
> hash, such that traditional futexes are still interleaved over the
> nodes.
Could we follow NUMA policies like with other metadata allocations during
systen call processing? If there is no NUMA task policy then the futex
should be placed on the local NUMA node.
That way the placement of the futex can be controlled by the tasks memory
policy. We could skip the FUTEX2_NUMA option.
> @@ -114,10 +137,29 @@ late_initcall(fail_futex_debugfs);
> */
> struct futex_hash_bucket *futex_hash(union futex_key *key)
> {
> - u32 hash = jhash2((u32 *)key, offsetof(typeof(*key), both.offset) / 4,
> + u32 hash = jhash2((u32 *)key,
> + offsetof(typeof(*key), both.offset) / sizeof(u32),
> key->both.offset);
> + int node = key->both.node;
>
> - return &futex_queues[hash & (futex_hashsize - 1)];
> + if (node == -1) {
> + /*
> + * In case of !FLAGS_NUMA, use some unused hash bits to pick a
> + * node -- this ensures regular futexes are interleaved across
> + * the nodes and avoids having to allocate multiple
> + * hash-tables.
> + *
> + * NOTE: this isn't perfectly uniform, but it is fast and
> + * handles sparse node masks.
> + */
> + node = (hash >> futex_hashshift) % nr_node_ids;
> + if (!node_possible(node)) {
> + node = find_next_bit_wrap(node_possible_map.bits,
> + nr_node_ids, node);
> + }
Use memory allocation policies here instead?
^ permalink raw reply
* Re: [PATCH v1 11/14] futex: Implement FUTEX2_NUMA
From: Christoph Lameter (Ampere) @ 2024-06-12 17:07 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Thomas Gleixner, axboe, linux-kernel, mingo, dvhart, dave,
andrealmeid, Andrew Morton, urezki, hch, lstoakes, Arnd Bergmann,
linux-api, linux-mm, linux-arch, malteskarupke
In-Reply-To: <20230731180320.GR29590@hirez.programming.kicks-ass.net>
On Mon, 31 Jul 2023, Peter Zijlstra wrote:
>> Is nr_node_ids guaranteed to be stable after init? It's marked
>> __read_mostly, but not __ro_after_init.
>
> AFAICT it is only ever written to in setup_nr_node_ids() and that is all
> __init code. So I'm thinking this could/should indeed be
> __ro_after_init. Esp. so since it is an exported variable.
>
> Mike?
Its stable and lots of other components depend on it like f.e. the size of
cpumasks.
^ permalink raw reply
* Re: termios constants should be unsigned
From: Alejandro Colomar @ 2024-06-12 16:28 UTC (permalink / raw)
To: Paul Eggert
Cc: Andrew Morton, Palmer Dabbelt, linux-api, libc-alpha, linux-man
In-Reply-To: <87af5e8f-0dcb-44a0-94de-757cad7d5ded@cs.ucla.edu>
[-- Attachment #1: Type: text/plain, Size: 1561 bytes --]
Hi Paul,
On Wed, Jun 12, 2024 at 07:55:14AM GMT, Paul Eggert wrote:
> On 2024-06-12 05:16, Alejandro Colomar wrote:
> > tcgets.c:53:24:
> > error: implicit conversion changes signedness: 'int' to 'tcflag_t' (aka
> > 'unsigned int') [clang-diagnostic-sign-conversion,-warnings-as-errors]
>
> This is a bug in Clang not glibc, and if you're worried about it I suggest
> sending a bug report to the Clang folks about the false positive.
>
> Even GCC's -Wsign-conversion, which is at least smart enough to not warn
> about benign conversions like that, is too often so chatty that it's best
> avoided.
>
> A lot of this stuff is pedanticism that dates back to the bad old days when
> the C standard allowed ones' complement and signed magnitude representations
> of signed integers. Although it can be amusing to worry about that
> possibility (I know I've done it) it's never been a practical worry, and
> even the motivation of pedanticism is going away now that C23 requires two's
> complement.
I know; I think I have -Weverything enabled in that run, which is known
for its pedanticity. I usually disable it when it triggers a warning,
since they are usually nonsense. But in this case, adding U is a net
improvement, without downsides (or I can't see them).
So, while the kernel and glibc are just fine with this implicit
conversion, they would be equally fine and even better without the
conversion. Not a bug, but rather a slight improvement.
Have a lovely day!
Alex
--
<https://www.alejandro-colomar.es/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH] uapi/asm/termbits: Use the U integer suffix for bit fields
From: Alejandro Colomar @ 2024-06-12 15:07 UTC (permalink / raw)
To: Greg KH; +Cc: linux-api, linux-man, Andrew Morton, Palmer Dabbelt, libc-alpha
In-Reply-To: <2024061223-saddlebag-gallantly-c35f@gregkh>
[-- Attachment #1: Type: text/plain, Size: 1314 bytes --]
Hi Greg,
On Wed, Jun 12, 2024 at 04:21:25PM GMT, Greg KH wrote:
> On Wed, Jun 12, 2024 at 04:00:18PM +0200, Alejandro Colomar wrote:
> > I expect that these specific values and the operations done on them
> > probably don't trigger UB, since the shifts are done by a controlled
> > amount, and there are justa few operations done on them.
>
> These, for the most part, are NOT used as shifts.
Quoting the EXAMPLES section in the manual page:
tio.c_cflag &= ~(CBAUD << IBSHIFT);
(And yeah, that shift is presumably controlled, so that it doesn't
overflow, which is why I mean these are presumably just fine.)
> > TL;DR: The kernel isn't broken, but improving this would allow users to
> > enable stricter warnings, which is a good thing.
>
> Enable it where?
I meant in user space programs that use termbits stuff. (That this may
also allow the kernel to eventually have stricter warnings, I don't
know. It might help. But mostly meant it for user space.)
So, if I have a user-space program (or more likely a library) which
wraps these ioctls, I'd prefer to be able to enable the warnings I
reported, to preclude any mistakes in my code. That would need the
constants to be unsigned, to avoid false negatives.
Cheers,
Alex
--
<https://www.alejandro-colomar.es/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: termios constants should be unsigned
From: Paul Eggert @ 2024-06-12 14:55 UTC (permalink / raw)
To: Alejandro Colomar, Andrew Morton, Palmer Dabbelt, linux-api,
libc-alpha, linux-man
In-Reply-To: <a7kfppfptkzvqys6cblwjudlpoghsycjglw57hxe2ywvruzkbd@e6nqpnxgwfnq>
On 2024-06-12 05:16, Alejandro Colomar wrote:
> tcgets.c:53:24:
> error: implicit conversion changes signedness: 'int' to 'tcflag_t' (aka
> 'unsigned int') [clang-diagnostic-sign-conversion,-warnings-as-errors]
This is a bug in Clang not glibc, and if you're worried about it I
suggest sending a bug report to the Clang folks about the false positive.
Even GCC's -Wsign-conversion, which is at least smart enough to not warn
about benign conversions like that, is too often so chatty that it's
best avoided.
A lot of this stuff is pedanticism that dates back to the bad old days
when the C standard allowed ones' complement and signed magnitude
representations of signed integers. Although it can be amusing to worry
about that possibility (I know I've done it) it's never been a practical
worry, and even the motivation of pedanticism is going away now that C23
requires two's complement.
^ permalink raw reply
* Re: [PATCH] uapi/asm/termbits: Use the U integer suffix for bit fields
From: Greg KH @ 2024-06-12 14:21 UTC (permalink / raw)
To: Alejandro Colomar
Cc: linux-api, linux-man, Andrew Morton, Palmer Dabbelt, libc-alpha
In-Reply-To: <tkfp53faunf5jlszu5k6jwwxgsl72faffiux7c3pykmnowapy6@wap3bvtvoebi>
On Wed, Jun 12, 2024 at 04:00:18PM +0200, Alejandro Colomar wrote:
> Hi Greg,
>
> On Wed, Jun 12, 2024 at 03:35:20PM GMT, Greg KH wrote:
> > On Wed, Jun 12, 2024 at 03:16:58PM +0200, Alejandro Colomar wrote:
> > > Constants that are to be used in bitwise operations should be unsigned,
> > > or a user could easily trigger Undefined Behavior.
> >
> > Wait, do we really have such broken compilers out there?
>
> I meant this as a generic statement that signed integers on bitwise ops
> are bad, not as a specific statement that these values would trigger UB.
>
> I expect that these specific values and the operations done on them
> probably don't trigger UB, since the shifts are done by a controlled
> amount, and there are justa few operations done on them.
These, for the most part, are NOT used as shifts.
> For example, a left shift where a set bit overflows the type (e.g.,
> 1<<32), causes UB.
Sure, but that's not in play here.
> The reason why it's better to avoid this at all even if we know these
> values work fine, is that programs using <asm/termbits.h> would need to
> disable those compiler warnings, which could silence warnings on other
> code which might be broken.
But again, you aren't using these as bit shifts, they are bit masks, or
values, only.
> TL;DR: The kernel isn't broken, but improving this would allow users to
> enable stricter warnings, which is a good thing.
Enable it where?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] uapi/asm/termbits: Use the U integer suffix for bit fields
From: Alejandro Colomar @ 2024-06-12 14:00 UTC (permalink / raw)
To: Greg KH; +Cc: linux-api, linux-man, Andrew Morton, Palmer Dabbelt, libc-alpha
In-Reply-To: <2024061214-absolute-deranged-14bf@gregkh>
[-- Attachment #1: Type: text/plain, Size: 1371 bytes --]
Hi Greg,
On Wed, Jun 12, 2024 at 03:35:20PM GMT, Greg KH wrote:
> On Wed, Jun 12, 2024 at 03:16:58PM +0200, Alejandro Colomar wrote:
> > Constants that are to be used in bitwise operations should be unsigned,
> > or a user could easily trigger Undefined Behavior.
>
> Wait, do we really have such broken compilers out there?
I meant this as a generic statement that signed integers on bitwise ops
are bad, not as a specific statement that these values would trigger UB.
I expect that these specific values and the operations done on them
probably don't trigger UB, since the shifts are done by a controlled
amount, and there are justa few operations done on them.
For example, a left shift where a set bit overflows the type (e.g.,
1<<32), causes UB.
The reason why it's better to avoid this at all even if we know these
values work fine, is that programs using <asm/termbits.h> would need to
disable those compiler warnings, which could silence warnings on other
code which might be broken.
TL;DR: The kernel isn't broken, but improving this would allow users to
enable stricter warnings, which is a good thing.
> With this change, can the glibc versions then be dropped to just rely on
> these instead?
I don't know. glibc is CCd, so they can answer that.
Have a lovely day!
Alex
--
<https://www.alejandro-colomar.es/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH] uapi/asm/termbits: Use the U integer suffix for bit fields
From: Greg KH @ 2024-06-12 13:35 UTC (permalink / raw)
To: Alejandro Colomar
Cc: linux-api, linux-man, Andrew Morton, Palmer Dabbelt, libc-alpha
In-Reply-To: <20240612131633.449937-2-alx@kernel.org>
On Wed, Jun 12, 2024 at 03:16:58PM +0200, Alejandro Colomar wrote:
> Constants that are to be used in bitwise operations should be unsigned,
> or a user could easily trigger Undefined Behavior.
Wait, do we really have such broken compilers out there? If so, how has
no one hit this before?
> Also, the types where these constants are to be assigned are unsigned,
> so this makes it more consistent.
Fair enough.
> alx@debian:/usr/include$ grepc -tt termios asm-generic/
> asm-generic/termbits.h:struct termios {
> tcflag_t c_iflag; /* input mode flags */
> tcflag_t c_oflag; /* output mode flags */
> tcflag_t c_cflag; /* control mode flags */
> tcflag_t c_lflag; /* local mode flags */
> cc_t c_line; /* line discipline */
> cc_t c_cc[NCCS]; /* control characters */
> };
> alx@debian:/usr/include$ grepc -tt tcflag_t asm-generic/
> asm-generic/termbits.h:typedef unsigned int tcflag_t;
> alx@debian:/usr/include$ grepc -tt cc_t asm-generic/
> asm-generic/termbits-common.h:typedef unsigned char cc_t;
> alx@debian:/usr/include$ grepc -tt speed_t asm-generic/
> asm-generic/termbits-common.h:typedef unsigned int speed_t;
>
> Link: <https://lore.kernel.org/linux-api/2024061222-scuttle-expanse-6438@gregkh/T/>
> Cc: Greg KH <gregkh@linuxfoundation.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Palmer Dabbelt <palmer@rivosinc.com>
> Cc: <linux-api@vger.kernel.org>
> Cc: <libc-alpha@sourceware.org>
> Signed-off-by: Alejandro Colomar <alx@kernel.org>
> ---
>
> Hi Greg,
>
> On Wed, Jun 12, 2024 at 02:22:37PM GMT, Greg KH wrote:
> > Have a proposed patch that you feel would resolve this?
> >
> > thanks,
> >
> > greg k-h
>
> Here it is. :)
>
> For reviewing it, I suggest using '--word-diff-regex=.'.
>
> I compiled the kernel, and it seems ok; didn't test more than that.
With this change, can the glibc versions then be dropped to just rely on
these instead?
thanks,
greg k-h
^ permalink raw reply
* [PATCH] uapi/asm/termbits: Use the U integer suffix for bit fields
From: Alejandro Colomar @ 2024-06-12 13:16 UTC (permalink / raw)
To: linux-api
Cc: Alejandro Colomar, linux-man, Greg KH, Andrew Morton,
Palmer Dabbelt, libc-alpha
In-Reply-To: <2024061222-scuttle-expanse-6438@gregkh>
[-- Attachment #1: Type: text/plain, Size: 34853 bytes --]
Constants that are to be used in bitwise operations should be unsigned,
or a user could easily trigger Undefined Behavior.
Also, the types where these constants are to be assigned are unsigned,
so this makes it more consistent.
alx@debian:/usr/include$ grepc -tt termios asm-generic/
asm-generic/termbits.h:struct termios {
tcflag_t c_iflag; /* input mode flags */
tcflag_t c_oflag; /* output mode flags */
tcflag_t c_cflag; /* control mode flags */
tcflag_t c_lflag; /* local mode flags */
cc_t c_line; /* line discipline */
cc_t c_cc[NCCS]; /* control characters */
};
alx@debian:/usr/include$ grepc -tt tcflag_t asm-generic/
asm-generic/termbits.h:typedef unsigned int tcflag_t;
alx@debian:/usr/include$ grepc -tt cc_t asm-generic/
asm-generic/termbits-common.h:typedef unsigned char cc_t;
alx@debian:/usr/include$ grepc -tt speed_t asm-generic/
asm-generic/termbits-common.h:typedef unsigned int speed_t;
Link: <https://lore.kernel.org/linux-api/2024061222-scuttle-expanse-6438@gregkh/T/>
Cc: Greg KH <gregkh@linuxfoundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Palmer Dabbelt <palmer@rivosinc.com>
Cc: <linux-api@vger.kernel.org>
Cc: <libc-alpha@sourceware.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
---
Hi Greg,
On Wed, Jun 12, 2024 at 02:22:37PM GMT, Greg KH wrote:
> Have a proposed patch that you feel would resolve this?
>
> thanks,
>
> greg k-h
Here it is. :)
For reviewing it, I suggest using '--word-diff-regex=.'.
I compiled the kernel, and it seems ok; didn't test more than that.
Have a lovely day!
Alex
Range-diff:
-: ------------ > 1: 588eb01ac153 uapi/asm/termbits: Use the U integer suffix for bit fields
arch/alpha/include/uapi/asm/termbits.h | 154 +++++++++---------
arch/mips/include/uapi/asm/termbits.h | 154 +++++++++---------
arch/parisc/include/uapi/asm/termbits.h | 152 +++++++++---------
arch/powerpc/include/uapi/asm/termbits.h | 156 +++++++++---------
arch/sparc/include/uapi/asm/termbits.h | 192 +++++++++++------------
include/uapi/asm-generic/termbits.h | 152 +++++++++---------
6 files changed, 480 insertions(+), 480 deletions(-)
diff --git a/arch/alpha/include/uapi/asm/termbits.h b/arch/alpha/include/uapi/asm/termbits.h
index f1290b22072b..965a8c93523d 100644
--- a/arch/alpha/include/uapi/asm/termbits.h
+++ b/arch/alpha/include/uapi/asm/termbits.h
@@ -70,39 +70,39 @@ struct ktermios {
#define VTIME 17
/* c_iflag bits */
-#define IXON 0x0200
-#define IXOFF 0x0400
-#define IUCLC 0x1000
-#define IMAXBEL 0x2000
-#define IUTF8 0x4000
+#define IXON 0x0200U
+#define IXOFF 0x0400U
+#define IUCLC 0x1000U
+#define IMAXBEL 0x2000U
+#define IUTF8 0x4000U
/* c_oflag bits */
-#define ONLCR 0x00002
-#define OLCUC 0x00004
-#define NLDLY 0x00300
-#define NL0 0x00000
-#define NL1 0x00100
-#define NL2 0x00200
-#define NL3 0x00300
-#define TABDLY 0x00c00
-#define TAB0 0x00000
-#define TAB1 0x00400
-#define TAB2 0x00800
-#define TAB3 0x00c00
-#define CRDLY 0x03000
-#define CR0 0x00000
-#define CR1 0x01000
-#define CR2 0x02000
-#define CR3 0x03000
-#define FFDLY 0x04000
-#define FF0 0x00000
-#define FF1 0x04000
-#define BSDLY 0x08000
-#define BS0 0x00000
-#define BS1 0x08000
-#define VTDLY 0x10000
-#define VT0 0x00000
-#define VT1 0x10000
+#define ONLCR 0x00002U
+#define OLCUC 0x00004U
+#define NLDLY 0x00300U
+#define NL0 0x00000U
+#define NL1 0x00100U
+#define NL2 0x00200U
+#define NL3 0x00300U
+#define TABDLY 0x00c00U
+#define TAB0 0x00000U
+#define TAB1 0x00400U
+#define TAB2 0x00800U
+#define TAB3 0x00c00U
+#define CRDLY 0x03000U
+#define CR0 0x00000U
+#define CR1 0x01000U
+#define CR2 0x02000U
+#define CR3 0x03000U
+#define FFDLY 0x04000U
+#define FF0 0x00000U
+#define FF1 0x04000U
+#define BSDLY 0x08000U
+#define BS0 0x00000U
+#define BS1 0x08000U
+#define VTDLY 0x10000U
+#define VT0 0x00000U
+#define VT1 0x10000U
/*
* Should be equivalent to TAB3, see description of TAB3 in
* POSIX.1-2008, Ch. 11.2.3 "Output Modes"
@@ -110,54 +110,54 @@ struct ktermios {
#define XTABS TAB3
/* c_cflag bit meaning */
-#define CBAUD 0x0000001f
-#define CBAUDEX 0x00000000
-#define BOTHER 0x0000001f
-#define B57600 0x00000010
-#define B115200 0x00000011
-#define B230400 0x00000012
-#define B460800 0x00000013
-#define B500000 0x00000014
-#define B576000 0x00000015
-#define B921600 0x00000016
-#define B1000000 0x00000017
-#define B1152000 0x00000018
-#define B1500000 0x00000019
-#define B2000000 0x0000001a
-#define B2500000 0x0000001b
-#define B3000000 0x0000001c
-#define B3500000 0x0000001d
-#define B4000000 0x0000001e
-#define CSIZE 0x00000300
-#define CS5 0x00000000
-#define CS6 0x00000100
-#define CS7 0x00000200
-#define CS8 0x00000300
-#define CSTOPB 0x00000400
-#define CREAD 0x00000800
-#define PARENB 0x00001000
-#define PARODD 0x00002000
-#define HUPCL 0x00004000
-#define CLOCAL 0x00008000
-#define CIBAUD 0x001f0000
+#define CBAUD 0x0000001fU
+#define CBAUDEX 0x00000000U
+#define BOTHER 0x0000001fU
+#define B57600 0x00000010U
+#define B115200 0x00000011U
+#define B230400 0x00000012U
+#define B460800 0x00000013U
+#define B500000 0x00000014U
+#define B576000 0x00000015U
+#define B921600 0x00000016U
+#define B1000000 0x00000017U
+#define B1152000 0x00000018U
+#define B1500000 0x00000019U
+#define B2000000 0x0000001aU
+#define B2500000 0x0000001bU
+#define B3000000 0x0000001cU
+#define B3500000 0x0000001dU
+#define B4000000 0x0000001eU
+#define CSIZE 0x00000300U
+#define CS5 0x00000000U
+#define CS6 0x00000100U
+#define CS7 0x00000200U
+#define CS8 0x00000300U
+#define CSTOPB 0x00000400U
+#define CREAD 0x00000800U
+#define PARENB 0x00001000U
+#define PARODD 0x00002000U
+#define HUPCL 0x00004000U
+#define CLOCAL 0x00008000U
+#define CIBAUD 0x001f0000U
/* c_lflag bits */
-#define ISIG 0x00000080
-#define ICANON 0x00000100
-#define XCASE 0x00004000
-#define ECHO 0x00000008
-#define ECHOE 0x00000002
-#define ECHOK 0x00000004
-#define ECHONL 0x00000010
-#define NOFLSH 0x80000000
-#define TOSTOP 0x00400000
-#define ECHOCTL 0x00000040
-#define ECHOPRT 0x00000020
-#define ECHOKE 0x00000001
-#define FLUSHO 0x00800000
-#define PENDIN 0x20000000
-#define IEXTEN 0x00000400
-#define EXTPROC 0x10000000
+#define ISIG 0x00000080U
+#define ICANON 0x00000100U
+#define XCASE 0x00004000U
+#define ECHO 0x00000008U
+#define ECHOE 0x00000002U
+#define ECHOK 0x00000004U
+#define ECHONL 0x00000010U
+#define NOFLSH 0x80000000U
+#define TOSTOP 0x00400000U
+#define ECHOCTL 0x00000040U
+#define ECHOPRT 0x00000020U
+#define ECHOKE 0x00000001U
+#define FLUSHO 0x00800000U
+#define PENDIN 0x20000000U
+#define IEXTEN 0x00000400U
+#define EXTPROC 0x10000000U
/* Values for the OPTIONAL_ACTIONS argument to `tcsetattr'. */
#define TCSANOW 0
diff --git a/arch/mips/include/uapi/asm/termbits.h b/arch/mips/include/uapi/asm/termbits.h
index 1eb60903d6f0..35eb388a176e 100644
--- a/arch/mips/include/uapi/asm/termbits.h
+++ b/arch/mips/include/uapi/asm/termbits.h
@@ -78,96 +78,96 @@ struct ktermios {
#define VEOL 17 /* End-of-line character [ICANON] */
/* c_iflag bits */
-#define IUCLC 0x0200 /* Map upper case to lower case on input */
-#define IXON 0x0400 /* Enable start/stop output control */
-#define IXOFF 0x1000 /* Enable start/stop input control */
-#define IMAXBEL 0x2000 /* Ring bell when input queue is full */
-#define IUTF8 0x4000 /* Input is UTF-8 */
+#define IUCLC 0x0200U /* Map upper case to lower case on input */
+#define IXON 0x0400U /* Enable start/stop output control */
+#define IXOFF 0x1000U /* Enable start/stop input control */
+#define IMAXBEL 0x2000U /* Ring bell when input queue is full */
+#define IUTF8 0x4000U /* Input is UTF-8 */
/* c_oflag bits */
-#define OLCUC 0x00002 /* Map lower case to upper case on output */
-#define ONLCR 0x00004 /* Map NL to CR-NL on output */
-#define NLDLY 0x00100
-#define NL0 0x00000
-#define NL1 0x00100
-#define CRDLY 0x00600
-#define CR0 0x00000
-#define CR1 0x00200
-#define CR2 0x00400
-#define CR3 0x00600
-#define TABDLY 0x01800
-#define TAB0 0x00000
-#define TAB1 0x00800
-#define TAB2 0x01000
-#define TAB3 0x01800
-#define XTABS 0x01800
-#define BSDLY 0x02000
-#define BS0 0x00000
-#define BS1 0x02000
-#define VTDLY 0x04000
-#define VT0 0x00000
-#define VT1 0x04000
-#define FFDLY 0x08000
-#define FF0 0x00000
-#define FF1 0x08000
+#define OLCUC 0x00002U /* Map lower case to upper case on output */
+#define ONLCR 0x00004U /* Map NL to CR-NL on output */
+#define NLDLY 0x00100U
+#define NL0 0x00000U
+#define NL1 0x00100U
+#define CRDLY 0x00600U
+#define CR0 0x00000U
+#define CR1 0x00200U
+#define CR2 0x00400U
+#define CR3 0x00600U
+#define TABDLY 0x01800U
+#define TAB0 0x00000U
+#define TAB1 0x00800U
+#define TAB2 0x01000U
+#define TAB3 0x01800U
+#define XTABS 0x01800U
+#define BSDLY 0x02000U
+#define BS0 0x00000U
+#define BS1 0x02000U
+#define VTDLY 0x04000U
+#define VT0 0x00000U
+#define VT1 0x04000U
+#define FFDLY 0x08000U
+#define FF0 0x00000U
+#define FF1 0x08000U
/*
#define PAGEOUT ???
#define WRAP ???
*/
/* c_cflag bit meaning */
-#define CBAUD 0x0000100f
-#define CSIZE 0x00000030 /* Number of bits per byte (mask) */
-#define CS5 0x00000000 /* 5 bits per byte */
-#define CS6 0x00000010 /* 6 bits per byte */
-#define CS7 0x00000020 /* 7 bits per byte */
-#define CS8 0x00000030 /* 8 bits per byte */
-#define CSTOPB 0x00000040 /* Two stop bits instead of one */
-#define CREAD 0x00000080 /* Enable receiver */
-#define PARENB 0x00000100 /* Parity enable */
-#define PARODD 0x00000200 /* Odd parity instead of even */
-#define HUPCL 0x00000400 /* Hang up on last close */
-#define CLOCAL 0x00000800 /* Ignore modem status lines */
-#define CBAUDEX 0x00001000
-#define BOTHER 0x00001000
-#define B57600 0x00001001
-#define B115200 0x00001002
-#define B230400 0x00001003
-#define B460800 0x00001004
-#define B500000 0x00001005
-#define B576000 0x00001006
-#define B921600 0x00001007
-#define B1000000 0x00001008
-#define B1152000 0x00001009
-#define B1500000 0x0000100a
-#define B2000000 0x0000100b
-#define B2500000 0x0000100c
-#define B3000000 0x0000100d
-#define B3500000 0x0000100e
-#define B4000000 0x0000100f
-#define CIBAUD 0x100f0000 /* input baud rate */
+#define CBAUD 0x0000100fU
+#define CSIZE 0x00000030U /* Number of bits per byte (mask) */
+#define CS5 0x00000000U /* 5 bits per byte */
+#define CS6 0x00000010U /* 6 bits per byte */
+#define CS7 0x00000020U /* 7 bits per byte */
+#define CS8 0x00000030U /* 8 bits per byte */
+#define CSTOPB 0x00000040U /* Two stop bits instead of one */
+#define CREAD 0x00000080U /* Enable receiver */
+#define PARENB 0x00000100U /* Parity enable */
+#define PARODD 0x00000200U /* Odd parity instead of even */
+#define HUPCL 0x00000400U /* Hang up on last close */
+#define CLOCAL 0x00000800U /* Ignore modem status lines */
+#define CBAUDEX 0x00001000U
+#define BOTHER 0x00001000U
+#define B57600 0x00001001U
+#define B115200 0x00001002U
+#define B230400 0x00001003U
+#define B460800 0x00001004U
+#define B500000 0x00001005U
+#define B576000 0x00001006U
+#define B921600 0x00001007U
+#define B1000000 0x00001008U
+#define B1152000 0x00001009U
+#define B1500000 0x0000100aU
+#define B2000000 0x0000100bU
+#define B2500000 0x0000100cU
+#define B3000000 0x0000100dU
+#define B3500000 0x0000100eU
+#define B4000000 0x0000100fU
+#define CIBAUD 0x100f0000U /* input baud rate */
/* c_lflag bits */
-#define ISIG 0x00001 /* Enable signals */
-#define ICANON 0x00002 /* Do erase and kill processing */
-#define XCASE 0x00004
-#define ECHO 0x00008 /* Enable echo */
-#define ECHOE 0x00010 /* Visual erase for ERASE */
-#define ECHOK 0x00020 /* Echo NL after KILL */
-#define ECHONL 0x00040 /* Echo NL even if ECHO is off */
-#define NOFLSH 0x00080 /* Disable flush after interrupt */
-#define IEXTEN 0x00100 /* Enable DISCARD and LNEXT */
-#define ECHOCTL 0x00200 /* Echo control characters as ^X */
-#define ECHOPRT 0x00400 /* Hardcopy visual erase */
-#define ECHOKE 0x00800 /* Visual erase for KILL */
-#define FLUSHO 0x02000
-#define PENDIN 0x04000 /* Retype pending input (state) */
-#define TOSTOP 0x08000 /* Send SIGTTOU for background output */
+#define ISIG 0x00001U /* Enable signals */
+#define ICANON 0x00002U /* Do erase and kill processing */
+#define XCASE 0x00004U
+#define ECHO 0x00008U /* Enable echo */
+#define ECHOE 0x00010U /* Visual erase for ERASE */
+#define ECHOK 0x00020U /* Echo NL after KILL */
+#define ECHONL 0x00040U /* Echo NL even if ECHO is off */
+#define NOFLSH 0x00080U /* Disable flush after interrupt */
+#define IEXTEN 0x00100U /* Enable DISCARD and LNEXT */
+#define ECHOCTL 0x00200U /* Echo control characters as ^X */
+#define ECHOPRT 0x00400U /* Hardcopy visual erase */
+#define ECHOKE 0x00800U /* Visual erase for KILL */
+#define FLUSHO 0x02000U
+#define PENDIN 0x04000U /* Retype pending input (state) */
+#define TOSTOP 0x08000U /* Send SIGTTOU for background output */
#define ITOSTOP TOSTOP
-#define EXTPROC 0x10000 /* External processing on pty */
+#define EXTPROC 0x10000U /* External processing on pty */
/* ioctl (fd, TIOCSERGETLSR, &result) where result may be as below */
-#define TIOCSER_TEMT 0x01 /* Transmitter physically empty */
+#define TIOCSER_TEMT 0x01U /* Transmitter physically empty */
/* tcsetattr uses these */
#define TCSANOW TCSETS /* Change immediately */
diff --git a/arch/parisc/include/uapi/asm/termbits.h b/arch/parisc/include/uapi/asm/termbits.h
index 3a8938d26fb4..d9090878c129 100644
--- a/arch/parisc/include/uapi/asm/termbits.h
+++ b/arch/parisc/include/uapi/asm/termbits.h
@@ -58,88 +58,88 @@ struct ktermios {
#define VEOL2 16
/* c_iflag bits */
-#define IUCLC 0x0200
-#define IXON 0x0400
-#define IXOFF 0x1000
-#define IMAXBEL 0x4000
-#define IUTF8 0x8000
+#define IUCLC 0x0200U
+#define IXON 0x0400U
+#define IXOFF 0x1000U
+#define IMAXBEL 0x4000U
+#define IUTF8 0x8000U
/* c_oflag bits */
-#define OLCUC 0x00002
-#define ONLCR 0x00004
-#define NLDLY 0x00100
-#define NL0 0x00000
-#define NL1 0x00100
-#define CRDLY 0x00600
-#define CR0 0x00000
-#define CR1 0x00200
-#define CR2 0x00400
-#define CR3 0x00600
-#define TABDLY 0x01800
-#define TAB0 0x00000
-#define TAB1 0x00800
-#define TAB2 0x01000
-#define TAB3 0x01800
-#define XTABS 0x01800
-#define BSDLY 0x02000
-#define BS0 0x00000
-#define BS1 0x02000
-#define VTDLY 0x04000
-#define VT0 0x00000
-#define VT1 0x04000
-#define FFDLY 0x08000
-#define FF0 0x00000
-#define FF1 0x08000
+#define OLCUC 0x00002U
+#define ONLCR 0x00004U
+#define NLDLY 0x00100U
+#define NL0 0x00000U
+#define NL1 0x00100U
+#define CRDLY 0x00600U
+#define CR0 0x00000U
+#define CR1 0x00200U
+#define CR2 0x00400U
+#define CR3 0x00600U
+#define TABDLY 0x01800U
+#define TAB0 0x00000U
+#define TAB1 0x00800U
+#define TAB2 0x01000U
+#define TAB3 0x01800U
+#define XTABS 0x01800U
+#define BSDLY 0x02000U
+#define BS0 0x00000U
+#define BS1 0x02000U
+#define VTDLY 0x04000U
+#define VT0 0x00000U
+#define VT1 0x04000U
+#define FFDLY 0x08000U
+#define FF0 0x00000U
+#define FF1 0x08000U
/* c_cflag bit meaning */
-#define CBAUD 0x0000100f
-#define CSIZE 0x00000030
-#define CS5 0x00000000
-#define CS6 0x00000010
-#define CS7 0x00000020
-#define CS8 0x00000030
-#define CSTOPB 0x00000040
-#define CREAD 0x00000080
-#define PARENB 0x00000100
-#define PARODD 0x00000200
-#define HUPCL 0x00000400
-#define CLOCAL 0x00000800
-#define CBAUDEX 0x00001000
-#define BOTHER 0x00001000
-#define B57600 0x00001001
-#define B115200 0x00001002
-#define B230400 0x00001003
-#define B460800 0x00001004
-#define B500000 0x00001005
-#define B576000 0x00001006
-#define B921600 0x00001007
-#define B1000000 0x00001008
-#define B1152000 0x00001009
-#define B1500000 0x0000100a
-#define B2000000 0x0000100b
-#define B2500000 0x0000100c
-#define B3000000 0x0000100d
-#define B3500000 0x0000100e
-#define B4000000 0x0000100f
-#define CIBAUD 0x100f0000 /* input baud rate */
+#define CBAUD 0x0000100fU
+#define CSIZE 0x00000030U
+#define CS5 0x00000000U
+#define CS6 0x00000010U
+#define CS7 0x00000020U
+#define CS8 0x00000030U
+#define CSTOPB 0x00000040U
+#define CREAD 0x00000080U
+#define PARENB 0x00000100U
+#define PARODD 0x00000200U
+#define HUPCL 0x00000400U
+#define CLOCAL 0x00000800U
+#define CBAUDEX 0x00001000U
+#define BOTHER 0x00001000U
+#define B57600 0x00001001U
+#define B115200 0x00001002U
+#define B230400 0x00001003U
+#define B460800 0x00001004U
+#define B500000 0x00001005U
+#define B576000 0x00001006U
+#define B921600 0x00001007U
+#define B1000000 0x00001008U
+#define B1152000 0x00001009U
+#define B1500000 0x0000100aU
+#define B2000000 0x0000100bU
+#define B2500000 0x0000100cU
+#define B3000000 0x0000100dU
+#define B3500000 0x0000100eU
+#define B4000000 0x0000100fU
+#define CIBAUD 0x100f0000U /* input baud rate */
/* c_lflag bits */
-#define ISIG 0x00001
-#define ICANON 0x00002
-#define XCASE 0x00004
-#define ECHO 0x00008
-#define ECHOE 0x00010
-#define ECHOK 0x00020
-#define ECHONL 0x00040
-#define NOFLSH 0x00080
-#define TOSTOP 0x00100
-#define ECHOCTL 0x00200
-#define ECHOPRT 0x00400
-#define ECHOKE 0x00800
-#define FLUSHO 0x01000
-#define PENDIN 0x04000
-#define IEXTEN 0x08000
-#define EXTPROC 0x10000
+#define ISIG 0x00001U
+#define ICANON 0x00002U
+#define XCASE 0x00004U
+#define ECHO 0x00008U
+#define ECHOE 0x00010U
+#define ECHOK 0x00020U
+#define ECHONL 0x00040U
+#define NOFLSH 0x00080U
+#define TOSTOP 0x00100U
+#define ECHOCTL 0x00200U
+#define ECHOPRT 0x00400U
+#define ECHOKE 0x00800U
+#define FLUSHO 0x01000U
+#define PENDIN 0x04000U
+#define IEXTEN 0x08000U
+#define EXTPROC 0x10000U
/* tcsetattr uses these */
#define TCSANOW 0
diff --git a/arch/powerpc/include/uapi/asm/termbits.h b/arch/powerpc/include/uapi/asm/termbits.h
index 21dc86dcb2f1..ae3c2ed3dae8 100644
--- a/arch/powerpc/include/uapi/asm/termbits.h
+++ b/arch/powerpc/include/uapi/asm/termbits.h
@@ -64,90 +64,90 @@ struct ktermios {
#define VDISCARD 16
/* c_iflag bits */
-#define IXON 0x0200
-#define IXOFF 0x0400
-#define IUCLC 0x1000
-#define IMAXBEL 0x2000
-#define IUTF8 0x4000
+#define IXON 0x0200U
+#define IXOFF 0x0400U
+#define IUCLC 0x1000U
+#define IMAXBEL 0x2000U
+#define IUTF8 0x4000U
/* c_oflag bits */
-#define ONLCR 0x00002
-#define OLCUC 0x00004
-#define NLDLY 0x00300
-#define NL0 0x00000
-#define NL1 0x00100
-#define NL2 0x00200
-#define NL3 0x00300
-#define TABDLY 0x00c00
-#define TAB0 0x00000
-#define TAB1 0x00400
-#define TAB2 0x00800
-#define TAB3 0x00c00
-#define XTABS 0x00c00 /* required by POSIX to == TAB3 */
-#define CRDLY 0x03000
-#define CR0 0x00000
-#define CR1 0x01000
-#define CR2 0x02000
-#define CR3 0x03000
-#define FFDLY 0x04000
-#define FF0 0x00000
-#define FF1 0x04000
-#define BSDLY 0x08000
-#define BS0 0x00000
-#define BS1 0x08000
-#define VTDLY 0x10000
-#define VT0 0x00000
-#define VT1 0x10000
+#define ONLCR 0x00002U
+#define OLCUC 0x00004U
+#define NLDLY 0x00300U
+#define NL0 0x00000U
+#define NL1 0x00100U
+#define NL2 0x00200U
+#define NL3 0x00300U
+#define TABDLY 0x00c00U
+#define TAB0 0x00000U
+#define TAB1 0x00400U
+#define TAB2 0x00800U
+#define TAB3 0x00c00U
+#define XTABS 0x00c00U /* required by POSIX to == TAB3 */
+#define CRDLY 0x03000U
+#define CR0 0x00000U
+#define CR1 0x01000U
+#define CR2 0x02000U
+#define CR3 0x03000U
+#define FFDLY 0x04000U
+#define FF0 0x00000U
+#define FF1 0x04000U
+#define BSDLY 0x08000U
+#define BS0 0x00000U
+#define BS1 0x08000U
+#define VTDLY 0x10000U
+#define VT0 0x00000U
+#define VT1 0x10000U
/* c_cflag bit meaning */
-#define CBAUD 0x000000ff
-#define CBAUDEX 0x00000000
-#define BOTHER 0x0000001f
-#define B57600 0x00000010
-#define B115200 0x00000011
-#define B230400 0x00000012
-#define B460800 0x00000013
-#define B500000 0x00000014
-#define B576000 0x00000015
-#define B921600 0x00000016
-#define B1000000 0x00000017
-#define B1152000 0x00000018
-#define B1500000 0x00000019
-#define B2000000 0x0000001a
-#define B2500000 0x0000001b
-#define B3000000 0x0000001c
-#define B3500000 0x0000001d
-#define B4000000 0x0000001e
-#define CSIZE 0x00000300
-#define CS5 0x00000000
-#define CS6 0x00000100
-#define CS7 0x00000200
-#define CS8 0x00000300
-#define CSTOPB 0x00000400
-#define CREAD 0x00000800
-#define PARENB 0x00001000
-#define PARODD 0x00002000
-#define HUPCL 0x00004000
-#define CLOCAL 0x00008000
-#define CIBAUD 0x00ff0000
+#define CBAUD 0x000000ffU
+#define CBAUDEX 0x00000000U
+#define BOTHER 0x0000001fU
+#define B57600 0x00000010U
+#define B115200 0x00000011U
+#define B230400 0x00000012U
+#define B460800 0x00000013U
+#define B500000 0x00000014U
+#define B576000 0x00000015U
+#define B921600 0x00000016U
+#define B1000000 0x00000017U
+#define B1152000 0x00000018U
+#define B1500000 0x00000019U
+#define B2000000 0x0000001aU
+#define B2500000 0x0000001bU
+#define B3000000 0x0000001cU
+#define B3500000 0x0000001dU
+#define B4000000 0x0000001eU
+#define CSIZE 0x00000300U
+#define CS5 0x00000000U
+#define CS6 0x00000100U
+#define CS7 0x00000200U
+#define CS8 0x00000300U
+#define CSTOPB 0x00000400U
+#define CREAD 0x00000800U
+#define PARENB 0x00001000U
+#define PARODD 0x00002000U
+#define HUPCL 0x00004000U
+#define CLOCAL 0x00008000U
+#define CIBAUD 0x00ff0000U
/* c_lflag bits */
-#define ISIG 0x00000080
-#define ICANON 0x00000100
-#define XCASE 0x00004000
-#define ECHO 0x00000008
-#define ECHOE 0x00000002
-#define ECHOK 0x00000004
-#define ECHONL 0x00000010
-#define NOFLSH 0x80000000
-#define TOSTOP 0x00400000
-#define ECHOCTL 0x00000040
-#define ECHOPRT 0x00000020
-#define ECHOKE 0x00000001
-#define FLUSHO 0x00800000
-#define PENDIN 0x20000000
-#define IEXTEN 0x00000400
-#define EXTPROC 0x10000000
+#define ISIG 0x00000080U
+#define ICANON 0x00000100U
+#define XCASE 0x00004000U
+#define ECHO 0x00000008U
+#define ECHOE 0x00000002U
+#define ECHOK 0x00000004U
+#define ECHONL 0x00000010U
+#define NOFLSH 0x80000000U
+#define TOSTOP 0x00400000U
+#define ECHOCTL 0x00000040U
+#define ECHOPRT 0x00000020U
+#define ECHOKE 0x00000001U
+#define FLUSHO 0x00800000U
+#define PENDIN 0x20000000U
+#define IEXTEN 0x00000400U
+#define EXTPROC 0x10000000U
/* Values for the OPTIONAL_ACTIONS argument to `tcsetattr'. */
#define TCSANOW 0
diff --git a/arch/sparc/include/uapi/asm/termbits.h b/arch/sparc/include/uapi/asm/termbits.h
index 0da2b1adc0f5..5cd0b28b16dc 100644
--- a/arch/sparc/include/uapi/asm/termbits.h
+++ b/arch/sparc/include/uapi/asm/termbits.h
@@ -75,121 +75,121 @@ struct ktermios {
#endif
/* c_iflag bits */
-#define IUCLC 0x0200
-#define IXON 0x0400
-#define IXOFF 0x1000
-#define IMAXBEL 0x2000
-#define IUTF8 0x4000
+#define IUCLC 0x0200U
+#define IXON 0x0400U
+#define IXOFF 0x1000U
+#define IMAXBEL 0x2000U
+#define IUTF8 0x4000U
/* c_oflag bits */
-#define OLCUC 0x00002
-#define ONLCR 0x00004
-#define NLDLY 0x00100
-#define NL0 0x00000
-#define NL1 0x00100
-#define CRDLY 0x00600
-#define CR0 0x00000
-#define CR1 0x00200
-#define CR2 0x00400
-#define CR3 0x00600
-#define TABDLY 0x01800
-#define TAB0 0x00000
-#define TAB1 0x00800
-#define TAB2 0x01000
-#define TAB3 0x01800
-#define XTABS 0x01800
-#define BSDLY 0x02000
-#define BS0 0x00000
-#define BS1 0x02000
-#define VTDLY 0x04000
-#define VT0 0x00000
-#define VT1 0x04000
-#define FFDLY 0x08000
-#define FF0 0x00000
-#define FF1 0x08000
-#define PAGEOUT 0x10000 /* SUNOS specific */
-#define WRAP 0x20000 /* SUNOS specific */
+#define OLCUC 0x00002U
+#define ONLCR 0x00004U
+#define NLDLY 0x00100U
+#define NL0 0x00000U
+#define NL1 0x00100U
+#define CRDLY 0x00600U
+#define CR0 0x00000U
+#define CR1 0x00200U
+#define CR2 0x00400U
+#define CR3 0x00600U
+#define TABDLY 0x01800U
+#define TAB0 0x00000U
+#define TAB1 0x00800U
+#define TAB2 0x01000U
+#define TAB3 0x01800U
+#define XTABS 0x01800U
+#define BSDLY 0x02000U
+#define BS0 0x00000U
+#define BS1 0x02000U
+#define VTDLY 0x04000U
+#define VT0 0x00000U
+#define VT1 0x04000U
+#define FFDLY 0x08000U
+#define FF0 0x00000U
+#define FF1 0x08000U
+#define PAGEOUT 0x10000U /* SUNOS specific */
+#define WRAP 0x20000U /* SUNOS specific */
/* c_cflag bit meaning */
-#define CBAUD 0x0000100f
-#define CSIZE 0x00000030
-#define CS5 0x00000000
-#define CS6 0x00000010
-#define CS7 0x00000020
-#define CS8 0x00000030
-#define CSTOPB 0x00000040
-#define CREAD 0x00000080
-#define PARENB 0x00000100
-#define PARODD 0x00000200
-#define HUPCL 0x00000400
-#define CLOCAL 0x00000800
-#define CBAUDEX 0x00001000
+#define CBAUD 0x0000100fU
+#define CSIZE 0x00000030U
+#define CS5 0x00000000U
+#define CS6 0x00000010U
+#define CS7 0x00000020U
+#define CS8 0x00000030U
+#define CSTOPB 0x00000040U
+#define CREAD 0x00000080U
+#define PARENB 0x00000100U
+#define PARODD 0x00000200U
+#define HUPCL 0x00000400U
+#define CLOCAL 0x00000800U
+#define CBAUDEX 0x00001000U
/* We'll never see these speeds with the Zilogs, but for completeness... */
-#define BOTHER 0x00001000
-#define B57600 0x00001001
-#define B115200 0x00001002
-#define B230400 0x00001003
-#define B460800 0x00001004
+#define BOTHER 0x00001000U
+#define B57600 0x00001001U
+#define B115200 0x00001002U
+#define B230400 0x00001003U
+#define B460800 0x00001004U
/* This is what we can do with the Zilogs. */
-#define B76800 0x00001005
+#define B76800 0x00001005U
/* This is what we can do with the SAB82532. */
-#define B153600 0x00001006
-#define B307200 0x00001007
-#define B614400 0x00001008
-#define B921600 0x00001009
+#define B153600 0x00001006U
+#define B307200 0x00001007U
+#define B614400 0x00001008U
+#define B921600 0x00001009U
/* And these are the rest... */
-#define B500000 0x0000100a
-#define B576000 0x0000100b
-#define B1000000 0x0000100c
-#define B1152000 0x0000100d
-#define B1500000 0x0000100e
-#define B2000000 0x0000100f
+#define B500000 0x0000100aU
+#define B576000 0x0000100bU
+#define B1000000 0x0000100cU
+#define B1152000 0x0000100dU
+#define B1500000 0x0000100eU
+#define B2000000 0x0000100fU
/* These have totally bogus values and nobody uses them
so far. Later on we'd have to use say 0x10000x and
adjust CBAUD constant and drivers accordingly.
-#define B2500000 0x00001010
-#define B3000000 0x00001011
-#define B3500000 0x00001012
-#define B4000000 0x00001013 */
-#define CIBAUD 0x100f0000 /* input baud rate (not used) */
+#define B2500000 0x00001010U
+#define B3000000 0x00001011U
+#define B3500000 0x00001012U
+#define B4000000 0x00001013U */
+#define CIBAUD 0x100f0000U /* input baud rate (not used) */
/* c_lflag bits */
-#define ISIG 0x00000001
-#define ICANON 0x00000002
-#define XCASE 0x00000004
-#define ECHO 0x00000008
-#define ECHOE 0x00000010
-#define ECHOK 0x00000020
-#define ECHONL 0x00000040
-#define NOFLSH 0x00000080
-#define TOSTOP 0x00000100
-#define ECHOCTL 0x00000200
-#define ECHOPRT 0x00000400
-#define ECHOKE 0x00000800
-#define DEFECHO 0x00001000 /* SUNOS thing, what is it? */
-#define FLUSHO 0x00002000
-#define PENDIN 0x00004000
-#define IEXTEN 0x00008000
-#define EXTPROC 0x00010000
+#define ISIG 0x00000001U
+#define ICANON 0x00000002U
+#define XCASE 0x00000004U
+#define ECHO 0x00000008U
+#define ECHOE 0x00000010U
+#define ECHOK 0x00000020U
+#define ECHONL 0x00000040U
+#define NOFLSH 0x00000080U
+#define TOSTOP 0x00000100U
+#define ECHOCTL 0x00000200U
+#define ECHOPRT 0x00000400U
+#define ECHOKE 0x00000800U
+#define DEFECHO 0x00001000U /* SUNOS thing, what is it? */
+#define FLUSHO 0x00002000U
+#define PENDIN 0x00004000U
+#define IEXTEN 0x00008000U
+#define EXTPROC 0x00010000U
/* modem lines */
-#define TIOCM_LE 0x001
-#define TIOCM_DTR 0x002
-#define TIOCM_RTS 0x004
-#define TIOCM_ST 0x008
-#define TIOCM_SR 0x010
-#define TIOCM_CTS 0x020
-#define TIOCM_CAR 0x040
-#define TIOCM_RNG 0x080
-#define TIOCM_DSR 0x100
+#define TIOCM_LE 0x001U
+#define TIOCM_DTR 0x002U
+#define TIOCM_RTS 0x004U
+#define TIOCM_ST 0x008U
+#define TIOCM_SR 0x010U
+#define TIOCM_CTS 0x020U
+#define TIOCM_CAR 0x040U
+#define TIOCM_RNG 0x080U
+#define TIOCM_DSR 0x100U
#define TIOCM_CD TIOCM_CAR
#define TIOCM_RI TIOCM_RNG
-#define TIOCM_OUT1 0x2000
-#define TIOCM_OUT2 0x4000
-#define TIOCM_LOOP 0x8000
+#define TIOCM_OUT1 0x2000U
+#define TIOCM_OUT2 0x4000U
+#define TIOCM_LOOP 0x8000U
/* ioctl (fd, TIOCSERGETLSR, &result) where result may be as below */
-#define TIOCSER_TEMT 0x01 /* Transmitter physically empty */
+#define TIOCSER_TEMT 0x01U /* Transmitter physically empty */
/* tcsetattr uses these */
#define TCSANOW 0
diff --git a/include/uapi/asm-generic/termbits.h b/include/uapi/asm-generic/termbits.h
index 890ef29053e2..10e01c77d2a6 100644
--- a/include/uapi/asm-generic/termbits.h
+++ b/include/uapi/asm-generic/termbits.h
@@ -58,88 +58,88 @@ struct ktermios {
#define VEOL2 16
/* c_iflag bits */
-#define IUCLC 0x0200
-#define IXON 0x0400
-#define IXOFF 0x1000
-#define IMAXBEL 0x2000
-#define IUTF8 0x4000
+#define IUCLC 0x0200U
+#define IXON 0x0400U
+#define IXOFF 0x1000U
+#define IMAXBEL 0x2000U
+#define IUTF8 0x4000U
/* c_oflag bits */
-#define OLCUC 0x00002
-#define ONLCR 0x00004
-#define NLDLY 0x00100
-#define NL0 0x00000
-#define NL1 0x00100
-#define CRDLY 0x00600
-#define CR0 0x00000
-#define CR1 0x00200
-#define CR2 0x00400
-#define CR3 0x00600
-#define TABDLY 0x01800
-#define TAB0 0x00000
-#define TAB1 0x00800
-#define TAB2 0x01000
-#define TAB3 0x01800
-#define XTABS 0x01800
-#define BSDLY 0x02000
-#define BS0 0x00000
-#define BS1 0x02000
-#define VTDLY 0x04000
-#define VT0 0x00000
-#define VT1 0x04000
-#define FFDLY 0x08000
-#define FF0 0x00000
-#define FF1 0x08000
+#define OLCUC 0x00002U
+#define ONLCR 0x00004U
+#define NLDLY 0x00100U
+#define NL0 0x00000U
+#define NL1 0x00100U
+#define CRDLY 0x00600U
+#define CR0 0x00000U
+#define CR1 0x00200U
+#define CR2 0x00400U
+#define CR3 0x00600U
+#define TABDLY 0x01800U
+#define TAB0 0x00000U
+#define TAB1 0x00800U
+#define TAB2 0x01000U
+#define TAB3 0x01800U
+#define XTABS 0x01800U
+#define BSDLY 0x02000U
+#define BS0 0x00000U
+#define BS1 0x02000U
+#define VTDLY 0x04000U
+#define VT0 0x00000U
+#define VT1 0x04000U
+#define FFDLY 0x08000U
+#define FF0 0x00000U
+#define FF1 0x08000U
/* c_cflag bit meaning */
-#define CBAUD 0x0000100f
-#define CSIZE 0x00000030
-#define CS5 0x00000000
-#define CS6 0x00000010
-#define CS7 0x00000020
-#define CS8 0x00000030
-#define CSTOPB 0x00000040
-#define CREAD 0x00000080
-#define PARENB 0x00000100
-#define PARODD 0x00000200
-#define HUPCL 0x00000400
-#define CLOCAL 0x00000800
-#define CBAUDEX 0x00001000
-#define BOTHER 0x00001000
-#define B57600 0x00001001
-#define B115200 0x00001002
-#define B230400 0x00001003
-#define B460800 0x00001004
-#define B500000 0x00001005
-#define B576000 0x00001006
-#define B921600 0x00001007
-#define B1000000 0x00001008
-#define B1152000 0x00001009
-#define B1500000 0x0000100a
-#define B2000000 0x0000100b
-#define B2500000 0x0000100c
-#define B3000000 0x0000100d
-#define B3500000 0x0000100e
-#define B4000000 0x0000100f
-#define CIBAUD 0x100f0000 /* input baud rate */
+#define CBAUD 0x0000100fU
+#define CSIZE 0x00000030U
+#define CS5 0x00000000U
+#define CS6 0x00000010U
+#define CS7 0x00000020U
+#define CS8 0x00000030U
+#define CSTOPB 0x00000040U
+#define CREAD 0x00000080U
+#define PARENB 0x00000100U
+#define PARODD 0x00000200U
+#define HUPCL 0x00000400U
+#define CLOCAL 0x00000800U
+#define CBAUDEX 0x00001000U
+#define BOTHER 0x00001000U
+#define B57600 0x00001001U
+#define B115200 0x00001002U
+#define B230400 0x00001003U
+#define B460800 0x00001004U
+#define B500000 0x00001005U
+#define B576000 0x00001006U
+#define B921600 0x00001007U
+#define B1000000 0x00001008U
+#define B1152000 0x00001009U
+#define B1500000 0x0000100aU
+#define B2000000 0x0000100bU
+#define B2500000 0x0000100cU
+#define B3000000 0x0000100dU
+#define B3500000 0x0000100eU
+#define B4000000 0x0000100fU
+#define CIBAUD 0x100f0000U /* input baud rate */
/* c_lflag bits */
-#define ISIG 0x00001
-#define ICANON 0x00002
-#define XCASE 0x00004
-#define ECHO 0x00008
-#define ECHOE 0x00010
-#define ECHOK 0x00020
-#define ECHONL 0x00040
-#define NOFLSH 0x00080
-#define TOSTOP 0x00100
-#define ECHOCTL 0x00200
-#define ECHOPRT 0x00400
-#define ECHOKE 0x00800
-#define FLUSHO 0x01000
-#define PENDIN 0x04000
-#define IEXTEN 0x08000
-#define EXTPROC 0x10000
+#define ISIG 0x00001U
+#define ICANON 0x00002U
+#define XCASE 0x00004U
+#define ECHO 0x00008U
+#define ECHOE 0x00010U
+#define ECHOK 0x00020U
+#define ECHONL 0x00040U
+#define NOFLSH 0x00080U
+#define TOSTOP 0x00100U
+#define ECHOCTL 0x00200U
+#define ECHOPRT 0x00400U
+#define ECHOKE 0x00800U
+#define FLUSHO 0x01000U
+#define PENDIN 0x04000U
+#define IEXTEN 0x08000U
+#define EXTPROC 0x10000U
/* tcsetattr uses these */
#define TCSANOW 0
base-commit: 83a7eefedc9b56fe7bfeff13b6c7356688ffa670
--
2.45.1
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* Re: termios constants should be unsigned
From: Greg KH @ 2024-06-12 12:22 UTC (permalink / raw)
To: Alejandro Colomar
Cc: Andrew Morton, Palmer Dabbelt, linux-api, libc-alpha, linux-man
In-Reply-To: <a7kfppfptkzvqys6cblwjudlpoghsycjglw57hxe2ywvruzkbd@e6nqpnxgwfnq>
On Wed, Jun 12, 2024 at 02:16:12PM +0200, Alejandro Colomar wrote:
> Hi!
>
> While compiling an example program in ioctl_tty(2) examples (now moving
> to TCSETS(2const)), I saw several warnings:
>
> tcgets.c:53:24: error: implicit conversion changes signedness: 'int' to 'tcflag_t' (aka 'unsigned int') [clang-diagnostic-sign-conversion,-warnings-as-errors]
> tio.c_cflag &= ~CBAUD;
> ~~ ^~~~~~
> tcgets.c:53:24: warning: use of a signed integer operand with a binary bitwise operator [hicpp-signed-bitwise]
> tio.c_cflag &= ~CBAUD;
> ~~ ^~~~~~
> tcgets.c:58:24: error: implicit conversion changes signedness: 'int' to 'tcflag_t' (aka 'unsigned int') [clang-diagnostic-sign-conversion,-warnings-as-errors]
> tio.c_cflag &= ~(CBAUD << IBSHIFT);
> ~~ ^~~~~~~~~~~~~~~~~~~
> tcgets.c:58:24: warning: use of a signed integer operand with a binary bitwise operator [hicpp-signed-bitwise]
> tio.c_cflag &= ~(CBAUD << IBSHIFT);
> ~~ ^~~~~~~~~~~~~~~~~~~
> tcgets.c:58:25: warning: use of a signed integer operand with a unary bitwise operator [hicpp-signed-bitwise]
> tio.c_cflag &= ~(CBAUD << IBSHIFT);
> ~^~~~~~~~~~~~~~~~~~
>
> Which seem reasonable warnings. Bitwise operations on signed integers
> is bad for ye health. Let's see the definitions of struct termios:
>
> alx@debian:/usr/include$ grepc termios x86_64-linux-gnu/ asm-generic/
> x86_64-linux-gnu/bits/termios-struct.h:struct termios
> {
> tcflag_t c_iflag; /* input mode flags */
> tcflag_t c_oflag; /* output mode flags */
> tcflag_t c_cflag; /* control mode flags */
> tcflag_t c_lflag; /* local mode flags */
> cc_t c_line; /* line discipline */
> cc_t c_cc[NCCS]; /* control characters */
> speed_t c_ispeed; /* input speed */
> speed_t c_ospeed; /* output speed */
> #define _HAVE_STRUCT_TERMIOS_C_ISPEED 1
> #define _HAVE_STRUCT_TERMIOS_C_OSPEED 1
> };
> asm-generic/termbits.h:struct termios {
> tcflag_t c_iflag; /* input mode flags */
> tcflag_t c_oflag; /* output mode flags */
> tcflag_t c_cflag; /* control mode flags */
> tcflag_t c_lflag; /* local mode flags */
> cc_t c_line; /* line discipline */
> cc_t c_cc[NCCS]; /* control characters */
> };
>
> Except for the speed members, they seem the same. The types of the
> members are correctly unsigned:
>
> alx@debian:/usr/include$ grepc tcflag_t x86_64-linux-gnu/ asm-generic/
> x86_64-linux-gnu/bits/termios.h:typedef unsigned int tcflag_t;
> asm-generic/termbits.h:typedef unsigned int tcflag_t;
>
> So, all of the constants that are to be used in these members, which
> will undergo bitwise operations, should use the U suffix. Here's the
> list for the UAPI:
>
> $ sed -n '/flag bits/,/^$/p' asm-generic/termbits.h
> /* c_iflag bits */
> #define IUCLC 0x0200
> #define IXON 0x0400
> #define IXOFF 0x1000
> #define IMAXBEL 0x2000
> #define IUTF8 0x4000
>
> /* c_oflag bits */
> #define OLCUC 0x00002
> #define ONLCR 0x00004
> #define NLDLY 0x00100
> #define NL0 0x00000
> #define NL1 0x00100
> #define CRDLY 0x00600
> #define CR0 0x00000
> #define CR1 0x00200
> #define CR2 0x00400
> #define CR3 0x00600
> #define TABDLY 0x01800
> #define TAB0 0x00000
> #define TAB1 0x00800
> #define TAB2 0x01000
> #define TAB3 0x01800
> #define XTABS 0x01800
> #define BSDLY 0x02000
> #define BS0 0x00000
> #define BS1 0x02000
> #define VTDLY 0x04000
> #define VT0 0x00000
> #define VT1 0x04000
> #define FFDLY 0x08000
> #define FF0 0x00000
> #define FF1 0x08000
>
> /* c_lflag bits */
> #define ISIG 0x00001
> #define ICANON 0x00002
> #define XCASE 0x00004
> #define ECHO 0x00008
> #define ECHOE 0x00010
> #define ECHOK 0x00020
> #define ECHONL 0x00040
> #define NOFLSH 0x00080
> #define TOSTOP 0x00100
> #define ECHOCTL 0x00200
> #define ECHOPRT 0x00400
> #define ECHOKE 0x00800
> #define FLUSHO 0x01000
> #define PENDIN 0x04000
> #define IEXTEN 0x08000
> #define EXTPROC 0x10000
>
> And glibc also (re)defines some of them, so those should also have the
> U suffix.
>
> Any opinions?
Have a proposed patch that you feel would resolve this?
thanks,
greg k-h
^ permalink raw reply
* termios constants should be unsigned
From: Alejandro Colomar @ 2024-06-12 12:16 UTC (permalink / raw)
To: Andrew Morton, Palmer Dabbelt, linux-api, libc-alpha, linux-man
[-- Attachment #1: Type: text/plain, Size: 4114 bytes --]
Hi!
While compiling an example program in ioctl_tty(2) examples (now moving
to TCSETS(2const)), I saw several warnings:
tcgets.c:53:24: error: implicit conversion changes signedness: 'int' to 'tcflag_t' (aka 'unsigned int') [clang-diagnostic-sign-conversion,-warnings-as-errors]
tio.c_cflag &= ~CBAUD;
~~ ^~~~~~
tcgets.c:53:24: warning: use of a signed integer operand with a binary bitwise operator [hicpp-signed-bitwise]
tio.c_cflag &= ~CBAUD;
~~ ^~~~~~
tcgets.c:58:24: error: implicit conversion changes signedness: 'int' to 'tcflag_t' (aka 'unsigned int') [clang-diagnostic-sign-conversion,-warnings-as-errors]
tio.c_cflag &= ~(CBAUD << IBSHIFT);
~~ ^~~~~~~~~~~~~~~~~~~
tcgets.c:58:24: warning: use of a signed integer operand with a binary bitwise operator [hicpp-signed-bitwise]
tio.c_cflag &= ~(CBAUD << IBSHIFT);
~~ ^~~~~~~~~~~~~~~~~~~
tcgets.c:58:25: warning: use of a signed integer operand with a unary bitwise operator [hicpp-signed-bitwise]
tio.c_cflag &= ~(CBAUD << IBSHIFT);
~^~~~~~~~~~~~~~~~~~
Which seem reasonable warnings. Bitwise operations on signed integers
is bad for ye health. Let's see the definitions of struct termios:
alx@debian:/usr/include$ grepc termios x86_64-linux-gnu/ asm-generic/
x86_64-linux-gnu/bits/termios-struct.h:struct termios
{
tcflag_t c_iflag; /* input mode flags */
tcflag_t c_oflag; /* output mode flags */
tcflag_t c_cflag; /* control mode flags */
tcflag_t c_lflag; /* local mode flags */
cc_t c_line; /* line discipline */
cc_t c_cc[NCCS]; /* control characters */
speed_t c_ispeed; /* input speed */
speed_t c_ospeed; /* output speed */
#define _HAVE_STRUCT_TERMIOS_C_ISPEED 1
#define _HAVE_STRUCT_TERMIOS_C_OSPEED 1
};
asm-generic/termbits.h:struct termios {
tcflag_t c_iflag; /* input mode flags */
tcflag_t c_oflag; /* output mode flags */
tcflag_t c_cflag; /* control mode flags */
tcflag_t c_lflag; /* local mode flags */
cc_t c_line; /* line discipline */
cc_t c_cc[NCCS]; /* control characters */
};
Except for the speed members, they seem the same. The types of the
members are correctly unsigned:
alx@debian:/usr/include$ grepc tcflag_t x86_64-linux-gnu/ asm-generic/
x86_64-linux-gnu/bits/termios.h:typedef unsigned int tcflag_t;
asm-generic/termbits.h:typedef unsigned int tcflag_t;
So, all of the constants that are to be used in these members, which
will undergo bitwise operations, should use the U suffix. Here's the
list for the UAPI:
$ sed -n '/flag bits/,/^$/p' asm-generic/termbits.h
/* c_iflag bits */
#define IUCLC 0x0200
#define IXON 0x0400
#define IXOFF 0x1000
#define IMAXBEL 0x2000
#define IUTF8 0x4000
/* c_oflag bits */
#define OLCUC 0x00002
#define ONLCR 0x00004
#define NLDLY 0x00100
#define NL0 0x00000
#define NL1 0x00100
#define CRDLY 0x00600
#define CR0 0x00000
#define CR1 0x00200
#define CR2 0x00400
#define CR3 0x00600
#define TABDLY 0x01800
#define TAB0 0x00000
#define TAB1 0x00800
#define TAB2 0x01000
#define TAB3 0x01800
#define XTABS 0x01800
#define BSDLY 0x02000
#define BS0 0x00000
#define BS1 0x02000
#define VTDLY 0x04000
#define VT0 0x00000
#define VT1 0x04000
#define FFDLY 0x08000
#define FF0 0x00000
#define FF1 0x08000
/* c_lflag bits */
#define ISIG 0x00001
#define ICANON 0x00002
#define XCASE 0x00004
#define ECHO 0x00008
#define ECHOE 0x00010
#define ECHOK 0x00020
#define ECHONL 0x00040
#define NOFLSH 0x00080
#define TOSTOP 0x00100
#define ECHOCTL 0x00200
#define ECHOPRT 0x00400
#define ECHOKE 0x00800
#define FLUSHO 0x01000
#define PENDIN 0x04000
#define IEXTEN 0x08000
#define EXTPROC 0x10000
And glibc also (re)defines some of them, so those should also have the
U suffix.
Any opinions?
Have a lovely day!
Alex
--
<https://www.alejandro-colomar.es/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v1 0/2] uapi/linux/prctl: Use the L and UL integer suffixes for certain constants
From: Alejandro Colomar @ 2024-06-12 12:02 UTC (permalink / raw)
To: linux-api, Andrew Morton, Palmer Dabbelt; +Cc: linux-man, libc-alpha
In-Reply-To: <20240528114750.106187-1-alx@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 809 bytes --]
[TO += Andrew, Palmer]
Hi Andrew, Palmer,
On Tue, May 28, 2024 at 01:48:05PM GMT, Alejandro Colomar wrote:
>
> Alejandro Colomar (2):
> uapi/linux/prctl: Use the L integer suffix for enumerations of width
> long
> uapi/linux/prctl: Use the UL integer suffix for bit fields of width
> long
Ping.
Have a lovely day!
Alex
> include/uapi/linux/prctl.h | 186 ++++++++++++++++++-------------------
> 1 file changed, 93 insertions(+), 93 deletions(-)
>
> Range-diff against v0:
> -: ------------ > 1: eb1cdf3e2f33 uapi/linux/prctl: Use the L integer suffix for enumerations of width long
> -: ------------ > 2: 16f5bd565191 uapi/linux/prctl: Use the UL integer suffix for bit fields of width long
> --
> 2.45.1
>
--
<https://www.alejandro-colomar.es/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCHv8 bpf-next 0/9] uprobe: uretprobe speed up
From: Andrii Nakryiko @ 2024-06-12 7:02 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Jiri Olsa, Steven Rostedt, Oleg Nesterov, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, linux-kernel,
linux-trace-kernel, linux-api, linux-man, x86, bpf, Song Liu,
Yonghong Song, John Fastabend, Peter Zijlstra, Thomas Gleixner,
Borislav Petkov (AMD), Ingo Molnar, Andy Lutomirski,
Edgecombe, Rick P, Deepak Gupta
In-Reply-To: <20240611235228.f70de698f8e43a3c177a15b8@kernel.org>
On Tue, Jun 11, 2024 at 3:52 PM Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> On Tue, 11 Jun 2024 13:21:49 +0200
> Jiri Olsa <jolsa@kernel.org> wrote:
>
> > hi,
> > as part of the effort on speeding up the uprobes [0] coming with
> > return uprobe optimization by using syscall instead of the trap
> > on the uretprobe trampoline.
> >
> > The speed up depends on instruction type that uprobe is installed
> > and depends on specific HW type, please check patch 1 for details.
> >
> > Patches 1-8 are based on bpf-next/master, but patch 2 and 3 are
> > apply-able on linux-trace.git tree probes/for-next branch.
> > Patch 9 is based on man-pages master.
> >
> > v8 changes:
> > - rebased (another new syscall got merged)
> > - added acks
> >
> > Also available at:
> > https://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
> > uretprobe_syscall
>
> Applied patch [1/9] - [8/9] on probes/for-next in
> git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace.git
>
Thanks, Masami, looks good! I also confirmed that probes/for-next and
bpf-next/master can be merged with no conflicts (hopefully it stays
this way till the next merge window).
> Thank you!
>
> >
> > thanks,
> > jirka
> >
> >
> > Notes to check list items in Documentation/process/adding-syscalls.rst:
> >
> > - System Call Alternatives
> > New syscall seems like the best way in here, because we need
> > just to quickly enter kernel with no extra arguments processing,
> > which we'd need to do if we decided to use another syscall.
> >
> > - Designing the API: Planning for Extension
> > The uretprobe syscall is very specific and most likely won't be
> > extended in the future.
> >
> > At the moment it does not take any arguments and even if it does
> > in future, it's allowed to be called only from trampoline prepared
> > by kernel, so there'll be no broken user.
> >
> > - Designing the API: Other Considerations
> > N/A because uretprobe syscall does not return reference to kernel
> > object.
> >
> > - Proposing the API
> > Wiring up of the uretprobe system call is in separate change,
> > selftests and man page changes are part of the patchset.
> >
> > - Generic System Call Implementation
> > There's no CONFIG option for the new functionality because it
> > keeps the same behaviour from the user POV.
> >
> > - x86 System Call Implementation
> > It's 64-bit syscall only.
> >
> > - Compatibility System Calls (Generic)
> > N/A uretprobe syscall has no arguments and is not supported
> > for compat processes.
> >
> > - Compatibility System Calls (x86)
> > N/A uretprobe syscall is not supported for compat processes.
> >
> > - System Calls Returning Elsewhere
> > N/A.
> >
> > - Other Details
> > N/A.
> >
> > - Testing
> > Adding new bpf selftests and ran ltp on top of this change.
> >
> > - Man Page
> > Attached.
> >
> > - Do not call System Calls in the Kernel
> > N/A.
> >
> >
> > [0] https://lore.kernel.org/bpf/ZeCXHKJ--iYYbmLj@krava/
> > ---
> > Jiri Olsa (8):
> > x86/shstk: Make return uprobe work with shadow stack
> > uprobe: Wire up uretprobe system call
> > uprobe: Add uretprobe syscall to speed up return probe
> > selftests/x86: Add return uprobe shadow stack test
> > selftests/bpf: Add uretprobe syscall test for regs integrity
> > selftests/bpf: Add uretprobe syscall test for regs changes
> > selftests/bpf: Add uretprobe syscall call from user space test
> > selftests/bpf: Add uretprobe shadow stack test
> >
> > arch/x86/entry/syscalls/syscall_64.tbl | 1 +
> > arch/x86/include/asm/shstk.h | 4 +
> > arch/x86/kernel/shstk.c | 16 ++++
> > arch/x86/kernel/uprobes.c | 124 ++++++++++++++++++++++++++++-
> > include/linux/syscalls.h | 2 +
> > include/linux/uprobes.h | 3 +
> > include/uapi/asm-generic/unistd.h | 5 +-
> > kernel/events/uprobes.c | 24 ++++--
> > kernel/sys_ni.c | 2 +
> > tools/include/linux/compiler.h | 4 +
> > tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c | 123 ++++++++++++++++++++++++++++-
> > tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c | 385 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > tools/testing/selftests/bpf/progs/uprobe_syscall.c | 15 ++++
> > tools/testing/selftests/bpf/progs/uprobe_syscall_executed.c | 17 ++++
> > tools/testing/selftests/x86/test_shadow_stack.c | 145 ++++++++++++++++++++++++++++++++++
> > 15 files changed, 860 insertions(+), 10 deletions(-)
> > create mode 100644 tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
> > create mode 100644 tools/testing/selftests/bpf/progs/uprobe_syscall.c
> > create mode 100644 tools/testing/selftests/bpf/progs/uprobe_syscall_executed.c
> >
> > Jiri Olsa (1):
> > man2: Add uretprobe syscall page
> >
> > man/man2/uretprobe.2 | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 56 insertions(+)
> > create mode 100644 man/man2/uretprobe.2
>
>
> --
> Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCHv8 bpf-next 0/9] uprobe: uretprobe speed up
From: Masami Hiramatsu @ 2024-06-11 14:52 UTC (permalink / raw)
To: Jiri Olsa
Cc: Steven Rostedt, Oleg Nesterov, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, linux-kernel,
linux-trace-kernel, linux-api, linux-man, x86, bpf, Song Liu,
Yonghong Song, John Fastabend, Peter Zijlstra, Thomas Gleixner,
Borislav Petkov (AMD), Ingo Molnar, Andy Lutomirski,
Edgecombe, Rick P, Deepak Gupta
In-Reply-To: <20240611112158.40795-1-jolsa@kernel.org>
On Tue, 11 Jun 2024 13:21:49 +0200
Jiri Olsa <jolsa@kernel.org> wrote:
> hi,
> as part of the effort on speeding up the uprobes [0] coming with
> return uprobe optimization by using syscall instead of the trap
> on the uretprobe trampoline.
>
> The speed up depends on instruction type that uprobe is installed
> and depends on specific HW type, please check patch 1 for details.
>
> Patches 1-8 are based on bpf-next/master, but patch 2 and 3 are
> apply-able on linux-trace.git tree probes/for-next branch.
> Patch 9 is based on man-pages master.
>
> v8 changes:
> - rebased (another new syscall got merged)
> - added acks
>
> Also available at:
> https://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
> uretprobe_syscall
Applied patch [1/9] - [8/9] on probes/for-next in
git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace.git
Thank you!
>
> thanks,
> jirka
>
>
> Notes to check list items in Documentation/process/adding-syscalls.rst:
>
> - System Call Alternatives
> New syscall seems like the best way in here, because we need
> just to quickly enter kernel with no extra arguments processing,
> which we'd need to do if we decided to use another syscall.
>
> - Designing the API: Planning for Extension
> The uretprobe syscall is very specific and most likely won't be
> extended in the future.
>
> At the moment it does not take any arguments and even if it does
> in future, it's allowed to be called only from trampoline prepared
> by kernel, so there'll be no broken user.
>
> - Designing the API: Other Considerations
> N/A because uretprobe syscall does not return reference to kernel
> object.
>
> - Proposing the API
> Wiring up of the uretprobe system call is in separate change,
> selftests and man page changes are part of the patchset.
>
> - Generic System Call Implementation
> There's no CONFIG option for the new functionality because it
> keeps the same behaviour from the user POV.
>
> - x86 System Call Implementation
> It's 64-bit syscall only.
>
> - Compatibility System Calls (Generic)
> N/A uretprobe syscall has no arguments and is not supported
> for compat processes.
>
> - Compatibility System Calls (x86)
> N/A uretprobe syscall is not supported for compat processes.
>
> - System Calls Returning Elsewhere
> N/A.
>
> - Other Details
> N/A.
>
> - Testing
> Adding new bpf selftests and ran ltp on top of this change.
>
> - Man Page
> Attached.
>
> - Do not call System Calls in the Kernel
> N/A.
>
>
> [0] https://lore.kernel.org/bpf/ZeCXHKJ--iYYbmLj@krava/
> ---
> Jiri Olsa (8):
> x86/shstk: Make return uprobe work with shadow stack
> uprobe: Wire up uretprobe system call
> uprobe: Add uretprobe syscall to speed up return probe
> selftests/x86: Add return uprobe shadow stack test
> selftests/bpf: Add uretprobe syscall test for regs integrity
> selftests/bpf: Add uretprobe syscall test for regs changes
> selftests/bpf: Add uretprobe syscall call from user space test
> selftests/bpf: Add uretprobe shadow stack test
>
> arch/x86/entry/syscalls/syscall_64.tbl | 1 +
> arch/x86/include/asm/shstk.h | 4 +
> arch/x86/kernel/shstk.c | 16 ++++
> arch/x86/kernel/uprobes.c | 124 ++++++++++++++++++++++++++++-
> include/linux/syscalls.h | 2 +
> include/linux/uprobes.h | 3 +
> include/uapi/asm-generic/unistd.h | 5 +-
> kernel/events/uprobes.c | 24 ++++--
> kernel/sys_ni.c | 2 +
> tools/include/linux/compiler.h | 4 +
> tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c | 123 ++++++++++++++++++++++++++++-
> tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c | 385 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> tools/testing/selftests/bpf/progs/uprobe_syscall.c | 15 ++++
> tools/testing/selftests/bpf/progs/uprobe_syscall_executed.c | 17 ++++
> tools/testing/selftests/x86/test_shadow_stack.c | 145 ++++++++++++++++++++++++++++++++++
> 15 files changed, 860 insertions(+), 10 deletions(-)
> create mode 100644 tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
> create mode 100644 tools/testing/selftests/bpf/progs/uprobe_syscall.c
> create mode 100644 tools/testing/selftests/bpf/progs/uprobe_syscall_executed.c
>
> Jiri Olsa (1):
> man2: Add uretprobe syscall page
>
> man/man2/uretprobe.2 | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 56 insertions(+)
> create mode 100644 man/man2/uretprobe.2
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox