Re: svn commit: r198706 - head/sys/sys
On Nov 2, 2009, at 12:34 PM, Kostik Belousov wrote:
On Mon, Nov 02, 2009 at 10:08:17AM -0800, Marcel Moolenaar wrote:
For a change that does change the ABI: revision 198506
tcsh(1) dumps core with a sig 11...
Can you provide some details ? Which architecture is it ?
What is the backtrace ? What is the ktrace before SIGSEGV ?
I'm working on it. I see it on ia64, so the root cause may be
an ia64-specific bug. The sig 11 happens when csh calls
sigsuspend:
(gdb) l *$b0
0x200483a0 is in pjwait (/nfs/freebsd/base/head/bin/csh/../../
contrib/tcsh/sh.proc.c:513).
508 while ((fp = (fp->p_friends)) != pp);
509 if ((jobflags & PRUNNING) == 0)
510 break;
511 jobdebug_xprintf(("%d starting to sigsuspend for SIGCHLD on
%d\n",
512 getpid(), fp->p_procid));
513 sigsuspend(&pause_mask);
514 }
515 cleanup_until(&oset);
516 jobdebug_xprintf(("%d returned from sigsuspend loop\n", getpid
()));
517 #ifdef BSDJOBS
(gdb)
FYI,
--
Marcel Moolenaar
xcl...@mac.com
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r198706 - head/sys/sys
On Mon, Nov 02, 2009 at 10:08:17AM -0800, Marcel Moolenaar wrote: > For a change that does change the ABI: revision 198506 > tcsh(1) dumps core with a sig 11... Can you provide some details ? Which architecture is it ? What is the backtrace ? What is the ktrace before SIGSEGV ? pgpCijm98aQWH.pgp Description: PGP signature
Re: svn commit: r198706 - head/sys/sys
On Nov 2, 2009, at 7:54 AM, Ed Schouten wrote: * M. Warner Losh wrote: And you haven't answered my question: Have you confirmed that there's no ABI changes on all platforms? Confirmed on all platforms? No. I've only tested it on i386 and amd64. I think someone also tested it on arm, so this makes me believe other architectures will also work as expected. When you bank on alignment, don't just test i386 and amd64. Please put in the extra effort to contact platform owners to give you a thumbs up. While I'm responding: please do a version bump as Warner suggested. The pain of needing one without having done so, is infinitely more than the effort or repo bloat can be together. Oh, and no, I don't think this changes the ABI :-) For a change that does change the ABI: revision 198506 tcsh(1) dumps core with a sig 11... Trick or treat... -- Marcel Moolenaar xcl...@mac.com ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r198706 - head/sys/sys
* M. Warner Losh wrote: > And you haven't answered my question: Have you confirmed that there's > no ABI changes on all platforms? Confirmed on all platforms? No. I've only tested it on i386 and amd64. I think someone also tested it on arm, so this makes me believe other architectures will also work as expected. -- Ed Schouten WWW: http://80386.nl/ pgpKvugEeEI9F.pgp Description: PGP signature
Re: svn commit: r198706 - head/sys/sys
In message: <20091102102404.gp1...@hoeg.nl> Ed Schouten writes: : Hello David, : : * David Malone wrote: : > Surely it is an API change, but not an ABI change? Code that used : > to do: : > : > d.d_uid = 3; : > : > will no longer compile, but code that was comipled with the old : > version will still run. I understand that the assignment doesn't : > do anything useful, but I suppose there still could be code that : > does it? : : Yes, in theory there could be pieces of code that do that, but keep in : mind that d_uid was never meant to be used by device drivers. It was : used by devfs internally, before cdevpriv existed. : : Looking at the SVN logs, it was introduced in March 2005, but it was : already rendered useless in September that same year, when devfs was : modified to just obtain the ownership/modes from the cdevpriv instead of : the cdevsw. : : Interesting commits: : : - http://svn.freebsd.org/viewvc/base?view=revision&revision=143746 : - http://svn.freebsd.org/viewvc/base?view=revision&revision=150342 True, but you've now spent way more time than a version bump would have taken on this. The first of these also does a version bump. :) And you haven't answered my question: Have you confirmed that there's no ABI changes on all platforms? Warner ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r198706 - head/sys/sys
Hello David, * David Malone wrote: > Surely it is an API change, but not an ABI change? Code that used > to do: > > d.d_uid = 3; > > will no longer compile, but code that was comipled with the old > version will still run. I understand that the assignment doesn't > do anything useful, but I suppose there still could be code that > does it? Yes, in theory there could be pieces of code that do that, but keep in mind that d_uid was never meant to be used by device drivers. It was used by devfs internally, before cdevpriv existed. Looking at the SVN logs, it was introduced in March 2005, but it was already rendered useless in September that same year, when devfs was modified to just obtain the ownership/modes from the cdevpriv instead of the cdevsw. Interesting commits: - http://svn.freebsd.org/viewvc/base?view=revision&revision=143746 - http://svn.freebsd.org/viewvc/base?view=revision&revision=150342 -- Ed Schouten WWW: http://80386.nl/ pgpLJh1tG7CbQ.pgp Description: PGP signature
Re: svn commit: r198706 - head/sys/sys
On Sun, Nov 01, 2009 at 02:12:12AM +0100, Ed Schouten wrote: > No, we don't. All these fields are not used by drivers, just some old > version of the devfs code. I made sure struct cdevsw didn't change in > size, so there should be no API nor ABI conflicts. Surely it is an API change, but not an ABI change? Code that used to do: d.d_uid = 3; will no longer compile, but code that was comipled with the old version will still run. I understand that the assignment doesn't do anything useful, but I suppose there still could be code that does it? David. ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r198706 - head/sys/sys
On 1 Nov 2009, at 15:33, Ed Schouten wrote: Because d_kind is a pointer, there will be a hole in the structure of at least 2 bytes (maybe even 6), which means we can safely extend d_mode to 32 bits as well. I could have decided to leave it at uint16_t, but that would only obfuscate it even more when we would try to reclaim some space there. d_list is just two pointers, so I merged it with d_kind into an array of three pointers No, you're right, and I was mistaken, it seems fine on i386 and amd64. And, at least here, d_devs is at the same offset on both architectures. The only really "weird" architecture is arm, which I believe aligns char and short to 32-bit, but that should be OK here I think as well. Robert ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r198706 - head/sys/sys
Hi Robert, * Robert Watson wrote: > The underlying change seems fine, especially in -CURRENT, but I'm > confused by the paragraph here on alignment and safety. What do you > mean by safe, and does it matter? With safe I mean that all useful members still begin at the same offset within the structure. I'm not sure whether it matters at all, because I can imagine d_devs could be moved around without having any effect, but I'm not going to make any predictions on that. > From a casual glance (perhaps mistaken), it looks like this changes > the KBI, so all modules declaring struct cdevsw will need to be > rebuilt. It shouldn't, right? - uid_t d_uid; - gid_t d_gid; - mode_t d_mode; - const char *d_kind; + + int32_t d_spare0[3]; + void*d_spare1[3]; /* These fields should not be messed with by drivers */ - LIST_ENTRY(cdevsw) d_list; d_uid, d_gid and d_mode is equal to: int32_t d_uid; int32_t d_gid; int16_t d_mode; Because d_kind is a pointer, there will be a hole in the structure of at least 2 bytes (maybe even 6), which means we can safely extend d_mode to 32 bits as well. I could have decided to leave it at uint16_t, but that would only obfuscate it even more when we would try to reclaim some space there. d_list is just two pointers, so I merged it with d_kind into an array of three pointers. -- Ed Schouten WWW: http://80386.nl/ pgp5dORJFjGWu.pgp Description: PGP signature
Re: svn commit: r198706 - head/sys/sys
On Sat, 31 Oct 2009, Ed Schouten wrote:
Turn unused structure fields of cdevsw into spares.
d_uid, d_gid and d_mode are unused, because permissions are stored in
cdevpriv nowadays. d_kind doesn't seem to be used at all. We no longer
keep a list of cdevsw's, so d_list is also unused.
uid_t and gid_t are 32 bits, but mode_t is 16 bits, Because of alignment
constraints of d_kind, we can safely turn it into three 32-bit integers.
d_kind and d_list is equal in size to three pointers.
The underlying change seems fine, especially in -CURRENT, but I'm confused by
the paragraph here on alignment and safety. What do you mean by safe, and
does it matter? From a casual glance (perhaps mistaken), it looks like this
changes the KBI, so all modules declaring struct cdevsw will need to be
rebuilt.
Robert N M Watson
Computer Laboratory
University of Cambridge
Discussed with:kib
Modified:
head/sys/sys/conf.h
Modified: head/sys/sys/conf.h
==
--- head/sys/sys/conf.h Sat Oct 31 09:03:48 2009(r198705)
+++ head/sys/sys/conf.h Sat Oct 31 10:35:41 2009(r198706)
@@ -210,15 +210,13 @@ struct cdevsw {
d_kqfilter_t*d_kqfilter;
d_purge_t *d_purge;
d_mmap_single_t *d_mmap_single;
- uid_t d_uid;
- gid_t d_gid;
- mode_t d_mode;
- const char *d_kind;
+
+ int32_t d_spare0[3];
+ void*d_spare1[3];
/* These fields should not be messed with by drivers */
- LIST_ENTRY(cdevsw) d_list;
LIST_HEAD(, cdev) d_devs;
- int d_spare3;
+ int d_spare2;
union {
struct cdevsw *gianttrick;
SLIST_ENTRY(cdevsw) postfree_list;
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r198706 - head/sys/sys
In message: <20091101011212.gg1...@hoeg.nl> Ed Schouten writes: : Hi Warner, : : * M. Warner Losh wrote: : > Don't we need a D_VERSION bump for this? : : No, we don't. All these fields are not used by drivers, just some old : version of the devfs code. I made sure struct cdevsw didn't change in : size, so there should be no API nor ABI conflicts. On all platforms? A version bump is trivial... Warner ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r198706 - head/sys/sys
Hi Warner, * M. Warner Losh wrote: > Don't we need a D_VERSION bump for this? No, we don't. All these fields are not used by drivers, just some old version of the devfs code. I made sure struct cdevsw didn't change in size, so there should be no API nor ABI conflicts. -- Ed Schouten WWW: http://80386.nl/ pgp8YKNrt9G1o.pgp Description: PGP signature
Re: svn commit: r198706 - head/sys/sys
In message: <200910311035.n9vazfib082...@svn.freebsd.org>
Ed Schouten writes:
: Author: ed
: Date: Sat Oct 31 10:35:41 2009
: New Revision: 198706
: URL: http://svn.freebsd.org/changeset/base/198706
:
: Log:
: Turn unused structure fields of cdevsw into spares.
:
: d_uid, d_gid and d_mode are unused, because permissions are stored in
: cdevpriv nowadays. d_kind doesn't seem to be used at all. We no longer
: keep a list of cdevsw's, so d_list is also unused.
:
: uid_t and gid_t are 32 bits, but mode_t is 16 bits, Because of alignment
: constraints of d_kind, we can safely turn it into three 32-bit integers.
: d_kind and d_list is equal in size to three pointers.
:
: Discussed with: kib
Don't we need a D_VERSION bump for this?
Warner
: Modified:
: head/sys/sys/conf.h
:
: Modified: head/sys/sys/conf.h
: ==
: --- head/sys/sys/conf.h Sat Oct 31 09:03:48 2009(r198705)
: +++ head/sys/sys/conf.h Sat Oct 31 10:35:41 2009(r198706)
: @@ -210,15 +210,13 @@ struct cdevsw {
: d_kqfilter_t*d_kqfilter;
: d_purge_t *d_purge;
: d_mmap_single_t *d_mmap_single;
: - uid_t d_uid;
: - gid_t d_gid;
: - mode_t d_mode;
: - const char *d_kind;
: +
: + int32_t d_spare0[3];
: + void*d_spare1[3];
:
: /* These fields should not be messed with by drivers */
: - LIST_ENTRY(cdevsw) d_list;
: LIST_HEAD(, cdev) d_devs;
: - int d_spare3;
: + int d_spare2;
: union {
: struct cdevsw *gianttrick;
: SLIST_ENTRY(cdevsw) postfree_list;
:
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
