svn commit: r264310 - head/sys/kern
Author: davidxu Date: Thu Apr 10 02:30:51 2014 New Revision: 264310 URL: http://svnweb.freebsd.org/changeset/base/264310 Log: Add kqueue support for devctl. Reviewed by: kib,mjg Modified: head/sys/kern/subr_bus.c Modified: head/sys/kern/subr_bus.c == --- head/sys/kern/subr_bus.cWed Apr 9 21:19:46 2014(r264309) +++ head/sys/kern/subr_bus.cThu Apr 10 02:30:51 2014(r264310) @@ -367,6 +367,7 @@ static d_close_tdevclose; static d_read_tdevread; static d_ioctl_t devioctl; static d_poll_tdevpoll; +static d_kqfilter_tdevkqfilter; static struct cdevsw dev_cdevsw = { .d_version =D_VERSION, @@ -375,6 +376,7 @@ static struct cdevsw dev_cdevsw = { .d_read = devread, .d_ioctl = devioctl, .d_poll = devpoll, + .d_kqfilter = devkqfilter, .d_name = devctl, }; @@ -399,6 +401,15 @@ static struct dev_softc struct sigio *sigio; } devsoftc; +static voidfilt_devctl_detach(struct knote *kn); +static int filt_devctl_read(struct knote *kn, long hint); + +struct filterops devctl_rfiltops = { + .f_isfd = 1, + .f_detach = filt_devctl_detach, + .f_event = filt_devctl_read, +}; + static struct cdev *devctl_dev; static void @@ -409,6 +420,7 @@ devinit(void) mtx_init(devsoftc.mtx, dev mtx, devd, MTX_DEF); cv_init(devsoftc.cv, dev cv); TAILQ_INIT(devsoftc.devq); + knlist_init_mtx(devsoftc.sel.si_note, devsoftc.mtx); } static int @@ -529,6 +541,34 @@ devpoll(struct cdev *dev, int events, st return (revents); } +static int +devkqfilter(struct cdev *dev, struct knote *kn) +{ + int error; + + if (kn-kn_filter == EVFILT_READ) { + kn-kn_fop = devctl_rfiltops; + knlist_add(devsoftc.sel.si_note, kn, 0); + error = 0; + } else + error = EINVAL; + return (error); +} + +static void +filt_devctl_detach(struct knote *kn) +{ + + knlist_remove(devsoftc.sel.si_note, kn, 0); +} + +static int +filt_devctl_read(struct knote *kn, long hint) +{ + kn-kn_data = devsoftc.queued; + return (kn-kn_data != 0); +} + /** * @brief Return whether the userland process is running */ @@ -576,6 +616,7 @@ devctl_queue_data_f(char *data, int flag TAILQ_INSERT_TAIL(devsoftc.devq, n1, dei_link); devsoftc.queued++; cv_broadcast(devsoftc.cv); + KNOTE_LOCKED(devsoftc.sel.si_note, 0); mtx_unlock(devsoftc.mtx); selwakeup(devsoftc.sel); if (devsoftc.async devsoftc.sigio != NULL) ___ 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
svn commit: r264114 - head/sys/kern
Author: davidxu Date: Fri Apr 4 12:31:13 2014 New Revision: 264114 URL: http://svnweb.freebsd.org/changeset/base/264114 Log: Fix SIGIO delivery. Use fsetown() to handle file descriptor owner ioctl and use pgsigio() to send SIGIO. Submitted by: truckman Reviewed by: mjg Modified: head/sys/kern/subr_bus.c Modified: head/sys/kern/subr_bus.c == --- head/sys/kern/subr_bus.cFri Apr 4 11:19:02 2014(r264113) +++ head/sys/kern/subr_bus.cFri Apr 4 12:31:13 2014(r264114) @@ -391,11 +391,12 @@ static struct dev_softc int inuse; int nonblock; int queued; + int async; struct mtx mtx; struct cv cv; struct selinfo sel; struct devq devq; - struct proc *async_proc; + struct sigio *sigio; } devsoftc; static struct cdev *devctl_dev; @@ -422,7 +423,7 @@ devopen(struct cdev *dev, int oflags, in /* move to init */ devsoftc.inuse = 1; devsoftc.nonblock = 0; - devsoftc.async_proc = NULL; + devsoftc.async = 0; mtx_unlock(devsoftc.mtx); return (0); } @@ -433,8 +434,8 @@ devclose(struct cdev *dev, int fflag, in mtx_lock(devsoftc.mtx); devsoftc.inuse = 0; - devsoftc.async_proc = NULL; cv_broadcast(devsoftc.cv); + funsetown(devsoftc.sigio); mtx_unlock(devsoftc.mtx); return (0); } @@ -490,33 +491,21 @@ devioctl(struct cdev *dev, u_long cmd, c devsoftc.nonblock = 0; return (0); case FIOASYNC: - /* -* FIXME: -* Since this is a simple assignment there is no guarantee that -* devsoftc.async_proc consumers will get a valid pointer. -* -* Example scenario where things break (processes A and B): -* 1. A opens devctl -* 2. A sends fd to B -* 3. B sets itself as async_proc -* 4. B exits -* -* However, normally this requires root privileges and the only -* in-tree consumer does not behave in a dangerous way so the -* issue is not critical. -*/ if (*(int*)data) - devsoftc.async_proc = td-td_proc; + devsoftc.async = 1; else - devsoftc.async_proc = NULL; + devsoftc.async = 0; + return (0); + case FIOSETOWN: + return fsetown(*(int *)data, devsoftc.sigio); + case FIOGETOWN: + *(int *)data = fgetown(devsoftc.sigio); return (0); /* (un)Support for other fcntl() calls. */ case FIOCLEX: case FIONCLEX: case FIONREAD: - case FIOSETOWN: - case FIOGETOWN: default: break; } @@ -560,7 +549,6 @@ void devctl_queue_data_f(char *data, int flags) { struct dev_event_info *n1 = NULL, *n2 = NULL; - struct proc *p; if (strlen(data) == 0) goto out; @@ -590,13 +578,8 @@ devctl_queue_data_f(char *data, int flag cv_broadcast(devsoftc.cv); mtx_unlock(devsoftc.mtx); selwakeup(devsoftc.sel); - /* XXX see a comment in devioctl */ - p = devsoftc.async_proc; - if (p != NULL) { - PROC_LOCK(p); - kern_psignal(p, SIGIO); - PROC_UNLOCK(p); - } + if (devsoftc.async devsoftc.sigio != NULL) + pgsigio(devsoftc.sigio, SIGIO, 0); return; out: /* ___ 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: r263755 - head/sys/kern
On 2014/03/29 15:52, Don Lewis wrote: On 29 Mar, Mateusz Guzik wrote: On Sat, Mar 29, 2014 at 11:52:09AM +0800, David Xu wrote: If fsetown handling like this is insecure this would bite us in that scenario (and few others). In short, if we can avoid giving another way to corrupt stuff in the kernel to userspace, we should. I can not see what you said, where is the security problem with fsetown ? if you have per-jail instance of devsoftc, they all are operating on their own instance. but I don't think this patch should address jail now, there are many things are not jail ready. I asked if multpiple concurrent calls to fsetown(.., pointer) could result in some corruption in the kernel - if they could, we would have a problem in the future. I decided to read the code once more and fsetown seems to be safe in this regard after all and with that in mind the patch looks good to me. The fsetown() implementation does sufficient locking to prevent the kernel from getting into a bad state. The issue is that the device can only have at most one owner (which may be a process group). If multiple processes are allowed to open the device, or if a process that opened the device shares the descriptor with another process, the last call to fsetown() wins. That means that one process could steal ownership from another if they both have the same device open. The reason that I suggested checking ownership when handling FIOASYNC is that in the case of two processes sharing access to a device, there is currently nothing that prevents a non-owner of the device from enabling this mode and causing SIGIO signals to be sent to the owner, which might not be expecting to receive them. I think if you add ownership checking, it will be incompatible with other code, people have to change their mind when dealing with this special file descriptor, my recommendation is people don't need to refresh their brain. OTOH, if it is a problem, we should have already been flooded by the problem, but in the past years, I saw zero complaining in the mailing lists. ___ 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: r263755 - head/sys/kern
On 2014/03/29 23:42, Warner Losh wrote: On Mar 29, 2014, at 6:31 AM, David Xu davi...@freebsd.org wrote: On 2014/03/29 15:52, Don Lewis wrote: On 29 Mar, Mateusz Guzik wrote: On Sat, Mar 29, 2014 at 11:52:09AM +0800, David Xu wrote: If fsetown handling like this is insecure this would bite us in that scenario (and few others). In short, if we can avoid giving another way to corrupt stuff in the kernel to userspace, we should. I can not see what you said, where is the security problem with fsetown ? if you have per-jail instance of devsoftc, they all are operating on their own instance. but I don't think this patch should address jail now, there are many things are not jail ready. I asked if multpiple concurrent calls to fsetown(.., pointer) could result in some corruption in the kernel - if they could, we would have a problem in the future. I decided to read the code once more and fsetown seems to be safe in this regard after all and with that in mind the patch looks good to me. The fsetown() implementation does sufficient locking to prevent the kernel from getting into a bad state. The issue is that the device can only have at most one owner (which may be a process group). If multiple processes are allowed to open the device, or if a process that opened the device shares the descriptor with another process, the last call to fsetown() wins. That means that one process could steal ownership from another if they both have the same device open. The reason that I suggested checking ownership when handling FIOASYNC is that in the case of two processes sharing access to a device, there is currently nothing that prevents a non-owner of the device from enabling this mode and causing SIGIO signals to be sent to the owner, which might not be expecting to receive them. I think if you add ownership checking, it will be incompatible with other code, people have to change their mind when dealing with this special file descriptor, my recommendation is people don't need to refresh their brain. OTOH, if it is a problem, we should have already been flooded by the problem, but in the past years, I saw zero complaining in the mailing lists. I believe that the SIGIO code was cut and pasted from a driver I was working on in the 4.x time frame. devd is the only consumer, and it doesn’t do the FIOASYNC stuff at all. So I’d be strongly biased to either (a) remove support for this or (b) make the support correct, even at the cost of speed or performance. Warner Even with my patch, I think there is a race condition, suppose owner was set, and now I turn on FIOASYNC, should SIGIO signal be sent now if input or output becomes possible ? if it is true, I found the bug in several piece of kernel code, socket, pipe etcs are all infected. ___ 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: r263755 - head/sys/kern
On 2014/03/28 06:31, Don Lewis wrote: On 27 Mar, Konstantin Belousov wrote: On Thu, Mar 27, 2014 at 04:05:12PM +0100, Mateusz Guzik wrote: On Thu, Mar 27, 2014 at 03:58:19PM +0100, Mateusz Guzik wrote: On Thu, Mar 27, 2014 at 04:46:57PM +0800, David Xu wrote: On 2014/03/27 16:37, Mateusz Guzik wrote: On Thu, Mar 27, 2014 at 03:45:17PM +0800, David Xu wrote: I think the async process pointer can be cleared when a process exits by registering an event handler. please see attached patch. Sure, but I'm not very fond of this solution. This is a rather obscure bug you wont hit unless you explicitly try, and even then you need root privs by default. OK, but I don't like the bug exists in kernel. It is not obscure for me, I can run shutdown now command, and insert a device, and then the kernel will write garbage data into freed memory space. Not sure what you mean. devd does not use this feature, and even if it did async_proc is cleared on close, which happens while signal delivery is still legal. That said, you are not going to encounter this bug unless you code something up to specifically trigger it. fwiw, I think we could axe this feature if there was no way to fix it without introducing a check for every process. As such writing a callback function which will be executed for all exiting processes seems unjustified for me. Ideally we would get some mechanism which would allow to register callbacks for events related to given entity. Then it could be used to provide a call this function when process p exits, amongst other things. Yes, but the callback itself is cheap enough and is not worth to be per-entity entry. There is other code in the kernel which would benefit from such functionality - dev/syscons/scmouse, dev/vt/vt_core.c, aio and possibly more. As such I think this is worth pursuing. We can hack around this one the way the other code is doing - apart from from proc pointer you store pid and then compare result of pfind(pid). This is still buggy as both proc and pid pointer can be recycled and end up being the same (but you have an entrirely new process). However, then in absolutely worst cae you send SIGIO to incorrect process, always an existing process so no more corruption. Would you be ok with such hack for the time being? Isn't p_sigiolist and fsetown(9) already provide the neccessary registration and cleanup on the process exit ? The KPI might require some generalization, but I think that the mechanism itself is enough. That's the correct mechanism, but it's not being used here. Something like the following untested patch should do the trick: Index: sys/kern/subr_bus.c === --- sys/kern/subr_bus.c (revision 263289) +++ sys/kern/subr_bus.c (working copy) @@ -402,7 +402,7 @@ struct cv cv; struct selinfo sel; struct devq devq; - struct proc *async_proc; + struct sigio *sigio; } devsoftc; static struct cdev *devctl_dev; @@ -425,7 +425,7 @@ /* move to init */ devsoftc.inuse = 1; devsoftc.nonblock = 0; - devsoftc.async_proc = NULL; + funsetown(devsoftc.sigio); return (0); } @@ -436,7 +436,7 @@ mtx_lock(devsoftc.mtx); cv_broadcast(devsoftc.cv); mtx_unlock(devsoftc.mtx); - devsoftc.async_proc = NULL; + funsetown(devsoftc.sigio); return (0); } @@ -492,9 +492,8 @@ return (0); case FIOASYNC: if (*(int*)data) - devsoftc.async_proc = td-td_proc; - else - devsoftc.async_proc = NULL; + return (fsetown(td-td_proc-p_pid, devsoftc.sigio)); + funsetown(devsoftc.sigio); return (0); /* (un)Support for other fcntl() calls. */ @@ -546,7 +545,6 @@ devctl_queue_data_f(char *data, int flags) { struct dev_event_info *n1 = NULL, *n2 = NULL; - struct proc *p; if (strlen(data) == 0) goto out; @@ -576,12 +574,8 @@ cv_broadcast(devsoftc.cv); mtx_unlock(devsoftc.mtx); selwakeup(devsoftc.sel); - p = devsoftc.async_proc; - if (p != NULL) { - PROC_LOCK(p); - kern_psignal(p, SIGIO); - PROC_UNLOCK(p); - } + if (devsoftc.sigio != NULL) + pgsigio(devsoftc.sigio, SIGIO, 0); return; out: /* I have tweaked it a bit, is this okay ? # HG changeset patch # Parent 53b614ff2cae108f27e4475989d3a86997017268 diff -r 53b614ff2cae sys/kern/subr_bus.c --- a/sys/kern/subr_bus.c Thu Mar 27 10:03:50 2014 +0800 +++ b/sys/kern/subr_bus.c Fri Mar 28 14:22:29 2014 +0800 @@ -391,11 +391,12 @@ int inuse; int nonblock; int queued; + int async; struct mtx mtx; struct cv cv; struct selinfo sel; struct devq
Re: svn commit: r263755 - head/sys/kern
On 2014/03/29 00:13, Don Lewis wrote: On 28 Mar, David Xu wrote: On 2014/03/28 06:31, Don Lewis wrote: On 27 Mar, Konstantin Belousov wrote: On Thu, Mar 27, 2014 at 04:05:12PM +0100, Mateusz Guzik wrote: On Thu, Mar 27, 2014 at 03:58:19PM +0100, Mateusz Guzik wrote: On Thu, Mar 27, 2014 at 04:46:57PM +0800, David Xu wrote: On 2014/03/27 16:37, Mateusz Guzik wrote: On Thu, Mar 27, 2014 at 03:45:17PM +0800, David Xu wrote: I think the async process pointer can be cleared when a process exits by registering an event handler. please see attached patch. Sure, but I'm not very fond of this solution. This is a rather obscure bug you wont hit unless you explicitly try, and even then you need root privs by default. OK, but I don't like the bug exists in kernel. It is not obscure for me, I can run shutdown now command, and insert a device, and then the kernel will write garbage data into freed memory space. Not sure what you mean. devd does not use this feature, and even if it did async_proc is cleared on close, which happens while signal delivery is still legal. That said, you are not going to encounter this bug unless you code something up to specifically trigger it. fwiw, I think we could axe this feature if there was no way to fix it without introducing a check for every process. As such writing a callback function which will be executed for all exiting processes seems unjustified for me. Ideally we would get some mechanism which would allow to register callbacks for events related to given entity. Then it could be used to provide a call this function when process p exits, amongst other things. Yes, but the callback itself is cheap enough and is not worth to be per-entity entry. There is other code in the kernel which would benefit from such functionality - dev/syscons/scmouse, dev/vt/vt_core.c, aio and possibly more. As such I think this is worth pursuing. We can hack around this one the way the other code is doing - apart from from proc pointer you store pid and then compare result of pfind(pid). This is still buggy as both proc and pid pointer can be recycled and end up being the same (but you have an entrirely new process). However, then in absolutely worst cae you send SIGIO to incorrect process, always an existing process so no more corruption. Would you be ok with such hack for the time being? Isn't p_sigiolist and fsetown(9) already provide the neccessary registration and cleanup on the process exit ? The KPI might require some generalization, but I think that the mechanism itself is enough. That's the correct mechanism, but it's not being used here. Something like the following untested patch should do the trick: Index: sys/kern/subr_bus.c === --- sys/kern/subr_bus.c (revision 263289) +++ sys/kern/subr_bus.c (working copy) @@ -402,7 +402,7 @@ struct cv cv; struct selinfo sel; struct devq devq; - struct proc *async_proc; + struct sigio *sigio; } devsoftc; static struct cdev *devctl_dev; @@ -425,7 +425,7 @@ /* move to init */ devsoftc.inuse = 1; devsoftc.nonblock = 0; - devsoftc.async_proc = NULL; + funsetown(devsoftc.sigio); return (0); } @@ -436,7 +436,7 @@ mtx_lock(devsoftc.mtx); cv_broadcast(devsoftc.cv); mtx_unlock(devsoftc.mtx); - devsoftc.async_proc = NULL; + funsetown(devsoftc.sigio); return (0); } @@ -492,9 +492,8 @@ return (0); case FIOASYNC: if (*(int*)data) - devsoftc.async_proc = td-td_proc; - else - devsoftc.async_proc = NULL; + return (fsetown(td-td_proc-p_pid, devsoftc.sigio)); + funsetown(devsoftc.sigio); return (0); /* (un)Support for other fcntl() calls. */ @@ -546,7 +545,6 @@ devctl_queue_data_f(char *data, int flags) { struct dev_event_info *n1 = NULL, *n2 = NULL; - struct proc *p; if (strlen(data) == 0) goto out; @@ -576,12 +574,8 @@ cv_broadcast(devsoftc.cv); mtx_unlock(devsoftc.mtx); selwakeup(devsoftc.sel); - p = devsoftc.async_proc; - if (p != NULL) { - PROC_LOCK(p); - kern_psignal(p, SIGIO); - PROC_UNLOCK(p); - } + if (devsoftc.sigio != NULL) + pgsigio(devsoftc.sigio, SIGIO, 0); return; out: /* I have tweaked it a bit, is this okay ? # HG changeset patch # Parent 53b614ff2cae108f27e4475989d3a86997017268 diff -r 53b614ff2cae sys/kern/subr_bus.c --- a/sys/kern/subr_bus.c Thu Mar 27 10:03:50 2014 +0800 +++ b/sys/kern/subr_bus.c Fri Mar 28 14:22:29 2014 +0800 @@ -391,11 +391,12 @@ int inuse; int nonblock; int queued; + int async; struct mtx mtx
Re: svn commit: r263755 - head/sys/kern
On 2014/03/29 10:56, Mateusz Guzik wrote: On Fri, Mar 28, 2014 at 09:13:20AM -0700, Don Lewis wrote: On 28 Mar, David Xu wrote: I have tweaked it a bit, is this okay ? # HG changeset patch # Parent 53b614ff2cae108f27e4475989d3a86997017268 diff -r 53b614ff2cae sys/kern/subr_bus.c --- a/sys/kern/subr_bus.c Thu Mar 27 10:03:50 2014 +0800 +++ b/sys/kern/subr_bus.c Fri Mar 28 14:22:29 2014 +0800 @@ -391,11 +391,12 @@ int inuse; int nonblock; int queued; + int async; struct mtx mtx; struct cv cv; struct selinfo sel; struct devq devq; - struct proc *async_proc; + struct sigio *sigio; } devsoftc; static struct cdev *devctl_dev; @@ -422,7 +423,8 @@ /* move to init */ devsoftc.inuse = 1; devsoftc.nonblock = 0; - devsoftc.async_proc = NULL; + devsoftc.async = 0; + devsoftc.sigio = NULL; mtx_unlock(devsoftc.mtx); return (0); } @@ -433,8 +435,9 @@ mtx_lock(devsoftc.mtx); devsoftc.inuse = 0; - devsoftc.async_proc = NULL; + devsoftc.async = 0; cv_broadcast(devsoftc.cv); + funsetown(devsoftc.sigio); mtx_unlock(devsoftc.mtx); return (0); } @@ -490,33 +493,21 @@ devsoftc.nonblock = 0; return (0); case FIOASYNC: - /* -* FIXME: -* Since this is a simple assignment there is no guarantee that -* devsoftc.async_proc consumers will get a valid pointer. -* -* Example scenario where things break (processes A and B): -* 1. A opens devctl -* 2. A sends fd to B -* 3. B sets itself as async_proc -* 4. B exits -* -* However, normally this requires root privileges and the only -* in-tree consumer does not behave in a dangerous way so the -* issue is not critical. -*/ if (*(int*)data) - devsoftc.async_proc = td-td_proc; + devsoftc.async = 1; else - devsoftc.async_proc = NULL; + devsoftc.async = 0; + return (0); + case FIOSETOWN: + return fsetown(*(int *)data, devsoftc.sigio); + case FIOGETOWN: + *(int *)data = fgetown(devsoftc.sigio); return (0); /* (un)Support for other fcntl() calls. */ case FIOCLEX: case FIONCLEX: case FIONREAD: - case FIOSETOWN: - case FIOGETOWN: default: break; } @@ -560,7 +551,6 @@ devctl_queue_data_f(char *data, int flags) { struct dev_event_info *n1 = NULL, *n2 = NULL; - struct proc *p; if (strlen(data) == 0) goto out; @@ -590,13 +580,8 @@ cv_broadcast(devsoftc.cv); mtx_unlock(devsoftc.mtx); selwakeup(devsoftc.sel); - /* XXX see a comment in devioctl */ - p = devsoftc.async_proc; - if (p != NULL) { - PROC_LOCK(p); - kern_psignal(p, SIGIO); - PROC_UNLOCK(p); - } + if (devsoftc.async devsoftc.sigio != NULL) + pgsigio(devsoftc.sigio, SIGIO, 0); return; out: /* That makes it work more like the other users of fsetown(), which is probably a good thing. The downside is that two syscalls are needed to activate it, which I was trying to avoid that with my patch. I noticed that logopen() in subr_log.c unconditionally calls fsetown(), which would avoid the need for an extra syscall. That also avoids the direct manipulation of the pointer in your patch, which makes me nervous about the possibility of a leak. I wonder if FIOASYNC should fail if td-td_proc != devsoftc.sigio.sio_proc (or the equivalent for other instances) to prevent a process from maniuplating the async flag for a device owned by another process. I think this check would need to be wrapped in SIGIO_LOCK()/SIGIO_UNLOCK() to be safe. But this patch would mean that current consumers (if any) would break - just calling FIOASYNC would not result in receiving SIGIO. The old behavior is inconsistent with other piece of code in the kernel and may be incompatible with POSIX. Original patch by Don seems to work fine though, but I'm unsure about one thing (present in this patch as well): There is one devsoftc.sigio instance and one can get multiple processes with devctl fd. Is it safe from kernel perspective to have multiple processes call fsetown(*(int *)data, devsoftc.sigio)? There is an inuse variable guarding this problem, you can not open it multiple times, you can only do it in the forked process which inherited the fd. if you don't trust the child process, you can close it before
Re: svn commit: r263755 - head/sys/kern
On 2014/03/29 11:25, Mateusz Guzik wrote: On Sat, Mar 29, 2014 at 11:09:34AM +0800, David Xu wrote: On 2014/03/29 10:56, Mateusz Guzik wrote: But this patch would mean that current consumers (if any) would break - just calling FIOASYNC would not result in receiving SIGIO. The old behavior is inconsistent with other piece of code in the kernel and may be incompatible with POSIX. Oh, I didn't know that. Unsure what to do in this case. Original patch by Don seems to work fine though, but I'm unsure about one thing (present in this patch as well): There is one devsoftc.sigio instance and one can get multiple processes with devctl fd. Is it safe from kernel perspective to have multiple processes call fsetown(*(int *)data, devsoftc.sigio)? There is an inuse variable guarding this problem, you can not open it multiple times, you can only do it in the forked process which inherited the fd. if you don't trust the child process, you can close it before executing real code in the child process. This does not answer my question. I can easily imagine devctl extended in the future so that there are per-jail instances (could be handy to monitor e.g. vnet related events like link changes). If fsetown handling like this is insecure this would bite us in that scenario (and few others). In short, if we can avoid giving another way to corrupt stuff in the kernel to userspace, we should. I can not see what you said, where is the security problem with fsetown ? if you have per-jail instance of devsoftc, they all are operating on their own instance. but I don't think this patch should address jail now, there are many things are not jail ready. ___ 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: r263755 - head/sys/kern
On 2014/03/29 12:14, Mateusz Guzik wrote: I asked if multpiple concurrent calls to fsetown(.., pointer) could result in some corruption in the kernel - if they could, we would have a problem in the future. I decided to read the code once more and fsetown seems to be safe in this regard after all and with that in mind the patch looks good to me. This thread is too long already, so I'm stepping down on this one in case there are some futher concerns. This thread is really long, but things must be clarified, so it was not long enough. :-) Previously I supposed you had read the fsetown code, if not, I was wrong, lol. ___ 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: r263755 - head/sys/kern
On 2014/03/26 07:30, Mateusz Guzik wrote: Author: mjg Date: Tue Mar 25 23:30:35 2014 New Revision: 263755 URL: http://svnweb.freebsd.org/changeset/base/263755 Log: Document a known problem with handling the process intended to receive SIGIO in /dev/devctl. Suggested by:adrian MFC after: 6 days Modified: head/sys/kern/subr_bus.c Modified: head/sys/kern/subr_bus.c == --- head/sys/kern/subr_bus.cTue Mar 25 23:19:45 2014(r263754) +++ head/sys/kern/subr_bus.cTue Mar 25 23:30:35 2014(r263755) @@ -490,6 +490,21 @@ devioctl(struct cdev *dev, u_long cmd, c devsoftc.nonblock = 0; return (0); case FIOASYNC: + /* +* FIXME: +* Since this is a simple assignment there is no guarantee that +* devsoftc.async_proc consumers will get a valid pointer. +* +* Example scenario where things break (processes A and B): +* 1. A opens devctl +* 2. A sends fd to B +* 3. B sets itself as async_proc +* 4. B exits +* +* However, normally this requires root privileges and the only +* in-tree consumer does not behave in a dangerous way so the +* issue is not critical. +*/ if (*(int*)data) devsoftc.async_proc = td-td_proc; else @@ -575,6 +590,7 @@ devctl_queue_data_f(char *data, int flag cv_broadcast(devsoftc.cv); mtx_unlock(devsoftc.mtx); selwakeup(devsoftc.sel); + /* XXX see a comment in devioctl */ p = devsoftc.async_proc; if (p != NULL) { PROC_LOCK(p); I think the async process pointer can be cleared when a process exits by registering an event handler. please see attached patch. # HG changeset patch # Parent 53b614ff2cae108f27e4475989d3a86997017268 diff -r 53b614ff2cae sys/kern/subr_bus.c --- a/sys/kern/subr_bus.c Thu Mar 27 10:03:50 2014 +0800 +++ b/sys/kern/subr_bus.c Thu Mar 27 15:38:40 2014 +0800 @@ -54,6 +54,7 @@ #include sys/uio.h #include sys/bus.h #include sys/interrupt.h +#include sys/eventhandler.h #include net/vnet.h @@ -399,6 +400,18 @@ } devsoftc; static struct cdev *devctl_dev; +static eventhandler_tag devctl_eh; + +static void +devctl_proc_clear(void *arg, struct proc *p, struct image_params *imgp __unused) +{ + if (devsoftc.async_proc == p) { + mtx_lock(devsoftc.mtx); + if (devsoftc.async_proc == p) + devsoftc.async_proc = NULL; + mtx_unlock(devsoftc.mtx); + } +} static void devinit(void) @@ -408,6 +421,8 @@ mtx_init(devsoftc.mtx, dev mtx, devd, MTX_DEF); cv_init(devsoftc.cv, dev cv); TAILQ_INIT(devsoftc.devq); + devctl_eh = EVENTHANDLER_REGISTER(process_exit, devctl_proc_clear, NULL, + EVENTHANDLER_PRI_ANY); } static int @@ -490,25 +505,12 @@ devsoftc.nonblock = 0; return (0); case FIOASYNC: - /* - * FIXME: - * Since this is a simple assignment there is no guarantee that - * devsoftc.async_proc consumers will get a valid pointer. - * - * Example scenario where things break (processes A and B): - * 1. A opens devctl - * 2. A sends fd to B - * 3. B sets itself as async_proc - * 4. B exits - * - * However, normally this requires root privileges and the only - * in-tree consumer does not behave in a dangerous way so the - * issue is not critical. - */ + mtx_lock(devsoftc.mtx); if (*(int*)data) devsoftc.async_proc = td-td_proc; else devsoftc.async_proc = NULL; + mtx_unlock(devsoftc.mtx); return (0); /* (un)Support for other fcntl() calls. */ @@ -588,15 +590,14 @@ TAILQ_INSERT_TAIL(devsoftc.devq, n1, dei_link); devsoftc.queued++; cv_broadcast(devsoftc.cv); - mtx_unlock(devsoftc.mtx); selwakeup(devsoftc.sel); - /* XXX see a comment in devioctl */ p = devsoftc.async_proc; if (p != NULL) { PROC_LOCK(p); kern_psignal(p, SIGIO); PROC_UNLOCK(p); } + mtx_unlock(devsoftc.mtx); return; out: /* @@ -4503,7 +4504,7 @@ return (0); case MOD_SHUTDOWN: - device_shutdown(root_bus); + EVENTHANDLER_DEREGISTER(process_exit, devctl_eh); return (0); default: return (EOPNOTSUPP); ___ 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: r263755 - head/sys/kern
On 2014/03/27 16:37, Mateusz Guzik wrote: On Thu, Mar 27, 2014 at 03:45:17PM +0800, David Xu wrote: I think the async process pointer can be cleared when a process exits by registering an event handler. please see attached patch. Sure, but I'm not very fond of this solution. This is a rather obscure bug you wont hit unless you explicitly try, and even then you need root privs by default. OK, but I don't like the bug exists in kernel. It is not obscure for me, I can run shutdown now command, and insert a device, and then the kernel will write garbage data into freed memory space. As such writing a callback function which will be executed for all exiting processes seems unjustified for me. Ideally we would get some mechanism which would allow to register callbacks for events related to given entity. Then it could be used to provide a call this function when process p exits, amongst other things. Yes, but the callback itself is cheap enough and is not worth to be per-entity entry. ___ 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: r263755 - head/sys/kern
On 2014/03/27 22:58, Mateusz Guzik wrote: On Thu, Mar 27, 2014 at 04:46:57PM +0800, David Xu wrote: On 2014/03/27 16:37, Mateusz Guzik wrote: On Thu, Mar 27, 2014 at 03:45:17PM +0800, David Xu wrote: I think the async process pointer can be cleared when a process exits by registering an event handler. please see attached patch. Sure, but I'm not very fond of this solution. This is a rather obscure bug you wont hit unless you explicitly try, and even then you need root privs by default. OK, but I don't like the bug exists in kernel. It is not obscure for me, I can run shutdown now command, and insert a device, and then the kernel will write garbage data into freed memory space. Not sure what you mean. devd does not use this feature, and even if it did async_proc is cleared on close, which happens while signal delivery is still legal. Thers is no race conidtion when using lock to protect it. That said, you are not going to encounter this bug unless you code something up to specifically trigger it. fwiw, I think we could axe this feature if there was no way to fix it without introducing a check for every process. If you added a feature, you can not assume people won't use it. As such writing a callback function which will be executed for all exiting processes seems unjustified for me. Ideally we would get some mechanism which would allow to register callbacks for events related to given entity. Then it could be used to provide a call this function when process p exits, amongst other things. Yes, but the callback itself is cheap enough and is not worth to be per-entity entry. There is other code in the kernel which would benefit from such functionality - dev/syscons/scmouse, dev/vt/vt_core.c, aio and possibly more. As such I think this is worth pursuing. I can not say more, it seems it is an extension. ___ 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: r263755 - head/sys/kern
On 2014/03/28 06:31, Don Lewis wrote: On 27 Mar, Konstantin Belousov wrote: On Thu, Mar 27, 2014 at 04:05:12PM +0100, Mateusz Guzik wrote: On Thu, Mar 27, 2014 at 03:58:19PM +0100, Mateusz Guzik wrote: On Thu, Mar 27, 2014 at 04:46:57PM +0800, David Xu wrote: On 2014/03/27 16:37, Mateusz Guzik wrote: On Thu, Mar 27, 2014 at 03:45:17PM +0800, David Xu wrote: I think the async process pointer can be cleared when a process exits by registering an event handler. please see attached patch. Sure, but I'm not very fond of this solution. This is a rather obscure bug you wont hit unless you explicitly try, and even then you need root privs by default. OK, but I don't like the bug exists in kernel. It is not obscure for me, I can run shutdown now command, and insert a device, and then the kernel will write garbage data into freed memory space. Not sure what you mean. devd does not use this feature, and even if it did async_proc is cleared on close, which happens while signal delivery is still legal. That said, you are not going to encounter this bug unless you code something up to specifically trigger it. fwiw, I think we could axe this feature if there was no way to fix it without introducing a check for every process. As such writing a callback function which will be executed for all exiting processes seems unjustified for me. Ideally we would get some mechanism which would allow to register callbacks for events related to given entity. Then it could be used to provide a call this function when process p exits, amongst other things. Yes, but the callback itself is cheap enough and is not worth to be per-entity entry. There is other code in the kernel which would benefit from such functionality - dev/syscons/scmouse, dev/vt/vt_core.c, aio and possibly more. As such I think this is worth pursuing. We can hack around this one the way the other code is doing - apart from from proc pointer you store pid and then compare result of pfind(pid). This is still buggy as both proc and pid pointer can be recycled and end up being the same (but you have an entrirely new process). However, then in absolutely worst cae you send SIGIO to incorrect process, always an existing process so no more corruption. Would you be ok with such hack for the time being? Isn't p_sigiolist and fsetown(9) already provide the neccessary registration and cleanup on the process exit ? The KPI might require some generalization, but I think that the mechanism itself is enough. That's the correct mechanism, but it's not being used here. Something like the following untested patch should do the trick: Index: sys/kern/subr_bus.c === --- sys/kern/subr_bus.c (revision 263289) +++ sys/kern/subr_bus.c (working copy) @@ -402,7 +402,7 @@ struct cv cv; struct selinfo sel; struct devq devq; - struct proc *async_proc; + struct sigio *sigio; } devsoftc; static struct cdev *devctl_dev; @@ -425,7 +425,7 @@ /* move to init */ devsoftc.inuse = 1; devsoftc.nonblock = 0; - devsoftc.async_proc = NULL; + funsetown(devsoftc.sigio); return (0); } @@ -436,7 +436,7 @@ mtx_lock(devsoftc.mtx); cv_broadcast(devsoftc.cv); mtx_unlock(devsoftc.mtx); - devsoftc.async_proc = NULL; + funsetown(devsoftc.sigio); return (0); } @@ -492,9 +492,8 @@ return (0); case FIOASYNC: if (*(int*)data) - devsoftc.async_proc = td-td_proc; - else - devsoftc.async_proc = NULL; + return (fsetown(td-td_proc-p_pid, devsoftc.sigio)); + funsetown(devsoftc.sigio); return (0); /* (un)Support for other fcntl() calls. */ @@ -546,7 +545,6 @@ devctl_queue_data_f(char *data, int flags) { struct dev_event_info *n1 = NULL, *n2 = NULL; - struct proc *p; if (strlen(data) == 0) goto out; @@ -576,12 +574,8 @@ cv_broadcast(devsoftc.cv); mtx_unlock(devsoftc.mtx); selwakeup(devsoftc.sel); - p = devsoftc.async_proc; - if (p != NULL) { - PROC_LOCK(p); - kern_psignal(p, SIGIO); - PROC_UNLOCK(p); - } + if (devsoftc.sigio != NULL) + pgsigio(devsoftc.sigio, SIGIO, 0); return; out: /* Hope this works. ___ 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
svn commit: r263255 - stable/9/lib/libc/gen
Author: davidxu Date: Mon Mar 17 02:10:45 2014 New Revision: 263255 URL: http://svnweb.freebsd.org/changeset/base/263255 Log: MFC r263107: To avoid missing a chance to cancel thread, call _pthread_testcancel at the beginning of _sem_timedwait. Submitted by: Eric van Gyzen lt; eric at vangyzen dot net gt; Modified: stable/9/lib/libc/gen/sem_new.c Directory Properties: stable/9/ (props changed) stable/9/lib/ (props changed) stable/9/lib/libc/ (props changed) Modified: stable/9/lib/libc/gen/sem_new.c == --- stable/9/lib/libc/gen/sem_new.c Sun Mar 16 22:56:22 2014 (r263254) +++ stable/9/lib/libc/gen/sem_new.c Mon Mar 17 02:10:45 2014 (r263255) @@ -393,6 +393,7 @@ _sem_timedwait(sem_t * __restrict sem, return (-1); retval = 0; + _pthread_testcancel(); for (;;) { while ((val = sem-_kern._count) 0) { if (atomic_cmpset_acq_int(sem-_kern._count, val, val - 1)) ___ 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
svn commit: r263258 - stable/10/lib/libc/gen
Author: davidxu Date: Mon Mar 17 05:03:53 2014 New Revision: 263258 URL: http://svnweb.freebsd.org/changeset/base/263258 Log: MFC r263107: To avoid missing a chance to cancel thread, call _pthread_testcancel at the beginning of _sem_timedwait. Submitted by: Eric van Gyzen lt; eric at vangyzen dot net gt; Modified: stable/10/lib/libc/gen/sem_new.c Directory Properties: stable/10/ (props changed) Modified: stable/10/lib/libc/gen/sem_new.c == --- stable/10/lib/libc/gen/sem_new.cMon Mar 17 04:38:10 2014 (r263257) +++ stable/10/lib/libc/gen/sem_new.cMon Mar 17 05:03:53 2014 (r263258) @@ -381,6 +381,7 @@ _sem_timedwait(sem_t * __restrict sem, return (-1); retval = 0; + _pthread_testcancel(); for (;;) { while ((val = sem-_kern._count) 0) { if (atomic_cmpset_acq_int(sem-_kern._count, val, val - 1)) ___ 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
svn commit: r263107 - head/lib/libc/gen
Author: davidxu Date: Thu Mar 13 06:54:10 2014 New Revision: 263107 URL: http://svnweb.freebsd.org/changeset/base/263107 Log: To avoid missing a chance to cancel thread, call _pthread_testcancel at the beginning of _sem_timedwait. Submitted by: Eric van Gyzen lt; eric at vangyzen dot net gt; MFC after:3 days Modified: head/lib/libc/gen/sem_new.c Modified: head/lib/libc/gen/sem_new.c == --- head/lib/libc/gen/sem_new.c Thu Mar 13 05:17:53 2014(r263106) +++ head/lib/libc/gen/sem_new.c Thu Mar 13 06:54:10 2014(r263107) @@ -381,6 +381,7 @@ _sem_timedwait(sem_t * __restrict sem, return (-1); retval = 0; + _pthread_testcancel(); for (;;) { while ((val = sem-_kern._count) 0) { if (atomic_cmpset_acq_int(sem-_kern._count, val, val - 1)) ___ 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
svn commit: r262544 - stable/10/libexec/rtld-elf
Author: davidxu Date: Thu Feb 27 02:36:09 2014 New Revision: 262544 URL: http://svnweb.freebsd.org/changeset/base/262544 Log: MFC r262277: malloc_aligned() may not leave enough space for pointer to allocated memory, saving the pointer will overwrite bytes belongs to another memory block unexpectly, to fix the problem, use (allocated address + sizeof(void *)) as initial value, and slip to next aligned address, so maximum extra bytes is sizeof(void *) + align - 1. Tested by: Andre Albsmeier mail at ma17 dot ata dot myota dot orgndre MFC r262334: Increase alignment to size of pointer if the alignment is too small. Some modules do not align data at least to size of pointer, they uses a smaller alignment, but our pointer should be aligned to its native boundary, otherwise on some platforms, hardware alignment checking will cause bus error. Modified: stable/10/libexec/rtld-elf/xmalloc.c Directory Properties: stable/10/ (props changed) Modified: stable/10/libexec/rtld-elf/xmalloc.c == --- stable/10/libexec/rtld-elf/xmalloc.cThu Feb 27 01:24:47 2014 (r262543) +++ stable/10/libexec/rtld-elf/xmalloc.cThu Feb 27 02:36:09 2014 (r262544) @@ -72,14 +72,12 @@ void * malloc_aligned(size_t size, size_t align) { void *mem, *res; - uintptr_t x; - size_t asize, r; - r = round(sizeof(void *), align); - asize = round(size, align) + r; - mem = xmalloc(asize); - x = (uintptr_t)mem; - res = (void *)round(x, align); + if (align sizeof(void *)) + align = sizeof(void *); + + mem = xmalloc(size + sizeof(void *) + align - 1); + res = (void *)round((uintptr_t)mem + sizeof(void *), align); *(void **)((uintptr_t)res - sizeof(void *)) = mem; return (res); } ___ 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
svn commit: r262545 - stable/9/libexec/rtld-elf
Author: davidxu Date: Thu Feb 27 02:41:41 2014 New Revision: 262545 URL: http://svnweb.freebsd.org/changeset/base/262545 Log: MFC r262277: malloc_aligned() may not leave enough space for pointer to allocated memory, saving the pointer will overwrite bytes belongs to another memory block unexpectly, to fix the problem, use (allocated address + sizeof(void *)) as initial value, and slip to next aligned address, so maximum extra bytes is sizeof(void *) + align - 1. Tested by: Andre Albsmeier mail at ma17 dot ata dot myota dot orgndre MFC r262334: Increase alignment to size of pointer if the alignment is too small. Some modules do not align data at least to size of pointer, they uses a smaller alignment, but our pointer should be aligned to its native boundary, otherwise on some platforms, hardware alignment checking will cause bus error. Modified: stable/9/libexec/rtld-elf/xmalloc.c Directory Properties: stable/9/ (props changed) stable/9/libexec/rtld-elf/ (props changed) Modified: stable/9/libexec/rtld-elf/xmalloc.c == --- stable/9/libexec/rtld-elf/xmalloc.c Thu Feb 27 02:36:09 2014 (r262544) +++ stable/9/libexec/rtld-elf/xmalloc.c Thu Feb 27 02:41:41 2014 (r262545) @@ -72,14 +72,12 @@ void * malloc_aligned(size_t size, size_t align) { void *mem, *res; - uintptr_t x; - size_t asize, r; - r = round(sizeof(void *), align); - asize = round(size, align) + r; - mem = xmalloc(asize); - x = (uintptr_t)mem; - res = (void *)round(x, align); + if (align sizeof(void *)) + align = sizeof(void *); + + mem = xmalloc(size + sizeof(void *) + align - 1); + res = (void *)round((uintptr_t)mem + sizeof(void *), align); *(void **)((uintptr_t)res - sizeof(void *)) = mem; return (res); } ___ 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
svn commit: r262334 - head/libexec/rtld-elf
Author: davidxu Date: Sat Feb 22 11:06:48 2014 New Revision: 262334 URL: http://svnweb.freebsd.org/changeset/base/262334 Log: Increase alignment to size of pointer if the alignment is too small. Some modules do not align data at least to size of pointer, they uses a smaller alignment, but our pointer should be aligned to its native boundary, otherwise on some platforms, hardware alignment checking will cause bus error. Modified: head/libexec/rtld-elf/xmalloc.c Modified: head/libexec/rtld-elf/xmalloc.c == --- head/libexec/rtld-elf/xmalloc.c Sat Feb 22 10:15:27 2014 (r262333) +++ head/libexec/rtld-elf/xmalloc.c Sat Feb 22 11:06:48 2014 (r262334) @@ -73,10 +73,8 @@ malloc_aligned(size_t size, size_t align { void *mem, *res; - if (align (sizeof(void *) -1)) { - rtld_fdputstr(STDERR_FILENO, Invalid alignment\n); - _exit(1); - } + if (align sizeof(void *)) + align = sizeof(void *); mem = xmalloc(size + sizeof(void *) + align - 1); res = (void *)round((uintptr_t)mem + sizeof(void *), align); ___ 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
svn commit: r262277 - head/libexec/rtld-elf
Author: davidxu Date: Fri Feb 21 03:36:16 2014 New Revision: 262277 URL: http://svnweb.freebsd.org/changeset/base/262277 Log: malloc_aligned() may not leave enough space for pointer to allocated memory, saving the pointer will overwrite bytes belongs to another memory block unexpectly, to fix the problem, use (allocated address + sizeof(void *)) as initial value, and slip to next aligned address, so maximum extra bytes is sizeof(void *) + align - 1. Tested by: Andre Albsmeier mail at ma17 dot ata dot myota dot orgndre Modified: head/libexec/rtld-elf/xmalloc.c Modified: head/libexec/rtld-elf/xmalloc.c == --- head/libexec/rtld-elf/xmalloc.c Fri Feb 21 03:35:43 2014 (r262276) +++ head/libexec/rtld-elf/xmalloc.c Fri Feb 21 03:36:16 2014 (r262277) @@ -72,14 +72,14 @@ void * malloc_aligned(size_t size, size_t align) { void *mem, *res; - uintptr_t x; - size_t asize, r; - r = round(sizeof(void *), align); - asize = round(size, align) + r; - mem = xmalloc(asize); - x = (uintptr_t)mem; - res = (void *)round(x, align); + if (align (sizeof(void *) -1)) { + rtld_fdputstr(STDERR_FILENO, Invalid alignment\n); + _exit(1); + } + + mem = xmalloc(size + sizeof(void *) + align - 1); + res = (void *)round((uintptr_t)mem + sizeof(void *), align); *(void **)((uintptr_t)res - sizeof(void *)) = mem; return (res); } ___ 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
svn commit: r254398 - in stable/9: include lib/libc/gen lib/libc/sys lib/libthr/thread sys/compat/freebsd32 sys/kern sys/sys
elf_utils.c erand48.c err.c errlst.c errno.c \ exec.c fdevname.c feature_present.c fmtcheck.c fmtmsg.c fnmatch.c \ Modified: stable/9/lib/libc/gen/Symbol.map == --- stable/9/lib/libc/gen/Symbol.mapFri Aug 16 05:30:13 2013 (r254397) +++ stable/9/lib/libc/gen/Symbol.mapFri Aug 16 06:40:12 2013 (r254398) @@ -380,6 +380,8 @@ FBSD_1.2 { }; FBSD_1.3 { + clock_getcpuclockid; + dirfd; fdlopen; __FreeBSD_libc_enter_restricted_mode; getcontextx; Copied: stable/9/lib/libc/gen/clock_getcpuclockid.c (from r239347, head/lib/libc/gen/clock_getcpuclockid.c) == --- /dev/null 00:00:00 1970 (empty, because file is newly added) +++ stable/9/lib/libc/gen/clock_getcpuclockid.c Fri Aug 16 06:40:12 2013 (r254398, copy of r239347, head/lib/libc/gen/clock_getcpuclockid.c) @@ -0,0 +1,39 @@ +/* + * Copyright (c) 2012 David Xu davi...@freebsd.org. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + *notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + *notice, this list of conditions and the following disclaimer in the + *documentation and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY DANIEL EISCHEN AND CONTRIBUTORS ``AS IS'' + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + */ + +#include sys/cdefs.h +__FBSDID($FreeBSD$); + +#include errno.h +#include time.h +#include unistd.h +#include sys/time.h + +clockid_t +clock_getcpuclockid(pid_t pid, clockid_t *clock_id) +{ + return clock_getcpuclockid2(pid, CPUCLOCK_WHICH_PID, clock_id); +} Modified: stable/9/lib/libc/gen/sysconf.c == --- stable/9/lib/libc/gen/sysconf.c Fri Aug 16 05:30:13 2013 (r254397) +++ stable/9/lib/libc/gen/sysconf.c Fri Aug 16 06:40:12 2013 (r254398) @@ -359,11 +359,7 @@ yesno: return (_POSIX_CLOCK_SELECTION); #endif case _SC_CPUTIME: -#if _POSIX_CPUTIME == 0 -#error _POSIX_CPUTIME -#else return (_POSIX_CPUTIME); -#endif #ifdef notdef case _SC_FILE_LOCKING: /* Modified: stable/9/lib/libc/sys/Symbol.map == --- stable/9/lib/libc/sys/Symbol.mapFri Aug 16 05:30:13 2013 (r254397) +++ stable/9/lib/libc/sys/Symbol.mapFri Aug 16 06:40:12 2013 (r254398) @@ -379,6 +379,7 @@ FBSD_1.2 { }; FBSD_1.3 { + clock_getcpuclockid2; posix_fadvise; wait6; }; @@ -488,6 +489,8 @@ FBSDprivate_1.0 { __sys_chown; _chroot; __sys_chroot; + _clock_getcpuclockid2; + __sys_clock_getcpuclockid2; _clock_getres; __sys_clock_getres; _clock_gettime; Modified: stable/9/lib/libthr/thread/thr_getcpuclockid.c == --- stable/9/lib/libthr/thread/thr_getcpuclockid.c Fri Aug 16 05:30:13 2013(r254397) +++ stable/9/lib/libthr/thread/thr_getcpuclockid.c Fri Aug 16 06:40:12 2013(r254398) @@ -39,9 +39,11 @@ __weak_reference(_pthread_getcpuclockid, int _pthread_getcpuclockid(pthread_t pthread, clockid_t *clock_id) { + if (pthread == NULL) return (EINVAL); - *clock_id = CLOCK_THREAD_CPUTIME_ID; + if (clock_getcpuclockid2(TID(pthread), CPUCLOCK_WHICH_TID, clock_id)) + return (errno); return (0); } Modified: stable/9/sys/compat/freebsd32/freebsd32_syscall.h == --- stable/9/sys/compat/freebsd32/freebsd32_syscall.h Fri Aug 16 05:30:13 2013(r254397) +++ stable/9/sys/compat/freebsd32/freebsd32_syscall.h Fri Aug 16 06:40:12 2013(r254398) @@ -212,6 +212,7 @@ #define
Re: svn commit: r251886 - in head: contrib/apr contrib/apr-util contrib/serf contrib/sqlite3 contrib/subversion share/mk usr.bin usr.bin/svn usr.bin/svn/lib usr.bin/svn/lib/libapr usr.bin/svn/lib/liba
On 2013/06/19 13:50, Alexey Dokuchaev wrote: On Tue, Jun 18, 2013 at 03:47:44PM -0400, Garance A Drosehn wrote: Note that a major change to the FreeBSD repo would require that users install a new 'svn' anyway, even if they did install 'svn' back when they first installed FreeBSD. IMO, I think this 'svnlite' idea is a good move. I wouldn't want a full-blown official 'svn' in the base system, but just enough that a user can immediately get freebsd-specific updates without first needing to install some port. Frankly I find it somehow wrong that user who wants to immediately get freebsd-specific updates should use any source-control tools and compile stuff from sources. That what developers usually do, not users. Users, in their majority, do not like to build things, and that makes perfect sense. As Ubuntu user at $work, I never had to worry about how and where shall we get updates, yet the system somehow gets them, installs them, only occasionally asking for a reboot. To do this, one has to put everything in a package managment system, let the system resolve what should be upgraded via package dependence. Now, if our user is showing developer's attitude and wants to make world, then it's fair to assume that he knows how to install things from ports, or has his own static svn client with him all the time, or mounts /usr/src over NFS -- whatever works better for him. ./danfe ___ 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
svn commit: r250691 - head/lib/libthr/thread
Author: davidxu Date: Thu May 16 03:01:04 2013 New Revision: 250691 URL: http://svnweb.freebsd.org/changeset/base/250691 Log: Return one-based key so that user can check if the key is ever allocated in the first place. Initial patch submitted by: phk Modified: head/lib/libthr/thread/thr_spec.c Modified: head/lib/libthr/thread/thr_spec.c == --- head/lib/libthr/thread/thr_spec.c Thu May 16 00:56:41 2013 (r250690) +++ head/lib/libthr/thread/thr_spec.c Thu May 16 03:01:04 2013 (r250691) @@ -70,7 +70,7 @@ _pthread_key_create(pthread_key_t *key, /* Unlock the key table: */ THR_LOCK_RELEASE(curthread, _keytable_lock); - *key = i; + *key = i + 1; return (0); } @@ -81,9 +81,10 @@ _pthread_key_create(pthread_key_t *key, } int -_pthread_key_delete(pthread_key_t key) +_pthread_key_delete(pthread_key_t userkey) { struct pthread *curthread = _get_curthread(); + int key = userkey - 1; int ret = 0; if ((unsigned int)key PTHREAD_KEYS_MAX) { @@ -178,9 +179,10 @@ pthread_key_allocate_data(void) } int -_pthread_setspecific(pthread_key_t key, const void *value) +_pthread_setspecific(pthread_key_t userkey, const void *value) { struct pthread *pthread; + pthread_key_t key = userkey - 1; int ret = 0; /* Point to the running thread: */ @@ -209,9 +211,10 @@ _pthread_setspecific(pthread_key_t key, } void * -_pthread_getspecific(pthread_key_t key) +_pthread_getspecific(pthread_key_t userkey) { struct pthread *pthread; + pthread_key_t key = userkey - 1; const void *data; /* Point to the running thread: */ ___ 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
svn commit: r250402 - head/lib/libthr/thread
Author: davidxu Date: Thu May 9 04:41:03 2013 New Revision: 250402 URL: http://svnweb.freebsd.org/changeset/base/250402 Log: Fix return value for setcontext and swapcontext. Modified: head/lib/libthr/thread/thr_sig.c Modified: head/lib/libthr/thread/thr_sig.c == --- head/lib/libthr/thread/thr_sig.cThu May 9 02:23:02 2013 (r250401) +++ head/lib/libthr/thread/thr_sig.cThu May 9 04:41:03 2013 (r250402) @@ -725,8 +725,10 @@ _setcontext(const ucontext_t *ucp) { ucontext_t uc; - if (ucp == NULL) - return (EINVAL); + if (ucp == NULL) { + errno = EINVAL; + return (-1); + } if (!SIGISMEMBER(uc.uc_sigmask, SIGCANCEL)) return __sys_setcontext(ucp); (void) memcpy(uc, ucp, sizeof(uc)); @@ -740,8 +742,10 @@ _swapcontext(ucontext_t *oucp, const uco { ucontext_t uc; - if (oucp == NULL || ucp == NULL) - return (EINVAL); + if (oucp == NULL || ucp == NULL) { + errno = EINVAL; + return (-1); + } if (SIGISMEMBER(ucp-uc_sigmask, SIGCANCEL)) { (void) memcpy(uc, ucp, sizeof(uc)); SIGDELSET(uc.uc_sigmask, SIGCANCEL); ___ 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: r249605 - head/sys/vm
On 2013/04/18 13:34, Alan Cox wrote: Author: alc Date: Thu Apr 18 05:34:33 2013 New Revision: 249605 URL: http://svnweb.freebsd.org/changeset/base/249605 Log: When calculating the number of reserved nodes, discount the pages that will be used to store the nodes. Sponsored by:EMC / Isilon Storage Division Modified: head/sys/vm/vm_radix.c Modified: head/sys/vm/vm_radix.c == --- head/sys/vm/vm_radix.c Thu Apr 18 05:12:11 2013(r249604) +++ head/sys/vm/vm_radix.c Thu Apr 18 05:34:33 2013(r249605) @@ -360,10 +360,17 @@ vm_radix_node_zone_init(void *mem, int s static void vm_radix_prealloc(void *arg __unused) { + int nodes; - if (!uma_zone_reserve_kva(vm_radix_node_zone, cnt.v_page_count)) + /* +* Calculate the number of reserved nodes, discounting the pages that +* are needed to store them. +*/ + nodes = ((vm_paddr_t)cnt.v_page_count * PAGE_SIZE) / (PAGE_SIZE + + sizeof(struct vm_radix_node)); + if (!uma_zone_reserve_kva(vm_radix_node_zone, nodes)) panic(%s: unable to create new zone, __func__); - uma_prealloc(vm_radix_node_zone, cnt.v_page_count); + uma_prealloc(vm_radix_node_zone, nodes); } SYSINIT(vm_radix_prealloc, SI_SUB_KMEM, SI_ORDER_SECOND, vm_radix_prealloc, NULL); FYI, after this change, my network card no longer works, the driver /sys/dev/if_msk.c reports watchdog timeout, backing out this change works again for me. Regards, David Xu ___ 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
svn commit: r250013 - head/lib/libthr/thread
Author: davidxu Date: Sun Apr 28 03:13:45 2013 New Revision: 250013 URL: http://svnweb.freebsd.org/changeset/base/250013 Log: Remove extra code for SA_RESETHAND, it is not needed because kernel has already done this. Modified: head/lib/libthr/thread/thr_sig.c Modified: head/lib/libthr/thread/thr_sig.c == --- head/lib/libthr/thread/thr_sig.cSun Apr 28 02:23:39 2013 (r250012) +++ head/lib/libthr/thread/thr_sig.cSun Apr 28 03:13:45 2013 (r250013) @@ -336,13 +336,6 @@ check_deferred_signal(struct pthread *cu memcpy(info, curthread-deferred_siginfo, sizeof(siginfo_t)); /* remove signal */ curthread-deferred_siginfo.si_signo = 0; - if (act.sa_flags SA_RESETHAND) { - struct sigaction tact; - - tact = act; - tact.sa_handler = SIG_DFL; - _sigaction(info.si_signo, tact, NULL); - } handle_signal(act, info.si_signo, info, uc); } } ___ 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
svn commit: r249604 - head/lib/libthr/thread
Author: davidxu Date: Thu Apr 18 05:12:11 2013 New Revision: 249604 URL: http://svnweb.freebsd.org/changeset/base/249604 Log: Revert revision 249323, the PR/177624 is confusing, that bug is caused by using buggy getcontext/setcontext on same stack, while swapcontext normally works on different stack, there is no such a problem. Modified: head/lib/libthr/thread/thr_sig.c Modified: head/lib/libthr/thread/thr_sig.c == --- head/lib/libthr/thread/thr_sig.cThu Apr 18 02:20:58 2013 (r249603) +++ head/lib/libthr/thread/thr_sig.cThu Apr 18 05:12:11 2013 (r249604) @@ -737,4 +737,13 @@ _setcontext(const ucontext_t *ucp) return __sys_setcontext(uc); } -__weak_reference(__sys_swapcontext, swapcontext); +__weak_reference(_swapcontext, swapcontext); +int +_swapcontext(ucontext_t *oucp, const ucontext_t *ucp) +{ + ucontext_t uc; + + (void) memcpy(uc, ucp, sizeof(uc)); + remove_thr_signals(uc.uc_sigmask); + return __sys_swapcontext(oucp, uc); +} ___ 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
svn commit: r249606 - head/lib/libthr/thread
Author: davidxu Date: Thu Apr 18 05:56:00 2013 New Revision: 249606 URL: http://svnweb.freebsd.org/changeset/base/249606 Log: Avoid copying memory if SIGCANCEL is not masked. Modified: head/lib/libthr/thread/thr_sig.c Modified: head/lib/libthr/thread/thr_sig.c == --- head/lib/libthr/thread/thr_sig.cThu Apr 18 05:34:33 2013 (r249605) +++ head/lib/libthr/thread/thr_sig.cThu Apr 18 05:56:00 2013 (r249606) @@ -732,8 +732,12 @@ _setcontext(const ucontext_t *ucp) { ucontext_t uc; + if (ucp == NULL) + return (EINVAL); + if (!SIGISMEMBER(uc.uc_sigmask, SIGCANCEL)) + return __sys_setcontext(ucp); (void) memcpy(uc, ucp, sizeof(uc)); - remove_thr_signals(uc.uc_sigmask); + SIGDELSET(uc.uc_sigmask, SIGCANCEL); return __sys_setcontext(uc); } @@ -743,7 +747,13 @@ _swapcontext(ucontext_t *oucp, const uco { ucontext_t uc; - (void) memcpy(uc, ucp, sizeof(uc)); - remove_thr_signals(uc.uc_sigmask); - return __sys_swapcontext(oucp, uc); + if (oucp == NULL || ucp == NULL) + return (EINVAL); + if (SIGISMEMBER(ucp-uc_sigmask, SIGCANCEL)) { + stdout_debug(remove SIGCANCEL\n); + (void) memcpy(uc, ucp, sizeof(uc)); + SIGDELSET(uc.uc_sigmask, SIGCANCEL); + ucp = uc; + } + return __sys_swapcontext(oucp, ucp); } ___ 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
svn commit: r249607 - head/lib/libthr/thread
Author: davidxu Date: Thu Apr 18 05:58:07 2013 New Revision: 249607 URL: http://svnweb.freebsd.org/changeset/base/249607 Log: Remove debug code. Modified: head/lib/libthr/thread/thr_sig.c Modified: head/lib/libthr/thread/thr_sig.c == --- head/lib/libthr/thread/thr_sig.cThu Apr 18 05:56:00 2013 (r249606) +++ head/lib/libthr/thread/thr_sig.cThu Apr 18 05:58:07 2013 (r249607) @@ -750,7 +750,6 @@ _swapcontext(ucontext_t *oucp, const uco if (oucp == NULL || ucp == NULL) return (EINVAL); if (SIGISMEMBER(ucp-uc_sigmask, SIGCANCEL)) { - stdout_debug(remove SIGCANCEL\n); (void) memcpy(uc, ucp, sizeof(uc)); SIGDELSET(uc.uc_sigmask, SIGCANCEL); ucp = uc; ___ 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
svn commit: r249323 - head/lib/libthr/thread
Author: davidxu Date: Wed Apr 10 02:40:03 2013 New Revision: 249323 URL: http://svnweb.freebsd.org/changeset/base/249323 Log: swapcontext wrapper can not be implemented in C, the stack pointer saved in the context becomes invalid when the function returns, same as setjmp, it must be implemented in assemble language, see discussions in PR misc/177624. Modified: head/lib/libthr/thread/thr_sig.c Modified: head/lib/libthr/thread/thr_sig.c == --- head/lib/libthr/thread/thr_sig.cWed Apr 10 02:18:17 2013 (r249322) +++ head/lib/libthr/thread/thr_sig.cWed Apr 10 02:40:03 2013 (r249323) @@ -737,13 +737,4 @@ _setcontext(const ucontext_t *ucp) return __sys_setcontext(uc); } -__weak_reference(_swapcontext, swapcontext); -int -_swapcontext(ucontext_t *oucp, const ucontext_t *ucp) -{ - ucontext_t uc; - - (void) memcpy(uc, ucp, sizeof(uc)); - remove_thr_signals(uc.uc_sigmask); - return __sys_swapcontext(oucp, uc); -} +__weak_reference(__sys_swapcontext, swapcontext); ___ 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
svn commit: r246894 - head/lib/libc/gen
Author: davidxu Date: Sun Feb 17 02:52:42 2013 New Revision: 246894 URL: http://svnweb.freebsd.org/changeset/base/246894 Log: Make more code be protected by internal mutex, and now it is fork-safe, in error case, the file exclusive lock is now released as soon as possible, in previous code, child process can still hold the exclusive lock. Modified: head/lib/libc/gen/sem_new.c Modified: head/lib/libc/gen/sem_new.c == --- head/lib/libc/gen/sem_new.c Sun Feb 17 02:15:19 2013(r246893) +++ head/lib/libc/gen/sem_new.c Sun Feb 17 02:52:42 2013(r246894) @@ -229,18 +229,18 @@ _sem_open(const char *name, int flags, . ni-open_count = 1; ni-sem = sem; LIST_INSERT_HEAD(sem_list, ni, next); - _pthread_mutex_unlock(sem_llock); _close(fd); + _pthread_mutex_unlock(sem_llock); return (sem); error: errsave = errno; - _pthread_mutex_unlock(sem_llock); if (fd != -1) _close(fd); if (sem != NULL) munmap(sem, sizeof(sem_t)); free(ni); + _pthread_mutex_unlock(sem_llock); errno = errsave; return (SEM_FAILED); } ___ 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
svn commit: r246872 - head/lib/libc/gen
Author: davidxu Date: Sat Feb 16 06:07:07 2013 New Revision: 246872 URL: http://svnweb.freebsd.org/changeset/base/246872 Log: Simplify code by using flag O_EXLOCK. PR: kern/175674 Modified: head/lib/libc/gen/sem_new.c Modified: head/lib/libc/gen/sem_new.c == --- head/lib/libc/gen/sem_new.c Sat Feb 16 05:22:48 2013(r246871) +++ head/lib/libc/gen/sem_new.c Sat Feb 16 06:07:07 2013(r246872) @@ -198,15 +198,11 @@ _sem_open(const char *name, int flags, . goto error; } - fd = _open(path, flags|O_RDWR|O_CLOEXEC, mode); + fd = _open(path, flags|O_RDWR|O_CLOEXEC|O_EXLOCK, mode); if (fd == -1) goto error; - if (flock(fd, LOCK_EX) == -1) + if (_fstat(fd, sb)) goto error; - if (_fstat(fd, sb)) { - flock(fd, LOCK_UN); - goto error; - } if (sb.st_size sizeof(sem_t)) { sem_t tmp; @@ -214,10 +210,8 @@ _sem_open(const char *name, int flags, . tmp._kern._has_waiters = 0; tmp._kern._count = value; tmp._kern._flags = USYNC_PROCESS_SHARED | SEM_NAMED; - if (_write(fd, tmp, sizeof(tmp)) != sizeof(tmp)) { - flock(fd, LOCK_UN); + if (_write(fd, tmp, sizeof(tmp)) != sizeof(tmp)) goto error; - } } flock(fd, LOCK_UN); sem = (sem_t *)mmap(NULL, sizeof(sem_t), PROT_READ|PROT_WRITE, ___ 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
svn commit: r245761 - head/gnu/usr.bin/grep
Author: davidxu Date: Tue Jan 22 03:23:14 2013 New Revision: 245761 URL: http://svnweb.freebsd.org/changeset/base/245761 Log: Make -D skip option work with FIFO by opening file in non-blocking mode. Reviewed by: jhb Tested by:delphij Modified: head/gnu/usr.bin/grep/grep.c Modified: head/gnu/usr.bin/grep/grep.c == --- head/gnu/usr.bin/grep/grep.cTue Jan 22 02:57:53 2013 (r245760) +++ head/gnu/usr.bin/grep/grep.cTue Jan 22 03:23:14 2013 (r245761) @@ -304,7 +304,7 @@ reset (int fd, char const *file, struct if (directories == SKIP_DIRECTORIES S_ISDIR (stats-stat.st_mode)) return 0; #ifndef DJGPP - if (devices == SKIP_DEVICES (S_ISCHR(stats-stat.st_mode) || S_ISBLK(stats-stat.st_mode) || S_ISSOCK(stats-stat.st_mode))) + if (devices == SKIP_DEVICES (S_ISCHR(stats-stat.st_mode) || S_ISBLK(stats-stat.st_mode) || S_ISSOCK(stats-stat.st_mode) || S_ISFIFO(stats-stat.st_mode))) #else if (devices == SKIP_DEVICES (S_ISCHR(stats-stat.st_mode) || S_ISBLK(stats-stat.st_mode))) #endif @@ -942,6 +942,7 @@ grepfile (char const *file, struct stats int desc; int count; int status; + int flags; if (! file) { @@ -950,7 +951,7 @@ grepfile (char const *file, struct stats } else { - while ((desc = open (file, O_RDONLY)) 0 errno == EINTR) + while ((desc = open (file, O_RDONLY | O_NONBLOCK)) 0 errno == EINTR) continue; if (desc 0) @@ -990,6 +991,9 @@ grepfile (char const *file, struct stats return 1; } + flags = fcntl(desc, F_GETFL); + flags = ~O_NONBLOCK; + fcntl(desc, F_SETFL, flags); filename = file; } ___ 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
svn commit: r245036 - head/sys/kern
Author: davidxu Date: Fri Jan 4 11:11:12 2013 New Revision: 245036 URL: http://svnweb.freebsd.org/changeset/base/245036 Log: Revert revision 244760 because strncpy pads trailing space with zero, this prevents kernel data from being leaked. Noticed by: Joerg Sonnenberger lt; joerg at britannica dot bec dot de gt; Modified: head/sys/kern/vfs_mount.c Modified: head/sys/kern/vfs_mount.c == --- head/sys/kern/vfs_mount.c Fri Jan 4 09:52:09 2013(r245035) +++ head/sys/kern/vfs_mount.c Fri Jan 4 11:11:12 2013(r245036) @@ -559,7 +559,7 @@ vfs_donmount(struct thread *td, uint64_t if (error || fstype[fstypelen - 1] != '\0') { error = EINVAL; if (errmsg != NULL) - strlcpy(errmsg, Invalid fstype, errmsg_len); + strncpy(errmsg, Invalid fstype, errmsg_len); goto bail; } fspathlen = 0; @@ -567,7 +567,7 @@ vfs_donmount(struct thread *td, uint64_t if (error || fspath[fspathlen - 1] != '\0') { error = EINVAL; if (errmsg != NULL) - strlcpy(errmsg, Invalid fspath, errmsg_len); + strncpy(errmsg, Invalid fspath, errmsg_len); goto bail; } @@ -1447,7 +1447,7 @@ vfs_filteropt(struct vfsoptlist *opts, c if (ret != 0) { TAILQ_FOREACH(opt, opts, link) { if (strcmp(opt-name, errmsg) == 0) { - strlcpy((char *)opt-value, errmsg, opt-len); + strncpy((char *)opt-value, errmsg, opt-len); break; } } ___ 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
svn commit: r244760 - head/sys/kern
Author: davidxu Date: Fri Dec 28 02:43:33 2012 New Revision: 244760 URL: http://svnweb.freebsd.org/changeset/base/244760 Log: Use strlcpy to NULL-terminate error message even if user provided a short buffer. Modified: head/sys/kern/vfs_mount.c Modified: head/sys/kern/vfs_mount.c == --- head/sys/kern/vfs_mount.c Fri Dec 28 01:42:32 2012(r244759) +++ head/sys/kern/vfs_mount.c Fri Dec 28 02:43:33 2012(r244760) @@ -559,7 +559,7 @@ vfs_donmount(struct thread *td, uint64_t if (error || fstype[fstypelen - 1] != '\0') { error = EINVAL; if (errmsg != NULL) - strncpy(errmsg, Invalid fstype, errmsg_len); + strlcpy(errmsg, Invalid fstype, errmsg_len); goto bail; } fspathlen = 0; @@ -567,7 +567,7 @@ vfs_donmount(struct thread *td, uint64_t if (error || fspath[fspathlen - 1] != '\0') { error = EINVAL; if (errmsg != NULL) - strncpy(errmsg, Invalid fspath, errmsg_len); + strlcpy(errmsg, Invalid fspath, errmsg_len); goto bail; } @@ -1447,7 +1447,7 @@ vfs_filteropt(struct vfsoptlist *opts, c if (ret != 0) { TAILQ_FOREACH(opt, opts, link) { if (strcmp(opt-name, errmsg) == 0) { - strncpy((char *)opt-value, errmsg, opt-len); + strlcpy((char *)opt-value, errmsg, opt-len); break; } } ___ 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
svn commit: r244695 - head/sys/dev/pci
Author: davidxu Date: Wed Dec 26 13:07:17 2012 New Revision: 244695 URL: http://svnweb.freebsd.org/changeset/base/244695 Log: Always initialize pattern_buf pointers to NULL, otherwise AMD64 machine panics with: free: address xxx(yyy) has not been allocated. it can be triggered by hald. Modified: head/sys/dev/pci/pci_user.c Modified: head/sys/dev/pci/pci_user.c == --- head/sys/dev/pci/pci_user.c Wed Dec 26 08:20:27 2012(r244694) +++ head/sys/dev/pci/pci_user.c Wed Dec 26 13:07:17 2012(r244695) @@ -425,12 +425,12 @@ pci_ioctl(struct cdev *dev, u_long cmd, #ifdef COMPAT_FREEBSD32 struct pci_conf_io32 *cio32 = NULL; struct pci_conf_old32 conf_old32; - struct pci_match_conf_old32 *pattern_buf_old32; + struct pci_match_conf_old32 *pattern_buf_old32 = NULL; #endif struct pci_conf_old conf_old; struct pci_io iodata; struct pci_io_old *io_old; - struct pci_match_conf_old *pattern_buf_old; + struct pci_match_conf_old *pattern_buf_old = NULL; io_old = NULL; @@ -470,10 +470,8 @@ pci_ioctl(struct cdev *dev, u_long cmd, #ifdef PRE7_COMPAT #ifdef COMPAT_FREEBSD32 case PCIOCGETCONF_OLD32: - pattern_buf_old32 = NULL; #endif case PCIOCGETCONF_OLD: - pattern_buf_old = NULL; #endif case PCIOCGETCONF: ___ 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
svn commit: r243938 - head/sys/kern
Author: davidxu Date: Thu Dec 6 06:29:08 2012 New Revision: 243938 URL: http://svnweb.freebsd.org/changeset/base/243938 Log: Eliminate superfluous code. Modified: head/sys/kern/subr_uio.c Modified: head/sys/kern/subr_uio.c == --- head/sys/kern/subr_uio.cThu Dec 6 05:08:34 2012(r243937) +++ head/sys/kern/subr_uio.cThu Dec 6 06:29:08 2012(r243938) @@ -389,7 +389,6 @@ again: case UIO_SYSSPACE: iov_base = iov-iov_base; *iov_base = c; - iov-iov_base = iov_base; break; case UIO_NOCOPY: ___ 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
svn commit: r243702 - stable/9/sys/kern
Author: davidxu Date: Fri Nov 30 05:30:31 2012 New Revision: 243702 URL: http://svnweb.freebsd.org/changeset/base/243702 Log: MFC r243599: Take first active vnode correctly. Modified: stable/9/sys/kern/vfs_subr.c Directory Properties: stable/9/sys/ (props changed) Modified: stable/9/sys/kern/vfs_subr.c == --- stable/9/sys/kern/vfs_subr.cFri Nov 30 04:56:39 2012 (r243701) +++ stable/9/sys/kern/vfs_subr.cFri Nov 30 05:30:31 2012 (r243702) @@ -4776,7 +4776,7 @@ __mnt_vnode_first_active(struct vnode ** MNT_REF(mp); (*mvp)-v_type = VMARKER; - vp = TAILQ_NEXT(*mvp, v_actfreelist); + vp = TAILQ_FIRST(mp-mnt_activevnodelist); while (vp != NULL) { VI_LOCK(vp); if (vp-v_mount == mp vp-v_type != VMARKER ___ 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
svn commit: r243615 - head/sys/net
Author: davidxu Date: Tue Nov 27 12:23:57 2012 New Revision: 243615 URL: http://svnweb.freebsd.org/changeset/base/243615 Log: Pass allocated unit number to make_dev, otherwise kernel panics later while cloning second tap. Reviewed by: kevlo,ed Modified: head/sys/net/if_tap.c Modified: head/sys/net/if_tap.c == --- head/sys/net/if_tap.c Tue Nov 27 11:30:39 2012(r243614) +++ head/sys/net/if_tap.c Tue Nov 27 12:23:57 2012(r243615) @@ -186,7 +186,7 @@ tap_clone_create(struct if_clone *ifc, i /* Find any existing device, or allocate new unit number. */ i = clone_create(tapclones, tap_cdevsw, unit, dev, 0); if (i) { - dev = make_dev(tap_cdevsw, 0, UID_ROOT, GID_WHEEL, 0600, + dev = make_dev(tap_cdevsw, unit, UID_ROOT, GID_WHEEL, 0600, %s%d, tapname, unit); } ___ 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: r243599 - head/sys/kern
On 2012/11/27 21:52, Attilio Rao wrote: On Tue, Nov 27, 2012 at 6:07 AM, David Xu davi...@freebsd.org wrote: Author: davidxu Date: Tue Nov 27 06:07:58 2012 New Revision: 243599 URL: http://svnweb.freebsd.org/changeset/base/243599 Log: Take first active vnode correctly. Thanks. I had send the exact same report to Kirk and Jeff one week ago, and I was still waiting for responses. Please note that vfs_msync() was completely broken because of this change (as not happening at all). I think that vinactive() and umount were enough to make the issue not really visible. Yes, I think so, except quota data ? Attilio ___ 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
svn commit: r243599 - head/sys/kern
Author: davidxu Date: Tue Nov 27 06:07:58 2012 New Revision: 243599 URL: http://svnweb.freebsd.org/changeset/base/243599 Log: Take first active vnode correctly. Reviewed by: kib MFC after:3 days Modified: head/sys/kern/vfs_subr.c Modified: head/sys/kern/vfs_subr.c == --- head/sys/kern/vfs_subr.cTue Nov 27 06:01:02 2012(r243598) +++ head/sys/kern/vfs_subr.cTue Nov 27 06:07:58 2012(r243599) @@ -4755,7 +4755,7 @@ __mnt_vnode_first_active(struct vnode ** MNT_REF(mp); (*mvp)-v_type = VMARKER; - vp = TAILQ_NEXT(*mvp, v_actfreelist); + vp = TAILQ_FIRST(mp-mnt_activevnodelist); while (vp != NULL) { VI_LOCK(vp); if (vp-v_mount == mp vp-v_type != VMARKER ___ 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
svn commit: r240295 - head/include
Author: davidxu Date: Mon Sep 10 05:00:29 2012 New Revision: 240295 URL: http://svn.freebsd.org/changeset/base/240295 Log: Add missing prototype for clock_getcpuclockid. Modified: head/include/time.h Modified: head/include/time.h == --- head/include/time.h Mon Sep 10 02:40:17 2012(r240294) +++ head/include/time.h Mon Sep 10 05:00:29 2012(r240295) @@ -88,6 +88,13 @@ typedef __timer_t timer_t; #include sys/timespec.h #endif /* __POSIX_VISIBLE = 199309 */ +#if __POSIX_VISIBLE = 200112 +#ifndef _PID_T_DECLARED +typedef__pid_t pid_t; +#define_PID_T_DECLARED +#endif +#endif + /* These macros are also in sys/time.h. */ #if !defined(CLOCK_REALTIME) __POSIX_VISIBLE = 200112 #define CLOCK_REALTIME 0 @@ -165,6 +172,10 @@ int clock_settime(clockid_t, const struc int nanosleep(const struct timespec *, struct timespec *); #endif /* __POSIX_VISIBLE = 199309 */ +#if __POSIX_VISIBLE = 200112 +int clock_getcpuclockid(pid_t, clockid_t *); +#endif + #if __POSIX_VISIBLE = 199506 char *asctime_r(const struct tm *, char *); char *ctime_r(const time_t *, char *); ___ 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
svn commit: r240296 - head/include
Author: davidxu Date: Mon Sep 10 05:09:39 2012 New Revision: 240296 URL: http://svn.freebsd.org/changeset/base/240296 Log: Process CPU-Time Clocks option is supported, define _POSIX_CPUTIME. Modified: head/include/unistd.h Modified: head/include/unistd.h == --- head/include/unistd.h Mon Sep 10 05:00:29 2012(r240295) +++ head/include/unistd.h Mon Sep 10 05:09:39 2012(r240296) @@ -100,6 +100,7 @@ typedef __useconds_tuseconds_t; * returns -1, the functions may be stubbed out. */ #define_POSIX_BARRIERS 200112L +#define_POSIX_CPUTIME 200112L #define_POSIX_READER_WRITER_LOCKS 200112L #define_POSIX_REGEXP 1 #define_POSIX_SHELL1 ___ 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
svn commit: r240297 - head/include
Author: davidxu Date: Mon Sep 10 05:12:45 2012 New Revision: 240297 URL: http://svn.freebsd.org/changeset/base/240297 Log: POSIX requires sigevent to be visible after mqueue.h is included. Modified: head/include/mqueue.h Modified: head/include/mqueue.h == --- head/include/mqueue.h Mon Sep 10 05:09:39 2012(r240296) +++ head/include/mqueue.h Mon Sep 10 05:12:45 2012(r240297) @@ -32,8 +32,8 @@ #include sys/cdefs.h #include sys/types.h #include sys/mqueue.h +#include sys/signal.h -struct sigevent; struct timespec; __BEGIN_DECLS ___ 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
svn commit: r239718 - head/lib/libthr/thread
Author: davidxu Date: Mon Aug 27 03:09:39 2012 New Revision: 239718 URL: http://svn.freebsd.org/changeset/base/239718 Log: In suspend_common(), don't wait for a thread which is in creation, because pthread_suspend_all_np() may have already suspended its parent thread. Add locking code in pthread_suspend_all_np() to only allow one thread to suspend other threads, this eliminates a deadlock where two or more threads try to suspend each others. Modified: head/lib/libthr/thread/thr_init.c head/lib/libthr/thread/thr_private.h head/lib/libthr/thread/thr_resume_np.c head/lib/libthr/thread/thr_sig.c head/lib/libthr/thread/thr_suspend_np.c Modified: head/lib/libthr/thread/thr_init.c == --- head/lib/libthr/thread/thr_init.c Mon Aug 27 02:56:58 2012 (r239717) +++ head/lib/libthr/thread/thr_init.c Mon Aug 27 03:09:39 2012 (r239718) @@ -120,6 +120,10 @@ struct umutex _rwlock_static_lock = DEFA struct umutex _keytable_lock = DEFAULT_UMUTEX; struct urwlock _thr_list_lock = DEFAULT_URWLOCK; struct umutex _thr_event_lock = DEFAULT_UMUTEX; +struct umutex _suspend_all_lock = DEFAULT_UMUTEX; +struct pthread *_single_thread; +int_suspend_all_cycle; +int_suspend_all_waiters; int__pthread_cond_wait(pthread_cond_t *, pthread_mutex_t *); int__pthread_mutex_lock(pthread_mutex_t *); @@ -441,11 +445,14 @@ init_private(void) _thr_umutex_init(_keytable_lock); _thr_urwlock_init(_thr_atfork_lock); _thr_umutex_init(_thr_event_lock); + _thr_umutex_init(_suspend_all_lock); _thr_once_init(); _thr_spinlock_init(); _thr_list_init(); _thr_wake_addr_init(); _sleepq_init(); + _single_thread = NULL; + _suspend_all_waiters = 0; /* * Avoid reinitializing some things if they don't need to be, Modified: head/lib/libthr/thread/thr_private.h == --- head/lib/libthr/thread/thr_private.hMon Aug 27 02:56:58 2012 (r239717) +++ head/lib/libthr/thread/thr_private.hMon Aug 27 03:09:39 2012 (r239718) @@ -721,6 +721,10 @@ extern struct umutex _rwlock_static_lock extern struct umutex _keytable_lock __hidden; extern struct urwlock _thr_list_lock __hidden; extern struct umutex _thr_event_lock __hidden; +extern struct umutex _suspend_all_lock __hidden; +extern int _suspend_all_waiters __hidden; +extern int _suspend_all_cycle __hidden; +extern struct pthread *_single_thread __hidden; /* * Function prototype definitions. @@ -777,6 +781,8 @@ int _thr_setscheduler(lwpid_t, int, cons void _thr_signal_prefork(void) __hidden; void _thr_signal_postfork(void) __hidden; void _thr_signal_postfork_child(void) __hidden; +void _thr_suspend_all_lock(struct pthread *) __hidden; +void _thr_suspend_all_unlock(struct pthread *) __hidden; void _thr_try_gc(struct pthread *, struct pthread *) __hidden; int_rtp_to_schedparam(const struct rtprio *rtp, int *policy, struct sched_param *param) __hidden; Modified: head/lib/libthr/thread/thr_resume_np.c == --- head/lib/libthr/thread/thr_resume_np.c Mon Aug 27 02:56:58 2012 (r239717) +++ head/lib/libthr/thread/thr_resume_np.c Mon Aug 27 03:09:39 2012 (r239718) @@ -63,7 +63,11 @@ _pthread_resume_all_np(void) { struct pthread *curthread = _get_curthread(); struct pthread *thread; + int old_nocancel; + old_nocancel = curthread-no_cancel; + curthread-no_cancel = 1; + _thr_suspend_all_lock(curthread); /* Take the thread list lock: */ THREAD_LIST_RDLOCK(curthread); @@ -77,6 +81,9 @@ _pthread_resume_all_np(void) /* Release the thread list lock: */ THREAD_LIST_UNLOCK(curthread); + _thr_suspend_all_unlock(curthread); + curthread-no_cancel = old_nocancel; + _thr_testcancel(curthread); } static void Modified: head/lib/libthr/thread/thr_sig.c == --- head/lib/libthr/thread/thr_sig.cMon Aug 27 02:56:58 2012 (r239717) +++ head/lib/libthr/thread/thr_sig.cMon Aug 27 03:09:39 2012 (r239718) @@ -356,7 +356,8 @@ check_suspend(struct pthread *curthread) (THR_FLAGS_NEED_SUSPEND | THR_FLAGS_SUSPENDED)) != THR_FLAGS_NEED_SUSPEND)) return; - + if (curthread == _single_thread) + return; if (curthread-force_exit) return; Modified: head/lib/libthr/thread/thr_suspend_np.c == --- head/lib/libthr/thread/thr_suspend_np.c Mon Aug 27 02:56:58 2012
svn commit: r239609 - head/lib/libthr/thread
Author: davidxu Date: Thu Aug 23 05:15:15 2012 New Revision: 239609 URL: http://svn.freebsd.org/changeset/base/239609 Log: Eliminate redundant code, _thr_spinlock_init() has already been called in init_private(), don't call it again in fork() wrapper. Modified: head/lib/libthr/thread/thr_fork.c Modified: head/lib/libthr/thread/thr_fork.c == --- head/lib/libthr/thread/thr_fork.c Thu Aug 23 04:57:56 2012 (r239608) +++ head/lib/libthr/thread/thr_fork.c Thu Aug 23 05:15:15 2012 (r239609) @@ -200,9 +200,6 @@ _fork(void) _rtld_atfork_post(rtld_locks); _thr_setthreaded(0); - /* reinitialize libc spinlocks. */ - _thr_spinlock_init(); - /* reinitalize library. */ _libpthread_init(curthread); ___ 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
svn commit: r239485 - head/lib/libc/gen
Author: davidxu Date: Tue Aug 21 09:17:13 2012 New Revision: 239485 URL: http://svn.freebsd.org/changeset/base/239485 Log: Fix prototype. Also the function should return error code instead of -1 on error. Modified: head/lib/libc/gen/clock_getcpuclockid.c Modified: head/lib/libc/gen/clock_getcpuclockid.c == --- head/lib/libc/gen/clock_getcpuclockid.c Tue Aug 21 09:14:34 2012 (r239484) +++ head/lib/libc/gen/clock_getcpuclockid.c Tue Aug 21 09:17:13 2012 (r239485) @@ -32,8 +32,10 @@ __FBSDID($FreeBSD$); #include unistd.h #include sys/time.h -clockid_t +int clock_getcpuclockid(pid_t pid, clockid_t *clock_id) { - return clock_getcpuclockid2(pid, CPUCLOCK_WHICH_PID, clock_id); + if (clock_getcpuclockid2(pid, CPUCLOCK_WHICH_PID, clock_id)) + return (errno); + return (0); } ___ 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
svn commit: r239486 - in head: lib/libc/gen share/man/man3
Author: davidxu Date: Tue Aug 21 09:18:28 2012 New Revision: 239486 URL: http://svn.freebsd.org/changeset/base/239486 Log: Add manual pages for clock_getcpuclockid and pthread_getcpuclockid. Added: head/lib/libc/gen/clock_getcpuclockid.3 (contents, props changed) head/share/man/man3/pthread_getcpuclockid.3 (contents, props changed) Modified: head/lib/libc/gen/Makefile.inc head/share/man/man3/Makefile Modified: head/lib/libc/gen/Makefile.inc == --- head/lib/libc/gen/Makefile.inc Tue Aug 21 09:17:13 2012 (r239485) +++ head/lib/libc/gen/Makefile.inc Tue Aug 21 09:18:28 2012 (r239486) @@ -52,7 +52,7 @@ SYM_MAPS+=${.CURDIR}/gen/Symbol.map .sinclude ${.CURDIR}/${LIBC_ARCH}/gen/Makefile.inc MAN+= alarm.3 arc4random.3 \ - basename.3 check_utility_compat.3 clock.3 \ + basename.3 check_utility_compat.3 clock.3 clock_getcpuclockid.3 \ confstr.3 ctermid.3 daemon.3 devname.3 directory.3 dirname.3 \ dl_iterate_phdr.3 dladdr.3 dlinfo.3 dllockinit.3 dlopen.3 \ err.3 exec.3 \ Added: head/lib/libc/gen/clock_getcpuclockid.3 == --- /dev/null 00:00:00 1970 (empty, because file is newly added) +++ head/lib/libc/gen/clock_getcpuclockid.3 Tue Aug 21 09:18:28 2012 (r239486) @@ -0,0 +1,95 @@ +.\ Copyright (c) 2012 David Xu davi...@freebsd.org +.\ All rights reserved. +.\ +.\ Redistribution and use in source and binary forms, with or without +.\ modification, are permitted provided that the following conditions +.\ are met: +.\ 1. Redistributions of source code must retain the above copyright +.\notice, this list of conditions and the following disclaimer. +.\ 2. Redistributions in binary form must reproduce the above copyright +.\notice, this list of conditions and the following disclaimer in the +.\documentation and/or other materials provided with the distribution. +.\ +.\ THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND +.\ ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +.\ IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +.\ ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE +.\ FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +.\ DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS +.\ OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) +.\ HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT +.\ LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY +.\ OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF +.\ SUCH DAMAGE. +.\ +.\ Portions of this text are reprinted and reproduced in electronic form +.\ from IEEE Std 1003.1, 2004 Edition, Standard for Information Technology -- +.\ Portable Operating System Interface (POSIX), The Open Group Base +.\ Specifications Issue 6, Copyright (C) 2001-2004 by the Institute of +.\ Electrical and Electronics Engineers, Inc and The Open Group. In the +.\ event of any discrepancy between this version and the original IEEE and +.\ The Open Group Standard, the original IEEE and The Open Group Standard is +.\ the referee document. The original Standard can be obtained online at +.\http://www.opengroup.org/unix/online.html. +.\ +.\ $FreeBSD$ +.\ +.Dd August 21, 2012 +.Dt CLOCK_GETCPUCLOCKID 3 +.Os +.Sh NAME +.Nm clock_getcpuclockid +.Nd access a process CPU-time clock +.Sh LIBRARY +.Lb libc +.Sh SYNOPSIS +.In time.h +.Ft int +.Fn clock_getcpuclockid pid_t pid clockid_t *clock_id +.Sh DESCRIPTION +The +.Fn clock_getcpuclockid +returns the clock ID of the CPU-time clock of the process specified by +.Fa pid . +If the process described by +.Fa pid +exists and the calling process has permission, the clock ID of this +clock will be returned in +.Fa clock_id . +.Pp +If +.Fa pid +is zero, the +.Fn clock_getcpuclockid +function returns the clock ID of the CPU-time clock of the process +making the call, in +.Fa clock_id . +.Sh RETURN VALUES +Upon successful completion, +.Fn clock_getcpuclockid +returns zero; otherwise, an error number is returned to indicate the +error. +.Sh ERRORS +The clock_getcpuclockid() function will fail if: +.Bl -tag -width Er +.It Bq Er EPERM +The requesting process does not have permission to access the CPU-time +clock for the process. +.It Bq Er ESRCH +No process can be found corresponding to the process specified by +.Fa pid . +.El +.Sh SEE ALSO +.Xr clock_gettime 2 +.Sh STANDARDS +The +.Fn clock_getcpuclockid +function conform to +.St -p1003.1-2001 . +.Sh HISTORY +The +.Fn clock_getcpuclockid +function first appeared in +.Fx 10.0 . +.Sh AUTHORS +.An David Xu Aq davi...@freebsd.org Modified: head/share/man/man3/Makefile
svn commit: r239347 - in head: lib/libc/gen lib/libc/sys lib/libthr/thread sys/compat/freebsd32 sys/kern sys/sys
Author: davidxu Date: Fri Aug 17 02:26:31 2012 New Revision: 239347 URL: http://svn.freebsd.org/changeset/base/239347 Log: Implement syscall clock_getcpuclockid2, so we can get a clock id for process, thread or others we want to support. Use the syscall to implement POSIX API clock_getcpuclock and pthread_getcpuclockid. PR: 168417 Added: head/lib/libc/gen/clock_getcpuclockid.c (contents, props changed) Modified: head/lib/libc/gen/Makefile.inc head/lib/libc/gen/Symbol.map head/lib/libc/gen/sysconf.c head/lib/libc/sys/Symbol.map head/lib/libthr/thread/thr_getcpuclockid.c head/sys/compat/freebsd32/freebsd32_syscall.h head/sys/compat/freebsd32/freebsd32_syscalls.c head/sys/compat/freebsd32/freebsd32_sysent.c head/sys/compat/freebsd32/freebsd32_systrace_args.c head/sys/compat/freebsd32/syscalls.master head/sys/kern/init_sysent.c head/sys/kern/kern_time.c head/sys/kern/syscalls.c head/sys/kern/syscalls.master head/sys/kern/systrace_args.c head/sys/sys/syscall.h head/sys/sys/syscall.mk head/sys/sys/sysproto.h head/sys/sys/time.h head/sys/sys/unistd.h Modified: head/lib/libc/gen/Makefile.inc == --- head/lib/libc/gen/Makefile.inc Fri Aug 17 01:49:51 2012 (r239346) +++ head/lib/libc/gen/Makefile.inc Fri Aug 17 02:26:31 2012 (r239347) @@ -8,7 +8,7 @@ SRCS+= __getosreldate.c __xuname.c \ _once_stub.c _pthread_stubs.c _rand48.c _spinlock_stub.c \ _thread_init.c \ alarm.c arc4random.c assert.c auxv.c basename.c check_utility_compat.c \ - clock.c closedir.c confstr.c \ + clock.c clock_getcpuclockid.c closedir.c confstr.c \ crypt.c ctermid.c daemon.c devname.c dirfd.c dirname.c disklabel.c \ dlfcn.c drand48.c elf_utils.c erand48.c err.c errlst.c errno.c \ exec.c fdevname.c feature_present.c fmtcheck.c fmtmsg.c fnmatch.c \ Modified: head/lib/libc/gen/Symbol.map == --- head/lib/libc/gen/Symbol.mapFri Aug 17 01:49:51 2012 (r239346) +++ head/lib/libc/gen/Symbol.mapFri Aug 17 02:26:31 2012 (r239347) @@ -382,6 +382,7 @@ FBSD_1.2 { }; FBSD_1.3 { + clock_getcpuclockid; dirfd; fdlopen; __FreeBSD_libc_enter_restricted_mode; Added: head/lib/libc/gen/clock_getcpuclockid.c == --- /dev/null 00:00:00 1970 (empty, because file is newly added) +++ head/lib/libc/gen/clock_getcpuclockid.c Fri Aug 17 02:26:31 2012 (r239347) @@ -0,0 +1,39 @@ +/* + * Copyright (c) 2012 David Xu davi...@freebsd.org. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + *notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + *notice, this list of conditions and the following disclaimer in the + *documentation and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY DANIEL EISCHEN AND CONTRIBUTORS ``AS IS'' + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + */ + +#include sys/cdefs.h +__FBSDID($FreeBSD$); + +#include errno.h +#include time.h +#include unistd.h +#include sys/time.h + +clockid_t +clock_getcpuclockid(pid_t pid, clockid_t *clock_id) +{ + return clock_getcpuclockid2(pid, CPUCLOCK_WHICH_PID, clock_id); +} Modified: head/lib/libc/gen/sysconf.c == --- head/lib/libc/gen/sysconf.c Fri Aug 17 01:49:51 2012(r239346) +++ head/lib/libc/gen/sysconf.c Fri Aug 17 02:26:31 2012(r239347) @@ -359,11 +359,7 @@ yesno: return (_POSIX_CLOCK_SELECTION); #endif case _SC_CPUTIME: -#if _POSIX_CPUTIME == 0 -#error _POSIX_CPUTIME -#else return (_POSIX_CPUTIME); -#endif #ifdef notdef case _SC_FILE_LOCKING: /* Modified: head/lib/libc/sys/Symbol.map
svn commit: r239349 - in head/sys: compat/freebsd32 kern sys
Author: davidxu Date: Fri Aug 17 02:47:16 2012 New Revision: 239349 URL: http://svn.freebsd.org/changeset/base/239349 Log: regen. Modified: head/sys/compat/freebsd32/freebsd32_proto.h head/sys/compat/freebsd32/freebsd32_syscall.h head/sys/compat/freebsd32/freebsd32_syscalls.c head/sys/compat/freebsd32/freebsd32_sysent.c head/sys/kern/init_sysent.c head/sys/kern/syscalls.c head/sys/sys/syscall.h head/sys/sys/syscall.mk head/sys/sys/sysproto.h Modified: head/sys/compat/freebsd32/freebsd32_proto.h == --- head/sys/compat/freebsd32/freebsd32_proto.h Fri Aug 17 02:27:17 2012 (r239348) +++ head/sys/compat/freebsd32/freebsd32_proto.h Fri Aug 17 02:47:16 2012 (r239349) @@ -3,7 +3,7 @@ * * DO NOT EDIT-- this file is automatically generated. * $FreeBSD$ - * created from FreeBSD: head/sys/compat/freebsd32/syscalls.master 239296 2012-08-15 15:17:56Z kib + * created from FreeBSD: head/sys/compat/freebsd32/syscalls.master 239347 2012-08-17 02:26:31Z davidxu */ #ifndef _FREEBSD32_SYSPROTO_H_ Modified: head/sys/compat/freebsd32/freebsd32_syscall.h == --- head/sys/compat/freebsd32/freebsd32_syscall.h Fri Aug 17 02:27:17 2012(r239348) +++ head/sys/compat/freebsd32/freebsd32_syscall.h Fri Aug 17 02:47:16 2012(r239349) @@ -3,7 +3,7 @@ * * DO NOT EDIT-- this file is automatically generated. * $FreeBSD$ - * created from FreeBSD: head/sys/compat/freebsd32/syscalls.master 239296 2012-08-15 15:17:56Z kib + * created from FreeBSD: head/sys/compat/freebsd32/syscalls.master 239347 2012-08-17 02:26:31Z davidxu */ #defineFREEBSD32_SYS_syscall 0 Modified: head/sys/compat/freebsd32/freebsd32_syscalls.c == --- head/sys/compat/freebsd32/freebsd32_syscalls.c Fri Aug 17 02:27:17 2012(r239348) +++ head/sys/compat/freebsd32/freebsd32_syscalls.c Fri Aug 17 02:47:16 2012(r239349) @@ -3,7 +3,7 @@ * * DO NOT EDIT-- this file is automatically generated. * $FreeBSD$ - * created from FreeBSD: head/sys/compat/freebsd32/syscalls.master 239296 2012-08-15 15:17:56Z kib + * created from FreeBSD: head/sys/compat/freebsd32/syscalls.master 239347 2012-08-17 02:26:31Z davidxu */ const char *freebsd32_syscallnames[] = { Modified: head/sys/compat/freebsd32/freebsd32_sysent.c == --- head/sys/compat/freebsd32/freebsd32_sysent.cFri Aug 17 02:27:17 2012(r239348) +++ head/sys/compat/freebsd32/freebsd32_sysent.cFri Aug 17 02:47:16 2012(r239349) @@ -3,7 +3,7 @@ * * DO NOT EDIT-- this file is automatically generated. * $FreeBSD$ - * created from FreeBSD: head/sys/compat/freebsd32/syscalls.master 239296 2012-08-15 15:17:56Z kib + * created from FreeBSD: head/sys/compat/freebsd32/syscalls.master 239347 2012-08-17 02:26:31Z davidxu */ #include opt_compat.h Modified: head/sys/kern/init_sysent.c == --- head/sys/kern/init_sysent.c Fri Aug 17 02:27:17 2012(r239348) +++ head/sys/kern/init_sysent.c Fri Aug 17 02:47:16 2012(r239349) @@ -3,7 +3,7 @@ * * DO NOT EDIT-- this file is automatically generated. * $FreeBSD$ - * created from FreeBSD: head/sys/kern/syscalls.master 236026 2012-05-25 21:50:48Z ed + * created from FreeBSD: head/sys/kern/syscalls.master 239347 2012-08-17 02:26:31Z davidxu */ #include opt_compat.h Modified: head/sys/kern/syscalls.c == --- head/sys/kern/syscalls.cFri Aug 17 02:27:17 2012(r239348) +++ head/sys/kern/syscalls.cFri Aug 17 02:47:16 2012(r239349) @@ -3,7 +3,7 @@ * * DO NOT EDIT-- this file is automatically generated. * $FreeBSD$ - * created from FreeBSD: head/sys/kern/syscalls.master 236026 2012-05-25 21:50:48Z ed + * created from FreeBSD: head/sys/kern/syscalls.master 239347 2012-08-17 02:26:31Z davidxu */ const char *syscallnames[] = { Modified: head/sys/sys/syscall.h == --- head/sys/sys/syscall.h Fri Aug 17 02:27:17 2012(r239348) +++ head/sys/sys/syscall.h Fri Aug 17 02:47:16 2012(r239349) @@ -3,7 +3,7 @@ * * DO NOT EDIT-- this file is automatically generated. * $FreeBSD$ - * created from FreeBSD: head/sys/kern/syscalls.master 236026 2012-05-25 21:50:48Z ed + * created from FreeBSD: head/sys/kern/syscalls.master 239347 2012-08-17 02:26:31Z davidxu */ #defineSYS_syscall 0 Modified: head/sys/sys/syscall.mk == ---
svn commit: r239200 - head/lib/libthr/thread
Author: davidxu Date: Sat Aug 11 23:17:02 2012 New Revision: 239200 URL: http://svn.freebsd.org/changeset/base/239200 Log: MFp4: Further decreases unexpected context switches by defering mutex wakeup until internal sleep queue lock is released. Modified: head/lib/libthr/thread/thr_cond.c head/lib/libthr/thread/thr_kern.c head/lib/libthr/thread/thr_mutex.c head/lib/libthr/thread/thr_private.h head/lib/libthr/thread/thr_umtx.h Modified: head/lib/libthr/thread/thr_cond.c == --- head/lib/libthr/thread/thr_cond.c Sat Aug 11 22:39:27 2012 (r239199) +++ head/lib/libthr/thread/thr_cond.c Sat Aug 11 23:17:02 2012 (r239200) @@ -217,6 +217,7 @@ cond_wait_user(struct pthread_cond *cvp, struct sleepqueue *sq; int recurse; int error; + int defered; if (curthread-wchan != NULL) PANIC(thread was already on queue.); @@ -230,13 +231,23 @@ cond_wait_user(struct pthread_cond *cvp, * us to check it without locking in pthread_cond_signal(). */ cvp-__has_user_waiters = 1; - curthread-will_sleep = 1; - (void)_mutex_cv_unlock(mp, recurse); + defered = 0; + (void)_mutex_cv_unlock(mp, recurse, defered); curthread-mutex_obj = mp; _sleepq_add(cvp, curthread); for(;;) { _thr_clear_wake(curthread); _sleepq_unlock(cvp); + if (defered) { + if ((mp-m_lock.m_owner UMUTEX_CONTESTED) == 0) + (void)_umtx_op_err(mp-m_lock, UMTX_OP_MUTEX_WAKE2, +mp-m_lock.m_flags, 0, 0); + } + if (curthread-nwaiter_defer 0) { + _thr_wake_all(curthread-defer_waiters, + curthread-nwaiter_defer); + curthread-nwaiter_defer = 0; + } if (cancel) { _thr_cancel_enter2(curthread, 0); Modified: head/lib/libthr/thread/thr_kern.c == --- head/lib/libthr/thread/thr_kern.c Sat Aug 11 22:39:27 2012 (r239199) +++ head/lib/libthr/thread/thr_kern.c Sat Aug 11 23:17:02 2012 (r239200) @@ -199,13 +199,6 @@ _thr_sleep(struct pthread *curthread, in const struct timespec *abstime) { - curthread-will_sleep = 0; - if (curthread-nwaiter_defer 0) { - _thr_wake_all(curthread-defer_waiters, - curthread-nwaiter_defer); - curthread-nwaiter_defer = 0; - } - if (curthread-wake_addr-value != 0) return (0); Modified: head/lib/libthr/thread/thr_mutex.c == --- head/lib/libthr/thread/thr_mutex.c Sat Aug 11 22:39:27 2012 (r239199) +++ head/lib/libthr/thread/thr_mutex.c Sat Aug 11 23:17:02 2012 (r239200) @@ -92,7 +92,7 @@ int __pthread_mutex_setyieldloops_np(pth static int mutex_self_trylock(pthread_mutex_t); static int mutex_self_lock(pthread_mutex_t, const struct timespec *abstime); -static int mutex_unlock_common(struct pthread_mutex *, int); +static int mutex_unlock_common(struct pthread_mutex *, int, int *); static int mutex_lock_sleep(struct pthread *, pthread_mutex_t, const struct timespec *); @@ -461,7 +461,7 @@ _pthread_mutex_unlock(pthread_mutex_t *m struct pthread_mutex *mp; mp = *mutex; - return (mutex_unlock_common(mp, 0)); + return (mutex_unlock_common(mp, 0, NULL)); } int @@ -476,7 +476,7 @@ _mutex_cv_lock(struct pthread_mutex *m, } int -_mutex_cv_unlock(struct pthread_mutex *m, int *count) +_mutex_cv_unlock(struct pthread_mutex *m, int *count, int *defer) { /* @@ -484,7 +484,7 @@ _mutex_cv_unlock(struct pthread_mutex *m */ *count = m-m_count; m-m_count = 0; - (void)mutex_unlock_common(m, 1); + (void)mutex_unlock_common(m, 1, defer); return (0); } @@ -629,7 +629,7 @@ mutex_self_lock(struct pthread_mutex *m, } static int -mutex_unlock_common(struct pthread_mutex *m, int cv) +mutex_unlock_common(struct pthread_mutex *m, int cv, int *mtx_defer) { struct pthread *curthread = _get_curthread(); uint32_t id; @@ -657,12 +657,12 @@ mutex_unlock_common(struct pthread_mutex defered = 1; m-m_flags = ~PMUTEX_FLAG_DEFERED; } else - defered = 0; + defered = 0; DEQUEUE_MUTEX(curthread, m); - _thr_umutex_unlock(m-m_lock, id); + _thr_umutex_unlock2(m-m_lock, id, mtx_defer); - if
svn commit: r239202 - head/sys/kern
Author: davidxu Date: Sat Aug 11 23:48:39 2012 New Revision: 239202 URL: http://svn.freebsd.org/changeset/base/239202 Log: Some style fixes inspired by @bde. Modified: head/sys/kern/kern_umtx.c Modified: head/sys/kern/kern_umtx.c == --- head/sys/kern/kern_umtx.c Sat Aug 11 23:26:19 2012(r239201) +++ head/sys/kern/kern_umtx.c Sat Aug 11 23:48:39 2012(r239202) @@ -587,7 +587,7 @@ abs_timeout_init2(struct abs_timeout *ti umtxtime-_timeout); } -static void +static inline void abs_timeout_update(struct abs_timeout *timo) { kern_clock_gettime(curthread, timo-clockid, timo-cur); @@ -598,10 +598,10 @@ abs_timeout_gethz(struct abs_timeout *ti { struct timespec tts; + if (timespeccmp(timo-end, timo-cur, =)) + return (-1); tts = timo-end; timespecsub(tts, timo-cur); - if (tts.tv_sec 0 || (tts.tv_sec == 0 tts.tv_nsec == 0)) - return (-1); return (tstohz(tts)); } @@ -610,29 +610,29 @@ abs_timeout_gethz(struct abs_timeout *ti * thread was removed from umtx queue. */ static inline int -umtxq_sleep(struct umtx_q *uq, const char *wmesg, struct abs_timeout *timo) +umtxq_sleep(struct umtx_q *uq, const char *wmesg, struct abs_timeout *abstime) { struct umtxq_chain *uc; - int error; - int pulse; + int error, timo; uc = umtxq_getchain(uq-uq_key); UMTXQ_LOCKED_ASSERT(uc); for (;;) { if (!(uq-uq_flags UQF_UMTXQ)) return (0); - if (timo != NULL) { - pulse = abs_timeout_gethz(timo); - if (pulse 0) + if (abstime != NULL) { + timo = abs_timeout_gethz(abstime); + if (timo 0) return (ETIMEDOUT); } else - pulse = 0; - error = msleep(uq, uc-uc_lock, PCATCH|PDROP, wmesg, pulse); + timo = 0; + error = msleep(uq, uc-uc_lock, PCATCH | PDROP, wmesg, timo); if (error != EWOULDBLOCK) { umtxq_lock(uq-uq_key); break; } - abs_timeout_update(timo); + if (abstime != NULL) + abs_timeout_update(abstime); umtxq_lock(uq-uq_key); } return (error); ___ 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
svn commit: r239206 - head/lib/libthr/thread
Author: davidxu Date: Sun Aug 12 00:56:56 2012 New Revision: 239206 URL: http://svn.freebsd.org/changeset/base/239206 Log: Do defered mutex wakeup once. Modified: head/lib/libthr/thread/thr_cond.c Modified: head/lib/libthr/thread/thr_cond.c == --- head/lib/libthr/thread/thr_cond.c Sun Aug 12 00:46:15 2012 (r239205) +++ head/lib/libthr/thread/thr_cond.c Sun Aug 12 00:56:56 2012 (r239206) @@ -239,6 +239,7 @@ cond_wait_user(struct pthread_cond *cvp, _thr_clear_wake(curthread); _sleepq_unlock(cvp); if (defered) { + defered = 0; if ((mp-m_lock.m_owner UMUTEX_CONTESTED) == 0) (void)_umtx_op_err(mp-m_lock, UMTX_OP_MUTEX_WAKE2, mp-m_lock.m_flags, 0, 0); ___ 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
svn commit: r239187 - head/sys/kern
Author: davidxu Date: Sat Aug 11 00:06:56 2012 New Revision: 239187 URL: http://svn.freebsd.org/changeset/base/239187 Log: tvtohz will print out an error message if a negative value is given to it, avoid this problem by detecting timeout earlier. Reported by: pho Modified: head/sys/kern/kern_umtx.c Modified: head/sys/kern/kern_umtx.c == --- head/sys/kern/kern_umtx.c Fri Aug 10 21:11:00 2012(r239186) +++ head/sys/kern/kern_umtx.c Sat Aug 11 00:06:56 2012(r239187) @@ -587,11 +587,10 @@ abs_timeout_init2(struct abs_timeout *ti umtxtime-_timeout); } -static int +static void abs_timeout_update(struct abs_timeout *timo) { kern_clock_gettime(curthread, timo-clockid, timo-cur); - return (timespeccmp(timo-cur, timo-end, =)); } static int @@ -601,6 +600,8 @@ abs_timeout_gethz(struct abs_timeout *ti tts = timo-end; timespecsub(tts, timo-cur); + if (tts.tv_sec 0 || (tts.tv_sec == 0 tts.tv_nsec == 0)) + return (-1); return (tstohz(tts)); } @@ -613,22 +614,25 @@ umtxq_sleep(struct umtx_q *uq, const cha { struct umtxq_chain *uc; int error; + int pulse; uc = umtxq_getchain(uq-uq_key); UMTXQ_LOCKED_ASSERT(uc); for (;;) { if (!(uq-uq_flags UQF_UMTXQ)) return (0); - error = msleep(uq, uc-uc_lock, PCATCH, wmesg, - timo == NULL ? 0 : abs_timeout_gethz(timo)); - if (error != EWOULDBLOCK) - break; - umtxq_unlock(uq-uq_key); - if (abs_timeout_update(timo)) { - error = ETIMEDOUT; + if (timo != NULL) { + pulse = abs_timeout_gethz(timo); + if (pulse 0) + return (ETIMEDOUT); + } else + pulse = 0; + error = msleep(uq, uc-uc_lock, PCATCH|PDROP, wmesg, pulse); + if (error != EWOULDBLOCK) { umtxq_lock(uq-uq_key); break; } + abs_timeout_update(timo); umtxq_lock(uq-uq_key); } return (error); ___ 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: r238936 - in head/sys: fs/fifofs kern sys
On 2012/8/2 16:12, Bruce Evans wrote: On Thu, 2 Aug 2012, David Xu wrote: On 2012/8/2 3:29, Bruce Evans wrote: On Wed, 1 Aug 2012, Giovanni Trematerra wrote: ... [gianni@bombay] /usr/src/tools/regression/poll#./pipepoll 1..20 not ok 17 FIFO state 6a: expected POLLHUP; got POLLIN | POLLHUP not ok 18 FIFO state 6b: poll result 0 expected 1. expected POLLHUP; got 0 not ok 19 FIFO state 6c: expected POLLHUP; got POLLIN | POLLHUP not ok 20 FIFO state 6d: expected POLLHUP; got POLLIN | POLLHUP As you can see, sub-tests 6c and 6d failed too on 9. So it's not a problem with new code though is irrelevant wrt the commit. The failure is very differnt. Failure to clear POLLIN in 6a, 6c and 6d is a normal bug in FreeBSD. I have attached a patch to fix it, it should make the regression tool happy. Is it worth to commit ? This is your patch quoted inline: % Index: sys_pipe.c % === % --- sys_pipe.c(revision 238936) % +++ sys_pipe.c(working copy) % @@ -1447,7 +1447,6 @@ % % if ((events POLLINIGNEOF) == 0) { % if (rpipe-pipe_state PIPE_EOF) { % -revents |= (events (POLLIN | POLLRDNORM)); % if (wpipe-pipe_present != PIPE_ACTIVE || % (wpipe-pipe_state PIPE_EOF)) % revents |= POLLHUP; My old patches use this: % Index: sys_pipe.c % === % RCS file: /home/ncvs/src/sys/kern/sys_pipe.c,v % retrieving revision 1.171 % diff -u -2 -r1.171 sys_pipe.c % --- sys_pipe.c27 Mar 2004 19:50:22 -1.171 % +++ sys_pipe.c13 Aug 2009 11:33:08 - % @@ -1296,6 +1295,5 @@ % if (events (POLLIN | POLLRDNORM)) % if ((rpipe-pipe_state PIPE_DIRECTW) || % -(rpipe-pipe_buffer.cnt 0) || % -(rpipe-pipe_state PIPE_EOF)) % +(rpipe-pipe_buffer.cnt 0)) % revents |= events (POLLIN | POLLRDNORM); % I'm not sure if there is any difference. The pipe code seems to have been changed to be more like the socket code. I made similar patches for sockets (to set POLLHUP on hangup (now in -current) and to not set POLLIN on hangup unless there is still data to be read). I started killing POLLINIGNEOF for sockets. -current added it for nameless pipes instead :-(. With the new fifo implementation, POLLINIGNEOF is even more of a mistake for sockets, but more needed for pipes since named pipes are fifos. % Index: uipc_socket.c % === % RCS file: /home/ncvs/src/sys/kern/uipc_socket.c,v % retrieving revision 1.189 % diff -u -2 -r1.189 uipc_socket.c % --- uipc_socket.c24 Jun 2004 04:28:30 -1.189 % +++ uipc_socket.c26 Aug 2009 22:49:12 - % @@ -1862,4 +1861,9 @@ % } % % +#definesoreadabledata(so) \ % +(((so)-so_rcv.sb_cc 0 \ % +(so)-so_rcv.sb_cc = (so)-so_rcv.sb_lowat) || \ % +!TAILQ_EMPTY((so)-so_comp) || (so)-so_error) % + % int % sopoll(struct socket *so, int events, struct ucred *active_cred, -current already has this in a header (don't count hangup as data). But -current still sets POLLIN for compatibility later, except in the bogus POLLINIGNEOF case. It only uses the above change to avoid setting POLLIN initially for hangup in the POLLINIGNEOF case. % @@ -1869,12 +1873,7 @@ % % if (events (POLLIN | POLLRDNORM)) % -if (soreadable(so)) % +if (soreadabledata(so)) % revents |= events (POLLIN | POLLRDNORM); Make POLLIN actually track data, not disconnection. % % -if (events POLLINIGNEOF) % -if (so-so_rcv.sb_cc = so-so_rcv.sb_lowat || % -!TAILQ_EMPTY(so-so_comp) || so-so_error) % -revents |= POLLINIGNEOF; % - % if (events (POLLOUT | POLLWRNORM)) % if (sowriteable(so)) Start killing this. % @@ -1885,8 +1884,15 @@ % revents |= events (POLLPRI | POLLRDBAND); % % +if ((events POLLINIGNEOF) == 0) { % +if (so-so_rcv.sb_state SBS_CANTRCVMORE) { % +if (so-so_snd.sb_state SBS_CANTSENDMORE) % +revents |= POLLHUP; % +else % +revents |= events (POLLIN | POLLRDNORM); % +} % +} % + Don't completely kill POLLINIGNEOF. I forget how the 2 socket state EOF flags work. Both this and -current set POLLHUP iff both socket state EOF flags are set. Then this never sets POLLIN, while -current always sets it. Both set POLLIN of SBS_CANTRCVMORE is set but SBS_CANTSENDMORE is not set. Note that POLLIN has already been set if there is actual data, so any setting of it here is bogus, but there seems to be a problem when only SBS_CANTRCVMORE is set. Then !SBS_CANTSENDMORE implies that output is still possible, so we must be able to return POLLOUT, but POLLOUT is incompatible with POLLHUP so we can't set POLLHUP. We apparently set POLLIN to fake this partial EOF. % if (revents == 0
Re: svn commit: r238936 - in head/sys: fs/fifofs kern sys
On 2012/8/2 22:17, Bruce Evans wrote: On Thu, 2 Aug 2012, David Xu wrote: On 2012/8/2 16:12, Bruce Evans wrote: ... I made similar patches for sockets (to set POLLHUP on hangup (now in -current) and to not set POLLIN on hangup unless there is still data to be read). I started killing POLLINIGNEOF for sockets. -current added it for nameless pipes instead :-(. With the new fifo implementation, POLLINIGNEOF is even more of a mistake for sockets, but more needed for pipes since named pipes are fifos. ... I think you can kill POLLINIGNEOF at all, I have grepped, and there is no external user, only pipe and socket code use it internally. The POLLINIGNEOF is confusing because it has same prefix with POLLIN, POLLOUT and other POLL flags. Did you grep all of google for it :-). All of ports should be enough. Uses of it in the kernel are certainly gone, but it was intentionally put in the user API to previde a workaround for the policy that was hard-coded in the kernel. The policy changed slightly, and you could set POLLINIGNEOF to either go back to the old policy or get the new policy for more cases. Hopefully this was never actually used except for testing and termporary workarounds. But it was expanded to work on nameless pipes as well as fifos and sockets. Bruce I don't know it is used by some ports. :-) Anyway, if people don't agree my patches, it is not a problem to me, because I always can apply them locally, though named pipe may be less useful from offical release. Thanks, ___ 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: r238936 - in head/sys: fs/fifofs kern sys
On 2012/8/2 3:29, Bruce Evans wrote: On Wed, 1 Aug 2012, Giovanni Trematerra wrote: On Tue, Jul 31, 2012 at 10:21 AM, David Xu listlog2...@gmail.com wrote: ... The old code broke some history semantic of FIFO pipe, you can try the test tool /usr/src/tools/regression/poll/pipepoll, try it before and after my commit, also compare the result with 8.3-STABLE, without this commit, both sub-tests 6c and 6d failed. This is on Vanilla 9.0-RELEASE where new fifo implementation weren't backported FreeBSD bombay 9.0-RELEASE FreeBSD 9.0-RELEASE #3: Tue Dec 27 21:59:00 UTC 2011 r...@build9x64.pcbsd.org:/usr/obj/builds/i386/pcbsd-build90/fbsd-source/9.0/sys/GENERIC i386 [gianni@bombay] /usr/src/tools/regression/poll#./pipepoll 1..20 not ok 17 FIFO state 6a: expected POLLHUP; got POLLIN | POLLHUP not ok 18 FIFO state 6b: poll result 0 expected 1. expected POLLHUP; got 0 not ok 19 FIFO state 6c: expected POLLHUP; got POLLIN | POLLHUP not ok 20 FIFO state 6d: expected POLLHUP; got POLLIN | POLLHUP As you can see, sub-tests 6c and 6d failed too on 9. So it's not a problem with new code though is irrelevant wrt the commit. The failure is very differnt. Failure to clear POLLIN in 6a, 6c and 6d is a normal bug in FreeBSD. I have attached a patch to fix it, it should make the regression tool happy. Is it worth to commit ? Index: sys_pipe.c === --- sys_pipe.c (revision 238936) +++ sys_pipe.c (working copy) @@ -1447,7 +1447,6 @@ if ((events POLLINIGNEOF) == 0) { if (rpipe-pipe_state PIPE_EOF) { - revents |= (events (POLLIN | POLLRDNORM)); if (wpipe-pipe_present != PIPE_ACTIVE || (wpipe-pipe_state PIPE_EOF)) revents |= POLLHUP; ___ 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: r238936 - in head/sys: fs/fifofs kern sys
On 2012/7/31 15:22, Giovanni Trematerra wrote: On Tue, Jul 31, 2012 at 7:48 AM, David Xu davi...@freebsd.org wrote: Author: davidxu Date: Tue Jul 31 05:48:35 2012 New Revision: 238936 URL: http://svn.freebsd.org/changeset/base/238936 Log: I am comparing current pipe code with the one in 8.3-STABLE r236165, I found 8.3 is a history BSD version using socket to implement FIFO pipe, it uses per-file seqcount to compare with writer generation stored in per-pipe object. The concept is after all writers are gone, the pipe enters next generation, all old readers have not closed the pipe should get the indication that the pipe is disconnected, result is they should get EPIPE, SIGPIPE or get POLLHUP in poll(). But newcomer should not know that previous writters were gone, it should treat it as a fresh session. I am trying to bring back FIFO pipe to history behavior. It is still unclear that if single EOF flag can represent SBS_CANTSENDMORE and SBS_CANTRCVMORE which socket-based version is using, but I have run the poll regression test in tool directory, output is same as the one on 8.3-STABLE now. I think the output not ok 18 FIFO state 6b: poll result 0 expected 1. expected POLLHUP; got 0 might be bogus, because newcomer should not know that old writers were gone. I got the same behavior on Linux. Our implementation always return POLLIN for disconnected pipe even it should return POLLHUP, but I think it is not wise to remove POLLIN for compatible reason, this is our history behavior. I'm sorry but I'm failing to understand the reason for this change. Can you point me out a test that confirm that the change is needed. The only thing I see is an increase in the memory footprint for the pipes. There was a lot of discussions on this topic on -arch mailing list http://lists.freebsd.org/pipermail/freebsd-arch/2012-January/012131.html http://lists.freebsd.org/pipermail/freebsd-arch/2012-February/012314.html Thank you -- Gianni The old code broke some history semantic of FIFO pipe, you can try the test tool /usr/src/tools/regression/poll/pipepoll, try it before and after my commit, also compare the result with 8.3-STABLE, without this commit, both sub-tests 6c and 6d failed. I think old code did not mimic original code correctly, in 8.3-STABLE code, seqcount is stored in each file, writer generation detection is based on each copy of seqcount, but your code stored single copy of seqcount in fifoinfo object which is store as vnode data, and made the writer generation flag global by setting PIPE_SAMEWGEN in pipe object and used this flag to determine if it should return POLLHUP/POLLIN or not, this is wrong, for example: when there is no writer but have old readers, new incoming reader will executes: line 174 and 175: fip-fi_seqcount = fip-fi_wgen - fip-fi_writers; FIFO_WPDWGEN(fip, fpipe); this causes fi_seqcount to be equal to fi_wgen because fi_writer is zero, and FIFO_WPDWGEN() turns on flag PIPE_SAMEWGEN. When PIPE_SAMEWGEN is on, pipe_poll() ignores EOF, this breaks old readers, it causes old reader to get nothing while it should get POLLHUP from poll(). The new incoming reader should get nothing, so I think sub-tests 6b is wrong. ___ 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
svn commit: r238928 - in head/sys: fs/fifofs kern sys
Author: davidxu Date: Tue Jul 31 02:00:37 2012 New Revision: 238928 URL: http://svn.freebsd.org/changeset/base/238928 Log: When a thread is blocked in direct write state, it only sets PIPE_DIRECTW flag but not PIPE_WANTW, but FIFO pipe code does not understand this internal state, when a FIFO peer reader closes the pipe, it wants to notify the writer, it checks PIPE_WANTW, if not set, it skips calling wakeup(), so blocked writer never noticed the case, but in general, the writer should return from the syscall with EPIPE error code and may get SIGPIPE signal. Setting the PIPE_WANTW fixed problem, or you can turn off direct write, it should fix the problem too. This bug is found by PR/170203. Another bug in FIFO pipe code is when peer closes the pipe, another end which is being blocked in select() or poll() is not notified, it missed to call pipeselwakeup(). Third problem is found in poll regression test, the existing code can not pass 6b,6c,6d tests, but FreeBSD-4 works. This commit does not fix the problem, I still need to study more to find the cause. PR: 170203 Tested by: Garrett Copper lt; yanegomi at gmail dot com gt; Modified: head/sys/fs/fifofs/fifo_vnops.c head/sys/kern/sys_pipe.c head/sys/sys/pipe.h Modified: head/sys/fs/fifofs/fifo_vnops.c == --- head/sys/fs/fifofs/fifo_vnops.c Tue Jul 31 00:46:19 2012 (r238927) +++ head/sys/fs/fifofs/fifo_vnops.c Tue Jul 31 02:00:37 2012 (r238928) @@ -283,8 +283,11 @@ fifo_close(ap) if (fip-fi_readers == 0) { PIPE_LOCK(cpipe); cpipe-pipe_state |= PIPE_EOF; - if (cpipe-pipe_state PIPE_WANTW) + if ((cpipe-pipe_state PIPE_WANTW)) { + cpipe-pipe_state = ~PIPE_WANTW; wakeup(cpipe); + } + pipeselwakeup(cpipe); PIPE_UNLOCK(cpipe); } } @@ -293,10 +296,13 @@ fifo_close(ap) if (fip-fi_writers == 0) { PIPE_LOCK(cpipe); cpipe-pipe_state |= PIPE_EOF; - if (cpipe-pipe_state PIPE_WANTR) + if ((cpipe-pipe_state PIPE_WANTR)) { + cpipe-pipe_state = ~PIPE_WANTR; wakeup(cpipe); + } fip-fi_wgen++; FIFO_UPDWGEN(fip, cpipe); + pipeselwakeup(cpipe); PIPE_UNLOCK(cpipe); } } Modified: head/sys/kern/sys_pipe.c == --- head/sys/kern/sys_pipe.cTue Jul 31 00:46:19 2012(r238927) +++ head/sys/kern/sys_pipe.cTue Jul 31 02:00:37 2012(r238928) @@ -227,7 +227,6 @@ static int pipe_create(struct pipe *pipe static int pipe_paircreate(struct thread *td, struct pipepair **p_pp); static __inline int pipelock(struct pipe *cpipe, int catch); static __inline void pipeunlock(struct pipe *cpipe); -static __inline void pipeselwakeup(struct pipe *cpipe); #ifndef PIPE_NODIRECT static int pipe_build_write_buffer(struct pipe *wpipe, struct uio *uio); static void pipe_destroy_write_buffer(struct pipe *wpipe); @@ -607,7 +606,7 @@ pipeunlock(cpipe) } } -static __inline void +void pipeselwakeup(cpipe) struct pipe *cpipe; { @@ -738,7 +737,7 @@ pipe_read(fp, uio, active_cred, flags, t rpipe-pipe_map.pos += size; rpipe-pipe_map.cnt -= size; if (rpipe-pipe_map.cnt == 0) { - rpipe-pipe_state = ~PIPE_DIRECTW; + rpipe-pipe_state = ~(PIPE_DIRECTW|PIPE_WANTW); wakeup(rpipe); } #endif @@ -1001,6 +1000,7 @@ retry: wakeup(wpipe); } pipeselwakeup(wpipe); + wpipe-pipe_state |= PIPE_WANTW; pipeunlock(wpipe); error = msleep(wpipe, PIPE_MTX(wpipe), PRIBIO | PCATCH, pipdwt, 0); Modified: head/sys/sys/pipe.h == --- head/sys/sys/pipe.h Tue Jul 31 00:46:19 2012(r238927) +++ head/sys/sys/pipe.h Tue Jul 31 02:00:37 2012(r238928) @@ -143,5 +143,5 @@ struct pipepair { void pipe_dtor(struct pipe *dpipe); intpipe_named_ctor(struct pipe **ppipe, struct thread *td); - +void pipeselwakeup(struct pipe *cpipe); #endif /* !_SYS_PIPE_H_ */ ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail
svn commit: r238936 - in head/sys: fs/fifofs kern sys
Author: davidxu Date: Tue Jul 31 05:48:35 2012 New Revision: 238936 URL: http://svn.freebsd.org/changeset/base/238936 Log: I am comparing current pipe code with the one in 8.3-STABLE r236165, I found 8.3 is a history BSD version using socket to implement FIFO pipe, it uses per-file seqcount to compare with writer generation stored in per-pipe object. The concept is after all writers are gone, the pipe enters next generation, all old readers have not closed the pipe should get the indication that the pipe is disconnected, result is they should get EPIPE, SIGPIPE or get POLLHUP in poll(). But newcomer should not know that previous writters were gone, it should treat it as a fresh session. I am trying to bring back FIFO pipe to history behavior. It is still unclear that if single EOF flag can represent SBS_CANTSENDMORE and SBS_CANTRCVMORE which socket-based version is using, but I have run the poll regression test in tool directory, output is same as the one on 8.3-STABLE now. I think the output not ok 18 FIFO state 6b: poll result 0 expected 1. expected POLLHUP; got 0 might be bogus, because newcomer should not know that old writers were gone. I got the same behavior on Linux. Our implementation always return POLLIN for disconnected pipe even it should return POLLHUP, but I think it is not wise to remove POLLIN for compatible reason, this is our history behavior. Regression test: /usr/src/tools/regression/poll Modified: head/sys/fs/fifofs/fifo_vnops.c head/sys/kern/sys_pipe.c head/sys/sys/pipe.h Modified: head/sys/fs/fifofs/fifo_vnops.c == --- head/sys/fs/fifofs/fifo_vnops.c Tue Jul 31 05:44:03 2012 (r238935) +++ head/sys/fs/fifofs/fifo_vnops.c Tue Jul 31 05:48:35 2012 (r238936) @@ -59,23 +59,13 @@ * Notes about locking: * - fi_pipe is invariant since init time. * - fi_readers and fi_writers are protected by the vnode lock. - * - fi_wgen and fi_seqcount are protected by the pipe mutex. */ struct fifoinfo { struct pipe *fi_pipe; longfi_readers; longfi_writers; - int fi_wgen; - int fi_seqcount; }; -#define FIFO_UPDWGEN(fip, pip) do { \ - if ((fip)-fi_wgen == (fip)-fi_seqcount) \ - (pip)-pipe_state |= PIPE_SAMEWGEN; \ - else\ - (pip)-pipe_state = ~PIPE_SAMEWGEN;\ -} while (0) - static vop_print_t fifo_print; static vop_open_t fifo_open; static vop_close_t fifo_close; @@ -161,7 +151,7 @@ fifo_open(ap) return (error); fip = malloc(sizeof(*fip), M_VNODE, M_WAITOK); fip-fi_pipe = fpipe; - fip-fi_wgen = fip-fi_readers = fip-fi_writers = 0; + fpipe-pipe_wgen = fip-fi_readers = fip-fi_writers = 0; KASSERT(vp-v_fifoinfo == NULL, (fifo_open: v_fifoinfo race)); vp-v_fifoinfo = fip; } @@ -181,8 +171,7 @@ fifo_open(ap) if (fip-fi_writers 0) wakeup(fip-fi_writers); } - fip-fi_seqcount = fip-fi_wgen - fip-fi_writers; - FIFO_UPDWGEN(fip, fpipe); + fp-f_seqcount = fpipe-pipe_wgen - fip-fi_writers; } if (ap-a_mode FWRITE) { if ((ap-a_mode O_NONBLOCK) fip-fi_readers == 0) { @@ -235,8 +224,7 @@ fifo_open(ap) fpipe-pipe_state |= PIPE_EOF; if (fpipe-pipe_state PIPE_WANTR) wakeup(fpipe); - fip-fi_wgen++; - FIFO_UPDWGEN(fip, fpipe); + fpipe-pipe_wgen++; PIPE_UNLOCK(fpipe); fifo_cleanup(vp); } @@ -300,8 +288,7 @@ fifo_close(ap) cpipe-pipe_state = ~PIPE_WANTR; wakeup(cpipe); } - fip-fi_wgen++; - FIFO_UPDWGEN(fip, cpipe); + cpipe-pipe_wgen++; pipeselwakeup(cpipe); PIPE_UNLOCK(cpipe); } Modified: head/sys/kern/sys_pipe.c == --- head/sys/kern/sys_pipe.cTue Jul 31 05:44:03 2012(r238935) +++ head/sys/kern/sys_pipe.cTue Jul 31 05:48:35 2012(r238936) @@ -1442,7 +1442,7 @@ pipe_poll(fp, events, active_cred, td) levents = events (POLLIN | POLLINIGNEOF | POLLPRI | POLLRDNORM | POLLRDBAND); if
Re: svn commit: r238828 - head/sys/sys
On 2012/7/27 21:30, Bruce Evans wrote: On Fri, 27 Jul 2012, Gleb Smirnoff wrote: On Fri, Jul 27, 2012 at 10:32:55PM +1000, Bruce Evans wrote: B I just noticed that there is a technical problem -- the count is read B unlocked in the KASSERT. And since the comparision is for equality, B if you lose the race reading the count when it reaches the overflow B threshold, then you won't see it overflow unless it wraps again and B you win the race next time (or later). atomic_cmpset could be used B to clamp the value at the max, but that is too much for an assertion. We have discussed that. As alternative I proposed: @@ -50,8 +51,14 @@ static __inline void refcount_acquire(volatile u_int *count) { +#ifdef INVARIANTS + u_int old; + old = atomic_fetchadd_int(count, 1); + KASSERT(old UINT_MAX, (refcount %p overflowed, count)); +#else atomic_add_acq_int(count, 1); +#endif } Konstantin didn't like that production code differs from INVARIANTS. So we ended with what I committed, advocating to the fact that although assertion is racy and bad panics still can occur, the good panics would occur much more often, and a single good panic is enough to show what's going on. Yes, it is excessive. So why do people even care about this particular overflow? There are many integers that can overflow in the kernel. Some binary wraparounds are even intentional. Bruce The overflow might not be important. if it is important, all code which use 32-bit generation number could also be a problem, how about if it is wrapped around to same value another thread has just read and waiting for a CPU to run. :D ___ 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
svn commit: r238760 - stable/8/lib/libthr/thread
Author: davidxu Date: Wed Jul 25 01:57:53 2012 New Revision: 238760 URL: http://svn.freebsd.org/changeset/base/238760 Log: Revert r238715, the revision breaks firefox. Reported by: dougb Modified: stable/8/lib/libthr/thread/thr_setprio.c (contents, props changed) stable/8/lib/libthr/thread/thr_setschedparam.c (contents, props changed) Modified: stable/8/lib/libthr/thread/thr_setprio.c == --- stable/8/lib/libthr/thread/thr_setprio.cWed Jul 25 01:05:49 2012 (r238759) +++ stable/8/lib/libthr/thread/thr_setprio.cWed Jul 25 01:57:53 2012 (r238760) @@ -45,22 +45,38 @@ _pthread_setprio(pthread_t pthread, int int ret; param.sched_priority = prio; - if (pthread == curthread) + if (pthread == curthread) { THR_LOCK(curthread); - else if ((ret = _thr_find_thread(curthread, pthread, /*include dead*/0))) - return (ret); - if (pthread-attr.sched_policy == SCHED_OTHER || - pthread-attr.prio == prio) { - pthread-attr.prio = prio; - ret = 0; - } else { - ret = _thr_setscheduler(pthread-tid, - pthread-attr.sched_policy, param); - if (ret == -1) - ret = errno; - else + if (curthread-attr.sched_policy == SCHED_OTHER || + curthread-attr.prio == prio) { + curthread-attr.prio = prio; + ret = 0; + } else { + ret = _thr_setscheduler(curthread-tid, + curthread-attr.sched_policy, param); + if (ret == -1) + ret = errno; + else + curthread-attr.prio = prio; + } + THR_UNLOCK(curthread); + } else if ((ret = _thr_ref_add(curthread, pthread, /*include dead*/0)) + == 0) { + THR_THREAD_LOCK(curthread, pthread); + if (pthread-attr.sched_policy == SCHED_OTHER || + pthread-attr.prio == prio) { pthread-attr.prio = prio; + ret = 0; + } else { + ret = _thr_setscheduler(pthread-tid, + curthread-attr.sched_policy, param); + if (ret == -1) + ret = errno; + else + pthread-attr.prio = prio; + } + THR_THREAD_UNLOCK(curthread, pthread); + _thr_ref_delete(curthread, pthread); } - THR_THREAD_UNLOCK(curthread, pthread); return (ret); } Modified: stable/8/lib/libthr/thread/thr_setschedparam.c == --- stable/8/lib/libthr/thread/thr_setschedparam.c Wed Jul 25 01:05:49 2012(r238759) +++ stable/8/lib/libthr/thread/thr_setschedparam.c Wed Jul 25 01:57:53 2012(r238760) @@ -53,25 +53,42 @@ _pthread_setschedparam(pthread_t pthread struct pthread *curthread = _get_curthread(); int ret; - if (pthread == curthread) + if (pthread == curthread) { THR_LOCK(curthread); - else if ((ret = _thr_find_thread(curthread, pthread, -/*include dead*/0)) != 0) - return (ret); - if (pthread-attr.sched_policy == policy - (policy == SCHED_OTHER || -pthread-attr.prio == param-sched_priority)) { - pthread-attr.prio = param-sched_priority; + if (curthread-attr.sched_policy == policy + (policy == SCHED_OTHER || +curthread-attr.prio == param-sched_priority)) { + pthread-attr.prio = param-sched_priority; + THR_UNLOCK(curthread); + return (0); + } + ret = _thr_setscheduler(curthread-tid, policy, param); + if (ret == -1) + ret = errno; + else { + curthread-attr.sched_policy = policy; + curthread-attr.prio = param-sched_priority; + } + THR_UNLOCK(curthread); + } else if ((ret = _thr_ref_add(curthread, pthread, /*include dead*/0)) + == 0) { + THR_THREAD_LOCK(curthread, pthread); + if (pthread-attr.sched_policy == policy + (policy == SCHED_OTHER || +pthread-attr.prio == param-sched_priority)) { + pthread-attr.prio = param-sched_priority; + THR_THREAD_UNLOCK(curthread, pthread); + return (0); + } +
svn commit: r238761 - stable/8/lib/libthr/thread
Author: davidxu Date: Wed Jul 25 02:05:59 2012 New Revision: 238761 URL: http://svn.freebsd.org/changeset/base/238761 Log: Release a reference count in case priority needn't to be changed. Modified: stable/8/lib/libthr/thread/thr_setschedparam.c Modified: stable/8/lib/libthr/thread/thr_setschedparam.c == --- stable/8/lib/libthr/thread/thr_setschedparam.c Wed Jul 25 01:57:53 2012(r238760) +++ stable/8/lib/libthr/thread/thr_setschedparam.c Wed Jul 25 02:05:59 2012(r238761) @@ -78,6 +78,7 @@ _pthread_setschedparam(pthread_t pthread pthread-attr.prio == param-sched_priority)) { pthread-attr.prio = param-sched_priority; THR_THREAD_UNLOCK(curthread, pthread); + _thr_ref_delete(curthread, pthread); return (0); } ret = _thr_setscheduler(pthread-tid, policy, param); ___ 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
svn commit: r238762 - stable/8/lib/libthr/thread
Author: davidxu Date: Wed Jul 25 02:09:06 2012 New Revision: 238762 URL: http://svn.freebsd.org/changeset/base/238762 Log: Use target thread's scheduling policy not current's. Modified: stable/8/lib/libthr/thread/thr_setprio.c Modified: stable/8/lib/libthr/thread/thr_setprio.c == --- stable/8/lib/libthr/thread/thr_setprio.cWed Jul 25 02:05:59 2012 (r238761) +++ stable/8/lib/libthr/thread/thr_setprio.cWed Jul 25 02:09:06 2012 (r238762) @@ -69,7 +69,7 @@ _pthread_setprio(pthread_t pthread, int ret = 0; } else { ret = _thr_setscheduler(pthread-tid, - curthread-attr.sched_policy, param); + pthread-attr.sched_policy, param); if (ret == -1) ret = errno; else ___ 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
svn commit: r238714 - stable/9/lib/libthr/thread
Author: davidxu Date: Mon Jul 23 09:33:31 2012 New Revision: 238714 URL: http://svn.freebsd.org/changeset/base/238714 Log: Merge r238637,r238640,r238641,r238642: r238637 | davidxu | 2012-07-20 09:56:14 +0800 (Fri, 20 Jul 2012) | 6 lines Don't forget to release a thread reference count, replace _thr_ref_add() with _thr_find_thread(), so reference count is no longer needed. r238640 | davidxu | 2012-07-20 11:00:41 +0800 (Fri, 20 Jul 2012) | 2 lines Eliminate duplicated code. r238641 | davidxu | 2012-07-20 11:16:52 +0800 (Fri, 20 Jul 2012) | 2 lines Eliminate duplicated code. r238642 | davidxu | 2012-07-20 11:22:17 +0800 (Fri, 20 Jul 2012) | 2 lines Don't assign same value. Approved by: re (kib) Modified: stable/9/lib/libthr/thread/thr_setprio.c (contents, props changed) stable/9/lib/libthr/thread/thr_setschedparam.c (contents, props changed) Modified: stable/9/lib/libthr/thread/thr_setprio.c == --- stable/9/lib/libthr/thread/thr_setprio.cMon Jul 23 09:19:14 2012 (r238713) +++ stable/9/lib/libthr/thread/thr_setprio.cMon Jul 23 09:33:31 2012 (r238714) @@ -45,38 +45,22 @@ _pthread_setprio(pthread_t pthread, int int ret; param.sched_priority = prio; - if (pthread == curthread) { + if (pthread == curthread) THR_LOCK(curthread); - if (curthread-attr.sched_policy == SCHED_OTHER || - curthread-attr.prio == prio) { - curthread-attr.prio = prio; - ret = 0; - } else { - ret = _thr_setscheduler(curthread-tid, - curthread-attr.sched_policy, param); - if (ret == -1) - ret = errno; - else - curthread-attr.prio = prio; - } - THR_UNLOCK(curthread); - } else if ((ret = _thr_ref_add(curthread, pthread, /*include dead*/0)) - == 0) { - THR_THREAD_LOCK(curthread, pthread); - if (pthread-attr.sched_policy == SCHED_OTHER || - pthread-attr.prio == prio) { + else if ((ret = _thr_find_thread(curthread, pthread, /*include dead*/0))) + return (ret); + if (pthread-attr.sched_policy == SCHED_OTHER || + pthread-attr.prio == prio) { + pthread-attr.prio = prio; + ret = 0; + } else { + ret = _thr_setscheduler(pthread-tid, + pthread-attr.sched_policy, param); + if (ret == -1) + ret = errno; + else pthread-attr.prio = prio; - ret = 0; - } else { - ret = _thr_setscheduler(pthread-tid, - curthread-attr.sched_policy, param); - if (ret == -1) - ret = errno; - else - pthread-attr.prio = prio; - } - THR_THREAD_UNLOCK(curthread, pthread); - _thr_ref_delete(curthread, pthread); } + THR_THREAD_UNLOCK(curthread, pthread); return (ret); } Modified: stable/9/lib/libthr/thread/thr_setschedparam.c == --- stable/9/lib/libthr/thread/thr_setschedparam.c Mon Jul 23 09:19:14 2012(r238713) +++ stable/9/lib/libthr/thread/thr_setschedparam.c Mon Jul 23 09:33:31 2012(r238714) @@ -53,42 +53,25 @@ _pthread_setschedparam(pthread_t pthread struct pthread *curthread = _get_curthread(); int ret; - if (pthread == curthread) { + if (pthread == curthread) THR_LOCK(curthread); - if (curthread-attr.sched_policy == policy - (policy == SCHED_OTHER || -curthread-attr.prio == param-sched_priority)) { - pthread-attr.prio = param-sched_priority; - THR_UNLOCK(curthread); - return (0); - } - ret = _thr_setscheduler(curthread-tid, policy, param); - if (ret == -1) - ret = errno; - else { - curthread-attr.sched_policy = policy; - curthread-attr.prio = param-sched_priority; - } - THR_UNLOCK(curthread); -
svn commit: r238715 - stable/8/lib/libthr/thread
Author: davidxu Date: Mon Jul 23 09:34:19 2012 New Revision: 238715 URL: http://svn.freebsd.org/changeset/base/238715 Log: Merge r238637,r238640,r238641,r238642: r238637 | davidxu | 2012-07-20 09:56:14 +0800 (Fri, 20 Jul 2012) | 6 lines Don't forget to release a thread reference count, replace _thr_ref_add() with _thr_find_thread(), so reference count is no longer needed. r238640 | davidxu | 2012-07-20 11:00:41 +0800 (Fri, 20 Jul 2012) | 2 lines Eliminate duplicated code. r238641 | davidxu | 2012-07-20 11:16:52 +0800 (Fri, 20 Jul 2012) | 2 lines Eliminate duplicated code. r238642 | davidxu | 2012-07-20 11:22:17 +0800 (Fri, 20 Jul 2012) | 2 lines Don't assign same value. Modified: stable/8/lib/libthr/thread/thr_setprio.c (contents, props changed) stable/8/lib/libthr/thread/thr_setschedparam.c (contents, props changed) Modified: stable/8/lib/libthr/thread/thr_setprio.c == --- stable/8/lib/libthr/thread/thr_setprio.cMon Jul 23 09:33:31 2012 (r238714) +++ stable/8/lib/libthr/thread/thr_setprio.cMon Jul 23 09:34:19 2012 (r238715) @@ -45,38 +45,22 @@ _pthread_setprio(pthread_t pthread, int int ret; param.sched_priority = prio; - if (pthread == curthread) { + if (pthread == curthread) THR_LOCK(curthread); - if (curthread-attr.sched_policy == SCHED_OTHER || - curthread-attr.prio == prio) { - curthread-attr.prio = prio; - ret = 0; - } else { - ret = _thr_setscheduler(curthread-tid, - curthread-attr.sched_policy, param); - if (ret == -1) - ret = errno; - else - curthread-attr.prio = prio; - } - THR_UNLOCK(curthread); - } else if ((ret = _thr_ref_add(curthread, pthread, /*include dead*/0)) - == 0) { - THR_THREAD_LOCK(curthread, pthread); - if (pthread-attr.sched_policy == SCHED_OTHER || - pthread-attr.prio == prio) { + else if ((ret = _thr_find_thread(curthread, pthread, /*include dead*/0))) + return (ret); + if (pthread-attr.sched_policy == SCHED_OTHER || + pthread-attr.prio == prio) { + pthread-attr.prio = prio; + ret = 0; + } else { + ret = _thr_setscheduler(pthread-tid, + pthread-attr.sched_policy, param); + if (ret == -1) + ret = errno; + else pthread-attr.prio = prio; - ret = 0; - } else { - ret = _thr_setscheduler(pthread-tid, - curthread-attr.sched_policy, param); - if (ret == -1) - ret = errno; - else - pthread-attr.prio = prio; - } - THR_THREAD_UNLOCK(curthread, pthread); - _thr_ref_delete(curthread, pthread); } + THR_THREAD_UNLOCK(curthread, pthread); return (ret); } Modified: stable/8/lib/libthr/thread/thr_setschedparam.c == --- stable/8/lib/libthr/thread/thr_setschedparam.c Mon Jul 23 09:33:31 2012(r238714) +++ stable/8/lib/libthr/thread/thr_setschedparam.c Mon Jul 23 09:34:19 2012(r238715) @@ -53,42 +53,25 @@ _pthread_setschedparam(pthread_t pthread struct pthread *curthread = _get_curthread(); int ret; - if (pthread == curthread) { + if (pthread == curthread) THR_LOCK(curthread); - if (curthread-attr.sched_policy == policy - (policy == SCHED_OTHER || -curthread-attr.prio == param-sched_priority)) { - pthread-attr.prio = param-sched_priority; - THR_UNLOCK(curthread); - return (0); - } - ret = _thr_setscheduler(curthread-tid, policy, param); - if (ret == -1) - ret = errno; - else { - curthread-attr.sched_policy = policy; - curthread-attr.prio = param-sched_priority; - } - THR_UNLOCK(curthread); - } else if ((ret =
svn commit: r238637 - head/lib/libthr/thread
Author: davidxu Date: Fri Jul 20 01:56:14 2012 New Revision: 238637 URL: http://svn.freebsd.org/changeset/base/238637 Log: Don't forget to release a thread reference count, replace _thr_ref_add() with _thr_find_thread(), so reference count is no longer needed. MFC after:3 days Modified: head/lib/libthr/thread/thr_setschedparam.c Modified: head/lib/libthr/thread/thr_setschedparam.c == --- head/lib/libthr/thread/thr_setschedparam.c Fri Jul 20 01:41:18 2012 (r238636) +++ head/lib/libthr/thread/thr_setschedparam.c Fri Jul 20 01:56:14 2012 (r238637) @@ -70,9 +70,8 @@ _pthread_setschedparam(pthread_t pthread curthread-attr.prio = param-sched_priority; } THR_UNLOCK(curthread); - } else if ((ret = _thr_ref_add(curthread, pthread, /*include dead*/0)) - == 0) { - THR_THREAD_LOCK(curthread, pthread); + } else if ((ret = _thr_find_thread(curthread, pthread, +/*include dead*/0)) == 0) { if (pthread-attr.sched_policy == policy (policy == SCHED_OTHER || pthread-attr.prio == param-sched_priority)) { @@ -88,7 +87,6 @@ _pthread_setschedparam(pthread_t pthread pthread-attr.prio = param-sched_priority; } THR_THREAD_UNLOCK(curthread, pthread); - _thr_ref_delete(curthread, pthread); } return (ret); } ___ 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
svn commit: r238640 - head/lib/libthr/thread
Author: davidxu Date: Fri Jul 20 03:00:41 2012 New Revision: 238640 URL: http://svn.freebsd.org/changeset/base/238640 Log: Eliminate duplicated code. Modified: head/lib/libthr/thread/thr_setschedparam.c Modified: head/lib/libthr/thread/thr_setschedparam.c == --- head/lib/libthr/thread/thr_setschedparam.c Fri Jul 20 02:18:47 2012 (r238639) +++ head/lib/libthr/thread/thr_setschedparam.c Fri Jul 20 03:00:41 2012 (r238640) @@ -54,39 +54,25 @@ _pthread_setschedparam(pthread_t pthread int ret; if (pthread == curthread) { + pthread = curthread; THR_LOCK(curthread); - if (curthread-attr.sched_policy == policy - (policy == SCHED_OTHER || -curthread-attr.prio == param-sched_priority)) { - pthread-attr.prio = param-sched_priority; - THR_UNLOCK(curthread); - return (0); - } - ret = _thr_setscheduler(curthread-tid, policy, param); - if (ret == -1) - ret = errno; - else { - curthread-attr.sched_policy = policy; - curthread-attr.prio = param-sched_priority; - } - THR_UNLOCK(curthread); } else if ((ret = _thr_find_thread(curthread, pthread, -/*include dead*/0)) == 0) { - if (pthread-attr.sched_policy == policy - (policy == SCHED_OTHER || -pthread-attr.prio == param-sched_priority)) { - pthread-attr.prio = param-sched_priority; - THR_THREAD_UNLOCK(curthread, pthread); - return (0); - } - ret = _thr_setscheduler(pthread-tid, policy, param); - if (ret == -1) - ret = errno; - else { - pthread-attr.sched_policy = policy; - pthread-attr.prio = param-sched_priority; - } +/*include dead*/0)) != 0) + return (ret); + if (pthread-attr.sched_policy == policy + (policy == SCHED_OTHER || +pthread-attr.prio == param-sched_priority)) { + pthread-attr.prio = param-sched_priority; THR_THREAD_UNLOCK(curthread, pthread); + return (0); } + ret = _thr_setscheduler(pthread-tid, policy, param); + if (ret == -1) + ret = errno; + else { + pthread-attr.sched_policy = policy; + pthread-attr.prio = param-sched_priority; + } + THR_THREAD_UNLOCK(curthread, pthread); return (ret); } ___ 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
svn commit: r238641 - head/lib/libthr/thread
Author: davidxu Date: Fri Jul 20 03:16:52 2012 New Revision: 238641 URL: http://svn.freebsd.org/changeset/base/238641 Log: Eliminate duplicated code. Modified: head/lib/libthr/thread/thr_setprio.c Modified: head/lib/libthr/thread/thr_setprio.c == --- head/lib/libthr/thread/thr_setprio.cFri Jul 20 03:00:41 2012 (r238640) +++ head/lib/libthr/thread/thr_setprio.cFri Jul 20 03:16:52 2012 (r238641) @@ -46,37 +46,22 @@ _pthread_setprio(pthread_t pthread, int param.sched_priority = prio; if (pthread == curthread) { + pthread = curthread; THR_LOCK(curthread); - if (curthread-attr.sched_policy == SCHED_OTHER || - curthread-attr.prio == prio) { - curthread-attr.prio = prio; - ret = 0; - } else { - ret = _thr_setscheduler(curthread-tid, - curthread-attr.sched_policy, param); - if (ret == -1) - ret = errno; - else - curthread-attr.prio = prio; - } - THR_UNLOCK(curthread); - } else if ((ret = _thr_ref_add(curthread, pthread, /*include dead*/0)) - == 0) { - THR_THREAD_LOCK(curthread, pthread); - if (pthread-attr.sched_policy == SCHED_OTHER || - pthread-attr.prio == prio) { + } else if ((ret = _thr_find_thread(curthread, pthread, /*include dead*/0))) + return (ret); + if (pthread-attr.sched_policy == SCHED_OTHER || + pthread-attr.prio == prio) { + pthread-attr.prio = prio; + ret = 0; + } else { + ret = _thr_setscheduler(pthread-tid, + pthread-attr.sched_policy, param); + if (ret == -1) + ret = errno; + else pthread-attr.prio = prio; - ret = 0; - } else { - ret = _thr_setscheduler(pthread-tid, - curthread-attr.sched_policy, param); - if (ret == -1) - ret = errno; - else - pthread-attr.prio = prio; - } - THR_THREAD_UNLOCK(curthread, pthread); - _thr_ref_delete(curthread, pthread); } + THR_THREAD_UNLOCK(curthread, pthread); return (ret); } ___ 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
svn commit: r238642 - head/lib/libthr/thread
Author: davidxu Date: Fri Jul 20 03:22:17 2012 New Revision: 238642 URL: http://svn.freebsd.org/changeset/base/238642 Log: Don't assign same value. Modified: head/lib/libthr/thread/thr_setprio.c head/lib/libthr/thread/thr_setschedparam.c Modified: head/lib/libthr/thread/thr_setprio.c == --- head/lib/libthr/thread/thr_setprio.cFri Jul 20 03:16:52 2012 (r238641) +++ head/lib/libthr/thread/thr_setprio.cFri Jul 20 03:22:17 2012 (r238642) @@ -45,10 +45,9 @@ _pthread_setprio(pthread_t pthread, int int ret; param.sched_priority = prio; - if (pthread == curthread) { - pthread = curthread; + if (pthread == curthread) THR_LOCK(curthread); - } else if ((ret = _thr_find_thread(curthread, pthread, /*include dead*/0))) + else if ((ret = _thr_find_thread(curthread, pthread, /*include dead*/0))) return (ret); if (pthread-attr.sched_policy == SCHED_OTHER || pthread-attr.prio == prio) { Modified: head/lib/libthr/thread/thr_setschedparam.c == --- head/lib/libthr/thread/thr_setschedparam.c Fri Jul 20 03:16:52 2012 (r238641) +++ head/lib/libthr/thread/thr_setschedparam.c Fri Jul 20 03:22:17 2012 (r238642) @@ -53,10 +53,9 @@ _pthread_setschedparam(pthread_t pthread struct pthread *curthread = _get_curthread(); int ret; - if (pthread == curthread) { - pthread = curthread; + if (pthread == curthread) THR_LOCK(curthread); - } else if ((ret = _thr_find_thread(curthread, pthread, + else if ((ret = _thr_find_thread(curthread, pthread, /*include dead*/0)) != 0) return (ret); if (pthread-attr.sched_policy == policy ___ 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
svn commit: r238643 - head/lib/libthr/thread
Author: davidxu Date: Fri Jul 20 03:27:07 2012 New Revision: 238643 URL: http://svn.freebsd.org/changeset/base/238643 Log: Eliminate duplicated code. Modified: head/lib/libthr/thread/thr_getschedparam.c Modified: head/lib/libthr/thread/thr_getschedparam.c == --- head/lib/libthr/thread/thr_getschedparam.c Fri Jul 20 03:22:17 2012 (r238642) +++ head/lib/libthr/thread/thr_getschedparam.c Fri Jul 20 03:27:07 2012 (r238643) @@ -53,25 +53,16 @@ _pthread_getschedparam(pthread_t pthread if (policy == NULL || param == NULL) return (EINVAL); - if (pthread == curthread) { - /* -* Avoid searching the thread list when it is the current -* thread. -*/ + /* +* Avoid searching the thread list when it is the current +* thread. +*/ + if (pthread == curthread) THR_LOCK(curthread); - *policy = curthread-attr.sched_policy; - param-sched_priority = curthread-attr.prio; - THR_UNLOCK(curthread); - ret = 0; - } - /* Find the thread in the list of active threads. */ - else if ((ret = _thr_ref_add(curthread, pthread, /*include dead*/0)) - == 0) { - THR_THREAD_LOCK(curthread, pthread); - *policy = pthread-attr.sched_policy; - param-sched_priority = pthread-attr.prio; - THR_THREAD_UNLOCK(curthread, pthread); - _thr_ref_delete(curthread, pthread); - } + else if ((ret = _thr_find_thread(curthread, pthread, /*include dead*/0))) + return (ret); + *policy = pthread-attr.sched_policy; + param-sched_priority = pthread-attr.prio; + THR_THREAD_UNLOCK(curthread, pthread); return (ret); } ___ 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
svn commit: r238644 - head/lib/libthr/thread
Author: davidxu Date: Fri Jul 20 03:37:19 2012 New Revision: 238644 URL: http://svn.freebsd.org/changeset/base/238644 Log: Simplify code by replacing _thr_ref_add() with _thr_find_thread(). Modified: head/lib/libthr/thread/thr_info.c Modified: head/lib/libthr/thread/thr_info.c == --- head/lib/libthr/thread/thr_info.c Fri Jul 20 03:27:07 2012 (r238643) +++ head/lib/libthr/thread/thr_info.c Fri Jul 20 03:37:19 2012 (r238644) @@ -51,16 +51,12 @@ _pthread_set_name_np(pthread_t thread, c if (thr_set_name(thread-tid, name)) ret = errno; } else { - if (_thr_ref_add(curthread, thread, 0) == 0) { - THR_THREAD_LOCK(curthread, thread); + if ((ret=_thr_find_thread(curthread, thread, 0)) == 0) { if (thread-state != PS_DEAD) { if (thr_set_name(thread-tid, name)) ret = errno; } THR_THREAD_UNLOCK(curthread, thread); - _thr_ref_delete(curthread, thread); - } else { - ret = ESRCH; } } #if 0 ___ 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
svn commit: r238645 - head/lib/libthr/thread
Author: davidxu Date: Fri Jul 20 05:47:12 2012 New Revision: 238645 URL: http://svn.freebsd.org/changeset/base/238645 Log: Don't forget to initialize return value. Modified: head/lib/libthr/thread/thr_getschedparam.c Modified: head/lib/libthr/thread/thr_getschedparam.c == --- head/lib/libthr/thread/thr_getschedparam.c Fri Jul 20 03:37:19 2012 (r238644) +++ head/lib/libthr/thread/thr_getschedparam.c Fri Jul 20 05:47:12 2012 (r238645) @@ -48,7 +48,7 @@ _pthread_getschedparam(pthread_t pthread struct sched_param *param) { struct pthread *curthread = _get_curthread(); - int ret; + int ret = 0; if (policy == NULL || param == NULL) return (EINVAL); ___ 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
svn commit: r238541 - stable/8/lib/libc/i386/gen
Author: davidxu Date: Tue Jul 17 02:02:40 2012 New Revision: 238541 URL: http://svn.freebsd.org/changeset/base/238541 Log: MFC r238328: Executing CPUID with EAX set to 1 to actually get feature flags. PR: 169730 Modified: stable/8/lib/libc/i386/gen/getcontextx.c Directory Properties: stable/8/lib/libc/ (props changed) Modified: stable/8/lib/libc/i386/gen/getcontextx.c == --- stable/8/lib/libc/i386/gen/getcontextx.cMon Jul 16 22:15:30 2012 (r238540) +++ stable/8/lib/libc/i386/gen/getcontextx.cTue Jul 17 02:02:40 2012 (r238541) @@ -68,7 +68,7 @@ __getcontextx_size(void) movl%%ebx,%1\n popl%%ebx\n : =a (p[0]), =r (p[1]), =c (p[2]), =d (p[3]) - : 0 (0x0)); + : 0 (0x1)); if ((p[2] CPUID2_OSXSAVE) != 0) { __asm __volatile( pushl %%ebx\n ___ 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
svn commit: r238513 - stable/9/lib/libc/i386/gen
Author: davidxu Date: Mon Jul 16 02:10:26 2012 New Revision: 238513 URL: http://svn.freebsd.org/changeset/base/238513 Log: MFC r238328: Executing CPUID with EAX set to 1 to actually get feature flags. PR: 169730 Approved by:re (kib) Modified: stable/9/lib/libc/i386/gen/getcontextx.c Directory Properties: stable/9/lib/libc/ (props changed) Modified: stable/9/lib/libc/i386/gen/getcontextx.c == --- stable/9/lib/libc/i386/gen/getcontextx.cMon Jul 16 00:21:05 2012 (r238512) +++ stable/9/lib/libc/i386/gen/getcontextx.cMon Jul 16 02:10:26 2012 (r238513) @@ -68,7 +68,7 @@ __getcontextx_size(void) movl%%ebx,%1\n popl%%ebx\n : =a (p[0]), =r (p[1]), =c (p[2]), =d (p[3]) - : 0 (0x0)); + : 0 (0x1)); if ((p[2] CPUID2_OSXSAVE) != 0) { __asm __volatile( pushl %%ebx\n ___ 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
svn commit: r238287 - head/sys/kern
Author: davidxu Date: Mon Jul 9 09:24:46 2012 New Revision: 238287 URL: http://svn.freebsd.org/changeset/base/238287 Log: If you have pressed CTRL+Z and a process is suspended, then you use gdb to attach to the process, it is surprising that the process is resumed without inputting any gdb commands, however ptrace manual said: The tracing process will see the newly-traced process stop and may then control it as if it had been traced all along. But the current code does not work in this way, unless traced process received a signal later, it will continue to run as a background task. To fix this problem, just send signal SIGSTOP to the traced process after we resumed it, this works like that you are attaching to a running process, it is not perfect but better than nothing. Modified: head/sys/kern/sys_process.c Modified: head/sys/kern/sys_process.c == --- head/sys/kern/sys_process.c Mon Jul 9 09:11:07 2012(r238286) +++ head/sys/kern/sys_process.c Mon Jul 9 09:24:46 2012(r238287) @@ -635,7 +635,7 @@ kern_ptrace(struct thread *td, int req, struct iovec iov; struct uio uio; struct proc *curp, *p, *pp; - struct thread *td2 = NULL; + struct thread *td2 = NULL, *td3; struct ptrace_io_desc *piod = NULL; struct ptrace_lwpinfo *pl; int error, write, tmp, num; @@ -953,10 +953,8 @@ kern_ptrace(struct thread *td, int req, td2-td_xsig = data; if (req == PT_DETACH) { - struct thread *td3; - FOREACH_THREAD_IN_PROC(p, td3) { + FOREACH_THREAD_IN_PROC(p, td3) td3-td_dbgflags = ~TDB_SUSPEND; - } } /* * unsuspend all threads, to not let a thread run, @@ -967,6 +965,8 @@ kern_ptrace(struct thread *td, int req, p-p_flag = ~(P_STOPPED_TRACE|P_STOPPED_SIG|P_WAITED); thread_unsuspend(p); PROC_SUNLOCK(p); + if (req == PT_ATTACH) + kern_psignal(p, data); } else { if (data) kern_psignal(p, data); ___ 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
svn commit: r238328 - head/lib/libc/i386/gen
Author: davidxu Date: Tue Jul 10 01:47:11 2012 New Revision: 238328 URL: http://svn.freebsd.org/changeset/base/238328 Log: Executing CPUID with EAX set to 1 to actually get feature flags. PR: 169730 Modified: head/lib/libc/i386/gen/getcontextx.c Modified: head/lib/libc/i386/gen/getcontextx.c == --- head/lib/libc/i386/gen/getcontextx.cTue Jul 10 01:32:52 2012 (r238327) +++ head/lib/libc/i386/gen/getcontextx.cTue Jul 10 01:47:11 2012 (r238328) @@ -68,7 +68,7 @@ __getcontextx_size(void) movl%%ebx,%1\n popl%%ebx\n : =a (p[0]), =r (p[1]), =c (p[2]), =d (p[3]) - : 0 (0x0)); + : 0 (0x1)); if ((p[2] CPUID2_OSXSAVE) != 0) { __asm __volatile( pushl %%ebx\n ___ 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
svn commit: r238336 - head/sys/kern
Author: davidxu Date: Tue Jul 10 05:45:13 2012 New Revision: 238336 URL: http://svn.freebsd.org/changeset/base/238336 Log: Always clear p_xthread if current thread no longer needs it, in theory, if debugger exited without calling ptrace(PT_DETACH), there is a time window that the p_xthread may be pointing to non-existing thread, in practical, this is not a problem because child process soon will be killed by parent process. Modified: head/sys/kern/kern_sig.c Modified: head/sys/kern/kern_sig.c == --- head/sys/kern/kern_sig.cTue Jul 10 05:39:06 2012(r238335) +++ head/sys/kern/kern_sig.cTue Jul 10 05:45:13 2012(r238336) @@ -2436,9 +2436,10 @@ ptracestop(struct thread *td, int sig) } stopme: thread_suspend_switch(td); - if (!(p-p_flag P_TRACED)) { + if (p-p_xthread == td) + p-p_xthread = NULL; + if (!(p-p_flag P_TRACED)) break; - } if (td-td_dbgflags TDB_SUSPEND) { if (p-p_flag P_SINGLE_EXIT) break; ___ 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: r237660 - head/lib/libc/gen
On 2012/6/28 15:53, Konstantin Belousov wrote: On Thu, Jun 28, 2012 at 10:53:03AM +0800, David Xu wrote: On 2012/6/28 10:32, Attilio Rao wrote: 2012/6/28, David Xulistlog2...@gmail.com: On 2012/6/28 10:21, Attilio Rao wrote: 2012/6/28, David Xulistlog2...@gmail.com: On 2012/6/28 4:32, Konstantin Belousov wrote: Author: kib Date: Wed Jun 27 20:32:45 2012 New Revision: 237660 URL: http://svn.freebsd.org/changeset/base/237660 Log: Optimize the handling of SC_NPROCESSORS_CONF, by using auxv AT_NCPU value if present. MFC after: 1 week Modified: head/lib/libc/gen/sysconf.c Modified: head/lib/libc/gen/sysconf.c == --- head/lib/libc/gen/sysconf.c Wed Jun 27 20:24:25 2012 (r237659) +++ head/lib/libc/gen/sysconf.c Wed Jun 27 20:32:45 2012 (r237660) @@ -42,6 +42,7 @@ __FBSDID($FreeBSD$); #includesys/resource.h #includesys/socket.h +#includeelf.h #includeerrno.h #includelimits.h #includepaths.h @@ -51,6 +52,7 @@ __FBSDID($FreeBSD$); #include ../stdlib/atexit.h #include tzfile.h /* from ../../../contrib/tzcode/stdtime */ +#include libc_private.h #define _PATH_ZONEINFO TZDIR /* from tzfile.h */ @@ -585,6 +587,8 @@ yesno: case _SC_NPROCESSORS_CONF: case _SC_NPROCESSORS_ONLN: + if (_elf_aux_info(AT_NCPUS,value, sizeof(value)) == 0) + return ((long)value); mib[0] = CTL_HW; mib[1] = HW_NCPU; break; Will this make controlling the number of CPU online or CPU hotplug be impossible on FreeBSD ? If I think about hotplug CPUs I can think of other 1000 problems/races/bad situations to be fixed before this one, really. These are problems only in kernel, but kib's change is about ABI between userland and kernel, I hope we don't introduce an ABI which is not extendable road stone. I'm not entirely sure I see the ABI breakage here. It is not breakage, it is the ABI thinks number of online cpu is fixed, obviously, it is not the case in future unless FreeBSD won't support dynamic number of online cpus. If the AT_NCPUS becames unconvenient and not correct at some point we can just fix sysconf() to not look into the aux vector anymoe. If you already know this will be a problem, why do you introduce it and later need to fix it ? Please note that AT_NCPUS is already exported nowadays. I think this is instead a clever optimization to avoid the sysctl() (usual way to retrieve the number of CPUs). But why don't you cache it in libc ? following code is enough: static int online_cpu; if (online_cpu == 0) online_cpu = sysctl return online_cpu; Thread did evolved somewhat while I was AFK. First, please note that the ABI which I designed there is fixable: if kernel does not export AT_NCPUS at all, then auxv correctly handles the situation returning an error, and libc falls back to sysctl(2). Do we really want to bypass sysctl and instead passing all info via auxv vector ? I found the sysconf() is a bunch of switch-case, which is already slow, before _SC_NPROCESSES_ONLN, it has already a quite number of case branches, and in your code, it calls _elf_aux_info() which also has some switch-cases branch, if you cache smp_cpus in libc, the call for _elf_aux_info is not needed, and you don't need code in kernel to passing it either, in any case, the code to call sysctl is still needed, so why don't we just use sysctl instead and cache the result in libc ? this at least can generate small code and a bit faster after first call to sysconf(_SC_NPROCESSES_ONLN). Second, sysconf(3) is very weird API. Note the following statement from SUSv4: The value shall not change during the lifetime of the calling process, [XSI] [Option Start] except that sysconf(_SC_OPEN_MAX) may return different values before and after a call to setrlimit() which changes the RLIMIT_NOFILE soft limit. Corresponding comment is also present in sysconf.c. So it declares the number of cpu is static and can not be changed. So I do not see an issue there, esp. for advisory value which NCPUS is anyway. ___ 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: r237660 - head/lib/libc/gen
On 2012/6/28 16:49, David Xu wrote: On 2012/6/28 15:53, Konstantin Belousov wrote: On Thu, Jun 28, 2012 at 10:53:03AM +0800, David Xu wrote: On 2012/6/28 10:32, Attilio Rao wrote: 2012/6/28, David Xulistlog2...@gmail.com: On 2012/6/28 10:21, Attilio Rao wrote: 2012/6/28, David Xulistlog2...@gmail.com: On 2012/6/28 4:32, Konstantin Belousov wrote: Author: kib Date: Wed Jun 27 20:32:45 2012 New Revision: 237660 URL: http://svn.freebsd.org/changeset/base/237660 Log: Optimize the handling of SC_NPROCESSORS_CONF, by using auxv AT_NCPU value if present. MFC after:1 week Modified: head/lib/libc/gen/sysconf.c Modified: head/lib/libc/gen/sysconf.c == --- head/lib/libc/gen/sysconf.cWed Jun 27 20:24:25 2012 (r237659) +++ head/lib/libc/gen/sysconf.cWed Jun 27 20:32:45 2012 (r237660) @@ -42,6 +42,7 @@ __FBSDID($FreeBSD$); #includesys/resource.h #includesys/socket.h +#includeelf.h #includeerrno.h #includelimits.h #includepaths.h @@ -51,6 +52,7 @@ __FBSDID($FreeBSD$); #include ../stdlib/atexit.h #include tzfile.h/* from ../../../contrib/tzcode/stdtime */ +#include libc_private.h #define_PATH_ZONEINFOTZDIR/* from tzfile.h */ @@ -585,6 +587,8 @@ yesno: case _SC_NPROCESSORS_CONF: case _SC_NPROCESSORS_ONLN: +if (_elf_aux_info(AT_NCPUS,value, sizeof(value)) == 0) +return ((long)value); mib[0] = CTL_HW; mib[1] = HW_NCPU; break; Will this make controlling the number of CPU online or CPU hotplug be impossible on FreeBSD ? If I think about hotplug CPUs I can think of other 1000 problems/races/bad situations to be fixed before this one, really. These are problems only in kernel, but kib's change is about ABI between userland and kernel, I hope we don't introduce an ABI which is not extendable road stone. I'm not entirely sure I see the ABI breakage here. It is not breakage, it is the ABI thinks number of online cpu is fixed, obviously, it is not the case in future unless FreeBSD won't support dynamic number of online cpus. If the AT_NCPUS becames unconvenient and not correct at some point we can just fix sysconf() to not look into the aux vector anymoe. If you already know this will be a problem, why do you introduce it and later need to fix it ? Please note that AT_NCPUS is already exported nowadays. I think this is instead a clever optimization to avoid the sysctl() (usual way to retrieve the number of CPUs). But why don't you cache it in libc ? following code is enough: static int online_cpu; if (online_cpu == 0) online_cpu = sysctl return online_cpu; Thread did evolved somewhat while I was AFK. First, please note that the ABI which I designed there is fixable: if kernel does not export AT_NCPUS at all, then auxv correctly handles the situation returning an error, and libc falls back to sysctl(2). Do we really want to bypass sysctl and instead passing all info via auxv vector ? I found the sysconf() is a bunch of switch-case, which is already slow, before _SC_NPROCESSES_ONLN, it has already a quite number of case branches, and in your code, it calls _elf_aux_info() which also has some switch-cases branch, if you cache smp_cpus in libc, the call for _elf_aux_info is not needed, and you don't need code in kernel to passing it either, in any case, the code to call sysctl is still needed, so why don't we just use sysctl instead and cache the result in libc ? this at least can generate small code and a bit faster after first call to sysconf(_SC_NPROCESSES_ONLN). And as a side note, I think we should not put non-critical code into fork/exec path, these two functions are rather critical path for any UNIX like system, anything slowing down these two functions will affect overall performance, so we should not waste cpu cycle trying to push data to user mode via auxv or other ways and yet the data is not used by user code in most time, such as the number of online cpu. And in rtld-elf or libc, we should not waste too much time before executing main(). Regards, David Xu ___ 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: r237660 - head/lib/libc/gen
On 2012/06/28 21:52, Kostik Belousov wrote: On Thu, Jun 28, 2012 at 4:00 PM, David Xu listlog2...@gmail.com wrote: On 2012/6/28 16:49, David Xu wrote: On 2012/6/28 15:53, Konstantin Belousov wrote: On Thu, Jun 28, 2012 at 10:53:03AM +0800, David Xu wrote: On 2012/6/28 10:32, Attilio Rao wrote: 2012/6/28, David Xulistlog2...@gmail.com: On 2012/6/28 10:21, Attilio Rao wrote: 2012/6/28, David Xulistlog2...@gmail.com: On 2012/6/28 4:32, Konstantin Belousov wrote: Author: kib Date: Wed Jun 27 20:32:45 2012 New Revision: 237660 URL: http://svn.freebsd.org/changeset/base/237660 Log: Optimize the handling of SC_NPROCESSORS_CONF, by using auxv AT_NCPU value if present. MFC after:1 week Modified: head/lib/libc/gen/sysconf.c Modified: head/lib/libc/gen/sysconf.c == --- head/lib/libc/gen/sysconf.cWed Jun 27 20:24:25 2012 (r237659) +++ head/lib/libc/gen/sysconf.cWed Jun 27 20:32:45 2012 (r237660) @@ -42,6 +42,7 @@ __FBSDID($FreeBSD$); #includesys/resource.h #includesys/socket.h +#includeelf.h #includeerrno.h #includelimits.h #includepaths.h @@ -51,6 +52,7 @@ __FBSDID($FreeBSD$); #include ../stdlib/atexit.h #include tzfile.h/* from ../../../contrib/tzcode/stdtime */ +#include libc_private.h #define_PATH_ZONEINFOTZDIR/* from tzfile.h */ @@ -585,6 +587,8 @@ yesno: case _SC_NPROCESSORS_CONF: case _SC_NPROCESSORS_ONLN: +if (_elf_aux_info(AT_NCPUS,value, sizeof(value)) == 0) +return ((long)value); mib[0] = CTL_HW; mib[1] = HW_NCPU; break; Will this make controlling the number of CPU online or CPU hotplug be impossible on FreeBSD ? If I think about hotplug CPUs I can think of other 1000 problems/races/bad situations to be fixed before this one, really. These are problems only in kernel, but kib's change is about ABI between userland and kernel, I hope we don't introduce an ABI which is not extendable road stone. I'm not entirely sure I see the ABI breakage here. It is not breakage, it is the ABI thinks number of online cpu is fixed, obviously, it is not the case in future unless FreeBSD won't support dynamic number of online cpus. If the AT_NCPUS becames unconvenient and not correct at some point we can just fix sysconf() to not look into the aux vector anymoe. If you already know this will be a problem, why do you introduce it and later need to fix it ? Please note that AT_NCPUS is already exported nowadays. I think this is instead a clever optimization to avoid the sysctl() (usual way to retrieve the number of CPUs). But why don't you cache it in libc ? following code is enough: static int online_cpu; if (online_cpu == 0) online_cpu = sysctl return online_cpu; Thread did evolved somewhat while I was AFK. First, please note that the ABI which I designed there is fixable: if kernel does not export AT_NCPUS at all, then auxv correctly handles the situation returning an error, and libc falls back to sysctl(2). Do we really want to bypass sysctl and instead passing all info via auxv vector ? I found the sysconf() is a bunch of switch-case, which is already slow, before _SC_NPROCESSES_ONLN, it has already a quite number of case branches, and in your code, it calls _elf_aux_info() which also has some switch-cases branch, if you cache smp_cpus in libc, the call for _elf_aux_info is not needed, and you don't need code in kernel to passing it either, in any case, the code to call sysctl is still needed, so why don't we just use sysctl instead and cache the result in libc ? this at least can generate small code and a bit faster after first call to sysconf(_SC_NPROCESSES_ONLN). And as a side note, I think we should not put non-critical code into fork/exec path, these two functions are rather critical path for any UNIX like system, anything slowing down these two functions will affect overall performance, so we should not waste cpu cycle trying to push data to user mode via auxv or other ways and yet the data is not used by user code in most time, such as the number of online cpu. And in rtld-elf or libc, we should not waste too much time before executing main(). My motivation for extending auxv vector and to develop auxv.c was exactly to shave around dozen of syscalls from the application startup sequence. If you look at the ktrace output of the binary startup on RELENG_8 libc, you should note exactly the sysctls to request ncpus, pagesizes, canary and so on. In RELENG_9 and HEAD, there are no sysctls in the trace, because the data is already pre-filled by kernel for libc consumption. The sysconf(3) commit you are commenting on was caused by jemalloc(3) initialization starting using _sysconf(3) to query ncpu (for older version, which used sysctl directly, I direct auxv call). So HEAD has temporary +1 sysctl syscall done on app startup, now
Re: svn commit: r237660 - head/lib/libc/gen
On 2012/6/28 10:21, Attilio Rao wrote: 2012/6/28, David Xulistlog2...@gmail.com: On 2012/6/28 4:32, Konstantin Belousov wrote: Author: kib Date: Wed Jun 27 20:32:45 2012 New Revision: 237660 URL: http://svn.freebsd.org/changeset/base/237660 Log: Optimize the handling of SC_NPROCESSORS_CONF, by using auxv AT_NCPU value if present. MFC after: 1 week Modified: head/lib/libc/gen/sysconf.c Modified: head/lib/libc/gen/sysconf.c == --- head/lib/libc/gen/sysconf.c Wed Jun 27 20:24:25 2012(r237659) +++ head/lib/libc/gen/sysconf.c Wed Jun 27 20:32:45 2012(r237660) @@ -42,6 +42,7 @@ __FBSDID($FreeBSD$); #includesys/resource.h #includesys/socket.h +#includeelf.h #includeerrno.h #includelimits.h #includepaths.h @@ -51,6 +52,7 @@ __FBSDID($FreeBSD$); #include ../stdlib/atexit.h #include tzfile.h/* from ../../../contrib/tzcode/stdtime */ +#include libc_private.h #define _PATH_ZONEINFO TZDIR /* from tzfile.h */ @@ -585,6 +587,8 @@ yesno: case _SC_NPROCESSORS_CONF: case _SC_NPROCESSORS_ONLN: + if (_elf_aux_info(AT_NCPUS,value, sizeof(value)) == 0) + return ((long)value); mib[0] = CTL_HW; mib[1] = HW_NCPU; break; Will this make controlling the number of CPU online or CPU hotplug be impossible on FreeBSD ? If I think about hotplug CPUs I can think of other 1000 problems/races/bad situations to be fixed before this one, really. These are problems only in kernel, but kib's change is about ABI between userland and kernel, I hope we don't introduce an ABI which is not extendable road stone. Attilio ___ 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: r237660 - head/lib/libc/gen
On 2012/6/28 10:32, Attilio Rao wrote: 2012/6/28, David Xulistlog2...@gmail.com: On 2012/6/28 10:21, Attilio Rao wrote: 2012/6/28, David Xulistlog2...@gmail.com: On 2012/6/28 4:32, Konstantin Belousov wrote: Author: kib Date: Wed Jun 27 20:32:45 2012 New Revision: 237660 URL: http://svn.freebsd.org/changeset/base/237660 Log: Optimize the handling of SC_NPROCESSORS_CONF, by using auxv AT_NCPU value if present. MFC after: 1 week Modified: head/lib/libc/gen/sysconf.c Modified: head/lib/libc/gen/sysconf.c == --- head/lib/libc/gen/sysconf.c Wed Jun 27 20:24:25 2012(r237659) +++ head/lib/libc/gen/sysconf.c Wed Jun 27 20:32:45 2012(r237660) @@ -42,6 +42,7 @@ __FBSDID($FreeBSD$); #includesys/resource.h #includesys/socket.h +#includeelf.h #includeerrno.h #includelimits.h #includepaths.h @@ -51,6 +52,7 @@ __FBSDID($FreeBSD$); #include ../stdlib/atexit.h #include tzfile.h /* from ../../../contrib/tzcode/stdtime */ +#include libc_private.h #define _PATH_ZONEINFO TZDIR /* from tzfile.h */ @@ -585,6 +587,8 @@ yesno: case _SC_NPROCESSORS_CONF: case _SC_NPROCESSORS_ONLN: + if (_elf_aux_info(AT_NCPUS,value, sizeof(value)) == 0) + return ((long)value); mib[0] = CTL_HW; mib[1] = HW_NCPU; break; Will this make controlling the number of CPU online or CPU hotplug be impossible on FreeBSD ? If I think about hotplug CPUs I can think of other 1000 problems/races/bad situations to be fixed before this one, really. These are problems only in kernel, but kib's change is about ABI between userland and kernel, I hope we don't introduce an ABI which is not extendable road stone. I'm not entirely sure I see the ABI breakage here. It is not breakage, it is the ABI thinks number of online cpu is fixed, obviously, it is not the case in future unless FreeBSD won't support dynamic number of online cpus. If the AT_NCPUS becames unconvenient and not correct at some point we can just fix sysconf() to not look into the aux vector anymoe. If you already know this will be a problem, why do you introduce it and later need to fix it ? Please note that AT_NCPUS is already exported nowadays. I think this is instead a clever optimization to avoid the sysctl() (usual way to retrieve the number of CPUs). But why don't you cache it in libc ? following code is enough: static int online_cpu; if (online_cpu == 0) online_cpu = sysctl return online_cpu; Attilio ___ 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: r236935 - head/sys/kern
On 2012/6/12 6:05, Pawel Jakub Dawidek wrote: Author: pjd Date: Mon Jun 11 22:05:26 2012 New Revision: 236935 URL: http://svn.freebsd.org/changeset/base/236935 Log: fdgrowtable() no longer drops the filedesc lock so it is enough to retry finding free file descriptor only once after fdgrowtable(). Spotted by: pluknet MFC after: 1 month Modified: head/sys/kern/kern_descrip.c Modified: head/sys/kern/kern_descrip.c == --- head/sys/kern/kern_descrip.cMon Jun 11 21:56:37 2012 (r236934) +++ head/sys/kern/kern_descrip.cMon Jun 11 22:05:26 2012 (r236935) @@ -1478,29 +1478,33 @@ fdalloc(struct thread *td, int minfd, in /* * Search the bitmap for a free descriptor. If none is found, try * to grow the file table. Keep at it until we either get a file -* descriptor or run into process or system limits; fdgrowtable() -* may drop the filedesc lock, so we're in a race. +* descriptor or run into process or system limits. */ - for (;;) { - fd = fd_first_free(fdp, minfd, fdp-fd_nfiles); - if (fd= maxfd) - return (EMFILE); - if (fd fdp-fd_nfiles) - break; + fd = fd_first_free(fdp, minfd, fdp-fd_nfiles); + if (fd= maxfd) + return (EMFILE); + if (fd= fdp-fd_nfiles) { #ifdef RACCT PROC_LOCK(p); - error = racct_set(p, RACCT_NOFILE, min(fdp-fd_nfiles * 2, maxfd)); + error = racct_set(p, RACCT_NOFILE, + min(fdp-fd_nfiles * 2, maxfd)); PROC_UNLOCK(p); if (error != 0) return (EMFILE); #endif fdgrowtable(fdp, min(fdp-fd_nfiles * 2, maxfd)); + /* Retry... */ + fd = fd_first_free(fdp, minfd, fdp-fd_nfiles); + if (fd= maxfd) + return (EMFILE); } /* * Perform some sanity checks, then mark the file descriptor as * used and return it to the caller. */ + KASSERT((unsigned int)fd min(maxfd, fdp-fd_nfiles), + (invalid descriptor %d, fd)); KASSERT(!fdisused(fdp, fd), (fd_first_free() returned non-free descriptor)); KASSERT(fdp-fd_ofiles[fd] == NULL, (file descriptor isn't free)); My machine crashed with this change. http://people.freebsd.org/~davidxu/fdcrash.jpg Regards, ___ 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: r236935 - head/sys/kern
On 2012/6/13 3:18, Mateusz Guzik wrote: On Tue, Jun 12, 2012 at 06:01:29PM +0200, Pawel Jakub Dawidek wrote: On Tue, Jun 12, 2012 at 03:49:50PM +0200, Mateusz Guzik wrote: On Tue, Jun 12, 2012 at 01:43:35PM +0200, Pawel Jakub Dawidek wrote: On Tue, Jun 12, 2012 at 12:47:49PM +0200, Mateusz Guzik wrote: The problem is that fdalloc grows to at most fdp-fd_nfiles * 2, which still may not be enough to have place for new fd with high number. I was under impression that fd_first_free() can return at most fdp-fd_nfiles, but indeed I missed this: if (low= size) return (low); So fd_first_free() can return number biffer than size... This fixed the problem for me, although I'm not sure whether it's ok to grow the table like this: http://people.freebsd.org/~mjg/patches/fdalloc.patch The patch looks good to me, could you please commit it, preferably after David's trying it and also update fd_first_free() comment, so it is clear that returned value can exceed 'size -1'? Given that you partially reverted r236935 I created a combined patch: http://people.freebsd.org/~mjg/patches/fdalloc%2bfd_first_free.patch Is this ok? Most changes are obiously yours, so I see no problem if you prefer to commit this yourself. Otherwise I plan to commit it with the following: Re-apply reverted parts of r236935 by pjd with some fixes. If fdalloc decides to grow fdtable it does it once and at most doubles the size. This still may be not enough for sufficiently large fd. Use fd in calculations of new size in order to fix this. Fix description of fd_first_free to note that it returns passed fd if it exceeds fdtable's size. == Look good and you can just add 'In co-operation with: pjd'. One minor thing is that fd_first_free() can return 'size' if there are no free slots available. Could you include that in the comment as well? http://people.freebsd.org/~mjg/patches/fdalloc%2bfd_first_free2.patch fd_last_used has very same problem with comment as fd_first_free. This function is static and the only caller always passes 0 as low. Given that, how about the following: http://people.freebsd.org/~mjg/patches/fd_last_used-cleanup.patch Looks good too. Updated in similar manner: http://people.freebsd.org/~mjg/patches/fd_last_used-cleanup2.patch I have tested it, the machine does not panic. Thanks, David Xu ___ 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
svn commit: r236275 - stable/9/lib/libthr/thread
Author: davidxu Date: Wed May 30 01:52:53 2012 New Revision: 236275 URL: http://svn.freebsd.org/changeset/base/236275 Log: MFC r236135: Return EBUSY for PTHREAD_MUTEX_ADAPTIVE_NP too when the mutex could not be acquired. PR: 168317 Modified: stable/9/lib/libthr/thread/thr_mutex.c Directory Properties: stable/9/lib/libthr/ (props changed) Modified: stable/9/lib/libthr/thread/thr_mutex.c == --- stable/9/lib/libthr/thread/thr_mutex.c Wed May 30 01:52:01 2012 (r236274) +++ stable/9/lib/libthr/thread/thr_mutex.c Wed May 30 01:52:53 2012 (r236275) @@ -538,6 +538,7 @@ mutex_self_trylock(struct pthread_mutex switch (PMUTEX_TYPE(m-m_flags)) { case PTHREAD_MUTEX_ERRORCHECK: case PTHREAD_MUTEX_NORMAL: + case PTHREAD_MUTEX_ADAPTIVE_NP: ret = EBUSY; break; ___ 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
svn commit: r236276 - stable/8/lib/libthr/thread
Author: davidxu Date: Wed May 30 01:54:14 2012 New Revision: 236276 URL: http://svn.freebsd.org/changeset/base/236276 Log: MFC r236135: Return EBUSY for PTHREAD_MUTEX_ADAPTIVE_NP too when the mutex could not be acquired. PR: 168317 Modified: stable/8/lib/libthr/thread/thr_mutex.c Directory Properties: stable/8/lib/libthr/ (props changed) Modified: stable/8/lib/libthr/thread/thr_mutex.c == --- stable/8/lib/libthr/thread/thr_mutex.c Wed May 30 01:52:53 2012 (r236275) +++ stable/8/lib/libthr/thread/thr_mutex.c Wed May 30 01:54:14 2012 (r236276) @@ -484,6 +484,7 @@ mutex_self_trylock(struct pthread_mutex switch (m-m_type) { case PTHREAD_MUTEX_ERRORCHECK: case PTHREAD_MUTEX_NORMAL: + case PTHREAD_MUTEX_ADAPTIVE_NP: ret = EBUSY; break; ___ 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
svn commit: r236135 - head/lib/libthr/thread
Author: davidxu Date: Sun May 27 01:24:51 2012 New Revision: 236135 URL: http://svn.freebsd.org/changeset/base/236135 Log: Return EBUSY for PTHREAD_MUTEX_ADAPTIVE_NP too when the mutex could not be acquired. PR: 168317 MFC after:3 days Modified: head/lib/libthr/thread/thr_mutex.c Modified: head/lib/libthr/thread/thr_mutex.c == --- head/lib/libthr/thread/thr_mutex.c Sun May 27 01:24:08 2012 (r236134) +++ head/lib/libthr/thread/thr_mutex.c Sun May 27 01:24:51 2012 (r236135) @@ -538,6 +538,7 @@ mutex_self_trylock(struct pthread_mutex switch (PMUTEX_TYPE(m-m_flags)) { case PTHREAD_MUTEX_ERRORCHECK: case PTHREAD_MUTEX_NORMAL: + case PTHREAD_MUTEX_ADAPTIVE_NP: ret = EBUSY; break; ___ 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
svn commit: r235714 - head/gnu/usr.bin/gdb/libgdb
Author: davidxu Date: Mon May 21 03:06:31 2012 New Revision: 235714 URL: http://svn.freebsd.org/changeset/base/235714 Log: Print key value, an index, otherwise we don't know which key is allocated. Modified: head/gnu/usr.bin/gdb/libgdb/fbsd-threads.c Modified: head/gnu/usr.bin/gdb/libgdb/fbsd-threads.c == --- head/gnu/usr.bin/gdb/libgdb/fbsd-threads.c Mon May 21 02:45:47 2012 (r235713) +++ head/gnu/usr.bin/gdb/libgdb/fbsd-threads.c Mon May 21 03:06:31 2012 (r235714) @@ -1311,7 +1311,7 @@ tsd_cb (thread_key_t key, void (*destruc else name = DEPRECATED_SYMBOL_NAME (ms); - printf_filtered (Destructor %p %s\n, destructor, name); + printf_filtered (Key %d, destructor %p %s\n, key, destructor, name); return 0; } ___ 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
svn commit: r235218 - head/lib/libthr/thread
Author: davidxu Date: Thu May 10 09:30:37 2012 New Revision: 235218 URL: http://svn.freebsd.org/changeset/base/235218 Log: Create a common function lookup() to search a chan, this eliminates redundant SC_LOOKUP() calling. Modified: head/lib/libthr/thread/thr_sleepq.c Modified: head/lib/libthr/thread/thr_sleepq.c == --- head/lib/libthr/thread/thr_sleepq.c Thu May 10 09:10:31 2012 (r235217) +++ head/lib/libthr/thread/thr_sleepq.c Thu May 10 09:30:37 2012 (r235218) @@ -94,19 +94,23 @@ _sleepq_unlock(void *wchan) THR_LOCK_RELEASE(curthread, sc-sc_lock); } -struct sleepqueue * -_sleepq_lookup(void *wchan) +static inline struct sleepqueue * +lookup(struct sleepqueue_chain *sc, void *wchan) { - struct sleepqueue_chain *sc; struct sleepqueue *sq; - sc = SC_LOOKUP(wchan); LIST_FOREACH(sq, sc-sc_queues, sq_hash) if (sq-sq_wchan == wchan) return (sq); return (NULL); } +struct sleepqueue * +_sleepq_lookup(void *wchan) +{ + return (lookup(SC_LOOKUP(wchan), wchan)); +} + void _sleepq_add(void *wchan, struct pthread *td) { @@ -114,7 +118,7 @@ _sleepq_add(void *wchan, struct pthread struct sleepqueue *sq; sc = SC_LOOKUP(wchan); - sq = _sleepq_lookup(wchan); + sq = lookup(sc, wchan); if (sq != NULL) { SLIST_INSERT_HEAD(sq-sq_freeq, td-sleepqueue, sq_flink); } else { ___ 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
svn commit: r235068 - head/lib/libthr/thread
Author: davidxu Date: Sat May 5 23:51:24 2012 New Revision: 235068 URL: http://svn.freebsd.org/changeset/base/235068 Log: Fix mis-merged line, move SC_LOOKUP() call to upper level. Modified: head/lib/libthr/thread/thr_sleepq.c Modified: head/lib/libthr/thread/thr_sleepq.c == --- head/lib/libthr/thread/thr_sleepq.c Sat May 5 22:44:08 2012 (r235067) +++ head/lib/libthr/thread/thr_sleepq.c Sat May 5 23:51:24 2012 (r235068) @@ -113,11 +113,11 @@ _sleepq_add(void *wchan, struct pthread struct sleepqueue_chain *sc; struct sleepqueue *sq; + sc = SC_LOOKUP(wchan); sq = _sleepq_lookup(wchan); if (sq != NULL) { SLIST_INSERT_HEAD(sq-sq_freeq, td-sleepqueue, sq_flink); } else { - sc = SC_LOOKUP(wchan); sq = td-sleepqueue; LIST_INSERT_HEAD(sc-sc_queues, sq, sq_hash); sq-sq_wchan = wchan; ___ 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
svn commit: r234947 - head/lib/libthr/thread
Author: davidxu Date: Thu May 3 09:17:31 2012 New Revision: 234947 URL: http://svn.freebsd.org/changeset/base/234947 Log: MFp4: Enqueue thread in LIFO, this can cause starvation, but it gives better performance. Use _thr_queuefifo to control the frequency of FIFO vs LIFO, you can use environment string LIBPTHREAD_QUEUE_FIFO to configure the variable. Modified: head/lib/libthr/thread/thr_init.c head/lib/libthr/thread/thr_private.h head/lib/libthr/thread/thr_sleepq.c Modified: head/lib/libthr/thread/thr_init.c == --- head/lib/libthr/thread/thr_init.c Thu May 3 08:56:43 2012 (r234946) +++ head/lib/libthr/thread/thr_init.c Thu May 3 09:17:31 2012 (r234947) @@ -112,6 +112,7 @@ size_t _thr_stack_initial = THR_STACK_I int_thr_page_size; int_thr_spinloops; int_thr_yieldloops; +int_thr_queuefifo = 4; int_gc_count; struct umutex _mutex_static_lock = DEFAULT_UMUTEX; struct umutex _cond_static_lock = DEFAULT_UMUTEX; @@ -470,6 +471,9 @@ init_private(void) env = getenv(LIBPTHREAD_YIELDLOOPS); if (env) _thr_yieldloops = atoi(env); + env = getenv(LIBPTHREAD_QUEUE_FIFO); + if (env) + _thr_queuefifo = atoi(env); TAILQ_INIT(_thr_atfork_list); } init_once = 1; Modified: head/lib/libthr/thread/thr_private.h == --- head/lib/libthr/thread/thr_private.hThu May 3 08:56:43 2012 (r234946) +++ head/lib/libthr/thread/thr_private.hThu May 3 09:17:31 2012 (r234947) @@ -710,6 +710,7 @@ extern size_t _thr_stack_initial __hidde extern int _thr_page_size __hidden; extern int _thr_spinloops __hidden; extern int _thr_yieldloops __hidden; +extern int _thr_queuefifo __hidden; /* Garbage thread count. */ extern int _gc_count __hidden; Modified: head/lib/libthr/thread/thr_sleepq.c == --- head/lib/libthr/thread/thr_sleepq.c Thu May 3 08:56:43 2012 (r234946) +++ head/lib/libthr/thread/thr_sleepq.c Thu May 3 09:17:31 2012 (r234947) @@ -39,6 +39,7 @@ struct sleepqueue_chain { struct umutex sc_lock; + int sc_enqcnt; LIST_HEAD(, sleepqueue) sc_queues; int sc_type; }; @@ -124,7 +125,10 @@ _sleepq_add(void *wchan, struct pthread } td-sleepqueue = NULL; td-wchan = wchan; - TAILQ_INSERT_TAIL(sq-sq_blocked, td, wle); + if (((++sc-sc_enqcnt _thr_queuefifo) 0xff) != 0) + TAILQ_INSERT_HEAD(sq-sq_blocked, td, wle); + else + TAILQ_INSERT_TAIL(sq-sq_blocked, td, wle); } int ___ 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
svn commit: r234937 - in stable/8: lib/libthr/thread sys/kern sys/sys
Author: davidxu Date: Thu May 3 03:05:18 2012 New Revision: 234937 URL: http://svn.freebsd.org/changeset/base/234937 Log: Merge 233103, 233912 from head: 233103: Some software think a mutex can be destroyed after it owned it, for example, it uses a serialization point like following: pthread_mutex_lock(mutex); pthread_mutex_unlock(mutex); pthread_mutex_destroy(muetx); They think a previous lock holder should have already left the mutex and is no longer referencing it, so they destroy it. To be maximum compatible with such code, we use IA64 version to unlock the mutex in kernel, remove the two steps unlocking code. 233912: umtx operation UMTX_OP_MUTEX_WAKE has a side-effect that it accesses a mutex after a thread has unlocked it, it event writes data to the mutex memory to clear contention bit, there is a race that other threads can lock it and unlock it, then destroy it, so it should not write data to the mutex memory if there isn't any waiter. The new operation UMTX_OP_MUTEX_WAKE2 try to fix the problem. It requires thread library to clear the lock word entirely, then call the WAKE2 operation to check if there is any waiter in kernel, and try to wake up a thread, if necessary, the contention bit is set again by the operation. This also mitgates the chance that other threads find the contention bit and try to enter kernel to compete with each other to wake up sleeping thread, this is unnecessary. With this change, the mutex owner is no longer holding the mutex until it reaches a point where kernel umtx queue is locked, it releases the mutex as soon as possible. Performance is improved when the mutex is contensted heavily. On Intel i3-2310M, the runtime of a benchmark program is reduced from 26.87 seconds to 2.39 seconds, it even is better than UMTX_OP_MUTEX_WAKE which is deprecated now. http://people.freebsd.org/~davidxu/bench/mutex_perf.c Special code for stable/8: And add code to detect if the UMTX_OP_MUTEX_WAKE2 is available. PR: threads/167308 Modified: stable/8/lib/libthr/thread/thr_private.h stable/8/lib/libthr/thread/thr_umtx.c stable/8/lib/libthr/thread/thr_umtx.h stable/8/sys/kern/kern_umtx.c stable/8/sys/sys/umtx.h Directory Properties: stable/8/lib/libthr/ (props changed) Modified: stable/8/lib/libthr/thread/thr_private.h == --- stable/8/lib/libthr/thread/thr_private.hThu May 3 01:41:12 2012 (r234936) +++ stable/8/lib/libthr/thread/thr_private.hThu May 3 03:05:18 2012 (r234937) @@ -706,8 +706,6 @@ ssize_t __sys_write(int, const void *, s void __sys_exit(int); #endif -int_umtx_op_err(void *, int op, u_long, void *, void *) __hidden; - static inline int _thr_isthreaded(void) { Modified: stable/8/lib/libthr/thread/thr_umtx.c == --- stable/8/lib/libthr/thread/thr_umtx.c Thu May 3 01:41:12 2012 (r234936) +++ stable/8/lib/libthr/thread/thr_umtx.c Thu May 3 03:05:18 2012 (r234937) @@ -112,6 +112,35 @@ __thr_umutex_timedlock(struct umutex *mt int __thr_umutex_unlock(struct umutex *mtx, uint32_t id) { + static int wake2_avail = 0; + + if (__predict_false(wake2_avail == 0)) { + struct umutex test = DEFAULT_UMUTEX; + + if (_umtx_op(test, UMTX_OP_MUTEX_WAKE2, test.m_flags, 0, 0) == -1) + wake2_avail = -1; + else + wake2_avail = 1; + } + + if (wake2_avail != 1) + goto unlock; + + uint32_t flags = mtx-m_flags; + + if ((flags (UMUTEX_PRIO_PROTECT | UMUTEX_PRIO_INHERIT)) == 0) { + uint32_t owner; + do { + owner = mtx-m_owner; + if (__predict_false((owner ~UMUTEX_CONTESTED) != id)) + return (EPERM); + } while (__predict_false(!atomic_cmpset_rel_32(mtx-m_owner, +owner, UMUTEX_UNOWNED))); + if ((owner UMUTEX_CONTESTED)) + (void)_umtx_op_err(mtx, UMTX_OP_MUTEX_WAKE2, flags, 0, 0); + return (0); + } +unlock: return _umtx_op_err(mtx, UMTX_OP_MUTEX_UNLOCK, 0, 0, 0); } Modified: stable/8/lib/libthr/thread/thr_umtx.h == --- stable/8/lib/libthr/thread/thr_umtx.h Thu May 3 01:41:12 2012 (r234936) +++ stable/8/lib/libthr/thread/thr_umtx.h Thu May 3 03:05:18 2012 (r234937) @@ -34,6 +34,7 @@ #define DEFAULT_UMUTEX {0,0, {0,0},{0,0,0,0}} +int _umtx_op_err(void *, int op, u_long, void *, void *) __hidden; int __thr_umutex_lock(struct umutex *mtx, uint32_t id) __hidden; int __thr_umutex_timedlock(struct umutex *mtx,
svn commit: r234371 - in stable/9/sys: kern sys
Author: davidxu Date: Tue Apr 17 09:02:55 2012 New Revision: 234371 URL: http://svn.freebsd.org/changeset/base/234371 Log: MFC r233912: mtx operation UMTX_OP_MUTEX_WAKE has a side-effect that it accesses a mutex after a thread has unlocked it, it event writes data to the mutex memory to clear contention bit, there is a race that other threads can lock it and unlock it, then destroy it, so it should not write data to the mutex memory if there isn't any waiter. The new operation UMTX_OP_MUTEX_WAKE2 try to fix the problem. It requires thread library to clear the lock word entirely, then call the WAKE2 operation to check if there is any waiter in kernel, and try to wake up a thread, if necessary, the contention bit is set again by the operation. This also mitgates the chance that other threads find the contention bit and try to enter kernel to compete with each other to wake up sleeping thread, this is unnecessary. With this change, the mutex owner is no longer holding the mutex until it reaches a point where kernel umtx queue is locked, it releases the mutex as soon as possible. Performance is improved when the mutex is contensted heavily. On Intel i3-2310M, the runtime of a benchmark program is reduced from 26.87 seconds to 2.39 seconds, it even is better than UMTX_OP_MUTEX_WAKE which is deprecated now. http://people.freebsd.org/~davidxu/bench/mutex_perf.c Modified: stable/9/sys/kern/kern_umtx.c stable/9/sys/sys/umtx.h Directory Properties: stable/9/sys/ (props changed) Modified: stable/9/sys/kern/kern_umtx.c == --- stable/9/sys/kern/kern_umtx.c Tue Apr 17 07:22:14 2012 (r234370) +++ stable/9/sys/kern/kern_umtx.c Tue Apr 17 09:02:55 2012 (r234371) @@ -1276,6 +1276,78 @@ do_wake_umutex(struct thread *td, struct return (0); } +/* + * Check if the mutex has waiters and tries to fix contention bit. + */ +static int +do_wake2_umutex(struct thread *td, struct umutex *m, uint32_t flags) +{ + struct umtx_key key; + uint32_t owner, old; + int type; + int error; + int count; + + switch(flags (UMUTEX_PRIO_INHERIT | UMUTEX_PRIO_PROTECT)) { + case 0: + type = TYPE_NORMAL_UMUTEX; + break; + case UMUTEX_PRIO_INHERIT: + type = TYPE_PI_UMUTEX; + break; + case UMUTEX_PRIO_PROTECT: + type = TYPE_PP_UMUTEX; + break; + default: + return (EINVAL); + } + if ((error = umtx_key_get(m, type, GET_SHARE(flags), + key)) != 0) + return (error); + + owner = 0; + umtxq_lock(key); + umtxq_busy(key); + count = umtxq_count(key); + umtxq_unlock(key); + /* +* Only repair contention bit if there is a waiter, this means the mutex +* is still being referenced by userland code, otherwise don't update +* any memory. +*/ + if (count 1) { + owner = fuword32(__DEVOLATILE(uint32_t *, m-m_owner)); + while ((owner UMUTEX_CONTESTED) ==0) { + old = casuword32(m-m_owner, owner, + owner|UMUTEX_CONTESTED); + if (old == owner) + break; + owner = old; + } + } else if (count == 1) { + owner = fuword32(__DEVOLATILE(uint32_t *, m-m_owner)); + while ((owner ~UMUTEX_CONTESTED) != 0 + (owner UMUTEX_CONTESTED) == 0) { + old = casuword32(m-m_owner, owner, + owner|UMUTEX_CONTESTED); + if (old == owner) + break; + owner = old; + } + } + umtxq_lock(key); + if (owner == -1) { + error = EFAULT; + umtxq_signal(key, INT_MAX); + } + else if (count != 0 (owner ~UMUTEX_CONTESTED) == 0) + umtxq_signal(key, 1); + umtxq_unbusy(key); + umtxq_unlock(key); + umtx_key_release(key); + return (error); +} + static inline struct umtx_pi * umtx_pi_alloc(int flags) { @@ -3183,6 +3255,12 @@ __umtx_op_sem_wake(struct thread *td, st return do_sem_wake(td, uap-obj); } +static int +__umtx_op_wake2_umutex(struct thread *td, struct _umtx_op_args *uap) +{ + return do_wake2_umutex(td, uap-obj, uap-val); +} + typedef int (*_umtx_op_func)(struct thread *td, struct _umtx_op_args *uap); static _umtx_op_func op_table[] = { @@ -3207,7 +3285,8 @@ static _umtx_op_func op_table[] = { __umtx_op_wake_umutex, /* UMTX_OP_UMUTEX_WAKE */ __umtx_op_sem_wait, /* UMTX_OP_SEM_WAIT */ __umtx_op_sem_wake, /* UMTX_OP_SEM_WAKE */ - __umtx_op_nwake_private
svn commit: r234372 - stable/9/lib/libthr/thread
Author: davidxu Date: Tue Apr 17 09:09:14 2012 New Revision: 234372 URL: http://svn.freebsd.org/changeset/base/234372 Log: Merge 233103, 233912 from head: 233103: Some software think a mutex can be destroyed after it owned it, for example, it uses a serialization point like following: pthread_mutex_lock(mutex); pthread_mutex_unlock(mutex); pthread_mutex_destroy(muetx); They think a previous lock holder should have already left the mutex and is no longer referencing it, so they destroy it. To be maximum compatible with such code, we use IA64 version to unlock the mutex in kernel, remove the two steps unlocking code. 233912: umtx operation UMTX_OP_MUTEX_WAKE has a side-effect that it accesses a mutex after a thread has unlocked it, it event writes data to the mutex memory to clear contention bit, there is a race that other threads can lock it and unlock it, then destroy it, so it should not write data to the mutex memory if there isn't any waiter. The new operation UMTX_OP_MUTEX_WAKE2 try to fix the problem. It requires thread library to clear the lock word entirely, then call the WAKE2 operation to check if there is any waiter in kernel, and try to wake up a thread, if necessary, the contention bit is set again by the operation. This also mitgates the chance that other threads find the contention bit and try to enter kernel to compete with each other to wake up sleeping thread, this is unnecessary. With this change, the mutex owner is no longer holding the mutex until it reaches a point where kernel umtx queue is locked, it releases the mutex as soon as possible. Performance is improved when the mutex is contensted heavily. On Intel i3-2310M, the runtime of a benchmark program is reduced from 26.87 seconds to 2.39 seconds, it even is better than UMTX_OP_MUTEX_WAKE which is deprecated now. http://people.freebsd.org/~davidxu/bench/mutex_perf.c Special code for stable/9: And add code to detect if the UMTX_OP_MUTEX_WAKE2 is available. Modified: stable/9/lib/libthr/thread/thr_private.h stable/9/lib/libthr/thread/thr_umtx.c stable/9/lib/libthr/thread/thr_umtx.h Directory Properties: stable/9/lib/libthr/ (props changed) Modified: stable/9/lib/libthr/thread/thr_private.h == --- stable/9/lib/libthr/thread/thr_private.hTue Apr 17 09:02:55 2012 (r234371) +++ stable/9/lib/libthr/thread/thr_private.hTue Apr 17 09:09:14 2012 (r234372) @@ -832,8 +832,6 @@ ssize_t __sys_write(int, const void *, s void __sys_exit(int); #endif -int_umtx_op_err(void *, int op, u_long, void *, void *) __hidden; - static inline int _thr_isthreaded(void) { Modified: stable/9/lib/libthr/thread/thr_umtx.c == --- stable/9/lib/libthr/thread/thr_umtx.c Tue Apr 17 09:02:55 2012 (r234371) +++ stable/9/lib/libthr/thread/thr_umtx.c Tue Apr 17 09:09:14 2012 (r234372) @@ -152,13 +152,35 @@ __thr_umutex_timedlock(struct umutex *mt int __thr_umutex_unlock(struct umutex *mtx, uint32_t id) { -#ifndef __ia64__ - /* XXX this logic has a race-condition on ia64. */ - if ((mtx-m_flags (UMUTEX_PRIO_PROTECT | UMUTEX_PRIO_INHERIT)) == 0) { - atomic_cmpset_rel_32(mtx-m_owner, id | UMUTEX_CONTESTED, UMUTEX_CONTESTED); - return _umtx_op_err(mtx, UMTX_OP_MUTEX_WAKE, 0, 0, 0); + static int wake2_avail = 0; + + if (__predict_false(wake2_avail == 0)) { + struct umutex test = DEFAULT_UMUTEX; + + if (_umtx_op(test, UMTX_OP_MUTEX_WAKE2, test.m_flags, 0, 0) == -1) + wake2_avail = -1; + else + wake2_avail = 1; + } + + if (wake2_avail != 1) + goto unlock; + + uint32_t flags = mtx-m_flags; + + if ((flags (UMUTEX_PRIO_PROTECT | UMUTEX_PRIO_INHERIT)) == 0) { + uint32_t owner; + do { + owner = mtx-m_owner; + if (__predict_false((owner ~UMUTEX_CONTESTED) != id)) + return (EPERM); + } while (__predict_false(!atomic_cmpset_rel_32(mtx-m_owner, +owner, UMUTEX_UNOWNED))); + if ((owner UMUTEX_CONTESTED)) + (void)_umtx_op_err(mtx, UMTX_OP_MUTEX_WAKE2, flags, 0, 0); + return (0); } -#endif /* __ia64__ */ +unlock: return _umtx_op_err(mtx, UMTX_OP_MUTEX_UNLOCK, 0, 0, 0); } Modified: stable/9/lib/libthr/thread/thr_umtx.h == --- stable/9/lib/libthr/thread/thr_umtx.h Tue Apr 17 09:02:55 2012 (r234371) +++ stable/9/lib/libthr/thread/thr_umtx.h Tue Apr 17 09:09:14 2012
svn commit: r234373 - stable/8/lib/libthr/thread
Author: davidxu Date: Tue Apr 17 09:18:06 2012 New Revision: 234373 URL: http://svn.freebsd.org/changeset/base/234373 Log: Merge 233103 from head: Some software think a mutex can be destroyed after it owned it, for example, it uses a serialization point like following: pthread_mutex_lock(mutex); pthread_mutex_unlock(mutex); pthread_mutex_destroy(muetx); They think a previous lock holder should have already left the mutex and is no longer referencing it, so they destroy it. To be maximum compatible with such code, we use IA64 version to unlock the mutex in kernel, remove the two steps unlocking code. Modified: stable/8/lib/libthr/thread/thr_umtx.c Directory Properties: stable/8/lib/libthr/ (props changed) Modified: stable/8/lib/libthr/thread/thr_umtx.c == --- stable/8/lib/libthr/thread/thr_umtx.c Tue Apr 17 09:09:14 2012 (r234372) +++ stable/8/lib/libthr/thread/thr_umtx.c Tue Apr 17 09:18:06 2012 (r234373) @@ -112,13 +112,6 @@ __thr_umutex_timedlock(struct umutex *mt int __thr_umutex_unlock(struct umutex *mtx, uint32_t id) { -#ifndef __ia64__ - /* XXX this logic has a race-condition on ia64. */ - if ((mtx-m_flags (UMUTEX_PRIO_PROTECT | UMUTEX_PRIO_INHERIT)) == 0) { - atomic_cmpset_rel_32(mtx-m_owner, id | UMUTEX_CONTESTED, UMUTEX_CONTESTED); - return _umtx_op_err(mtx, UMTX_OP_MUTEX_WAKE, 0, 0, 0); - } -#endif /* __ia64__ */ return _umtx_op_err(mtx, UMTX_OP_MUTEX_UNLOCK, 0, 0, 0); } ___ 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
svn commit: r233912 - in head: lib/libthr/thread sys/kern sys/sys
Author: davidxu Date: Thu Apr 5 02:24:08 2012 New Revision: 233912 URL: http://svn.freebsd.org/changeset/base/233912 Log: umtx operation UMTX_OP_MUTEX_WAKE has a side-effect that it accesses a mutex after a thread has unlocked it, it event writes data to the mutex memory to clear contention bit, there is a race that other threads can lock it and unlock it, then destroy it, so it should not write data to the mutex memory if there isn't any waiter. The new operation UMTX_OP_MUTEX_WAKE2 try to fix the problem. It requires thread library to clear the lock word entirely, then call the WAKE2 operation to check if there is any waiter in kernel, and try to wake up a thread, if necessary, the contention bit is set again by the operation. This also mitgates the chance that other threads find the contention bit and try to enter kernel to compete with each other to wake up sleeping thread, this is unnecessary. With this change, the mutex owner is no longer holding the mutex until it reaches a point where kernel umtx queue is locked, it releases the mutex as soon as possible. Performance is improved when the mutex is contensted heavily. On Intel i3-2310M, the runtime of a benchmark program is reduced from 26.87 seconds to 2.39 seconds, it even is better than UMTX_OP_MUTEX_WAKE which is deprecated now. http://people.freebsd.org/~davidxu/bench/mutex_perf.c Modified: head/lib/libthr/thread/thr_private.h head/lib/libthr/thread/thr_umtx.h head/sys/kern/kern_umtx.c head/sys/sys/umtx.h Modified: head/lib/libthr/thread/thr_private.h == --- head/lib/libthr/thread/thr_private.hThu Apr 5 00:53:21 2012 (r233911) +++ head/lib/libthr/thread/thr_private.hThu Apr 5 02:24:08 2012 (r233912) @@ -834,8 +834,6 @@ ssize_t __sys_write(int, const void *, s void __sys_exit(int); #endif -int_umtx_op_err(void *, int op, u_long, void *, void *) __hidden; - static inline int _thr_isthreaded(void) { Modified: head/lib/libthr/thread/thr_umtx.h == --- head/lib/libthr/thread/thr_umtx.h Thu Apr 5 00:53:21 2012 (r233911) +++ head/lib/libthr/thread/thr_umtx.h Thu Apr 5 02:24:08 2012 (r233912) @@ -35,6 +35,7 @@ #define DEFAULT_UMUTEX {0,0,{0,0},{0,0,0,0}} #define DEFAULT_URWLOCK {0,0,0,0,{0,0,0,0}} +int _umtx_op_err(void *, int op, u_long, void *, void *) __hidden; int __thr_umutex_lock(struct umutex *mtx, uint32_t id) __hidden; int __thr_umutex_lock_spin(struct umutex *mtx, uint32_t id) __hidden; int __thr_umutex_timedlock(struct umutex *mtx, uint32_t id, @@ -121,9 +122,23 @@ _thr_umutex_timedlock(struct umutex *mtx static inline int _thr_umutex_unlock(struct umutex *mtx, uint32_t id) { -if (atomic_cmpset_rel_32(mtx-m_owner, id, UMUTEX_UNOWNED)) - return (0); -return (__thr_umutex_unlock(mtx, id)); + uint32_t flags = mtx-m_flags; + + if ((flags (UMUTEX_PRIO_PROTECT | UMUTEX_PRIO_INHERIT)) == 0) { + uint32_t owner; + do { + owner = mtx-m_owner; + if (__predict_false((owner ~UMUTEX_CONTESTED) != id)) + return (EPERM); + } while (__predict_false(!atomic_cmpset_rel_32(mtx-m_owner, +owner, UMUTEX_UNOWNED))); + if ((owner UMUTEX_CONTESTED)) + (void)_umtx_op_err(mtx, UMTX_OP_MUTEX_WAKE2, flags, 0, 0); + return (0); + } + if (atomic_cmpset_rel_32(mtx-m_owner, id, UMUTEX_UNOWNED)) + return (0); + return (__thr_umutex_unlock(mtx, id)); } static inline int Modified: head/sys/kern/kern_umtx.c == --- head/sys/kern/kern_umtx.c Thu Apr 5 00:53:21 2012(r233911) +++ head/sys/kern/kern_umtx.c Thu Apr 5 02:24:08 2012(r233912) @@ -1319,6 +1319,78 @@ do_wake_umutex(struct thread *td, struct return (0); } +/* + * Check if the mutex has waiters and tries to fix contention bit. + */ +static int +do_wake2_umutex(struct thread *td, struct umutex *m, uint32_t flags) +{ + struct umtx_key key; + uint32_t owner, old; + int type; + int error; + int count; + + switch(flags (UMUTEX_PRIO_INHERIT | UMUTEX_PRIO_PROTECT)) { + case 0: + type = TYPE_NORMAL_UMUTEX; + break; + case UMUTEX_PRIO_INHERIT: + type = TYPE_PI_UMUTEX; + break; + case UMUTEX_PRIO_PROTECT: + type = TYPE_PP_UMUTEX; + break; + default: + return (EINVAL); + } + if ((error = umtx_key_get(m, type, GET_SHARE(flags), + key)) != 0) + return (error); + + owner = 0; +
svn commit: r233913 - in head: lib/libc/gen sys/kern
Author: davidxu Date: Thu Apr 5 03:05:02 2012 New Revision: 233913 URL: http://svn.freebsd.org/changeset/base/233913 Log: In sem_post, the field _has_waiters is no longer used, because some application destroys semaphore after sem_wait returns. Just enter kernel to wake up sleeping threads, only update _has_waiters if it is safe. While here, check if the value exceed SEM_VALUE_MAX and return EOVERFLOW if this is true. Modified: head/lib/libc/gen/sem_new.c head/sys/kern/kern_umtx.c Modified: head/lib/libc/gen/sem_new.c == --- head/lib/libc/gen/sem_new.c Thu Apr 5 02:24:08 2012(r233912) +++ head/lib/libc/gen/sem_new.c Thu Apr 5 03:05:02 2012(r233913) @@ -332,9 +332,6 @@ _sem_getvalue(sem_t * __restrict sem, in static __inline int usem_wake(struct _usem *sem) { - rmb(); - if (!sem-_has_waiters) - return (0); return _umtx_op(sem, UMTX_OP_SEM_WAKE, 0, NULL, NULL); } @@ -374,17 +371,6 @@ _sem_trywait(sem_t *sem) return (-1); } -#define TIMESPEC_SUB(dst, src, val) \ -do {\ -(dst)-tv_sec = (src)-tv_sec - (val)-tv_sec; \ -(dst)-tv_nsec = (src)-tv_nsec - (val)-tv_nsec; \ -if ((dst)-tv_nsec 0) { \ -(dst)-tv_sec--;\ -(dst)-tv_nsec += 10; \ -} \ -} while (0) - - int _sem_timedwait(sem_t * __restrict sem, const struct timespec * __restrict abstime) @@ -438,10 +424,16 @@ _sem_wait(sem_t *sem) int _sem_post(sem_t *sem) { + unsigned int count; if (sem_check_validity(sem) != 0) return (-1); - atomic_add_rel_int(sem-_kern._count, 1); - return usem_wake(sem-_kern); + do { + count = sem-_kern._count; + if (count + 1 SEM_VALUE_MAX) + return (EOVERFLOW); + } while(!atomic_cmpset_rel_int(sem-_kern._count, count, count+1)); + (void)usem_wake(sem-_kern); + return (0); } Modified: head/sys/kern/kern_umtx.c == --- head/sys/kern/kern_umtx.c Thu Apr 5 02:24:08 2012(r233912) +++ head/sys/kern/kern_umtx.c Thu Apr 5 03:05:02 2012(r233913) @@ -2840,9 +2840,7 @@ do_sem_wait(struct thread *td, struct _u umtxq_busy(uq-uq_key); umtxq_insert(uq); umtxq_unlock(uq-uq_key); - casuword32(__DEVOLATILE(uint32_t *, sem-_has_waiters), 0, 1); - rmb(); count = fuword32(__DEVOLATILE(uint32_t *, sem-_count)); if (count != 0) { umtxq_lock(uq-uq_key); @@ -2876,7 +2874,7 @@ static int do_sem_wake(struct thread *td, struct _usem *sem) { struct umtx_key key; - int error, cnt, nwake; + int error, cnt; uint32_t flags; flags = fuword32(sem-_flags); @@ -2885,12 +2883,19 @@ do_sem_wake(struct thread *td, struct _u umtxq_lock(key); umtxq_busy(key); cnt = umtxq_count(key); - nwake = umtxq_signal(key, 1); - if (cnt = nwake) { - umtxq_unlock(key); - error = suword32( - __DEVOLATILE(uint32_t *, sem-_has_waiters), 0); - umtxq_lock(key); + if (cnt 0) { + umtxq_signal(key, 1); + /* +* Check if count is greater than 0, this means the memory is +* still being referenced by user code, so we can safely +* update _has_waiters flag. +*/ + if (cnt == 1) { + umtxq_unlock(key); + error = suword32( + __DEVOLATILE(uint32_t *, sem-_has_waiters), 0); + umtxq_lock(key); + } } umtxq_unbusy(key); umtxq_unlock(key); ___ 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
svn commit: r233729 - head/sys/kern
Author: davidxu Date: Sat Mar 31 06:48:41 2012 New Revision: 233729 URL: http://svn.freebsd.org/changeset/base/233729 Log: Remove stale comments. Modified: head/sys/kern/kern_umtx.c Modified: head/sys/kern/kern_umtx.c == --- head/sys/kern/kern_umtx.c Sat Mar 31 06:44:48 2012(r233728) +++ head/sys/kern/kern_umtx.c Sat Mar 31 06:48:41 2012(r233729) @@ -1216,9 +1216,6 @@ do_lock_normal(struct thread *td, struct } /* - * Lock PTHREAD_PRIO_NONE protocol POSIX mutex. - */ -/* * Unlock PTHREAD_PRIO_NONE protocol POSIX mutex. */ static int ___ 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
svn commit: r233693 - head/sys/kern
Author: davidxu Date: Fri Mar 30 09:03:53 2012 New Revision: 233693 URL: http://svn.freebsd.org/changeset/base/233693 Log: Fix COMPAT_FREEBSD32 build. Submitted by: Andreas Tobler andreast at fgznet dot ch Modified: head/sys/kern/kern_umtx.c Modified: head/sys/kern/kern_umtx.c == --- head/sys/kern/kern_umtx.c Fri Mar 30 08:33:08 2012(r233692) +++ head/sys/kern/kern_umtx.c Fri Mar 30 09:03:53 2012(r233693) @@ -957,7 +957,7 @@ do_lock_umtx32(struct thread *td, uint32 umtxq_lock(uq-uq_key); if (old == owner) error = umtxq_sleep(uq, umtx, timeout == NULL ? - NULL : timo); + NULL : timo); umtxq_remove(uq); umtxq_unlock(uq-uq_key); umtx_key_release(uq-uq_key); @@ -3372,7 +3372,7 @@ __umtx_op_rw_rdlock_compat32(struct thre (size_t)uap-uaddr1, timeout); if (error != 0) return (error); - error = do_rw_rdlock2(td, uap-obj, uap-val, timeout); + error = do_rw_rdlock(td, uap-obj, uap-val, timeout); } return (error); } @@ -3391,7 +3391,7 @@ __umtx_op_rw_wrlock_compat32(struct thre (size_t)uap-uaddr1, timeout); if (error != 0) return (error); - error = do_rw_wrlock2(td, uap-obj, timeout); + error = do_rw_wrlock(td, uap-obj, timeout); } return (error); } ___ 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