Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 9 Feb 2026 19:49:46 +0200
From:      Konstantin Belousov <kostikbel@gmail.com>
To:        Minsoo Choo <minsoochoo0122@proton.me>
Cc:        John Baldwin <jhb@freebsd.org>, src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org
Subject:   Re: git: 8f23665fed2f - main - pcb.h: mark struct pcb to be preserved
Message-ID:  <aYoeOlGN_MRl-zbc@kib.kiev.ua>
In-Reply-To: <ZU_n2ErkGNuLyBqn3vKMEeDN0oquRSTYh1S-gIk81J1h1sDd3jHwimn-l7NTE80AHjXlV4oc7ODVj32EkwULLjof3bEauWXyHH1xDIV_5ik=@proton.me>
References:  <69896027.2112e.2852c0dd@gitrepo.freebsd.org> <5bec84ad-a01e-4931-98e6-4ba06adb1237@FreeBSD.org> <bafAtYtkiQ-8-cLlyxu4WUlhcBp6PPxb6y5XE1KZw-qvN8xi-2wjc8uL_6OZJAW2cg9OqyCorL4knqNazkRt44btbusFZU_I53vAVWlxdKY=@proton.me> <eed3e673-ce22-414b-8787-84981492869a@FreeBSD.org> <ZU_n2ErkGNuLyBqn3vKMEeDN0oquRSTYh1S-gIk81J1h1sDd3jHwimn-l7NTE80AHjXlV4oc7ODVj32EkwULLjof3bEauWXyHH1xDIV_5ik=@proton.me>

index | next in thread | previous in thread | raw e-mail

On Mon, Feb 09, 2026 at 04:55:41PM +0000, Minsoo Choo wrote:
> On Monday, February 9th, 2026 at 11:45 AM, John Baldwin <jhb@FreeBSD.org> wrote:
> 
> > On 2/9/26 10:56, Minsoo Choo wrote:
> > 
> > > On Monday, February 9th, 2026 at 10:12 AM, John Baldwin jhb@FreeBSD.org wrote:
> > > 
> > > > On 2/8/26 23:18, Konstantin Belousov wrote:
> > > > 
> > > > > The branch main has been updated by kib:
> > > > > 
> > > > > URL: https://cgit.FreeBSD.org/src/commit/?id=8f23665fed2fbaf4481359b4d2fcdd7b9feb40e3
> > > > > 
> > > > > commit 8f23665fed2fbaf4481359b4d2fcdd7b9feb40e3
> > > > > Author: Minsoo Choo minsoochoo0122@proton.me
> > > > > AuthorDate: 2026-02-06 20:03:53 +0000
> > > > > Commit: Konstantin Belousov kib@FreeBSD.org
> > > > > CommitDate: 2026-02-09 04:17:16 +0000
> > > > > 
> > > > > pcb.h: mark struct pcb to be preserved
> > > > > 
> > > > > There are programs that depend on this structure (e.g. kernel debuggers)
> > > > > that breaks when the ABI changes.
> > > > > 
> > > > > Signed-off-by: Minsoo Choo minsoochoo0122@proton.me
> > > > > Reviewed by: kib
> > > > > MFC after: 1 week
> > > > > Differential Revision: https://reviews.freebsd.org/D55149
> > > > 
> > > > Hmmm, I think it is useful to know which fields debuggers use (typically just
> > > > registers) vs which ones can be safely changed without impacting debuggers.
> > > > 
> > > > --
> > > > John Baldwin
> > > 
> > > In the review, kib said:
> > > 
> > > > I had to add and preserve the padding fields pcb_pad in the amd64 structure to be compatible with kgdb at least. This makes me sure that the whole pcb layout is fixed by ABI.
> > 
> > 
> > It is no longer needed and hasn't been needed in a decade. Back when the field was
> > first added, kgdb did indeed assume a fixed size for `struct pcb`. However, back in
> > 2015 I added a pcb_size variable that debuggers can use to determine this size
> > (to properly index the stoppcbs[] array) even if debug symbols weren't present:
> > 
> > commit 645743ea99829e6d578cca5a580cf0a4102fb7dd
> > Author: John Baldwin jhb@FreeBSD.org
> > 
> > Date: Thu Nov 12 22:00:59 2015 +0000
> > 
> > Export various helper variables describing the layout and size of
> > certain kernel structures for use by debuggers. This mostly aids
> > in examining cores from a kernel without debug symbols as a debugger
> > can infer these values if debug symbols are available.
> > 
> > One set of variables describes the layout of 'struct linker_file' to
> > walk the list of loaded kernel modules.
> > 
> > A second set of variables describes the layout of 'struct proc' and
> > 'struct thread' to walk the list of processes in the kernel and the
> > threads in each process.
> > 
> > The 'pcb_size' variable is used to index into the stoppcbs[] array.
> > 
> > The 'vm_maxuser_address' is used to distinguish kernel virtual addresses
> > from user addresses. This doesn't have to be perfect, and
> > 'vm_maxuser_address' is a cheap and simple way to differentiate kernel
> > pointers from simple values like TIDs and PIDs.
> > 
> > While here, annotate the fields in struct pcb used by kgdb on amd64
> > and i386 to note that their ABI should be preserved. Annotations for
> > other platforms will be added in the future.
> > 
> > This was even the commit that added the annotations just removed, so a simple
> > blame for why the annotations were present would have turned this up.
> > 
> > --
> > John Baldwin
> 
> If this is correct, we should revert this commit. I want to hear kib@ thoughts on this and once he confirms someone with commit access can revert this change.
> 
> My original patch was marking pcb fields as stable for all architectures (lldb aarch64 kernel debugger was broken because of this). I'll create new revision with original patch once revert is pushed.

First, I do not mind reverting this, will do in a hour unless we get to some
other conclusion in the thread.

Second, I want to discuss something related to the struct pcb.  Right now
the structure is used for three quite unrelated purposes:
- as effectively part of the struct thread, despite being a separate
  allocation when the thread is created
- as wakepcb for saving some part of BSP state suspend/resume
- as dumppcb for saving all CPUs state for vmcore
With these consumers, it seems that fields are only added to the struct,
and never removed.  For instance, I do not see any use of pcb_sfmask.

I really want to split pcb into 'thread pcb' and 'kpcb', and define
that kpcb is KBI, while thread pcb would be only stable on the stable
branches.  kgdb and possibly klldb need kpcb for starting frame information
on CPUs when kernel dump does not have dwarf, am I right?


home | help

Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?aYoeOlGN_MRl-zbc>