Date: Tue, 15 May 2012 21:53:48 +0100 From: Attilio Rao <attilio@freebsd.org> To: Konstantin Belousov <kostikbel@gmail.com> Cc: svn-src-stable@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, svn-src-stable-9@freebsd.org, Andriy Gapon <avg@freebsd.org> Subject: Re: svn commit: r235480 - in stable/9/sys: dev/usb geom geom/mountver i386/conf kern sys Message-ID: <CAJ-FndCGS6jbKumFRTcV8o1EkMAPzK1CCqRy4wemD1CsPYPU0A@mail.gmail.com> In-Reply-To: <20120515203832.GE2358@deviant.kiev.zoral.com.ua> References: <201205151721.q4FHLlgq097452@svn.freebsd.org> <20120515182747.GD2358@deviant.kiev.zoral.com.ua> <4FB2B7C5.2080604@FreeBSD.org> <20120515203832.GE2358@deviant.kiev.zoral.com.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
2012/5/15 Konstantin Belousov <kostikbel@gmail.com>: > On Tue, May 15, 2012 at 11:08:37PM +0300, Andriy Gapon wrote: >> on 15/05/2012 21:27 Konstantin Belousov said the following: >> > On Tue, May 15, 2012 at 05:21:47PM +0000, Andriy Gapon wrote: >> >> Author: avg Date: Tue May 15 17:21:46 2012 New Revision: 235480 URL: >> >> http://svn.freebsd.org/changeset/base/235480 >> >> >> >> Log: MFC r230643: stop_scheduler -> td_stopsched Modified: >> >> stable/9/sys/sys/proc.h >> >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D >> >> >> >> >> --- stable/9/sys/sys/proc.h =C2=A0 Tue May 15 17:11:00 2012 =C2=A0 =C2= =A0 =C2=A0 =C2=A0(r235479) >> >> +++ stable/9/sys/sys/proc.h =C2=A0 =C2=A0 =C2=A0 =C2=A0Tue May 15 17:= 21:46 2012 =C2=A0 =C2=A0 =C2=A0 =C2=A0(r235480) @@ -235,6 >> >> +235,7 @@ struct thread { short =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0td_locks; =C2=A0 =C2=A0 =C2=A0 /* (k) Count of non-spin >> >> locks. */ short =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0td_rw_rlocks= ; =C2=A0 /* (k) Count of rwlock read locks. */ >> >> short =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0td_lk_slocks; = =C2=A0 /* (k) Count of lockmgr shared locks. */ + =C2=A0 =C2=A0 =C2=A0short >> >> td_stopsched; =C2=A0 =C2=A0 =C2=A0/* (k) Scheduler stopped. */ struct= turnstile *td_blocked; >> >> /* (t) Lock thread is blocked on. */ const char =C2=A0 =C2=A0*td_lock= name; =C2=A0 /* (t) Name >> >> of lock blocked on. */ LIST_HEAD(, turnstile) td_contested; =C2=A0 = =C2=A0 =C2=A0 =C2=A0/* (q) >> >> Contested locks. */ >> >> >> > Did you verified that there is a gap between td_lk_slocks and td_block= ed an >> > all tier-1 architectures large enough to adopt td_stopsched without >> > changing the layout of struct thread after the new member ? >> >> I haven't verified that. but I presumed that the gap was either 2 bytes = on >> 32-bit architectures or 6 bytes on 64-bit ones. >> What is the proper and doable way of verifying this? > I think that p sizeof(struct thread) from gdb on previous and current > version of the kernel would be a good indicator. If sizes differ, it is > bad. > > To be completely sure, calculate offset of the next member in the structu= re: > in gdb, > p &(((struct thread *)0)->td_blocked) > for previous and current kernels. > >> >> > If not, this breaks KBI. >> >> I can just revert the commit to not risk an obscure breakage. >> What would you suggest? > Move the new member to the end of the struct thread, and do explicit > zeroing on the thread allocation, if commit changed the layout. That member position is strategic. It is left as close as possible to the recurse flags because they will very likely be already cached when td_schedstop is hit, which means td_schedstop will be already cached likely and this will avoid to pollute an extra cache line. Attilio --=20 Peace can only be achieved by understanding - A. Einstein
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAJ-FndCGS6jbKumFRTcV8o1EkMAPzK1CCqRy4wemD1CsPYPU0A>