On Mon, May 3, 2021 at 2:26 PM Jens Axboe <axboe@xxxxxxxxx> wrote: > > On 5/3/21 2:37 PM, Linus Torvalds wrote: > > On Mon, May 3, 2021 at 1:15 PM Andy Lutomirski <luto@xxxxxxxxxx> wrote: > >> > >> On Mon, May 3, 2021 at 12:15 PM Linus Torvalds > >> <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote: > >>> So generally, the IO threads are now 100% normal threads - it's > >>> literally just that they never return to user space because they are > >>> always just doing the IO offload on the kernel side. > >>> > >>> That part is lovely, but part of the "100% IO threads" really is that > >>> they share the signal struct too, which in turn means that they very > >>> much show up as normal threads. Again, not a problem: they really > >>> _are_ normal threads for all intents and purposes. > >> > >> I'm a bit confused, though. All the ptrace register access (AFAICS) > >> goes through ptrace_check_attach(), which should wait until the tracee > >> is stopped. Does the io_uring thread now stop in response to ptrace > >> stop requests? > > > > Yup. They really are 100% regular threads. Things like ^Z and friends > > also stop them now, and the freezer freezes them etc. > > > > And making PTRACE_ATTACH fail just causes gdb to fail. > > > >> Fair enough. But I would really, really rather that gdb starts fixing > >> its amazingly broken assumptions about bitness. > > > > "Preach it, Brother" > > That's actually what the original code did, and the "only" problem with > it was that gdb shits itself and just go into an infinite loop trying to > attach. And yes, that's most certainly a gdb bug, and we entertained a > few options for making that work. One was hiding the threads, but nobody > (myself included) liked that, because then we're special casing > something again, and for no other reason than gdb is buggy. > > On principle, I think it's arguably the right change to just -EINVAL the > attach. However, a part of me also finds it very annoying that anyone > attempting to debug any program that uses io_uring will not be able to > do so with a buggy gdb. That's regardless of whether or not you want to > look at the io threads or not, or even if you don't care about debugging > the io_uring side of things. And I'm assuming this will take a while to > get fixed, and then even longer to make its way back to distros. > > So... You should just make the call. At least then I can just tell > people that Linus made that decision :-) > > >>> So I think Stefan's patch is reasonable, if not pretty. Literally > >>> becasue of that "make these threads look even more normal" > >> > >> I think it's reasonable except for the bit about copying the segment > >> regs. Can we hardcode __USER_CS, etc, and, when gdb crashes or > >> otherwise malfunctions for compat programs, we can say that gdb needs > >> to stop sucking. > > > > So that was actually my initial suggestion. Stefan really does seem to > > care about compat programs. > > > > Any "gdb breaks" would be good to motivate people to fix gdb, but the > > thing is, presumably nobody actually wants to touch gdb with a ten > > foot pole. > > > > And a "let's break gdb to encourage people to fix it" only works if > > people actually _do_ fit it. Which doesn't seem to be happening. > > > > Two lines of kernel code seems to be the better option than hoping for > > gdb to be fixed. > > As far as I'm concerned, gdb works "well enough" with io threads as it > stands. Yes, it'll complain a bit, but nothing that prevents you from > attaching to a progrem. If we -EINVAL, then gdb will become useless for > debugging a program that uses io_uring. And I'm not holding my breath > while someone fixes that. To be clear, I'm suggesting that we -EINVAL the PTRACE_GETREGS calls and such, not the ATTACH. I have no idea what gdb will do if this happens, though. --Andy