Re: [PATCH] io_thread/x86: don't reset 'cs', 'ss', 'ds' and 'es' registers for io_threads

From: Simon Marchi <simon.marchi@polymtl.ca>
To: Stefan Metzmacher <metze@samba.org>,
	Borislav Petkov <bp@alien8.de>,
	Peter Zijlstra <peterz@infradead.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Andy Lutomirski <luto@amacapital.net>,
	Thomas Gleixner <tglx@linutronix.de>,
	Jens Axboe <axboe@kernel.dk>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	io-uring <io-uring@vger.kernel.org>,
	the arch/x86 maintainers <x86@kernel.org>,
	linux-toolchains@vger.kernel.org
Subject: Re: [PATCH] io_thread/x86: don't reset 'cs', 'ss', 'ds' and 'es' registers for io_threads
Date: Wed, 5 May 2021 17:59:12 -0400	[thread overview]
Message-ID: <30e248aa-534d-37ff-2954-a70a454391fc@polymtl.ca> (raw)
In-Reply-To: <932d65e1-5a8f-c86a-8673-34f0e006c27f@samba.org>

On 2021-05-05 7:29 a.m., Stefan Metzmacher wrote:
> See https://lore.kernel.org/io-uring/0375b37f-2e1e-7999-53b8-c567422aa181@samba.org/
> and https://lore.kernel.org/io-uring/20210411152705.2448053-1-metze@samba.org/T/#m461f280e8c3d32a49bc7da7bb5e214e90d97cf65
> 
> The question is why does inferior_ptid doesn't represent the thread
> that was specified by 'gdb --pid PIDVAL'

Hi Stefan,

When you attach to PIDVAL (assuming that PIDVAL is a thread-group
leader), GDB attaches to all the threads of that thread group.  The
inferior_ptid global variable is "the thread we are currently working
with", and changes whenever GDB wants to deal with a different thread.

After attaching to all threads, GDB wants to know more about that
process' architecture (that read_description call mentioned in [1]).
The way this is implemented varies from arch to arch, but as you found
out, for x86-64 it consists of peeking into a thread's CS/DS to
determine whether the process is x86-64, x32 or i386.  The thread used
for this - the inferior_ptid value - just happens to be the latest
thread we switched inferior_ptid to (presumably because we iterated on
the thread list to do something and that was the last thread in the
list).  And up to now, this was working under the assumption that
picking any thread of the process would yield the same values for that
purpose.  I don't think it was intentionally done this way, but it
worked and didn't cause any trouble until now.

With io threads, that assumption doesn't hold anymore.  From what I
understand, your v1 patch made it so that the kernel puts the CS/DS
values GDB expects in the io threads (even though they are never
actually used otherwise).  I don't understand how your v2 patch
addresses the problem though.

I don't think it would be a problem on the GDB-side to make sure to
fetch these values from a "standard" thread.  Most likely the thread
group leader, like Tom has proposed in [1].

Thanks,

Simon

[1] https://sourceware.org/bugzilla/show_bug.cgi?id=27822#c0