Re: svn commit: r368769 - head/sys/net
> > NET_EPOCH_ASSERT(); > > - if (w->w_op == NET_RT_FLAGS && !(rt->rte_flags & w->w_arg)) > - return 0; > - if (!can_export_rte(w->w_req->td->td_ucred, rt)) > + export_rtaddrs(rt, w->dst, w->mask); > + if (!can_export_rte(w->w_req->td->td_ucred, rt_is_host(rt), w->dst)) > return (0); > - nh = rt->rt_nhop; > + nh = rt_get_raw_nhop(rt); > #ifdef ROUTE_MPATH > if (NH_IS_NHGRP(nh)) { > struct weightened_nhop *wn; > @@ -1783,13 +1848,17 @@ sysctl_dumpnhop(struct rtentry *rt, struct > nhop_object > { > struct rt_addrinfo info; > int error = 0, size; > - struct sockaddr_storage ss; > + uint32_t rtflags; > > + rtflags = nhop_get_rtflags(nh); > + > + if (w->w_op == NET_RT_FLAGS && !(rtflags & w->w_arg)) > + return (0); > + > bzero((caddr_t), sizeof(info)); > - info.rti_info[RTAX_DST] = rt_key(rt); > + info.rti_info[RTAX_DST] = w->dst; > info.rti_info[RTAX_GATEWAY] = >gw_sa; > - info.rti_info[RTAX_NETMASK] = rtsock_fix_netmask(rt_key(rt), > - rt_mask(rt), ); > + info.rti_info[RTAX_NETMASK] = (rtflags & RTF_HOST) ? NULL : w->mask; > info.rti_info[RTAX_GENMASK] = 0; > if (nh->nh_ifp && !(nh->nh_ifp->if_flags & IFF_DYING)) { > info.rti_info[RTAX_IFP] = nh->nh_ifp->if_addr->ifa_addr; > @@ -1804,12 +1873,16 @@ sysctl_dumpnhop(struct rtentry *rt, struct > nhop_object > > bzero(>rtm_index, > sizeof(*rtm) - offsetof(struct rt_msghdr, rtm_index)); > - if (rt->rte_flags & RTF_GWFLAG_COMPAT) > + > + /* > + * rte flags may consist of RTF_HOST (duplicated in nhop > rtflags) > + * and RTF_UP (if entry is linked, which is always true here). > + * Given that, use nhop rtflags & add RTF_UP. > + */ > + rtm->rtm_flags = rtflags | RTF_UP; > + if (rtm->rtm_flags & RTF_GWFLAG_COMPAT) > rtm->rtm_flags = RTF_GATEWAY | > - (rt->rte_flags & ~RTF_GWFLAG_COMPAT); > - else > - rtm->rtm_flags = rt->rte_flags; > - rtm->rtm_flags |= nhop_get_rtflags(nh); > + (rtm->rtm_flags & ~RTF_GWFLAG_COMPAT); > rt_getmetrics(rt, nh, >rtm_rmx); > rtm->rtm_rmx.rmx_weight = weight; > rtm->rtm_index = nh->nh_ifp->if_index; > @@ -2075,10 +2148,23 @@ sysctl_ifmalist(int af, struct walkarg *w) > return (error); > } > > +static void > +rtable_sysctl_dump(uint32_t fibnum, int family, struct walkarg *w) > +{ > + union sockaddr_union sa_dst, sa_mask; > + > + w->family = family; > + w->dst = (struct sockaddr *)_dst; > + w->mask = (struct sockaddr *)_mask; > + > + init_sockaddrs_family(family, w->dst, w->mask); > + > + rib_walk(fibnum, family, false, sysctl_dumpentry, w); > +} > + > static int > sysctl_rtsock(SYSCTL_HANDLER_ARGS) > { > - RIB_RLOCK_TRACKER; > struct epoch_tracker et; > int *name = (int *)arg1; > u_int namelen = arg2; > @@ -2151,10 +2237,7 @@ sysctl_rtsock(SYSCTL_HANDLER_ARGS) > for (error = 0; error == 0 && i <= lim; i++) { > rnh = rt_tables_get_rnh(fib, i); > if (rnh != NULL) { > - RIB_RLOCK(rnh); > - error = rnh->rnh_walktree(>head, > - sysctl_dumpentry, ); > - RIB_RUNLOCK(rnh); > + rtable_sysctl_dump(fib, i, ); > } else if (af != 0) > error = EAFNOSUPPORT; > } > ___ > svn-src-...@freebsd.org mailing list > https://lists.freebsd.org/mailman/listinfo/svn-src-all > To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org" > -- Mateusz Guzik ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r368732 - head/sys/kern
Author: mjg Date: Thu Dec 17 18:52:30 2020 New Revision: 368732 URL: https://svnweb.freebsd.org/changeset/base/368732 Log: fd: reimplement close_range to avoid spurious relocking Modified: head/sys/kern/kern_descrip.c Modified: head/sys/kern/kern_descrip.c == --- head/sys/kern/kern_descrip.cThu Dec 17 18:52:04 2020 (r368731) +++ head/sys/kern/kern_descrip.cThu Dec 17 18:52:30 2020 (r368732) @@ -307,6 +307,7 @@ fdfree(struct filedesc *fdp, int fd) { struct filedescent *fde; + FILEDESC_XLOCK_ASSERT(fdp); fde = >fd_ofiles[fd]; #ifdef CAPABILITIES seqc_write_begin(>fde_seqc); @@ -1367,12 +1368,10 @@ int kern_close_range(struct thread *td, u_int lowfd, u_int highfd) { struct filedesc *fdp; - int fd, ret, lastfile; + const struct fdescenttbl *fdt; + struct file *fp; + int fd; - ret = 0; - fdp = td->td_proc->p_fd; - FILEDESC_SLOCK(fdp); - /* * Check this prior to clamping; closefrom(3) with only fd 0, 1, and 2 * open should not be a usage error. From a close_range() perspective, @@ -1380,30 +1379,36 @@ kern_close_range(struct thread *td, u_int lowfd, u_int * be a usage error as all fd above 3 are in-fact already closed. */ if (highfd < lowfd) { - ret = EINVAL; - goto out; + return (EINVAL); } - /* -* If lastfile == -1, we're dealing with either a fresh file -* table or one in which every fd has been closed. Just return -* successful; there's nothing left to do. -*/ - lastfile = fdlastfile(fdp); - if (lastfile == -1) - goto out; - /* Clamped to [lowfd, lastfile] */ - highfd = MIN(highfd, lastfile); - for (fd = lowfd; fd <= highfd; fd++) { - if (fdp->fd_ofiles[fd].fde_file != NULL) { - FILEDESC_SUNLOCK(fdp); - (void)kern_close(td, fd); - FILEDESC_SLOCK(fdp); + fdp = td->td_proc->p_fd; + FILEDESC_XLOCK(fdp); + fdt = atomic_load_ptr(>fd_files); + highfd = MIN(highfd, fdt->fdt_nfiles - 1); + fd = lowfd; + if (__predict_false(fd > highfd)) { + goto out_locked; + } + for (;;) { + fp = fdt->fdt_ofiles[fd].fde_file; + if (fp == NULL) { + if (fd == highfd) + goto out_locked; + } else { + fdfree(fdp, fd); + (void) closefp(fdp, fd, fp, td, true, true); + if (fd == highfd) + goto out_unlocked; + FILEDESC_XLOCK(fdp); + fdt = atomic_load_ptr(>fd_files); } + fd++; } -out: - FILEDESC_SUNLOCK(fdp); - return (ret); +out_locked: + FILEDESC_XUNLOCK(fdp); +out_unlocked: + return (0); } #ifndef _SYS_SYSPROTO_H_ ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r368731 - in head/sys: kern security/audit
Author: mjg Date: Thu Dec 17 18:52:04 2020 New Revision: 368731 URL: https://svnweb.freebsd.org/changeset/base/368731 Log: audit: rework AUDIT_SYSCLOSE This in particular avoids spurious lookups on close. Modified: head/sys/kern/kern_descrip.c head/sys/security/audit/audit.h head/sys/security/audit/audit_arg.c Modified: head/sys/kern/kern_descrip.c == --- head/sys/kern/kern_descrip.cThu Dec 17 18:51:09 2020 (r368730) +++ head/sys/kern/kern_descrip.cThu Dec 17 18:52:04 2020 (r368731) @@ -107,7 +107,7 @@ __read_mostly uma_zone_t pwd_zone; VFS_SMR_DECLARE; static int closefp(struct filedesc *fdp, int fd, struct file *fp, - struct thread *td, bool holdleaders); + struct thread *td, bool holdleaders, bool audit); static int fd_first_free(struct filedesc *fdp, int low, int size); static voidfdgrowtable(struct filedesc *fdp, int nfd); static voidfdgrowtable_exp(struct filedesc *fdp, int nfd); @@ -998,7 +998,7 @@ kern_dup(struct thread *td, u_int mode, int flags, int error = 0; if (delfp != NULL) { - (void) closefp(fdp, new, delfp, td, true); + (void) closefp(fdp, new, delfp, td, true, false); FILEDESC_UNLOCK_ASSERT(fdp); } else { unlock: @@ -1240,7 +1240,8 @@ fgetown(struct sigio **sigiop) } static int -closefp_impl(struct filedesc *fdp, int fd, struct file *fp, struct thread *td) +closefp_impl(struct filedesc *fdp, int fd, struct file *fp, struct thread *td, +bool audit) { int error; @@ -1262,6 +1263,10 @@ closefp_impl(struct filedesc *fdp, int fd, struct file mq_fdclose(td, fd, fp); FILEDESC_XUNLOCK(fdp); +#ifdef AUDIT + if (AUDITING_TD(td) && audit) + audit_sysclose(td, fd, fp); +#endif error = closef(fp, td); /* @@ -1277,7 +1282,7 @@ closefp_impl(struct filedesc *fdp, int fd, struct file static int closefp_hl(struct filedesc *fdp, int fd, struct file *fp, struct thread *td, -bool holdleaders) +bool holdleaders, bool audit) { int error; @@ -1295,7 +1300,7 @@ closefp_hl(struct filedesc *fdp, int fd, struct file * } } - error = closefp_impl(fdp, fd, fp, td); + error = closefp_impl(fdp, fd, fp, td, audit); if (holdleaders) { FILEDESC_XLOCK(fdp); fdp->fd_holdleaderscount--; @@ -1311,15 +1316,15 @@ closefp_hl(struct filedesc *fdp, int fd, struct file * static int closefp(struct filedesc *fdp, int fd, struct file *fp, struct thread *td, -bool holdleaders) +bool holdleaders, bool audit) { FILEDESC_XLOCK_ASSERT(fdp); if (__predict_false(td->td_proc->p_fdtol != NULL)) { - return (closefp_hl(fdp, fd, fp, td, holdleaders)); + return (closefp_hl(fdp, fd, fp, td, holdleaders, audit)); } else { - return (closefp_impl(fdp, fd, fp, td)); + return (closefp_impl(fdp, fd, fp, td, audit)); } } @@ -1347,8 +1352,6 @@ kern_close(struct thread *td, int fd) fdp = td->td_proc->p_fd; - AUDIT_SYSCLOSE(td, fd); - FILEDESC_XLOCK(fdp); if ((fp = fget_locked(fdp, fd)) == NULL) { FILEDESC_XUNLOCK(fdp); @@ -1357,7 +1360,7 @@ kern_close(struct thread *td, int fd) fdfree(fdp, fd); /* closefp() drops the FILEDESC lock for us. */ - return (closefp(fdp, fd, fp, td, true)); + return (closefp(fdp, fd, fp, td, true, true)); } int @@ -2671,7 +2674,7 @@ fdcloseexec(struct thread *td) (fde->fde_flags & UF_EXCLOSE))) { FILEDESC_XLOCK(fdp); fdfree(fdp, i); - (void) closefp(fdp, i, fp, td, false); + (void) closefp(fdp, i, fp, td, false, false); FILEDESC_UNLOCK_ASSERT(fdp); } } Modified: head/sys/security/audit/audit.h == --- head/sys/security/audit/audit.h Thu Dec 17 18:51:09 2020 (r368730) +++ head/sys/security/audit/audit.h Thu Dec 17 18:52:04 2020 (r368731) @@ -140,7 +140,7 @@ void audit_arg_argv(char *argv, int argc, int length) voidaudit_arg_envv(char *envv, int envc, int length); voidaudit_arg_rights(cap_rights_t *rightsp); voidaudit_arg_fcntl_rights(uint32_t fcntlrights); -voidaudit_sysclose(struct thread *td, int fd); +voidaudit_sysclose(struct thread *td, int fd, struct file *fp); voidaudit_cred_copy(struct ucred *src, struct ucred *dest); voidaudit_cred_destroy(struct ucred *cred); voidaudit_cred_init(struct ucred *cred); Modified: head/sys/security/audit/audit_arg.c
svn commit: r368730 - head/sys/kern
Author: mjg Date: Thu Dec 17 18:51:09 2020 New Revision: 368730 URL: https://svnweb.freebsd.org/changeset/base/368730 Log: fd: refactor closefp in preparation for close_range rework Modified: head/sys/kern/kern_descrip.c Modified: head/sys/kern/kern_descrip.c == --- head/sys/kern/kern_descrip.cThu Dec 17 18:29:30 2020 (r368729) +++ head/sys/kern/kern_descrip.cThu Dec 17 18:51:09 2020 (r368730) @@ -107,7 +107,7 @@ __read_mostly uma_zone_t pwd_zone; VFS_SMR_DECLARE; static int closefp(struct filedesc *fdp, int fd, struct file *fp, - struct thread *td, int holdleaders); + struct thread *td, bool holdleaders); static int fd_first_free(struct filedesc *fdp, int low, int size); static voidfdgrowtable(struct filedesc *fdp, int nfd); static voidfdgrowtable_exp(struct filedesc *fdp, int nfd); @@ -998,7 +998,7 @@ kern_dup(struct thread *td, u_int mode, int flags, int error = 0; if (delfp != NULL) { - (void) closefp(fdp, new, delfp, td, 1); + (void) closefp(fdp, new, delfp, td, true); FILEDESC_UNLOCK_ASSERT(fdp); } else { unlock: @@ -1239,29 +1239,13 @@ fgetown(struct sigio **sigiop) return (pgid); } -/* - * Function drops the filedesc lock on return. - */ static int -closefp(struct filedesc *fdp, int fd, struct file *fp, struct thread *td, -int holdleaders) +closefp_impl(struct filedesc *fdp, int fd, struct file *fp, struct thread *td) { int error; FILEDESC_XLOCK_ASSERT(fdp); - if (holdleaders) { - if (td->td_proc->p_fdtol != NULL) { - /* -* Ask fdfree() to sleep to ensure that all relevant -* process leaders can be traversed in closef(). -*/ - fdp->fd_holdleaderscount++; - } else { - holdleaders = 0; - } - } - /* * We now hold the fp reference that used to be owned by the * descriptor array. We have to unlock the FILEDESC *AFTER* @@ -1288,7 +1272,31 @@ closefp(struct filedesc *fdp, int fd, struct file *fp, if (error == ERESTART) error = EINTR; + return (error); +} + +static int +closefp_hl(struct filedesc *fdp, int fd, struct file *fp, struct thread *td, +bool holdleaders) +{ + int error; + + FILEDESC_XLOCK_ASSERT(fdp); + if (holdleaders) { + if (td->td_proc->p_fdtol != NULL) { + /* +* Ask fdfree() to sleep to ensure that all relevant +* process leaders can be traversed in closef(). +*/ + fdp->fd_holdleaderscount++; + } else { + holdleaders = false; + } + } + + error = closefp_impl(fdp, fd, fp, td); + if (holdleaders) { FILEDESC_XLOCK(fdp); fdp->fd_holdleaderscount--; if (fdp->fd_holdleaderscount == 0 && @@ -1301,6 +1309,20 @@ closefp(struct filedesc *fdp, int fd, struct file *fp, return (error); } +static int +closefp(struct filedesc *fdp, int fd, struct file *fp, struct thread *td, +bool holdleaders) +{ + + FILEDESC_XLOCK_ASSERT(fdp); + + if (__predict_false(td->td_proc->p_fdtol != NULL)) { + return (closefp_hl(fdp, fd, fp, td, holdleaders)); + } else { + return (closefp_impl(fdp, fd, fp, td)); + } +} + /* * Close a file descriptor. */ @@ -1335,7 +1357,7 @@ kern_close(struct thread *td, int fd) fdfree(fdp, fd); /* closefp() drops the FILEDESC lock for us. */ - return (closefp(fdp, fd, fp, td, 1)); + return (closefp(fdp, fd, fp, td, true)); } int @@ -2649,7 +2671,7 @@ fdcloseexec(struct thread *td) (fde->fde_flags & UF_EXCLOSE))) { FILEDESC_XLOCK(fdp); fdfree(fdp, i); - (void) closefp(fdp, i, fp, td, 0); + (void) closefp(fdp, i, fp, td, false); FILEDESC_UNLOCK_ASSERT(fdp); } } ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r368703 - head/sys/kern
Author: mjg Date: Wed Dec 16 18:01:41 2020 New Revision: 368703 URL: https://svnweb.freebsd.org/changeset/base/368703 Log: fd: remove redundant saturation check from fget_unlocked_seq refcount_acquire_if_not_zero returns true on saturation. The case of 0 is handled by looping again, after which the originally found pointer will no longer be there. Noted by: kib Modified: head/sys/kern/kern_descrip.c Modified: head/sys/kern/kern_descrip.c == --- head/sys/kern/kern_descrip.cWed Dec 16 17:09:38 2020 (r368702) +++ head/sys/kern/kern_descrip.cWed Dec 16 18:01:41 2020 (r368703) @@ -3022,13 +3022,6 @@ fget_unlocked_seq(struct filedesc *fdp, int fd, cap_ri #endif if (__predict_false(!refcount_acquire_if_not_zero(>f_count))) { /* -* The count was found either saturated or zero. -* This re-read is not any more racy than using the -* return value from fcmpset. -*/ - if (refcount_load(>f_count) != 0) - return (EBADF); - /* * Force a reload. Other thread could reallocate the * table before this fd was closed, so it is possible * that there is a stale fp pointer in cached version. ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r368617 - head/sys/kern
Author: mjg Date: Sun Dec 13 21:32:19 2020 New Revision: 368617 URL: https://svnweb.freebsd.org/changeset/base/368617 Log: uipc: disable prediction in unp_pcb_lock_peer The branch is not very predictable one way or the other, at least during buildkernel where it only correctly matched 57% of calls. Modified: head/sys/kern/uipc_usrreq.c Modified: head/sys/kern/uipc_usrreq.c == --- head/sys/kern/uipc_usrreq.c Sun Dec 13 21:30:42 2020(r368616) +++ head/sys/kern/uipc_usrreq.c Sun Dec 13 21:32:19 2020(r368617) @@ -382,7 +382,7 @@ unp_pcb_lock_peer(struct unpcb *unp) UNP_PCB_LOCK_ASSERT(unp); unp2 = unp->unp_conn; - if (__predict_false(unp2 == NULL)) + if (unp2 == NULL) return (NULL); if (__predict_false(unp == unp2)) return (unp); ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r368616 - head/sys/sys
Author: mjg Date: Sun Dec 13 21:30:42 2020 New Revision: 368616 URL: https://svnweb.freebsd.org/changeset/base/368616 Log: Patch annotation in sigdeferstop Probability flipped since sigdefer handling was moved away from regular VOP calls. Modified: head/sys/sys/signalvar.h Modified: head/sys/sys/signalvar.h == --- head/sys/sys/signalvar.hSun Dec 13 21:29:39 2020(r368615) +++ head/sys/sys/signalvar.hSun Dec 13 21:30:42 2020(r368616) @@ -367,7 +367,7 @@ static inline int sigdeferstop(int mode) { - if (__predict_true(mode == SIGDEFERSTOP_NOP)) + if (__predict_false(mode == SIGDEFERSTOP_NOP)) return (SIGDEFERSTOP_VAL_NCHG); return (sigdeferstop_impl(mode)); } ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r368615 - head/sys/kern
Author: mjg Date: Sun Dec 13 21:29:39 2020 New Revision: 368615 URL: https://svnweb.freebsd.org/changeset/base/368615 Log: cache: fix ups bad predicts - last level fallback normally sees CREATE; the code should be optimized to not get there for said case - fast path commonly fails with ENOENT Modified: head/sys/kern/vfs_cache.c Modified: head/sys/kern/vfs_cache.c == --- head/sys/kern/vfs_cache.c Sun Dec 13 21:28:15 2020(r368614) +++ head/sys/kern/vfs_cache.c Sun Dec 13 21:29:39 2020(r368615) @@ -1824,7 +1824,10 @@ retry: } return (-1); negative_success: - if (__predict_false(cnp->cn_nameiop == CREATE)) { + /* +* We don't get here with regular lookup apart from corner cases. +*/ + if (__predict_true(cnp->cn_nameiop == CREATE)) { if (cnp->cn_flags & ISLASTCN) { counter_u64_add(numnegzaps, 1); error = cache_zap_locked_bucket(ncp, cnp, hash, blp); @@ -1927,7 +1930,7 @@ cache_lookup(struct vnode *dvp, struct vnode **vpp, st } return (-1); negative_success: - if (__predict_false(cnp->cn_nameiop == CREATE)) { + if (cnp->cn_nameiop == CREATE) { if (cnp->cn_flags & ISLASTCN) { vfs_smr_exit(); goto out_fallback; @@ -4589,7 +4592,10 @@ out: case CACHE_FPL_STATUS_HANDLED: MPASS(error != CACHE_FPL_FAILED); cache_fpl_smr_assert_not_entered(fpl); - if (__predict_false(error != 0)) { + /* +* A common error is ENOENT. +*/ + if (error != 0) { ndp->ni_dvp = NULL; ndp->ni_vp = NULL; cache_fpl_cleanup_cnp(cnp); ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r368614 - head/sys/kern
Author: mjg Date: Sun Dec 13 21:28:15 2020 New Revision: 368614 URL: https://svnweb.freebsd.org/changeset/base/368614 Log: vfs: correctly predict last fdrop on failed open Arguably since the count is guaranteed to be 1 the code should be modified to avoid the work. Modified: head/sys/kern/vfs_syscalls.c Modified: head/sys/kern/vfs_syscalls.c == --- head/sys/kern/vfs_syscalls.cSun Dec 13 19:45:42 2020 (r368613) +++ head/sys/kern/vfs_syscalls.cSun Dec 13 21:28:15 2020 (r368614) @@ -1229,7 +1229,7 @@ success: return (0); bad: KASSERT(indx == -1, ("indx=%d, should be -1", indx)); - fdrop(fp, td); + fdrop_close(fp, td); return (error); } ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r368609 - in head/sys: kern sys
Unfortunately inlines mess with __FILE__/__LINE__ by showing the implementation instead of the consumer. This in particular matters with https://reviews.freebsd.org/D27600 I failed to find replacements for __ macros which don't suffer the problem. On 12/13/20, Alexander Richardson wrote: > On Sun, 13 Dec 2020 at 18:06, Mateusz Guzik wrote: >> >> Author: mjg >> Date: Sun Dec 13 18:06:24 2020 >> New Revision: 368609 >> URL: https://svnweb.freebsd.org/changeset/base/368609 >> >> Log: >> fd: fix fdrop prediction when closing a fd >> >> Most of the time this is the last reference, contrary to typical fdrop >> use. >> >> Modified: >> head/sys/kern/kern_descrip.c >> head/sys/sys/file.h >> >> Modified: head/sys/kern/kern_descrip.c >> == >> --- head/sys/kern/kern_descrip.cSun Dec 13 16:26:37 2020 >> (r368608) >> +++ head/sys/kern/kern_descrip.cSun Dec 13 18:06:24 2020 >> (r368609) >> @@ -2766,7 +2766,7 @@ closef(struct file *fp, struct thread *td) >> FILEDESC_XUNLOCK(fdp); >> } >> } >> - return (fdrop(fp, td)); >> + return (fdrop_close(fp, td)); >> } >> >> /* >> >> Modified: head/sys/sys/file.h >> == >> --- head/sys/sys/file.h Sun Dec 13 16:26:37 2020(r368608) >> +++ head/sys/sys/file.h Sun Dec 13 18:06:24 2020(r368609) >> @@ -299,6 +299,17 @@ fhold(struct file *fp) >> _error; \ >> }) >> >> +#definefdrop_close(fp, td) ({ \ >> + struct file *_fp; \ >> + int _error; \ >> + \ >> + _error = 0; \ >> + _fp = (fp); \ >> + if (__predict_true(refcount_release(&_fp->f_count)))\ >> + _error = _fdrop(_fp, td); \ >> + _error; \ >> +}) >> + >> static __inline fo_rdwr_t fo_read; >> static __inline fo_rdwr_t fo_write; >> static __inline fo_truncate_t fo_truncate; > > Wouldn't this be more readable as a static __inline function (or if > you are concerned about it not being inlined, __always_inline)? Also > means you can drop the temporary _fp variable that's there to avoid > side-effects in macro args. > > Regards, > Alex > -- Mateusz Guzik ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r368609 - in head/sys: kern sys
Author: mjg Date: Sun Dec 13 18:06:24 2020 New Revision: 368609 URL: https://svnweb.freebsd.org/changeset/base/368609 Log: fd: fix fdrop prediction when closing a fd Most of the time this is the last reference, contrary to typical fdrop use. Modified: head/sys/kern/kern_descrip.c head/sys/sys/file.h Modified: head/sys/kern/kern_descrip.c == --- head/sys/kern/kern_descrip.cSun Dec 13 16:26:37 2020 (r368608) +++ head/sys/kern/kern_descrip.cSun Dec 13 18:06:24 2020 (r368609) @@ -2766,7 +2766,7 @@ closef(struct file *fp, struct thread *td) FILEDESC_XUNLOCK(fdp); } } - return (fdrop(fp, td)); + return (fdrop_close(fp, td)); } /* Modified: head/sys/sys/file.h == --- head/sys/sys/file.h Sun Dec 13 16:26:37 2020(r368608) +++ head/sys/sys/file.h Sun Dec 13 18:06:24 2020(r368609) @@ -299,6 +299,17 @@ fhold(struct file *fp) _error; \ }) +#definefdrop_close(fp, td) ({ \ + struct file *_fp; \ + int _error; \ + \ + _error = 0; \ + _fp = (fp); \ + if (__predict_true(refcount_release(&_fp->f_count)))\ + _error = _fdrop(_fp, td); \ + _error; \ +}) + static __inline fo_rdwr_t fo_read; static __inline fo_rdwr_t fo_write; static __inline fo_truncate_t fo_truncate; ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r368571 - head/sys/netpfil/ipfw
k6 = (struct sockaddr_in6 *)pmask; > - memcpy(>k.addr6, >sin6_addr, > - sizeof(struct in6_addr)); > - len = 128; > - if (mask6 != NULL) > - len = contigmask((uint8_t *)>sin6_addr, 128); > - if (len == -1) > - len = 0; > - tent->masklen = len; > +#ifdef INET > + if (family == AF_INET6) { > + rt_get_inet6_prefix_plen(rt, >k.addr6, , ); > + tent->masklen = plen; > tent->subtype = AF_INET6; > tent->v.kidx = 0; > } > #endif > - > return (0); > } > > @@ -3985,66 +3935,61 @@ static int > ta_find_kfib_tentry(void *ta_state, struct table_info *ti, > ipfw_obj_tentry *tent) > { > - struct rt_addrinfo info; > - struct sockaddr_in6 key6, dst6, mask6; > - struct sockaddr *dst, *key, *mask; > + struct rtentry *rt; > + struct route_nhop_data rnd; > + struct epoch_tracker et; > + int error; > > - /* Prepare sockaddr for prefix/mask and info */ > - bzero(, sizeof(dst6)); > - dst6.sin6_len = sizeof(dst6); > - dst = (struct sockaddr *) > - bzero(, sizeof(mask6)); > - mask6.sin6_len = sizeof(mask6); > - mask = (struct sockaddr *) > - > - bzero(, sizeof(info)); > - info.rti_info[RTAX_DST] = dst; > - info.rti_info[RTAX_NETMASK] = mask; > - > - /* Prepare the lookup key */ > - bzero(, sizeof(key6)); > - key6.sin6_family = tent->subtype; > - key = (struct sockaddr *) > - > + NET_EPOCH_ENTER(et); > if (tent->subtype == AF_INET) { > - ((struct sockaddr_in *))->sin_addr = tent->k.addr; > - key6.sin6_len = sizeof(struct sockaddr_in); > + rt = fib4_lookup_rt(ti->data, tent->k.addr, 0, 0, ); > } else { > - key6.sin6_addr = tent->k.addr6; > - key6.sin6_len = sizeof(struct sockaddr_in6); > + rt = fib6_lookup_rt(ti->data, >k.addr6, 0, 0, ); > } > + if (rt != NULL) > + error = ta_dump_kfib_tentry_int(tent->subtype, rt, tent); > + else > + error = ENOENT; > + NET_EPOCH_EXIT(et); > > - if (rib_lookup_info(ti->data, key, 0, 0, ) != 0) > - return (ENOENT); > - if ((info.rti_addrs & RTA_NETMASK) == 0) > - mask = NULL; > + return (error); > +} > > - ta_dump_kfib_tentry_int(dst, mask, tent); > +struct kfib_dump_arg { > + struct rtentry *rt; > + int family; > + ta_foreach_f*f; > + void*arg; > +}; > > - return (0); > +static int > +ta_dump_kfib_tentry(void *ta_state, struct table_info *ti, void *e, > +ipfw_obj_tentry *tent) > +{ > + struct kfib_dump_arg *karg = (struct kfib_dump_arg *)e; > + > + return (ta_dump_kfib_tentry_int(karg->family, karg->rt, tent)); > } > > +static int > +walk_wrapper_f(struct rtentry *rt, void *arg) > +{ > + struct kfib_dump_arg *karg = (struct kfib_dump_arg *)arg; > + > + karg->rt = rt; > + return (karg->f(karg, karg->arg)); > +} > + > static void > ta_foreach_kfib(void *ta_state, struct table_info *ti, ta_foreach_f *f, > void *arg) > { > - RIB_RLOCK_TRACKER; > - struct rib_head *rh; > - int error; > + struct kfib_dump_arg karg = { .f = f, .arg = arg }; > > - rh = rt_tables_get_rnh(ti->data, AF_INET); > - if (rh != NULL) { > - RIB_RLOCK(rh); > - error = rh->rnh_walktree(>head, (walktree_f_t *)f, arg); > - RIB_RUNLOCK(rh); > - } > - > - rh = rt_tables_get_rnh(ti->data, AF_INET6); > - if (rh != NULL) { > - RIB_RLOCK(rh); > - error = rh->rnh_walktree(>head, (walktree_f_t *)f, arg); > - RIB_RUNLOCK(rh); > - } > + karg.family = AF_INET; > + rib_walk(ti->data, AF_INET, false, walk_wrapper_f, ); > + karg.family = AF_INET6; > + rib_walk(ti->data, AF_INET6, false, walk_wrapper_f, ); > } > > struct table_algo addr_kfib = { > ___ > svn-src-...@freebsd.org mailing list > https://lists.freebsd.org/mailman/listinfo/svn-src-all > To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org" > -- Mateusz Guzik ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r368516 - head/sys/kern
Author: mjg Date: Thu Dec 10 17:17:22 2020 New Revision: 368516 URL: https://svnweb.freebsd.org/changeset/base/368516 Log: fd: make serialization in fdescfree_fds conditional on hold count p_fd nullification in fdescfree serializes against new threads transitioning the count 1 -> 2, meaning that fdescfree_fds observing the count of 1 can safely assume there is nobody else using the table. Losing the race and observing > 1 is harmless. Reviewed by: markj Differential Revision:https://reviews.freebsd.org/D27522 Modified: head/sys/kern/kern_descrip.c Modified: head/sys/kern/kern_descrip.c == --- head/sys/kern/kern_descrip.cThu Dec 10 13:32:51 2020 (r368515) +++ head/sys/kern/kern_descrip.cThu Dec 10 17:17:22 2020 (r368516) @@ -2466,9 +2466,13 @@ fdescfree_fds(struct thread *td, struct filedesc *fdp, KASSERT(refcount_load(>fd_refcnt) == 0, ("%s: fd table %p carries references", __func__, fdp)); - /* Serialize with threads iterating over the table. */ - FILEDESC_XLOCK(fdp); - FILEDESC_XUNLOCK(fdp); + /* +* Serialize with threads iterating over the table, if any. +*/ + if (refcount_load(>fd_holdcnt) > 1) { + FILEDESC_XLOCK(fdp); + FILEDESC_XUNLOCK(fdp); + } lastfile = fdlastfile_single(fdp); for (i = 0; i <= lastfile; i++) { ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r368375 - head/sys/kern
Thanks for the report. Fixed in r368395. On 12/6/20, Jessica Clarke wrote: > Hi Mateusz, > This looks like a behavioural change to me. Was that intended and a bug > fix (in which case, it should have been documented in the commit > message) or not? See below for the exact change. > > On 6 Dec 2020, at 04:59, Mateusz Guzik wrote: >> +static int >> +namei_getpath(struct nameidata *ndp) >> +{ >> +struct componentname *cnp; >> +int error; >> + >> +cnp = >ni_cnd; >> + >> +/* >> + * Get a buffer for the name to be translated, and copy the >> + * name into the buffer. >> + */ >> +cnp->cn_pnbuf = uma_zalloc(namei_zone, M_WAITOK); >> +if (ndp->ni_segflg == UIO_SYSSPACE) { >> +error = copystr(ndp->ni_dirp, cnp->cn_pnbuf, MAXPATHLEN, >> +>ni_pathlen); >> +} else { >> +error = copyinstr(ndp->ni_dirp, cnp->cn_pnbuf, MAXPATHLEN, >> +>ni_pathlen); >> +} >> + >> +if (__predict_false(error != 0)) { >> +return (error); > > This does not call namei_cleanup_cnp. > >> @@ -531,31 +568,11 @@ namei(struct nameidata *ndp) >> ndp->ni_lcf = 0; >> ndp->ni_vp = NULL; >> >> -/* >> - * Get a buffer for the name to be translated, and copy the >> - * name into the buffer. >> - */ >> -cnp->cn_pnbuf = uma_zalloc(namei_zone, M_WAITOK); >> -if (ndp->ni_segflg == UIO_SYSSPACE) >> -error = copystr(ndp->ni_dirp, cnp->cn_pnbuf, MAXPATHLEN, >> ->ni_pathlen); >> -else >> -error = copyinstr(ndp->ni_dirp, cnp->cn_pnbuf, MAXPATHLEN, >> ->ni_pathlen); >> - >> +error = namei_getpath(ndp); >> if (__predict_false(error != 0)) { >> -namei_cleanup_cnp(cnp); > > But it used to be called in that case here. > >> return (error); >> } > > Jess > > -- Mateusz Guzik ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r368395 - head/sys/kern
Author: mjg Date: Sun Dec 6 19:24:38 2020 New Revision: 368395 URL: https://svnweb.freebsd.org/changeset/base/368395 Log: vfs: add cleanup on error missed in r368375 Noted by: jrtc27 Modified: head/sys/kern/vfs_lookup.c Modified: head/sys/kern/vfs_lookup.c == --- head/sys/kern/vfs_lookup.c Sun Dec 6 18:43:12 2020(r368394) +++ head/sys/kern/vfs_lookup.c Sun Dec 6 19:24:38 2020(r368395) @@ -486,6 +486,7 @@ namei_getpath(struct nameidata *ndp) } if (__predict_false(error != 0)) { + namei_cleanup_cnp(cnp); return (error); } ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r368375 - head/sys/kern
Author: mjg Date: Sun Dec 6 04:59:24 2020 New Revision: 368375 URL: https://svnweb.freebsd.org/changeset/base/368375 Log: vfs: factor buffer allocation/copyin out of namei Modified: head/sys/kern/vfs_lookup.c Modified: head/sys/kern/vfs_lookup.c == --- head/sys/kern/vfs_lookup.c Sat Dec 5 22:04:30 2020(r368374) +++ head/sys/kern/vfs_lookup.c Sun Dec 6 04:59:24 2020(r368375) @@ -464,6 +464,43 @@ namei_setup(struct nameidata *ndp, struct vnode **dpp, return (0); } +static int +namei_getpath(struct nameidata *ndp) +{ + struct componentname *cnp; + int error; + + cnp = >ni_cnd; + + /* +* Get a buffer for the name to be translated, and copy the +* name into the buffer. +*/ + cnp->cn_pnbuf = uma_zalloc(namei_zone, M_WAITOK); + if (ndp->ni_segflg == UIO_SYSSPACE) { + error = copystr(ndp->ni_dirp, cnp->cn_pnbuf, MAXPATHLEN, + >ni_pathlen); + } else { + error = copyinstr(ndp->ni_dirp, cnp->cn_pnbuf, MAXPATHLEN, + >ni_pathlen); + } + + if (__predict_false(error != 0)) { + return (error); + } + + /* +* Don't allow empty pathnames. +*/ + if (__predict_false(*cnp->cn_pnbuf == '\0')) { + namei_cleanup_cnp(cnp); + return (ENOENT); + } + + cnp->cn_nameptr = cnp->cn_pnbuf; + return (0); +} + /* * Convert a pathname into a pointer to a locked vnode. * @@ -531,31 +568,11 @@ namei(struct nameidata *ndp) ndp->ni_lcf = 0; ndp->ni_vp = NULL; - /* -* Get a buffer for the name to be translated, and copy the -* name into the buffer. -*/ - cnp->cn_pnbuf = uma_zalloc(namei_zone, M_WAITOK); - if (ndp->ni_segflg == UIO_SYSSPACE) - error = copystr(ndp->ni_dirp, cnp->cn_pnbuf, MAXPATHLEN, - >ni_pathlen); - else - error = copyinstr(ndp->ni_dirp, cnp->cn_pnbuf, MAXPATHLEN, - >ni_pathlen); - + error = namei_getpath(ndp); if (__predict_false(error != 0)) { - namei_cleanup_cnp(cnp); return (error); } - /* -* Don't allow empty pathnames. -*/ - if (__predict_false(*cnp->cn_pnbuf == '\0')) { - namei_cleanup_cnp(cnp); - return (ENOENT); - } - #ifdef KTRACE if (KTRPOINT(td, KTR_NAMEI)) { KASSERT(cnp->cn_thread == curthread, @@ -563,8 +580,6 @@ namei(struct nameidata *ndp) ktrnamei(cnp->cn_pnbuf); } #endif - - cnp->cn_nameptr = cnp->cn_pnbuf; /* * First try looking up the target without locking any vnodes. ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r368159 - in head/sys: arm/conf conf
On 11/29/20, Michal Meloun wrote: > Author: mmel > Date: Sun Nov 29 17:42:32 2020 > New Revision: 368159 > URL: https://svnweb.freebsd.org/changeset/base/368159 > > Log: > Remove unused options. > > Marvell files and their related SOC_MV_ options should be cleaned up > in another pass. > Since this change tinderbox fails to build VERSATILEPB and EFIKA_MX: /usr/src/sys/dev/syscons/scgfbrndr.c:35:10: fatal error: 'opt_gfb.h' file not found #include "opt_gfb.h" ^~~ > Modified: > head/sys/arm/conf/NOTES > head/sys/arm/conf/std.armv6 > head/sys/arm/conf/std.armv7 > head/sys/conf/options.arm > > Modified: head/sys/arm/conf/NOTES > == > --- head/sys/arm/conf/NOTES Sun Nov 29 16:44:22 2020(r368158) > +++ head/sys/arm/conf/NOTES Sun Nov 29 17:42:32 2020(r368159) > @@ -9,7 +9,6 @@ makeoptions CONF_CFLAGS+="-march=armv7a" > > # Add options for armv7 that are not in sys/conf/NOTES... > > -options ARM_L2_PIPT # Only L2 PIPT is supported > options FDT # Flattened device tree support > options FREEBSD_BOOT_LOADER # Process metadata passed from loader(8) > options INTRNG # Include INTRNG framework > > Modified: head/sys/arm/conf/std.armv6 > == > --- head/sys/arm/conf/std.armv6 Sun Nov 29 16:44:22 2020 > (r368158) > +++ head/sys/arm/conf/std.armv6 Sun Nov 29 17:42:32 2020 > (r368159) > @@ -3,7 +3,6 @@ > # $FreeBSD$ > > options HZ=1000 > -options ARM_L2_PIPT # Only L2 PIPT is supported > options INTRNG # All arm systems use INTRNG these days > options PREEMPTION # Enable kernel thread preemption > options VIMAGE # Subsystem virtualization, e.g. VNET > > Modified: head/sys/arm/conf/std.armv7 > == > --- head/sys/arm/conf/std.armv7 Sun Nov 29 16:44:22 2020 > (r368158) > +++ head/sys/arm/conf/std.armv7 Sun Nov 29 17:42:32 2020 > (r368159) > @@ -3,7 +3,6 @@ > # $FreeBSD$ > > options HZ=1000 > -options ARM_L2_PIPT # Only L2 PIPT is supported > options INTRNG # All arm systems use INTRNG these days > options PREEMPTION # Enable kernel thread preemption > options VIMAGE # Subsystem virtualization, e.g. VNET > > Modified: head/sys/conf/options.arm > == > --- head/sys/conf/options.arm Sun Nov 29 16:44:22 2020(r368158) > +++ head/sys/conf/options.arm Sun Nov 29 17:42:32 2020(r368159) > @@ -1,13 +1,7 @@ > #$FreeBSD$ > ARMV6opt_global.h > ARMV7opt_global.h > -ARM_CACHE_LOCK_ENABLEopt_global.h > -ARM_KERN_DIRECTMAP opt_vm.h > -ARM_L2_PIPT opt_global.h > -ARM_MANY_BOARD opt_global.h > -ARM_WANT_TP_ADDRESS opt_global.h > CPSW_ETHERSWITCH opt_cpsw.h > -CPU_ARM9Eopt_global.h > CPU_ARM1176 opt_global.h > CPU_CORTEXA opt_global.h > CPU_KRAITopt_global.h > @@ -23,7 +17,6 @@ FREEBSD_BOOT_LOADER opt_global.h > KERNBASE opt_global.h > KERNVIRTADDR opt_global.h > LINUX_BOOT_ABI opt_global.h > -LOADERRAMADDRopt_global.h > LOCORE_MAP_MBopt_locore.h > NKPT2PG opt_pmap.h > PHYSADDR opt_global.h > @@ -31,7 +24,6 @@ PLATFORMopt_global.h > SOCDEV_PAopt_global.h > SOCDEV_VAopt_global.h > PV_STATS opt_pmap.h > -QEMU_WORKAROUNDS opt_global.h > SOC_ALLWINNER_A10opt_global.h > SOC_ALLWINNER_A13opt_global.h > SOC_ALLWINNER_A20opt_global.h > @@ -56,13 +48,6 @@ SOC_MV_KIRKWOODopt_global.h > SOC_MV_ORION opt_global.h > SOC_OMAP3opt_global.h > SOC_OMAP4opt_global.h > -SOC_ROCKCHIP_RK3188 opt_global.h > SOC_TI_AM335Xopt_global.h > -SOC_TEGRA2 opt_global.h > -XSCALE_CACHE_READ_WRITE_ALLOCATE opt_global.h > -VERBOSE_INIT_ARM opt_global.h > VM_MAXUSER_ADDRESS opt_global.h > -GFB_DEBUGopt_gfb.h > -GFB_NO_FONT_LOADING opt_gfb.h > -GFB_NO_MODE_CHANGE opt_gfb.h > VFP opt_global.h > ___ > sv
svn commit: r368360 - head/sys/kern
Author: mjg Date: Sat Dec 5 05:56:23 2020 New Revision: 368360 URL: https://svnweb.freebsd.org/changeset/base/368360 Log: vfs: keep bad ops on vnode reclaim They were only modified to accomodate a redundant assertion. This runs into problems as lockless lookup can still try to use the vnode and crash instead of getting an error. The bug was only present in kernels with INVARIANTS. Reported by: kevans Modified: head/sys/kern/vfs_subr.c Modified: head/sys/kern/vfs_subr.c == --- head/sys/kern/vfs_subr.cSat Dec 5 03:18:48 2020(r368359) +++ head/sys/kern/vfs_subr.cSat Dec 5 05:56:23 2020(r368360) @@ -1816,10 +1816,6 @@ freevnode(struct vnode *vp) destroy_vpollinfo(vp->v_pollinfo); vp->v_pollinfo = NULL; } -#ifdef INVARIANTS - /* XXX Elsewhere we detect an already freed vnode via NULL v_op. */ - vp->v_op = NULL; -#endif vp->v_mountedhere = NULL; vp->v_unpcb = NULL; vp->v_rdev = NULL; @@ -3458,8 +3454,6 @@ vdrop_deactivate(struct vnode *vp) */ VNASSERT(!VN_IS_DOOMED(vp), vp, ("vdrop: returning doomed vnode")); - VNASSERT(vp->v_op != NULL, vp, - ("vdrop: vnode already reclaimed.")); VNASSERT((vp->v_iflag & VI_OWEINACT) == 0, vp, ("vnode with VI_OWEINACT set")); VNASSERT((vp->v_iflag & VI_DEFINACT) == 0, vp, ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r367714 - head/sys/kern
zoo.freebsd.org crashed with what appears to be the same bug, so I went ahead and committed the fix in r368271. On 12/1/20, Hans Petter Selasky wrote: > On 12/1/20 12:26 PM, Mateusz Guzik wrote: >> Does this fix it for you?https://people.freebsd.org/~mjg/poll.diff > > Will take some time to reproduce. Testing right now. > > --HPS > -- Mateusz Guzik ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r368271 - head/sys/kern
On 12/2/20, Mateusz Guzik wrote: > Author: mjg > Date: Wed Dec 2 00:48:15 2020 > New Revision: 368271 > URL: https://svnweb.freebsd.org/changeset/base/368271 > > Log: > select: make sure there are no wakeup attempts after selfdfree returns > > Prior to the patch returning selfdfree could still be racing against > doselwakeup > which set sf_si = NULL and now locks stp to wake up the other thread. > > A sufficiently unlucky pair can end up going all the way down to freeing > select-related structures before the lock/wakeup/unlock finishes. > > This started manifesting itself as crashes since select data started > getting > freed in r367714. > Reported by: hps, mike tancsa > Modified: > head/sys/kern/sys_generic.c > > Modified: head/sys/kern/sys_generic.c > == > --- head/sys/kern/sys_generic.c Wed Dec 2 00:45:35 2020 > (r368270) > +++ head/sys/kern/sys_generic.c Wed Dec 2 00:48:15 2020 > (r368271) > @@ -1820,14 +1820,17 @@ doselwakeup(struct selinfo *sip, int pri) >*/ > TAILQ_REMOVE(>si_tdlist, sfp, sf_threads); > stp = sfp->sf_td; > - /* > - * Paired with selfdfree. > - */ > - atomic_store_rel_ptr((uintptr_t *)>sf_si, (uintptr_t)NULL); > mtx_lock(>st_mtx); > stp->st_flags |= SELTD_PENDING; > cv_broadcastpri(>st_wait, pri); > mtx_unlock(>st_mtx); > + /* > + * Paired with selfdfree. > + * > + * Storing this only after the wakeup provides an invariant that > + * stp is not used after selfdfree returns. > + */ > + atomic_store_rel_ptr((uintptr_t *)>sf_si, (uintptr_t)NULL); > } > mtx_unlock(sip->si_mtx); > } > @@ -1837,14 +1840,18 @@ seltdinit(struct thread *td) > { > struct seltd *stp; > > - if ((stp = td->td_sel) != NULL) > - goto out; > - td->td_sel = stp = malloc(sizeof(*stp), M_SELECT, M_WAITOK|M_ZERO); > + stp = td->td_sel; > + if (stp != NULL) { > + MPASS(stp->st_flags == 0); > + MPASS(STAILQ_EMPTY(>st_selq)); > + return; > + } > + stp = malloc(sizeof(*stp), M_SELECT, M_WAITOK|M_ZERO); > mtx_init(>st_mtx, "sellck", NULL, MTX_DEF); > cv_init(>st_wait, "select"); > -out: > stp->st_flags = 0; > STAILQ_INIT(>st_selq); > + td->td_sel = stp; > } > > static int > @@ -1887,6 +1894,8 @@ seltdfini(struct thread *td) > stp = td->td_sel; > if (stp == NULL) > return; > + MPASS(stp->st_flags == 0); > + MPASS(STAILQ_EMPTY(>st_selq)); > if (stp->st_free1) > free(stp->st_free1, M_SELFD); > if (stp->st_free2) > -- Mateusz Guzik ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r368271 - head/sys/kern
Author: mjg Date: Wed Dec 2 00:48:15 2020 New Revision: 368271 URL: https://svnweb.freebsd.org/changeset/base/368271 Log: select: make sure there are no wakeup attempts after selfdfree returns Prior to the patch returning selfdfree could still be racing against doselwakeup which set sf_si = NULL and now locks stp to wake up the other thread. A sufficiently unlucky pair can end up going all the way down to freeing select-related structures before the lock/wakeup/unlock finishes. This started manifesting itself as crashes since select data started getting freed in r367714. Modified: head/sys/kern/sys_generic.c Modified: head/sys/kern/sys_generic.c == --- head/sys/kern/sys_generic.c Wed Dec 2 00:45:35 2020(r368270) +++ head/sys/kern/sys_generic.c Wed Dec 2 00:48:15 2020(r368271) @@ -1820,14 +1820,17 @@ doselwakeup(struct selinfo *sip, int pri) */ TAILQ_REMOVE(>si_tdlist, sfp, sf_threads); stp = sfp->sf_td; - /* -* Paired with selfdfree. -*/ - atomic_store_rel_ptr((uintptr_t *)>sf_si, (uintptr_t)NULL); mtx_lock(>st_mtx); stp->st_flags |= SELTD_PENDING; cv_broadcastpri(>st_wait, pri); mtx_unlock(>st_mtx); + /* +* Paired with selfdfree. +* +* Storing this only after the wakeup provides an invariant that +* stp is not used after selfdfree returns. +*/ + atomic_store_rel_ptr((uintptr_t *)>sf_si, (uintptr_t)NULL); } mtx_unlock(sip->si_mtx); } @@ -1837,14 +1840,18 @@ seltdinit(struct thread *td) { struct seltd *stp; - if ((stp = td->td_sel) != NULL) - goto out; - td->td_sel = stp = malloc(sizeof(*stp), M_SELECT, M_WAITOK|M_ZERO); + stp = td->td_sel; + if (stp != NULL) { + MPASS(stp->st_flags == 0); + MPASS(STAILQ_EMPTY(>st_selq)); + return; + } + stp = malloc(sizeof(*stp), M_SELECT, M_WAITOK|M_ZERO); mtx_init(>st_mtx, "sellck", NULL, MTX_DEF); cv_init(>st_wait, "select"); -out: stp->st_flags = 0; STAILQ_INIT(>st_selq); + td->td_sel = stp; } static int @@ -1887,6 +1894,8 @@ seltdfini(struct thread *td) stp = td->td_sel; if (stp == NULL) return; + MPASS(stp->st_flags == 0); + MPASS(STAILQ_EMPTY(>st_selq)); if (stp->st_free1) free(stp->st_free1, M_SELFD); if (stp->st_free2) ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r367714 - head/sys/kern
Does this fix it for you? https://people.freebsd.org/~mjg/poll.diff On 12/1/20, Hans Petter Selasky wrote: > On 12/1/20 12:06 PM, Mateusz Guzik wrote: >> I see what the bug is, will think about the right fix. >> >> Is this reproducible for you? > > Yes, I have a crash dump. > > --HPS > > -- Mateusz Guzik ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r367714 - head/sys/kern
I see what the bug is, will think about the right fix. Is this reproducible for you? On 12/1/20, Hans Petter Selasky wrote: > On 11/16/20 4:12 AM, Mateusz Guzik wrote: >> Author: mjg >> Date: Mon Nov 16 03:12:21 2020 >> New Revision: 367714 >> URL: https://svnweb.freebsd.org/changeset/base/367714 >> >> Log: >>select: call seltdfini on process and thread exit >> >>Since thread_zone is marked NOFREE the thread_fini callback is never >>executed, meaning memory allocated by seltdinit is never released. >> >>Adding the call to thread_dtor is not sufficient as exiting processes >>cache the main thread. >> >> Modified: >>head/sys/kern/kern_exit.c >>head/sys/kern/kern_thread.c >> >> Modified: head/sys/kern/kern_exit.c >> == >> --- head/sys/kern/kern_exit.cMon Nov 16 03:09:18 2020 >> (r367713) >> +++ head/sys/kern/kern_exit.cMon Nov 16 03:12:21 2020 >> (r367714) >> @@ -355,6 +355,7 @@ exit1(struct thread *td, int rval, int signo) >> PROC_UNLOCK(p); >> >> umtx_thread_exit(td); >> +seltdfini(td); >> >> /* >> * Reset any sigio structures pointing to us as a result of >> >> Modified: head/sys/kern/kern_thread.c >> == >> --- head/sys/kern/kern_thread.c Mon Nov 16 03:09:18 2020 >> (r367713) >> +++ head/sys/kern/kern_thread.c Mon Nov 16 03:12:21 2020 >> (r367714) >> @@ -329,6 +329,7 @@ thread_ctor(void *mem, int size, void *arg, int >> flags) >> audit_thread_alloc(td); >> #endif >> umtx_thread_alloc(td); >> +MPASS(td->td_sel == NULL); >> return (0); >> } >> >> @@ -369,6 +370,7 @@ thread_dtor(void *mem, int size, void *arg) >> osd_thread_exit(td); >> td_softdep_cleanup(td); >> MPASS(td->td_su == NULL); >> +seltdfini(td); >> } >> >> /* >> @@ -405,7 +407,7 @@ thread_fini(void *mem, int size) >> turnstile_free(td->td_turnstile); >> sleepq_free(td->td_sleepqueue); >> umtx_thread_fini(td); >> -seltdfini(td); >> +MPASS(td->td_sel == NULL); >> } >> >> /* > > Hi, > > The following panic() has been observed after this change: > > panic: Assertion mtx_unowned(m) failed at /usr/src/sys/kern/kern_mutex:1181 > cpuid = 6 > > panic() > _mtx_destroy() > seltdfini() > exit1() > postsig() > ast() > doreti_ast() > > --HPS > > -- Mateusz Guzik ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r368048 - in head/sys: kern sys
Author: mjg Date: Thu Nov 26 06:59:27 2020 New Revision: 368048 URL: https://svnweb.freebsd.org/changeset/base/368048 Log: thread: staticize thread_reap and move td_allocdomain thread_init is a much better fit as the the value is constant after initialization. Modified: head/sys/kern/kern_thread.c head/sys/sys/proc.h Modified: head/sys/kern/kern_thread.c == --- head/sys/kern/kern_thread.c Thu Nov 26 05:58:55 2020(r368047) +++ head/sys/kern/kern_thread.c Thu Nov 26 06:59:27 2020(r368048) @@ -143,6 +143,7 @@ static struct task thread_reap_task; static struct callout thread_reap_callout; static void thread_zombie(struct thread *); +static void thread_reap(void); static void thread_reap_all(void); static void thread_reap_task_cb(void *, int); static void thread_reap_callout_cb(void *); @@ -347,7 +348,6 @@ thread_ctor(void *mem, int size, void *arg, int flags) td = (struct thread *)mem; td->td_state = TDS_INACTIVE; td->td_lastcpu = td->td_oncpu = NOCPU; - td->td_allocdomain = vm_phys_domain(vtophys(td)); /* * Note that td_critnest begins life as 1 because the thread is not @@ -420,6 +420,7 @@ thread_init(void *mem, int size, int flags) td = (struct thread *)mem; + td->td_allocdomain = vm_phys_domain(vtophys(td)); td->td_sleepqueue = sleepq_alloc(); td->td_turnstile = turnstile_alloc(); td->td_rlqe = NULL; @@ -663,7 +664,7 @@ thread_reap_all(void) /* * Reap zombies from local domain. */ -void +static void thread_reap(void) { struct thread_domain_data *tdd; Modified: head/sys/sys/proc.h == --- head/sys/sys/proc.h Thu Nov 26 05:58:55 2020(r368047) +++ head/sys/sys/proc.h Thu Nov 26 06:59:27 2020(r368048) @@ -1142,7 +1142,6 @@ int thread_create(struct thread *td, struct rtprio *rt void thread_exit(void) __dead2; void thread_free(struct thread *td); void thread_link(struct thread *td, struct proc *p); -void thread_reap(void); intthread_single(struct proc *p, int how); void thread_single_end(struct proc *p, int how); void thread_stash(struct thread *td); ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r368039 - head/sys/kern
Author: mjg Date: Wed Nov 25 22:53:21 2020 New Revision: 368039 URL: https://svnweb.freebsd.org/changeset/base/368039 Log: pipe: follow up cleanup to previous The commited patch was incomplete. - add back missing goto retry, noted by jhb - 'if (error)' -> 'if (error != 0)' - consistently do: if (error != 0) break; continue; instead of: if (error != 0) break; else continue; This adds some 'continue' uses which are not needed, but line up with the rest of pipe_write. Modified: head/sys/kern/sys_pipe.c Modified: head/sys/kern/sys_pipe.c == --- head/sys/kern/sys_pipe.cWed Nov 25 21:41:23 2020(r368038) +++ head/sys/kern/sys_pipe.cWed Nov 25 22:53:21 2020(r368039) @@ -994,8 +994,9 @@ retry: error = msleep(wpipe, PIPE_MTX(wpipe), PRIBIO | PCATCH, "pipdww", 0); pipelock(wpipe, 0); - if (error) + if (error != 0) goto error1; + goto retry; } if (wpipe->pipe_buffer.cnt > 0) { if (wpipe->pipe_state & PIPE_WANTR) { @@ -1008,10 +1009,9 @@ retry: error = msleep(wpipe, PIPE_MTX(wpipe), PRIBIO | PCATCH, "pipdwc", 0); pipelock(wpipe, 0); - if (error) + if (error != 0) goto error1; - else - goto retry; + goto retry; } error = pipe_build_write_buffer(wpipe, uio); @@ -1142,7 +1142,7 @@ pipe_write(struct file *fp, struct uio *uio, struct uc wpipe->pipe_buffer.size >= PIPE_MINDIRECT && (fp->f_flag & FNONBLOCK) == 0) { error = pipe_direct_write(wpipe, uio); - if (error) + if (error != 0) break; continue; } @@ -1166,10 +1166,9 @@ pipe_write(struct file *fp, struct uio *uio, struct uc error = msleep(wpipe, PIPE_MTX(rpipe), PRIBIO | PCATCH, "pipbww", 0); pipelock(wpipe, 0); - if (error) + if (error != 0) break; - else - continue; + continue; } space = wpipe->pipe_buffer.size - wpipe->pipe_buffer.cnt; @@ -1243,6 +1242,7 @@ pipe_write(struct file *fp, struct uio *uio, struct uc } if (error != 0) break; + continue; } else { /* * If the "read-side" has been blocked, wake it up now. ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r368038 - head/sys/kern
Author: mjg Date: Wed Nov 25 21:41:23 2020 New Revision: 368038 URL: https://svnweb.freebsd.org/changeset/base/368038 Log: pipe: drop spurious pipeunlock/pipelock cycle on write Modified: head/sys/kern/sys_pipe.c Modified: head/sys/kern/sys_pipe.c == --- head/sys/kern/sys_pipe.cWed Nov 25 21:25:17 2020(r368037) +++ head/sys/kern/sys_pipe.cWed Nov 25 21:41:23 2020(r368038) @@ -979,12 +979,8 @@ pipe_direct_write(struct pipe *wpipe, struct uio *uio) retry: PIPE_LOCK_ASSERT(wpipe, MA_OWNED); - error = pipelock(wpipe, 1); - if (error != 0) - goto error1; if ((wpipe->pipe_state & PIPE_EOF) != 0) { error = EPIPE; - pipeunlock(wpipe); goto error1; } if (wpipe->pipe_state & PIPE_DIRECTW) { @@ -997,10 +993,9 @@ retry: pipeunlock(wpipe); error = msleep(wpipe, PIPE_MTX(wpipe), PRIBIO | PCATCH, "pipdww", 0); + pipelock(wpipe, 0); if (error) goto error1; - else - goto retry; } if (wpipe->pipe_buffer.cnt > 0) { if (wpipe->pipe_state & PIPE_WANTR) { @@ -1012,6 +1007,7 @@ retry: pipeunlock(wpipe); error = msleep(wpipe, PIPE_MTX(wpipe), PRIBIO | PCATCH, "pipdwc", 0); + pipelock(wpipe, 0); if (error) goto error1; else @@ -1020,7 +1016,6 @@ retry: error = pipe_build_write_buffer(wpipe, uio); if (error) { - pipeunlock(wpipe); goto error1; } @@ -1050,7 +1045,6 @@ retry: } else { pipe_destroy_write_buffer(wpipe); } - pipeunlock(wpipe); KASSERT((wpipe->pipe_state & PIPE_DIRECTW) == 0, ("pipe %p leaked PIPE_DIRECTW", wpipe)); return (error); @@ -1124,16 +1118,12 @@ pipe_write(struct file *fp, struct uio *uio, struct uc } MPASS(wpipe->pipe_buffer.size != 0); - pipeunlock(wpipe); - orig_resid = uio->uio_resid; while (uio->uio_resid) { int space; - pipelock(wpipe, 0); if (wpipe->pipe_state & PIPE_EOF) { - pipeunlock(wpipe); error = EPIPE; break; } @@ -1151,7 +1141,6 @@ pipe_write(struct file *fp, struct uio *uio, struct uc uio->uio_iov->iov_len >= PIPE_MINDIRECT && wpipe->pipe_buffer.size >= PIPE_MINDIRECT && (fp->f_flag & FNONBLOCK) == 0) { - pipeunlock(wpipe); error = pipe_direct_write(wpipe, uio); if (error) break; @@ -1176,6 +1165,7 @@ pipe_write(struct file *fp, struct uio *uio, struct uc pipeunlock(wpipe); error = msleep(wpipe, PIPE_MTX(rpipe), PRIBIO | PCATCH, "pipbww", 0); + pipelock(wpipe, 0); if (error) break; else @@ -1251,7 +1241,6 @@ pipe_write(struct file *fp, struct uio *uio, struct uc wpipe->pipe_buffer.size, ("Pipe buffer overflow")); } - pipeunlock(wpipe); if (error != 0) break; } else { @@ -1268,7 +1257,6 @@ pipe_write(struct file *fp, struct uio *uio, struct uc */ if (fp->f_flag & FNONBLOCK) { error = EAGAIN; - pipeunlock(wpipe); break; } @@ -1282,12 +1270,13 @@ pipe_write(struct file *fp, struct uio *uio, struct uc pipeunlock(wpipe); error = msleep(wpipe, PIPE_MTX(rpipe), PRIBIO | PCATCH, "pipewr", 0); + pipelock(wpipe, 0); if (error != 0) break; + continue; } } - pipelock(wpipe, 0); --wpipe->pipe_busy; if ((wpipe->pipe_busy == 0) && (wpipe->pipe_state & PIPE_WANT)) { ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r367978 - head/sys/kern
Author: mjg Date: Tue Nov 24 03:49:37 2020 New Revision: 367978 URL: https://svnweb.freebsd.org/changeset/base/367978 Log: locks: push lock_delay_arg_init calls down Minor cleanup to skip doing them when recursing on locks and so that they can act on found lock value if need be. Modified: head/sys/kern/kern_lock.c head/sys/kern/kern_mutex.c head/sys/kern/kern_rwlock.c head/sys/kern/kern_sx.c Modified: head/sys/kern/kern_lock.c == --- head/sys/kern/kern_lock.c Tue Nov 24 03:48:44 2020(r367977) +++ head/sys/kern/kern_lock.c Tue Nov 24 03:49:37 2020(r367978) @@ -603,10 +603,10 @@ lockmgr_slock_hard(struct lock *lk, u_int flags, struc if (LK_CAN_WITNESS(flags)) WITNESS_CHECKORDER(>lock_object, LOP_NEWORDER, file, line, flags & LK_INTERLOCK ? ilk : NULL); + x = lockmgr_read_value(lk); lock_delay_arg_init(, _delay); if (!lk_adaptive) flags &= ~LK_ADAPTIVE; - x = lockmgr_read_value(lk); /* * The lock may already be locked exclusive by curthread, * avoid deadlock. Modified: head/sys/kern/kern_mutex.c == --- head/sys/kern/kern_mutex.c Tue Nov 24 03:48:44 2020(r367977) +++ head/sys/kern/kern_mutex.c Tue Nov 24 03:49:37 2020(r367978) @@ -535,12 +535,6 @@ __mtx_lock_sleep(volatile uintptr_t *c, uintptr_t v) if (SCHEDULER_STOPPED_TD(td)) return; -#if defined(ADAPTIVE_MUTEXES) - lock_delay_arg_init(, _delay); -#elif defined(KDTRACE_HOOKS) - lock_delay_arg_init_noadapt(); -#endif - if (__predict_false(v == MTX_UNOWNED)) v = MTX_READ_VALUE(m); @@ -562,6 +556,12 @@ __mtx_lock_sleep(volatile uintptr_t *c, uintptr_t v) opts &= ~MTX_RECURSE; #endif +#if defined(ADAPTIVE_MUTEXES) + lock_delay_arg_init(, _delay); +#elif defined(KDTRACE_HOOKS) + lock_delay_arg_init_noadapt(); +#endif + #ifdef HWPMC_HOOKS PMC_SOFT_CALL( , , lock, failed); #endif @@ -746,12 +746,12 @@ _mtx_lock_spin_cookie(volatile uintptr_t *c, uintptr_t if (SCHEDULER_STOPPED()) return; - lock_delay_arg_init(, _spin_delay); - if (LOCK_LOG_TEST(>lock_object, opts)) CTR1(KTR_LOCK, "_mtx_lock_spin: %p spinning", m); KTR_STATE1(KTR_SCHED, "thread", sched_tdname((struct thread *)tid), "spinning", "lockname:\"%s\"", m->lock_object.lo_name); + + lock_delay_arg_init(, _spin_delay); #ifdef HWPMC_HOOKS PMC_SOFT_CALL( , , lock, failed); Modified: head/sys/kern/kern_rwlock.c == --- head/sys/kern/kern_rwlock.c Tue Nov 24 03:48:44 2020(r367977) +++ head/sys/kern/kern_rwlock.c Tue Nov 24 03:49:37 2020(r367978) @@ -948,11 +948,6 @@ __rw_wlock_hard(volatile uintptr_t *c, uintptr_t v LOC if (SCHEDULER_STOPPED()) return; -#if defined(ADAPTIVE_RWLOCKS) - lock_delay_arg_init(, _delay); -#elif defined(KDTRACE_HOOKS) - lock_delay_arg_init_noadapt(); -#endif if (__predict_false(v == RW_UNLOCKED)) v = RW_READ_VALUE(rw); @@ -970,6 +965,12 @@ __rw_wlock_hard(volatile uintptr_t *c, uintptr_t v LOC if (LOCK_LOG_TEST(>lock_object, 0)) CTR5(KTR_LOCK, "%s: %s contested (lock=%p) at %s:%d", __func__, rw->lock_object.lo_name, (void *)rw->rw_lock, file, line); + +#if defined(ADAPTIVE_RWLOCKS) + lock_delay_arg_init(, _delay); +#elif defined(KDTRACE_HOOKS) + lock_delay_arg_init_noadapt(); +#endif #ifdef HWPMC_HOOKS PMC_SOFT_CALL( , , lock, failed); Modified: head/sys/kern/kern_sx.c == --- head/sys/kern/kern_sx.c Tue Nov 24 03:48:44 2020(r367977) +++ head/sys/kern/kern_sx.c Tue Nov 24 03:49:37 2020(r367978) @@ -620,12 +620,6 @@ _sx_xlock_hard(struct sx *sx, uintptr_t x, int opts LO if (SCHEDULER_STOPPED()) return (0); -#if defined(ADAPTIVE_SX) - lock_delay_arg_init(, _delay); -#elif defined(KDTRACE_HOOKS) - lock_delay_arg_init_noadapt(); -#endif - if (__predict_false(x == SX_LOCK_UNLOCKED)) x = SX_READ_VALUE(sx); @@ -644,6 +638,12 @@ _sx_xlock_hard(struct sx *sx, uintptr_t x, int opts LO if (LOCK_LOG_TEST(>lock_object, 0)) CTR5(KTR_LOCK, "%s: %s contested (lock=%p) at %s:%d", __func__, sx->lock_object.lo_name, (void *)sx->sx_lock, file, line); + +#if defined(ADAPTIVE_SX) + lock_delay_arg_init(, _delay); +#elif defined(KDTRACE_HOOKS) + lock_delay_arg_init_noadapt(); +#endif #ifdef HWPMC_HOOKS PMC_SOFT_CALL( , , lock, failed);
svn commit: r367977 - head/sys/kern
Author: mjg Date: Tue Nov 24 03:48:44 2020 New Revision: 367977 URL: https://svnweb.freebsd.org/changeset/base/367977 Log: sx: drop spurious volatile keyword Modified: head/sys/kern/kern_sx.c Modified: head/sys/kern/kern_sx.c == --- head/sys/kern/kern_sx.c Tue Nov 24 02:51:45 2020(r367976) +++ head/sys/kern/kern_sx.c Tue Nov 24 03:48:44 2020(r367977) @@ -573,7 +573,7 @@ _sx_xlock_hard(struct sx *sx, uintptr_t x, int opts LO GIANT_DECLARE; uintptr_t tid, setx; #ifdef ADAPTIVE_SX - volatile struct thread *owner; + struct thread *owner; u_int i, n, spintries = 0; enum { READERS, WRITER } sleep_reason = READERS; bool in_critical = false; @@ -1020,7 +1020,7 @@ _sx_slock_hard(struct sx *sx, int opts, uintptr_t x LO GIANT_DECLARE; struct thread *td; #ifdef ADAPTIVE_SX - volatile struct thread *owner; + struct thread *owner; u_int i, n, spintries = 0; #endif #ifdef LOCK_PROFILING ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r367962 - in head/sys: kern sys
Author: mjg Date: Mon Nov 23 18:27:21 2020 New Revision: 367962 URL: https://svnweb.freebsd.org/changeset/base/367962 Log: dtrace: stop using eventhandlers for the part compiled into the kernel Reviewed by: kib, markj Differential Revision:https://reviews.freebsd.org/D27311 Modified: head/sys/kern/init_main.c head/sys/kern/kern_dtrace.c head/sys/kern/kern_proc.c head/sys/kern/kern_thread.c head/sys/sys/dtrace_bsd.h Modified: head/sys/kern/init_main.c == --- head/sys/kern/init_main.c Mon Nov 23 18:26:47 2020(r367961) +++ head/sys/kern/init_main.c Mon Nov 23 18:27:21 2020(r367962) @@ -65,6 +65,7 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include #include #include #include @@ -607,6 +608,10 @@ proc0_init(void *dummy __unused) */ EVENTHANDLER_DIRECT_INVOKE(process_init, p); EVENTHANDLER_DIRECT_INVOKE(thread_init, td); +#ifdef KDTRACE_HOOKS + kdtrace_proc_ctor(p); + kdtrace_thread_ctor(td); +#endif EVENTHANDLER_DIRECT_INVOKE(process_ctor, p); EVENTHANDLER_DIRECT_INVOKE(thread_ctor, td); Modified: head/sys/kern/kern_dtrace.c == --- head/sys/kern/kern_dtrace.c Mon Nov 23 18:26:47 2020(r367961) +++ head/sys/kern/kern_dtrace.c Mon Nov 23 18:27:21 2020(r367962) @@ -67,21 +67,19 @@ kdtrace_proc_size() return (KDTRACE_PROC_SIZE); } -static void -kdtrace_proc_ctor(void *arg __unused, struct proc *p) +void +kdtrace_proc_ctor(struct proc *p) { p->p_dtrace = malloc(KDTRACE_PROC_SIZE, M_KDTRACE, M_WAITOK|M_ZERO); } -static void -kdtrace_proc_dtor(void *arg __unused, struct proc *p) +void +kdtrace_proc_dtor(struct proc *p) { - if (p->p_dtrace != NULL) { - free(p->p_dtrace, M_KDTRACE); - p->p_dtrace = NULL; - } + free(p->p_dtrace, M_KDTRACE); + p->p_dtrace = NULL; } /* Return the DTrace thread data size compiled in the kernel hooks. */ @@ -92,38 +90,17 @@ kdtrace_thread_size() return (KDTRACE_THREAD_SIZE); } -static void -kdtrace_thread_ctor(void *arg __unused, struct thread *td) +void +kdtrace_thread_ctor(struct thread *td) { td->td_dtrace = malloc(KDTRACE_THREAD_SIZE, M_KDTRACE, M_WAITOK|M_ZERO); } -static void -kdtrace_thread_dtor(void *arg __unused, struct thread *td) +void +kdtrace_thread_dtor(struct thread *td) { - if (td->td_dtrace != NULL) { - free(td->td_dtrace, M_KDTRACE); - td->td_dtrace = NULL; - } + free(td->td_dtrace, M_KDTRACE); + td->td_dtrace = NULL; } - -/* - * Initialise the kernel DTrace hooks. - */ -static void -init_dtrace(void *dummy __unused) -{ - - EVENTHANDLER_REGISTER(process_ctor, kdtrace_proc_ctor, NULL, - EVENTHANDLER_PRI_ANY); - EVENTHANDLER_REGISTER(process_dtor, kdtrace_proc_dtor, NULL, - EVENTHANDLER_PRI_ANY); - EVENTHANDLER_REGISTER(thread_ctor, kdtrace_thread_ctor, NULL, - EVENTHANDLER_PRI_ANY); - EVENTHANDLER_REGISTER(thread_dtor, kdtrace_thread_dtor, NULL, - EVENTHANDLER_PRI_ANY); -} - -SYSINIT(kdtrace, SI_SUB_KDTRACE, SI_ORDER_FIRST, init_dtrace, NULL); Modified: head/sys/kern/kern_proc.c == --- head/sys/kern/kern_proc.c Mon Nov 23 18:26:47 2020(r367961) +++ head/sys/kern/kern_proc.c Mon Nov 23 18:27:21 2020(r367962) @@ -65,6 +65,7 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include #include #include #include @@ -207,6 +208,9 @@ proc_ctor(void *mem, int size, void *arg, int flags) struct thread *td; p = (struct proc *)mem; +#ifdef KDTRACE_HOOKS + kdtrace_proc_ctor(p); +#endif EVENTHANDLER_DIRECT_INVOKE(process_ctor, p); td = FIRST_THREAD_IN_PROC(p); if (td != NULL) { @@ -243,6 +247,9 @@ proc_dtor(void *mem, int size, void *arg) EVENTHANDLER_DIRECT_INVOKE(thread_dtor, td); } EVENTHANDLER_DIRECT_INVOKE(process_dtor, p); +#ifdef KDTRACE_HOOKS + kdtrace_proc_dtor(p); +#endif if (p->p_ksi != NULL) KASSERT(! KSI_ONQ(p->p_ksi), ("SIGCHLD queue")); } Modified: head/sys/kern/kern_thread.c == --- head/sys/kern/kern_thread.c Mon Nov 23 18:26:47 2020(r367961) +++ head/sys/kern/kern_thread.c Mon Nov 23 18:27:21 2020(r367962) @@ -50,6 +50,7 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include #include #include #include @@ -358,6 +359,9 @@ thread_ctor(void *mem, int size, void *arg, int flags) #ifdef AUDIT audit_thread_alloc(td); #endif +#ifdef KDTRACE_HOOKS + kdtrace_thread_ctor(td); +#endif umtx_thread_alloc(td);
svn commit: r367961 - in head/sys: kern sys
Author: mjg Date: Mon Nov 23 18:26:47 2020 New Revision: 367961 URL: https://svnweb.freebsd.org/changeset/base/367961 Log: thread: stash domain id to work around vtophys problems on ppc64 Adding to zombie list can be perfomed by idle threads, which on ppc64 leads to panics as it requires a sleepable lock. Reported by: alfredo Reviewed by: kib, markj Fixes:r367842 ("thread: numa-aware zombie reaping") Differential Revision:https://reviews.freebsd.org/D27288 Modified: head/sys/kern/kern_thread.c head/sys/sys/proc.h Modified: head/sys/kern/kern_thread.c == --- head/sys/kern/kern_thread.c Mon Nov 23 18:18:16 2020(r367960) +++ head/sys/kern/kern_thread.c Mon Nov 23 18:26:47 2020(r367961) @@ -346,6 +346,7 @@ thread_ctor(void *mem, int size, void *arg, int flags) td = (struct thread *)mem; td->td_state = TDS_INACTIVE; td->td_lastcpu = td->td_oncpu = NOCPU; + td->td_allocdomain = vm_phys_domain(vtophys(td)); /* * Note that td_critnest begins life as 1 because the thread is not @@ -544,7 +545,7 @@ thread_zombie(struct thread *td) struct thread_domain_data *tdd; struct thread *ztd; - tdd = _domain_data[vm_phys_domain(vtophys(td))]; + tdd = _domain_data[td->td_allocdomain]; ztd = atomic_load_ptr(>tdd_zombies); for (;;) { td->td_zombie = ztd; Modified: head/sys/sys/proc.h == --- head/sys/sys/proc.h Mon Nov 23 18:18:16 2020(r367960) +++ head/sys/sys/proc.h Mon Nov 23 18:26:47 2020(r367961) @@ -246,6 +246,7 @@ struct thread { sigqueue_t td_sigqueue;/* (c) Sigs arrived, not delivered. */ #definetd_siglist td_sigqueue.sq_signals u_char td_lend_user_pri; /* (t) Lend user pri. */ + u_char td_allocdomain; /* (b) NUMA domain backing this struct thread. */ /* Cleared during fork1() */ #definetd_startzero td_flags ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r367852 - in head/sys: kern sys
Author: mjg Date: Thu Nov 19 19:25:47 2020 New Revision: 367852 URL: https://svnweb.freebsd.org/changeset/base/367852 Log: pipe: thundering herd problem in pipelock All reads and writes are serialized with a hand-rolled lock, but unlocking it always wakes up all waiters. Existing flag fields get resized to make room for introduction of waiter counter without growing the struct. Reviewed by: kib Differential Revision:https://reviews.freebsd.org/D27273 Modified: head/sys/kern/sys_pipe.c head/sys/sys/pipe.h Modified: head/sys/kern/sys_pipe.c == --- head/sys/kern/sys_pipe.cThu Nov 19 19:05:16 2020(r367851) +++ head/sys/kern/sys_pipe.cThu Nov 19 19:25:47 2020(r367852) @@ -622,9 +622,13 @@ pipelock(struct pipe *cpipe, int catch) if (catch) prio |= PCATCH; while (cpipe->pipe_state & PIPE_LOCKFL) { - cpipe->pipe_state |= PIPE_LWANT; + KASSERT(cpipe->pipe_waiters >= 0, + ("%s: bad waiter count %d", __func__, + cpipe->pipe_waiters)); + cpipe->pipe_waiters++; error = msleep(cpipe, PIPE_MTX(cpipe), prio, "pipelk", 0); + cpipe->pipe_waiters--; if (error != 0) return (error); } @@ -642,10 +646,12 @@ pipeunlock(struct pipe *cpipe) PIPE_LOCK_ASSERT(cpipe, MA_OWNED); KASSERT(cpipe->pipe_state & PIPE_LOCKFL, ("Unlocked pipe passed to pipeunlock")); + KASSERT(cpipe->pipe_waiters >= 0, + ("%s: bad waiter count %d", __func__, + cpipe->pipe_waiters)); cpipe->pipe_state &= ~PIPE_LOCKFL; - if (cpipe->pipe_state & PIPE_LWANT) { - cpipe->pipe_state &= ~PIPE_LWANT; - wakeup(cpipe); + if (cpipe->pipe_waiters > 0) { + wakeup_one(cpipe); } } Modified: head/sys/sys/pipe.h == --- head/sys/sys/pipe.h Thu Nov 19 19:05:16 2020(r367851) +++ head/sys/sys/pipe.h Thu Nov 19 19:25:47 2020(r367852) @@ -116,9 +116,10 @@ struct pipe { struct pipe *pipe_peer;/* link with other direction */ struct pipepair *pipe_pair;/* container structure pointer */ u_short pipe_state; /* pipe status info */ - u_short pipe_type; /* pipe type info */ + u_char pipe_type; /* pipe type info */ + u_char pipe_present; /* still present? */ + int pipe_waiters; /* pipelock waiters */ int pipe_busy; /* busy flag, mostly to handle rundown sanely */ - int pipe_present; /* still present? */ int pipe_wgen; /* writer generation for named pipe */ ino_t pipe_ino; /* fake inode for stat(2) */ }; ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r367842 - head/sys/kern
Author: mjg Date: Thu Nov 19 10:00:48 2020 New Revision: 367842 URL: https://svnweb.freebsd.org/changeset/base/367842 Log: thread: numa-aware zombie reaping The current global list is a significant problem, in particular induces a lot of cross-domain thread frees. When running poudriere on a 2 domain box about half of all frees were of that nature. Patch below introduces per-domain thread data containing zombie lists and domain-aware reaping. By default it only reaps from the current domain, only reaping from others if there is free TID shortage. A dedicated callout is introduced to reap lingering threads if there happens to be no activity. Reviewed by: kib, markj Differential Revision:https://reviews.freebsd.org/D27185 Modified: head/sys/kern/kern_thread.c Modified: head/sys/kern/kern_thread.c == --- head/sys/kern/kern_thread.c Thu Nov 19 09:26:51 2020(r367841) +++ head/sys/kern/kern_thread.c Thu Nov 19 10:00:48 2020(r367842) @@ -52,6 +52,7 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include #include #include #include @@ -64,9 +65,11 @@ __FBSDID("$FreeBSD$"); #include +#include #include #include #include +#include #include /* @@ -128,9 +131,20 @@ SDT_PROBE_DEFINE(proc, , , lwp__exit); */ static uma_zone_t thread_zone; -static __exclusive_cache_line struct thread *thread_zombies; +struct thread_domain_data { + struct thread *tdd_zombies; + int tdd_reapticks; +} __aligned(CACHE_LINE_SIZE); +static struct thread_domain_data thread_domain_data[MAXMEMDOM]; + +static struct task thread_reap_task; +static struct callout thread_reap_callout; + static void thread_zombie(struct thread *); +static void thread_reap_all(void); +static void thread_reap_task_cb(void *, int); +static void thread_reap_callout_cb(void *); static int thread_unsuspend_one(struct thread *td, struct proc *p, bool boundary); static void thread_free_batched(struct thread *td); @@ -159,30 +173,45 @@ EVENTHANDLER_LIST_DEFINE(thread_init); EVENTHANDLER_LIST_DEFINE(thread_fini); static bool -thread_count_inc(void) +thread_count_inc_try(void) { - static struct timeval lastfail; - static int curfail; int nthreads_new; - thread_reap(); - nthreads_new = atomic_fetchadd_int(, 1) + 1; if (nthreads_new >= maxthread - 100) { if (priv_check_cred(curthread->td_ucred, PRIV_MAXPROC) != 0 || nthreads_new >= maxthread) { atomic_subtract_int(, 1); - if (ppsratecheck(, , 1)) { - printf("maxthread limit exceeded by uid %u " - "(pid %d); consider increasing kern.maxthread\n", - curthread->td_ucred->cr_ruid, curproc->p_pid); - } return (false); } } return (true); } +static bool +thread_count_inc(void) +{ + static struct timeval lastfail; + static int curfail; + + thread_reap(); + if (thread_count_inc_try()) { + return (true); + } + + thread_reap_all(); + if (thread_count_inc_try()) { + return (true); + } + + if (ppsratecheck(, , 1)) { + printf("maxthread limit exceeded by uid %u " + "(pid %d); consider increasing kern.maxthread\n", + curthread->td_ucred->cr_ruid, curproc->p_pid); + } + return (false); +} + static void thread_count_sub(int n) { @@ -500,6 +529,10 @@ threadinit(void) M_TIDHASH, M_WAITOK | M_ZERO); for (i = 0; i < tidhashlock + 1; i++) rw_init(_lock[i], "tidhash"); + + TASK_INIT(_reap_task, 0, thread_reap_task_cb, NULL); + callout_init(_reap_callout, 1); + callout_reset(_reap_callout, 5 * hz, thread_reap_callout_cb, NULL); } /* @@ -508,12 +541,14 @@ threadinit(void) void thread_zombie(struct thread *td) { + struct thread_domain_data *tdd; struct thread *ztd; - ztd = atomic_load_ptr(_zombies); + tdd = _domain_data[vm_phys_domain(vtophys(td))]; + ztd = atomic_load_ptr(>tdd_zombies); for (;;) { td->td_zombie = ztd; - if (atomic_fcmpset_rel_ptr((uintptr_t *)_zombies, + if (atomic_fcmpset_rel_ptr((uintptr_t *)>tdd_zombies, (uintptr_t *), (uintptr_t)td)) break; continue; @@ -531,10 +566,10 @@ thread_stash(struct thread *td) } /* - * Reap zombie threads. + * Reap zombies from passed domain. */ -void -thread_reap(void) +static void +thread_reap_domain(struct thread_domain_data *tdd) { struct thread *itd, *ntd; struct tidbatch tidbatch; @@ -547,19 +582,26 @@
svn commit: r367835 - head/sys/kern
Author: mjg Date: Thu Nov 19 08:16:45 2020 New Revision: 367835 URL: https://svnweb.freebsd.org/changeset/base/367835 Log: pipe: tidy up pipelock Modified: head/sys/kern/sys_pipe.c Modified: head/sys/kern/sys_pipe.c == --- head/sys/kern/sys_pipe.cThu Nov 19 07:23:39 2020(r367834) +++ head/sys/kern/sys_pipe.cThu Nov 19 08:16:45 2020(r367835) @@ -614,14 +614,17 @@ pipespace(struct pipe *cpipe, int size) static __inline int pipelock(struct pipe *cpipe, int catch) { - int error; + int error, prio; PIPE_LOCK_ASSERT(cpipe, MA_OWNED); + + prio = PRIBIO; + if (catch) + prio |= PCATCH; while (cpipe->pipe_state & PIPE_LOCKFL) { cpipe->pipe_state |= PIPE_LWANT; error = msleep(cpipe, PIPE_MTX(cpipe), - catch ? (PRIBIO | PCATCH) : PRIBIO, - "pipelk", 0); + prio, "pipelk", 0); if (error != 0) return (error); } ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r367833 - in head/sys: kern security/mac sys
Author: mjg Date: Thu Nov 19 06:30:25 2020 New Revision: 367833 URL: https://svnweb.freebsd.org/changeset/base/367833 Log: pipe: allow for lockless pipe_stat pipes get stated all thet time and this avoidably contributed to contention. The pipe lock is only held to accomodate MAC and to check the type. Since normally there is no probe for pipe stat depessimize this by having the flag. The pipe_state field gets modified with locks held all the time and it's not feasible to convert them to use atomic store. Move the type flag away to a separate variable as a simple cleanup and to provide stable field to read. Use short for both fields to avoid growing the struct. While here short-circuit MAC for pipe_poll as well. Modified: head/sys/kern/sys_pipe.c head/sys/security/mac/mac_framework.c head/sys/security/mac/mac_framework.h head/sys/security/mac/mac_pipe.c head/sys/sys/pipe.h Modified: head/sys/kern/sys_pipe.c == --- head/sys/kern/sys_pipe.cThu Nov 19 05:46:59 2020(r367832) +++ head/sys/kern/sys_pipe.cThu Nov 19 06:30:25 2020(r367833) @@ -140,7 +140,7 @@ __FBSDID("$FreeBSD$"); /* #define PIPE_NODIRECT */ #define PIPE_PEER(pipe)\ - (((pipe)->pipe_state & PIPE_NAMED) ? (pipe) : ((pipe)->pipe_peer)) + (((pipe)->pipe_type & PIPE_TYPE_NAMED) ? (pipe) : ((pipe)->pipe_peer)) /* * interfaces to the outside world @@ -403,7 +403,7 @@ pipe_named_ctor(struct pipe **ppipe, struct thread *td error = pipe_paircreate(td, ); if (error != 0) return (error); - pp->pp_rpipe.pipe_state |= PIPE_NAMED; + pp->pp_rpipe.pipe_type |= PIPE_TYPE_NAMED; *ppipe = >pp_rpipe; return (0); } @@ -413,7 +413,7 @@ pipe_dtor(struct pipe *dpipe) { struct pipe *peer; - peer = (dpipe->pipe_state & PIPE_NAMED) != 0 ? dpipe->pipe_peer : NULL; + peer = (dpipe->pipe_type & PIPE_TYPE_NAMED) != 0 ? dpipe->pipe_peer : NULL; funsetown(>pipe_sigio); pipeclose(dpipe); if (peer != NULL) { @@ -1328,7 +1328,7 @@ pipe_truncate(struct file *fp, off_t length, struct uc int error; cpipe = fp->f_data; - if (cpipe->pipe_state & PIPE_NAMED) + if (cpipe->pipe_type & PIPE_TYPE_NAMED) error = vnops.fo_truncate(fp, length, active_cred, td); else error = invfo_truncate(fp, length, active_cred, td); @@ -1443,7 +1443,7 @@ pipe_poll(struct file *fp, int events, struct ucred *a levents = events & (POLLIN | POLLINIGNEOF | POLLPRI | POLLRDNORM | POLLRDBAND); - if (rpipe->pipe_state & PIPE_NAMED && fp->f_flag & FREAD && levents && + if (rpipe->pipe_type & PIPE_TYPE_NAMED && fp->f_flag & FREAD && levents && fp->f_pipegen == rpipe->pipe_wgen) events |= POLLINIGNEOF; @@ -1496,23 +1496,22 @@ pipe_stat(struct file *fp, struct stat *ub, struct ucr #endif pipe = fp->f_data; - PIPE_LOCK(pipe); #ifdef MAC - error = mac_pipe_check_stat(active_cred, pipe->pipe_pair); - if (error) { + if (mac_pipe_check_stat_enabled()) { + PIPE_LOCK(pipe); + error = mac_pipe_check_stat(active_cred, pipe->pipe_pair); PIPE_UNLOCK(pipe); - return (error); + if (error) { + return (error); + } } #endif /* For named pipes ask the underlying filesystem. */ - if (pipe->pipe_state & PIPE_NAMED) { - PIPE_UNLOCK(pipe); + if (pipe->pipe_type & PIPE_TYPE_NAMED) { return (vnops.fo_stat(fp, ub, active_cred, td)); } - PIPE_UNLOCK(pipe); - bzero(ub, sizeof(*ub)); ub->st_mode = S_IFIFO; ub->st_blksize = PAGE_SIZE; @@ -1554,7 +1553,7 @@ pipe_chmod(struct file *fp, mode_t mode, struct ucred int error; cpipe = fp->f_data; - if (cpipe->pipe_state & PIPE_NAMED) + if (cpipe->pipe_type & PIPE_TYPE_NAMED) error = vn_chmod(fp, mode, active_cred, td); else error = invfo_chmod(fp, mode, active_cred, td); @@ -1569,7 +1568,7 @@ pipe_chown(struct file *fp, uid_t uid, gid_t gid, stru int error; cpipe = fp->f_data; - if (cpipe->pipe_state & PIPE_NAMED) + if (cpipe->pipe_type & PIPE_TYPE_NAMED) error = vn_chown(fp, uid, gid, active_cred, td); else error = invfo_chown(fp, uid, gid, active_cred, td); @@ -1758,7 +1757,7 @@ filt_piperead(struct knote *kn, long hint) kn->kn_data = rpipe->pipe_pages.cnt; if ((rpipe->pipe_state & PIPE_EOF) != 0 && - ((rpipe->pipe_state & PIPE_NAMED) == 0 || + ((rpipe->pipe_type & PIPE_TYPE_NAMED) == 0 || fp->f_pipegen != rpipe->pipe_wgen)) { kn->kn_flags |=
Re: svn commit: r367695 - in head/sys: kern sys
On 11/19/20, John Baldwin wrote: > On 11/18/20 2:16 PM, Mateusz Guzik wrote: >> On 11/17/20, John Baldwin wrote: >>> On 11/14/20 11:22 AM, Mateusz Guzik wrote: >> Interested parties can check the consumer (also seen in the diff) to >> see this is for consistency. I don't think any comments are warranted >> in the header. > > I did read the consumer, and there didn't seem tremendous value in the > extra line there. > One thing to note is that there are more thing to batch than currently implemented, meaning the established pattern is going get more users. With everyone implementing the same routines, even if nops, it is pretty clear what's going on. In contrast if random calls are missing the reader is left wondering if there is a bug. Either way I see no reason to either comment add a comment in the header nor to remove the nop func. >>> These changes would benefit from review. >>> >> >> I don't think it's feasible to ask for review for everything lest it >> degardes to rubber stamping and I don't think this change warranted >> it, regardless of the cosmetic issues which can always show up. > > That is not consistent with the direction the project is moving. If you > check the commit logs of other high-volume committers such as markj@, > kib@, or myself, you will find that a substantial number of those commits > are reviewed (typically in phabricator) without preventing us from > making useful progress. Also, while the previous core did not mandate > reviews, we moved closer to it when the Pre-Commit Review chapter was > added to the Committer's Guide: > > https://www.freebsd.org/doc/en_US.ISO8859-1/articles/committers-guide/pre-commit-review.html > > In the related thread on developers@ we indicated that while weren't yet > making pre-commit review mandatory, we collectively want to move in that > direction. > If you exclude bite-size commits, you will see I am getting reviews for thing which are outside of my work area or which include design choices. Past that other people keep committing without reviews anyway. That said, it may be this patch indeed should have been reviewed, but as it is I don't think this is the case. -- Mateusz Guzik ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r367830 - in head/sys: kern sys
Author: mjg Date: Thu Nov 19 04:28:39 2020 New Revision: 367830 URL: https://svnweb.freebsd.org/changeset/base/367830 Log: cred: fix minor nits in r367695 Noted by: jhb Modified: head/sys/kern/kern_prot.c head/sys/sys/ucred.h Modified: head/sys/kern/kern_prot.c == --- head/sys/kern/kern_prot.c Thu Nov 19 04:27:51 2020(r367829) +++ head/sys/kern/kern_prot.c Thu Nov 19 04:28:39 2020(r367830) @@ -2082,6 +2082,7 @@ crfree_final(struct ucred *cr) __func__, cr->cr_users, cr)); KASSERT(cr->cr_ref == 0, ("%s: ref %d not == 0 on cred %p", __func__, cr->cr_ref, cr)); + /* * Some callers of crget(), such as nfs_statfs(), allocate a temporary * credential, but don't allocate a uidinfo structure. Modified: head/sys/sys/ucred.h == --- head/sys/sys/ucred.hThu Nov 19 04:27:51 2020(r367829) +++ head/sys/sys/ucred.hThu Nov 19 04:28:39 2020(r367830) @@ -128,12 +128,13 @@ credbatch_prep(struct credbatch *crb) crb->ref = 0; } void credbatch_add(struct credbatch *crb, struct thread *td); + static inline void credbatch_process(struct credbatch *crb __unused) { } -void credbatch_add(struct credbatch *crb, struct thread *td); + void credbatch_final(struct credbatch *crb); void change_egid(struct ucred *newcred, gid_t egid); ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r367829 - head/sys/kern
Author: mjg Date: Thu Nov 19 04:27:51 2020 New Revision: 367829 URL: https://svnweb.freebsd.org/changeset/base/367829 Log: smp: fix smp_rendezvous_cpus_retry usage before smp starts Since none of the other CPUs are running there is nobody to clear their entries and the routine spins indefinitely. Modified: head/sys/kern/subr_smp.c Modified: head/sys/kern/subr_smp.c == --- head/sys/kern/subr_smp.cThu Nov 19 03:59:21 2020(r367828) +++ head/sys/kern/subr_smp.cThu Nov 19 04:27:51 2020(r367829) @@ -896,6 +896,21 @@ smp_rendezvous_cpus_retry(cpuset_t map, int cpu; /* +* Only one CPU to execute on. +*/ + if (!smp_started) { + spinlock_enter(); + if (setup_func != NULL) + setup_func(arg); + if (action_func != NULL) + action_func(arg); + if (teardown_func != NULL) + teardown_func(arg); + spinlock_exit(); + return; + } + + /* * Execute an action on all specified CPUs while retrying until they * all acknowledge completion. */ ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r367813 - head/lib/libutil
On 11/19/20, Stefan Esser wrote: > Am 18.11.20 um 23:39 schrieb Jessica Clarke: >> On 18 Nov 2020, at 22:32, Stefan Esser wrote: >>> >>> Am 18.11.20 um 22:40 schrieb Mateusz Guzik: >>>>> +{ >>>>> + static const int localbase_oid[2] = {CTL_USER, USER_LOCALBASE}; >>>> There is no use for this to be static. >>> >>> Why not? This makes it part of the constant initialized data of >>> the library, which is more efficient under run-time and memory >>> space aspects than initializing them on the stack. >>> >>> What do I miss? >> >> What is more efficient is not so clear-cut, it depends on things like >> the architecture, microarchitecture and ABI. Allocating a small buffer >> on the stack is extremely cheap (the memory will almost certainly be in >> the L1 cache), whereas globally-allocated storage is less likely to be >> in the cache due to being spread out, and on some architecture/ABI >> combinations will incur additional indirection through the GOT. Also, 8 >> bytes of additional stack use is lost in the noise. > > The memory latency of the extra access to the constant will be hidden in > the noise. The data will probably be in a page that has already been > accessed (so no TLB load penalty) and modern CPUs will be able to deal > with the cache miss (if any, because the cache line may already be > loaded depending on other data near-by). > > Yes, I do agree that a stack local variable could have been used and > it could have been created with little effort by a modern multi-issue > CPU. The code size would have been larger, though, by some 10 to 20 > bytes, I'd assume - but I doubt a single path through this code is > measurable, much less observable in practice. > > We are talking about nano-seconds here (even if the cache line did > not contain the constant data, it would probably be accessed just a > few instructions further down and incur the same latency then). > > I have followed CPU development over more than 45 years and know many > architectures and their specifics, but the time were I have programmed > drivers in assembly and counted instruction cycles is long gone. > > This is nitpicking at a level that I do not want to continue. I'm not > opposed to achieving efficiency where it is relevant. This function is > providing useful functionality and I do not mind a wasted microsecond, > it is not relevant here. (This was different if it was wasted within > a tight loop - but it is not, it is typically called once if at all). > > Feel free to replace my code with your implementation, if you think it > is not acceptable as written by me. > > I just wanted to provide an implementation of this functionality to > be used in a number of programs where other developers had expressed > interest in such a feature (and one of these programs has been worked > on by me in recent weeks, so I'm now able to make use of it myself). > The entire localbase saga is getting way out of hand. To address this e-mail and things you wrote in another reply, my comlaints are very simple and are not getting less valid for not being raised sooner. I just was not watching any of this until recent fallout. For the change at hand, there has to be a reason to use a static symbol. Standard example is catching otherwise expensive to obtain data. For that the static localbase pointer makes perfect sense, while the static lookup array does not have any justification that I see. Bringing cache, TLB or whatever microarchitectural details into the discussion is beyond not warranted. More importantly though the commit comes with a self-confessed memory leak and is a show stopper. That said, I'll see about patching this up. -- Mateusz Guzik ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r367695 - in head/sys: kern sys
On 11/17/20, John Baldwin wrote: > On 11/14/20 11:22 AM, Mateusz Guzik wrote: >> Author: mjg >> Date: Sat Nov 14 19:22:02 2020 >> New Revision: 367695 >> URL: https://svnweb.freebsd.org/changeset/base/367695 >> >> Log: >> thread: batch credential freeing >> >> Modified: >> head/sys/kern/kern_prot.c >> head/sys/kern/kern_thread.c >> head/sys/sys/ucred.h >> >> Modified: head/sys/kern/kern_prot.c >> == >> --- head/sys/kern/kern_prot.cSat Nov 14 19:21:46 2020 >> (r367694) >> +++ head/sys/kern/kern_prot.cSat Nov 14 19:22:02 2020 >> (r367695) >> @@ -2007,6 +2071,17 @@ crfree(struct ucred *cr) >> mtx_unlock(>cr_mtx); >> return; >> } >> +crfree_final(cr); >> +} >> + >> +static void >> +crfree_final(struct ucred *cr) >> +{ >> + >> +KASSERT(cr->cr_users == 0, ("%s: users %d not == 0 on cred %p", >> +__func__, cr->cr_users, cr)); >> +KASSERT(cr->cr_ref == 0, ("%s: ref %d not == 0 on cred %p", >> +__func__, cr->cr_ref, cr)); >> /* > > Please add blank lines before comments. It's in style(9) and I've noticed > a pattern in your changes of not including them. > Ok. >> Modified: head/sys/sys/ucred.h >> == >> --- head/sys/sys/ucred.h Sat Nov 14 19:21:46 2020(r367694) >> +++ head/sys/sys/ucred.h Sat Nov 14 19:22:02 2020(r367695) >> @@ -114,6 +114,28 @@ struct xucred { >> struct proc; >> struct thread; >> >> +struct credbatch { >> +struct ucred *cred; >> +int users; >> +int ref; >> +}; >> + >> +static inline void >> +credbatch_prep(struct credbatch *crb) >> +{ >> +crb->cred = NULL; >> +crb->users = 0; >> +crb->ref = 0; >> +} >> +voidcredbatch_add(struct credbatch *crb, struct thread *td); >> +static inline void >> +credbatch_process(struct credbatch *crb) >> +{ >> + >> +} >> +voidcredbatch_add(struct credbatch *crb, struct thread *td); >> +voidcredbatch_final(struct credbatch *crb); >> + > > Do not mix prototypes and inlines, especially without spaces > around the prototype in the middle. Also, the kernel uses __inline > rather than inline (for better or for worse). The kernel has a huge mix of inline and __inline, to the point where my impression was that __inline is obsolete. > Better would be: > > static __inline void > credbatch_prep() > { > ... > } > > static __inline void > credbatch_process() > { > ... > } > > void credbatch_add(); > void credbatch_final(); > I disagree. The current header order follows how these routines are used. > It seems you just have a duplicate credbatch_add() in fact. > Indeed, I'll whack it. > Also, why have an empty inline function? > Interested parties can check the consumer (also seen in the diff) to see this is for consistency. I don't think any comments are warranted in the header. > These changes would benefit from review. > I don't think it's feasible to ask for review for everything lest it degardes to rubber stamping and I don't think this change warranted it, regardless of the cosmetic issues which can always show up. > -- > John Baldwin > -- Mateusz Guzik ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r367813 - head/lib/libutil
eck for some other thread already having > + * set localbase - this should use atomic ops. > + * The amount of memory allocated above may leak, > + * if a parallel update in another thread is not > + * detected and the non-NULL pointer is overwritten. > + */ > + if (tmppath[0] != '\0' && > + (volatile const char*)localbase == NULL) > + localbase = tmppath; > + else > + free((void*)tmppath); > + return (localbase); > + } > + return (_PATH_LOCALBASE); > +} > > Modified: head/lib/libutil/libutil.h > == > --- head/lib/libutil/libutil.hWed Nov 18 19:35:30 2020 > (r367812) > +++ head/lib/libutil/libutil.hWed Nov 18 19:44:30 2020 > (r367813) > @@ -98,6 +98,8 @@ int flopen(const char *_path, int _flags, ...); > int flopenat(int _dirfd, const char *_path, int _flags, ...); > int forkpty(int *_amaster, char *_name, > struct termios *_termp, struct winsize *_winp); > +const char * > + getlocalbase(void); > void hexdump(const void *_ptr, int _length, const char *_hdr, int _flags); > int humanize_number(char *_buf, size_t _len, int64_t _number, > const char *_suffix, int _scale, int _flags); > -- Mateusz Guzik ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r367819 - in head: sys/kern sys/sys usr.sbin/jail
*cred; > + int descend, error, enabled; > + > + cred = req->td->td_ucred; > + enabled = suser_enabled(cred); > + > + error = sysctl_handle_int(oidp, , 0, req); > + if (error || !req->newptr) > + return (error); > + > + pr = cred->cr_prison; > + sx_slock(_lock); > + mtx_lock(>pr_mtx); > + > + prison_suser_set(pr, enabled); > + if (!enabled) { > + FOREACH_PRISON_DESCENDANT_LOCKED(pr, cpr, descend) { > + prison_suser_set(cpr, 0); > + } > + } > + mtx_unlock(>pr_mtx); > + sx_sunlock(_lock); > + return (0); > +} > + > +SYSCTL_PROC(_security_bsd, OID_AUTO, suser_enabled, CTLTYPE_INT | > +CTLFLAG_RWTUN | CTLFLAG_PRISON, 0, 0, _kern_suser_enabled, "I", > +"Processes with uid 0 have privilege"); > + This lacks CTLFLAG_MPSAFE. > static int unprivileged_mlock = 1; > SYSCTL_INT(_security_bsd, OID_AUTO, unprivileged_mlock, CTLFLAG_RWTUN, > _mlock, 0, "Allow non-root users to call mlock(2)"); > @@ -186,7 +238,7 @@ priv_check_cred(struct ucred *cred, int priv) >* superuser policy to be globally disabled, although this is >* currenty of limited utility. >*/ > - if (suser_enabled) { > + if (suser_enabled(cred)) { > switch (priv) { > case PRIV_MAXFILES: > case PRIV_MAXPROC: > @@ -258,7 +310,7 @@ priv_check_cred_vfs_lookup_slow(struct ucred *cred) > if (error) > goto out; > > - if (cred->cr_uid == 0 && suser_enabled) { > + if (cred->cr_uid == 0 && suser_enabled(cred)) { > error = 0; > goto out; > } > @@ -279,7 +331,7 @@ priv_check_cred_vfs_lookup(struct ucred *cred) > return (priv_check_cred_vfs_lookup_slow(cred)); > > error = EPERM; > - if (cred->cr_uid == 0 && suser_enabled) > + if (cred->cr_uid == 0 && suser_enabled(cred)) > error = 0; > return (error); > } > @@ -294,7 +346,7 @@ priv_check_cred_vfs_lookup_nomac(struct ucred *cred) > return (EAGAIN); > > error = EPERM; > - if (cred->cr_uid == 0 && suser_enabled) > + if (cred->cr_uid == 0 && suser_enabled(cred)) > error = 0; > return (error); > } > @@ -313,7 +365,7 @@ priv_check_cred_vfs_generation_slow(struct ucred *cred > goto out; > } > > - if (cred->cr_uid == 0 && suser_enabled) { > + if (cred->cr_uid == 0 && suser_enabled(cred)) { > error = 0; > goto out; > } > @@ -334,7 +386,7 @@ priv_check_cred_vfs_generation(struct ucred *cred) > return (priv_check_cred_vfs_generation_slow(cred)); > > error = EPERM; > - if (!jailed(cred) && cred->cr_uid == 0 && suser_enabled) > + if (!jailed(cred) && cred->cr_uid == 0 && suser_enabled(cred)) > error = 0; > return (error); > } > > Modified: head/sys/sys/jail.h > == > --- head/sys/sys/jail.h Wed Nov 18 20:59:58 2020(r367818) > +++ head/sys/sys/jail.h Wed Nov 18 21:07:08 2020(r367819) > @@ -232,9 +232,10 @@ struct prison_racct { > #define PR_ALLOW_MLOCK 0x0080 > #define PR_ALLOW_READ_MSGBUF0x0100 > #define PR_ALLOW_UNPRIV_DEBUG 0x0200 > +#define PR_ALLOW_SUSER 0x0400 > #define PR_ALLOW_RESERVED_PORTS 0x8000 > #define PR_ALLOW_KMEM_ACCESS0x0001 /* reserved, > not used yet */ > -#define PR_ALLOW_ALL_STATIC 0x000183ff > +#define PR_ALLOW_ALL_STATIC 0x000187ff > > /* > * PR_ALLOW_DIFFERENCES determines which flags are able to be > > Modified: head/usr.sbin/jail/jail.8 > == > --- head/usr.sbin/jail/jail.8 Wed Nov 18 20:59:58 2020(r367818) > +++ head/usr.sbin/jail/jail.8 Wed Nov 18 21:07:08 2020(r367819) > @@ -25,7 +25,7 @@ > .\" > .\" $FreeBSD$ > .\" > -.Dd May 14, 2020 > +.Dd November 18, 2020 > .Dt JAIL 8 > .Os > .Sh NAME > @@ -587,6 +587,13 @@ and resource limits. > The jail root may bind to ports lower than 1024. > .It Va allow.unprivileged_proc_debug > Unprivileged processes in the jail may use debugging facilities. > +.It Va allow.suser > +The value of the jail's > +.Va security.bsd.suser_enabled > +sysctl. > +The super-user will be disabled automatically if its parent system has it > +disabled. > +The super-user is enabled by default. > .El > .El > .Pp > @@ -1267,6 +1274,7 @@ Changes to these variables by a jailed process do not > > environment, only the jail environment. > These variables are > .Va kern.securelevel , > +.Va security.bsd.suser_enabled , > .Va kern.hostname , > .Va kern.domainname , > .Va kern.hostid , > -- Mateusz Guzik ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r367814 - head/sys/sys
Author: mjg Date: Wed Nov 18 19:47:24 2020 New Revision: 367814 URL: https://svnweb.freebsd.org/changeset/base/367814 Log: fd: reorder struct file to reduce false sharing The size on LP64 is 80 bytes, which is just more than a cacheline, does not lend itself to easy shrinking and rounding up to 2 would be a huge waste given NOFREE marker. The least which can be done is to reorder it so that most commonly used fields are less likely to span different lines, and consequently suffer less false sharing. With the change at hand most commonly used fields land in the same line about 3/4 of the time, as opposed to 2/4. Modified: head/sys/sys/file.h Modified: head/sys/sys/file.h == --- head/sys/sys/file.h Wed Nov 18 19:44:30 2020(r367813) +++ head/sys/sys/file.h Wed Nov 18 19:47:24 2020(r367814) @@ -177,14 +177,14 @@ struct fadvise_info { }; struct file { + volatile u_int f_flag; /* see fcntl.h */ + volatile u_int f_count;/* reference count */ void*f_data;/* file descriptor specific data */ struct fileops *f_ops; /* File operations */ - struct ucred*f_cred;/* associated credentials. */ struct vnode*f_vnode; /* NULL or applicable vnode */ + struct ucred*f_cred;/* associated credentials. */ short f_type; /* descriptor type */ short f_vnread_flags; /* (f) Sleep lock for f_offset */ - volatile u_int f_flag; /* see fcntl.h */ - volatile u_int f_count;/* reference count */ /* * DTYPE_VNODE specific fields. */ ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r367713 - head/sys/kern
On 11/18/20, Konstantin Belousov wrote: > On Tue, Nov 17, 2020 at 03:36:31PM +0100, Mateusz Guzik wrote: >> On 11/17/20, Konstantin Belousov wrote: >> > On Tue, Nov 17, 2020 at 04:15:12AM +0100, Mateusz Guzik wrote: >> >> On 11/17/20, Konstantin Belousov wrote: >> >> > On Mon, Nov 16, 2020 at 03:09:19AM +, Mateusz Guzik wrote: >> >> >> Author: mjg >> >> >> Date: Mon Nov 16 03:09:18 2020 >> >> >> New Revision: 367713 >> >> >> URL: https://svnweb.freebsd.org/changeset/base/367713 >> >> >> >> >> >> Log: >> >> >> select: replace reference counting with memory barriers in selfd >> >> >> >> >> >> Refcounting was added to combat a race between selfdfree and >> >> >> doselwakup, >> >> >> but it adds avoidable overhead. >> >> >> >> >> >> selfdfree detects it can free the object by ->sf_si == NULL, thus >> >> >> we >> >> >> can >> >> >> ensure that the condition only holds after all accesses are >> >> >> completed. >> >> >> >> >> >> Modified: >> >> >> head/sys/kern/sys_generic.c >> >> >> >> >> >> Modified: head/sys/kern/sys_generic.c >> >> >> == >> >> >> --- head/sys/kern/sys_generic.cSun Nov 15 22:49:28 2020 >> >> >> (r367712) >> >> >> +++ head/sys/kern/sys_generic.cMon Nov 16 03:09:18 2020 >> >> >> (r367713) >> >> >> @@ -156,7 +156,6 @@ struct selfd { >> >> >>struct mtx *sf_mtx;/* Pointer to selinfo >> >> >> mtx. */ >> >> >>struct seltd*sf_td; /* (k) owning seltd. */ >> >> >>void*sf_cookie; /* (k) fd or pollfd. */ >> >> >> - u_int sf_refs; >> >> >> }; >> >> >> >> >> >> MALLOC_DEFINE(M_SELFD, "selfd", "selfd"); >> >> >> @@ -1704,16 +1703,17 @@ static void >> >> >> selfdfree(struct seltd *stp, struct selfd *sfp) >> >> >> { >> >> >>STAILQ_REMOVE(>st_selq, sfp, selfd, sf_link); >> >> >> - if (sfp->sf_si != NULL) { >> >> >> + /* >> >> >> + * Paired with doselwakeup. >> >> >> + */ >> >> >> + if (atomic_load_acq_ptr((uintptr_t *)>sf_si) != >> >> >> (uintptr_t)NULL) >> >> >> { >> >> > This could be != 0. >> >> > >> >> >>mtx_lock(sfp->sf_mtx); >> >> >>if (sfp->sf_si != NULL) { >> >> >>TAILQ_REMOVE(>sf_si->si_tdlist, sfp, >> >> >> sf_threads); >> >> >> - refcount_release(>sf_refs); >> >> >>} >> >> >>mtx_unlock(sfp->sf_mtx); >> >> >>} >> >> >> - if (refcount_release(>sf_refs)) >> >> >> - free(sfp, M_SELFD); >> >> >> + free(sfp, M_SELFD); >> >> > What guarantees that doselwakeup() finished with sfp ? >> >> > >> >> >> >> Release semantics provided by atomic_store_rel_ptr -- it means the >> >> NULL store is the last access, neither CPU nor the compiler are going >> >> to reorder preceding loads/stores past it. >> > It only guarantees that if we observed NULL as the result of load. >> > >> >> If that did not happen selfdfree takes the lock. If the entry is still >> there, it removes it and doselwakeup will not see it after it takes >> the lock. If the entry is not there, doselwaekup is done with it >> because it released the lock. > > I still do not understand it. selfdfree() indeed takes sf_mtx and rechecks > sf_si. But what prevents doselwakeup() to store NULL into sf_si at any > moment. e.g. after free() is done ? selfdfree() takes seltd mutex, not > selinfo. > That's the same lock. In selrecord: mtxp = sip->si_mtx; if (mtxp == NULL) mtxp = mtx_pool_find(mtxpool_select, sip); /* * Initialize the sfp and queue it in the thread. */ sfp->sf_si = sip; sfp->sf_mtx = mtxp; STAILQ_INSERT_TAIL(>st_selq, sfp, sf_link); /* * Now that we've locked the sip, check for initialization. */ mtx_lock(mtxp); if (sip->si_mtx == NULL) { sip->si_mtx = mtxp; TAILQ_INIT(>si_tdlist); } Then doselwakeup mtx_lock(sip->si_mtx) serializes against mtx_lock(sfp->sf_mtx) -- Mateusz Guzik ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r367713 - head/sys/kern
On 11/17/20, Konstantin Belousov wrote: > On Tue, Nov 17, 2020 at 04:15:12AM +0100, Mateusz Guzik wrote: >> On 11/17/20, Konstantin Belousov wrote: >> > On Mon, Nov 16, 2020 at 03:09:19AM +0000, Mateusz Guzik wrote: >> >> Author: mjg >> >> Date: Mon Nov 16 03:09:18 2020 >> >> New Revision: 367713 >> >> URL: https://svnweb.freebsd.org/changeset/base/367713 >> >> >> >> Log: >> >> select: replace reference counting with memory barriers in selfd >> >> >> >> Refcounting was added to combat a race between selfdfree and >> >> doselwakup, >> >> but it adds avoidable overhead. >> >> >> >> selfdfree detects it can free the object by ->sf_si == NULL, thus we >> >> can >> >> ensure that the condition only holds after all accesses are >> >> completed. >> >> >> >> Modified: >> >> head/sys/kern/sys_generic.c >> >> >> >> Modified: head/sys/kern/sys_generic.c >> >> == >> >> --- head/sys/kern/sys_generic.c Sun Nov 15 22:49:28 2020 >> >> (r367712) >> >> +++ head/sys/kern/sys_generic.c Mon Nov 16 03:09:18 2020 >> >> (r367713) >> >> @@ -156,7 +156,6 @@ struct selfd { >> >> struct mtx *sf_mtx;/* Pointer to selinfo mtx. */ >> >> struct seltd*sf_td; /* (k) owning seltd. */ >> >> void*sf_cookie; /* (k) fd or pollfd. */ >> >> - u_int sf_refs; >> >> }; >> >> >> >> MALLOC_DEFINE(M_SELFD, "selfd", "selfd"); >> >> @@ -1704,16 +1703,17 @@ static void >> >> selfdfree(struct seltd *stp, struct selfd *sfp) >> >> { >> >> STAILQ_REMOVE(>st_selq, sfp, selfd, sf_link); >> >> - if (sfp->sf_si != NULL) { >> >> + /* >> >> + * Paired with doselwakeup. >> >> + */ >> >> + if (atomic_load_acq_ptr((uintptr_t *)>sf_si) != (uintptr_t)NULL) >> >> { >> > This could be != 0. >> > >> >> mtx_lock(sfp->sf_mtx); >> >> if (sfp->sf_si != NULL) { >> >> TAILQ_REMOVE(>sf_si->si_tdlist, sfp, sf_threads); >> >> - refcount_release(>sf_refs); >> >> } >> >> mtx_unlock(sfp->sf_mtx); >> >> } >> >> - if (refcount_release(>sf_refs)) >> >> - free(sfp, M_SELFD); >> >> + free(sfp, M_SELFD); >> > What guarantees that doselwakeup() finished with sfp ? >> > >> >> Release semantics provided by atomic_store_rel_ptr -- it means the >> NULL store is the last access, neither CPU nor the compiler are going >> to reorder preceding loads/stores past it. > It only guarantees that if we observed NULL as the result of load. > If that did not happen selfdfree takes the lock. If the entry is still there, it removes it and doselwakeup will not see it after it takes the lock. If the entry is not there, doselwaekup is done with it because it released the lock. -- Mateusz Guzik ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r367713 - head/sys/kern
On 11/17/20, Konstantin Belousov wrote: > On Mon, Nov 16, 2020 at 03:09:19AM +0000, Mateusz Guzik wrote: >> Author: mjg >> Date: Mon Nov 16 03:09:18 2020 >> New Revision: 367713 >> URL: https://svnweb.freebsd.org/changeset/base/367713 >> >> Log: >> select: replace reference counting with memory barriers in selfd >> >> Refcounting was added to combat a race between selfdfree and >> doselwakup, >> but it adds avoidable overhead. >> >> selfdfree detects it can free the object by ->sf_si == NULL, thus we >> can >> ensure that the condition only holds after all accesses are completed. >> >> Modified: >> head/sys/kern/sys_generic.c >> >> Modified: head/sys/kern/sys_generic.c >> == >> --- head/sys/kern/sys_generic.c Sun Nov 15 22:49:28 2020 >> (r367712) >> +++ head/sys/kern/sys_generic.c Mon Nov 16 03:09:18 2020 >> (r367713) >> @@ -156,7 +156,6 @@ struct selfd { >> struct mtx *sf_mtx;/* Pointer to selinfo mtx. */ >> struct seltd*sf_td; /* (k) owning seltd. */ >> void*sf_cookie; /* (k) fd or pollfd. */ >> -u_int sf_refs; >> }; >> >> MALLOC_DEFINE(M_SELFD, "selfd", "selfd"); >> @@ -1704,16 +1703,17 @@ static void >> selfdfree(struct seltd *stp, struct selfd *sfp) >> { >> STAILQ_REMOVE(>st_selq, sfp, selfd, sf_link); >> -if (sfp->sf_si != NULL) { >> +/* >> + * Paired with doselwakeup. >> + */ >> +if (atomic_load_acq_ptr((uintptr_t *)>sf_si) != (uintptr_t)NULL) { > This could be != 0. > >> mtx_lock(sfp->sf_mtx); >> if (sfp->sf_si != NULL) { >> TAILQ_REMOVE(>sf_si->si_tdlist, sfp, sf_threads); >> -refcount_release(>sf_refs); >> } >> mtx_unlock(sfp->sf_mtx); >> } >> -if (refcount_release(>sf_refs)) >> -free(sfp, M_SELFD); >> +free(sfp, M_SELFD); > What guarantees that doselwakeup() finished with sfp ? > Release semantics provided by atomic_store_rel_ptr -- it means the NULL store is the last access, neither CPU nor the compiler are going to reorder preceding loads/stores past it. -- Mateusz Guzik ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r367738 - head/sys/sys
Author: mjg Date: Tue Nov 17 00:04:30 2020 New Revision: 367738 URL: https://svnweb.freebsd.org/changeset/base/367738 Log: cpuset: reorder so that cs_mask does not share cacheline with cs_ref Modified: head/sys/sys/cpuset.h Modified: head/sys/sys/cpuset.h == --- head/sys/sys/cpuset.h Tue Nov 17 00:04:05 2020(r367737) +++ head/sys/sys/cpuset.h Tue Nov 17 00:04:30 2020(r367738) @@ -111,15 +111,15 @@ LIST_HEAD(setlist, cpuset); * to deal with inconsistent results. */ struct cpuset { - cpuset_tcs_mask;/* bitmask of valid cpus. */ - struct domainset*cs_domain; /* (c) NUMA policy. */ volatile u_int cs_ref; /* (a) Reference count. */ int cs_flags; /* (s) Flags from below. */ - cpusetid_t cs_id; /* (s) Id or INVALID. */ - struct cpuset *cs_parent; /* (s) Pointer to our parent. */ LIST_ENTRY(cpuset) cs_link;/* (c) All identified sets. */ LIST_ENTRY(cpuset) cs_siblings;/* (c) Sibling set link. */ struct setlist cs_children;/* (c) List of children. */ + struct domainset*cs_domain; /* (c) NUMA policy. */ + cpusetid_t cs_id; /* (s) Id or INVALID. */ + struct cpuset *cs_parent; /* (s) Pointer to our parent. */ + cpuset_tcs_mask;/* bitmask of valid cpus. */ }; #define CPU_SET_ROOT0x0001 /* Set is a root set. */ ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r367737 - head/sys/kern
Author: mjg Date: Tue Nov 17 00:04:05 2020 New Revision: 367737 URL: https://svnweb.freebsd.org/changeset/base/367737 Log: cpuset: refcount-clean Modified: head/sys/kern/kern_cpuset.c Modified: head/sys/kern/kern_cpuset.c == --- head/sys/kern/kern_cpuset.c Mon Nov 16 21:55:52 2020(r367736) +++ head/sys/kern/kern_cpuset.c Tue Nov 17 00:04:05 2020(r367737) @@ -1482,7 +1482,7 @@ cpuset_thread0(void) CPU_COPY(_cpus, >cs_mask); LIST_INIT(>cs_children); LIST_INSERT_HEAD(_ids, set, cs_link); - set->cs_ref = 1; + refcount_init(>cs_ref, 1); set->cs_flags = CPU_SET_ROOT | CPU_SET_RDONLY; set->cs_domain = cpuset_zero = set; @@ -2281,7 +2281,7 @@ DB_SHOW_COMMAND(cpusets, db_show_cpusets) LIST_FOREACH(set, _ids, cs_link) { db_printf("set=%p id=%-6u ref=%-6d flags=0x%04x parent id=%d\n", - set, set->cs_id, set->cs_ref, set->cs_flags, + set, set->cs_id, refcount_load(>cs_ref), set->cs_flags, (set->cs_parent != NULL) ? set->cs_parent->cs_id : 0); db_printf(" cpu mask="); ddb_display_cpuset(>cs_mask); ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r367733 - head/sys/kern
Author: mjg Date: Mon Nov 16 17:56:58 2020 New Revision: 367733 URL: https://svnweb.freebsd.org/changeset/base/367733 Log: malloc: make malloc_large closer to standalone This moves entire large alloc handling out of all consumers, apart from deciding to go there. This is a step towards creating a fast path. Reviewed by: markj Differential Revision:https://reviews.freebsd.org/D27198 Modified: head/sys/kern/kern_malloc.c Modified: head/sys/kern/kern_malloc.c == --- head/sys/kern/kern_malloc.c Mon Nov 16 17:45:42 2020(r367732) +++ head/sys/kern/kern_malloc.c Mon Nov 16 17:56:58 2020(r367733) @@ -110,6 +110,14 @@ dtrace_malloc_probe_func_t __read_mostly dtrace_malloc #defineMALLOC_DEBUG1 #endif +#ifdef DEBUG_REDZONE +#defineDEBUG_REDZONE_ARG_DEF , unsigned long osize +#defineDEBUG_REDZONE_ARG , osize +#else +#defineDEBUG_REDZONE_ARG_DEF +#defineDEBUG_REDZONE_ARG +#endif + /* * When realloc() is called, if the new size is sufficiently smaller than * the old size, realloc() will allocate a new, smaller block to avoid @@ -574,21 +582,33 @@ malloc_large_size(uma_slab_t slab) return (va >> 1); } -static caddr_t -malloc_large(size_t *size, struct domainset *policy, int flags) +static caddr_t __noinline +malloc_large(size_t *size, struct malloc_type *mtp, struct domainset *policy, +int flags DEBUG_REDZONE_ARG_DEF) { - vm_offset_t va; + vm_offset_t kva; + caddr_t va; size_t sz; sz = roundup(*size, PAGE_SIZE); - va = kmem_malloc_domainset(policy, sz, flags); - if (va != 0) { + kva = kmem_malloc_domainset(policy, sz, flags); + if (kva != 0) { /* The low bit is unused for slab pointers. */ - vsetzoneslab(va, NULL, (void *)((sz << 1) | 1)); + vsetzoneslab(kva, NULL, (void *)((sz << 1) | 1)); uma_total_inc(sz); *size = sz; } - return ((caddr_t)va); + va = (caddr_t)kva; + malloc_type_allocated(mtp, va == NULL ? 0 : sz); + if (__predict_false(va == NULL)) { + KASSERT((flags & M_WAITOK) == 0, + ("malloc(M_WAITOK) returned NULL")); + } +#ifdef DEBUG_REDZONE + if (va != NULL) + va = redzone_setup(va, osize); +#endif + return (va); } static void @@ -613,30 +633,30 @@ void * int indx; caddr_t va; uma_zone_t zone; -#if defined(DEBUG_REDZONE) +#ifdef DEBUG_REDZONE unsigned long osize = size; #endif MPASS((flags & M_EXEC) == 0); + #ifdef MALLOC_DEBUG va = NULL; if (malloc_dbg(, , mtp, flags) != 0) return (va); #endif - if (size <= kmem_zmax) { - if (size & KMEM_ZMASK) - size = (size & ~KMEM_ZMASK) + KMEM_ZBASE; - indx = kmemsize[size >> KMEM_ZSHIFT]; - zone = kmemzones[indx].kz_zone[mtp_get_subzone(mtp)]; - va = uma_zalloc(zone, flags); - if (va != NULL) - size = zone->uz_size; - malloc_type_zone_allocated(mtp, va == NULL ? 0 : size, indx); - } else { - va = malloc_large(, DOMAINSET_RR(), flags); - malloc_type_allocated(mtp, va == NULL ? 0 : size); - } + if (__predict_false(size > kmem_zmax)) + return (malloc_large(, mtp, DOMAINSET_RR(), flags + DEBUG_REDZONE_ARG)); + + if (size & KMEM_ZMASK) + size = (size & ~KMEM_ZMASK) + KMEM_ZBASE; + indx = kmemsize[size >> KMEM_ZSHIFT]; + zone = kmemzones[indx].kz_zone[mtp_get_subzone(mtp)]; + va = uma_zalloc(zone, flags); + if (va != NULL) + size = zone->uz_size; + malloc_type_zone_allocated(mtp, va == NULL ? 0 : size, indx); if (__predict_false(va == NULL)) { KASSERT((flags & M_WAITOK) == 0, ("malloc(M_WAITOK) returned NULL")); @@ -679,28 +699,27 @@ malloc_domainset(size_t size, struct malloc_type *mtp, caddr_t va; int domain; int indx; - -#if defined(DEBUG_REDZONE) +#ifdef DEBUG_REDZONE unsigned long osize = size; #endif + MPASS((flags & M_EXEC) == 0); + #ifdef MALLOC_DEBUG va = NULL; if (malloc_dbg(, , mtp, flags) != 0) return (va); #endif - if (size <= kmem_zmax) { - vm_domainset_iter_policy_init(, ds, , ); - do { - va = malloc_domain(, , mtp, domain, flags); - } while (va == NULL && - vm_domainset_iter_policy(, ) == 0); - malloc_type_zone_allocated(mtp, va == NULL ? 0 : size, indx); - } else { - /* Policy is handled by kmem. */ - va = malloc_large(, ds,
svn commit: r367714 - head/sys/kern
Author: mjg Date: Mon Nov 16 03:12:21 2020 New Revision: 367714 URL: https://svnweb.freebsd.org/changeset/base/367714 Log: select: call seltdfini on process and thread exit Since thread_zone is marked NOFREE the thread_fini callback is never executed, meaning memory allocated by seltdinit is never released. Adding the call to thread_dtor is not sufficient as exiting processes cache the main thread. Modified: head/sys/kern/kern_exit.c head/sys/kern/kern_thread.c Modified: head/sys/kern/kern_exit.c == --- head/sys/kern/kern_exit.c Mon Nov 16 03:09:18 2020(r367713) +++ head/sys/kern/kern_exit.c Mon Nov 16 03:12:21 2020(r367714) @@ -355,6 +355,7 @@ exit1(struct thread *td, int rval, int signo) PROC_UNLOCK(p); umtx_thread_exit(td); + seltdfini(td); /* * Reset any sigio structures pointing to us as a result of Modified: head/sys/kern/kern_thread.c == --- head/sys/kern/kern_thread.c Mon Nov 16 03:09:18 2020(r367713) +++ head/sys/kern/kern_thread.c Mon Nov 16 03:12:21 2020(r367714) @@ -329,6 +329,7 @@ thread_ctor(void *mem, int size, void *arg, int flags) audit_thread_alloc(td); #endif umtx_thread_alloc(td); + MPASS(td->td_sel == NULL); return (0); } @@ -369,6 +370,7 @@ thread_dtor(void *mem, int size, void *arg) osd_thread_exit(td); td_softdep_cleanup(td); MPASS(td->td_su == NULL); + seltdfini(td); } /* @@ -405,7 +407,7 @@ thread_fini(void *mem, int size) turnstile_free(td->td_turnstile); sleepq_free(td->td_sleepqueue); umtx_thread_fini(td); - seltdfini(td); + MPASS(td->td_sel == NULL); } /* ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r367713 - head/sys/kern
Author: mjg Date: Mon Nov 16 03:09:18 2020 New Revision: 367713 URL: https://svnweb.freebsd.org/changeset/base/367713 Log: select: replace reference counting with memory barriers in selfd Refcounting was added to combat a race between selfdfree and doselwakup, but it adds avoidable overhead. selfdfree detects it can free the object by ->sf_si == NULL, thus we can ensure that the condition only holds after all accesses are completed. Modified: head/sys/kern/sys_generic.c Modified: head/sys/kern/sys_generic.c == --- head/sys/kern/sys_generic.c Sun Nov 15 22:49:28 2020(r367712) +++ head/sys/kern/sys_generic.c Mon Nov 16 03:09:18 2020(r367713) @@ -156,7 +156,6 @@ struct selfd { struct mtx *sf_mtx;/* Pointer to selinfo mtx. */ struct seltd*sf_td; /* (k) owning seltd. */ void*sf_cookie; /* (k) fd or pollfd. */ - u_int sf_refs; }; MALLOC_DEFINE(M_SELFD, "selfd", "selfd"); @@ -1704,16 +1703,17 @@ static void selfdfree(struct seltd *stp, struct selfd *sfp) { STAILQ_REMOVE(>st_selq, sfp, selfd, sf_link); - if (sfp->sf_si != NULL) { + /* +* Paired with doselwakeup. +*/ + if (atomic_load_acq_ptr((uintptr_t *)>sf_si) != (uintptr_t)NULL) { mtx_lock(sfp->sf_mtx); if (sfp->sf_si != NULL) { TAILQ_REMOVE(>sf_si->si_tdlist, sfp, sf_threads); - refcount_release(>sf_refs); } mtx_unlock(sfp->sf_mtx); } - if (refcount_release(>sf_refs)) - free(sfp, M_SELFD); + free(sfp, M_SELFD); } /* Drain the waiters tied to all the selfd belonging the specified selinfo. */ @@ -1766,7 +1766,6 @@ selrecord(struct thread *selector, struct selinfo *sip */ sfp->sf_si = sip; sfp->sf_mtx = mtxp; - refcount_init(>sf_refs, 2); STAILQ_INSERT_TAIL(>st_selq, sfp, sf_link); /* * Now that we've locked the sip, check for initialization. @@ -1820,14 +1819,15 @@ doselwakeup(struct selinfo *sip, int pri) * sf_si seltdclear will know to ignore this si. */ TAILQ_REMOVE(>si_tdlist, sfp, sf_threads); - sfp->sf_si = NULL; stp = sfp->sf_td; + /* +* Paired with selfdfree. +*/ + atomic_store_rel_ptr((uintptr_t *)>sf_si, (uintptr_t)NULL); mtx_lock(>st_mtx); stp->st_flags |= SELTD_PENDING; cv_broadcastpri(>st_wait, pri); mtx_unlock(>st_mtx); - if (refcount_release(>sf_refs)) - free(sfp, M_SELFD); } mtx_unlock(sip->si_mtx); } ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r367700 - head/sys/kern
Author: mjg Date: Sun Nov 15 01:54:44 2020 New Revision: 367700 URL: https://svnweb.freebsd.org/changeset/base/367700 Log: sched: fix an incorrect comparison in sched_lend_user_prio_cond Compare with sched_lend_user_prio. Modified: head/sys/kern/sched_4bsd.c head/sys/kern/sched_ule.c Modified: head/sys/kern/sched_4bsd.c == --- head/sys/kern/sched_4bsd.c Sat Nov 14 20:45:12 2020(r367699) +++ head/sys/kern/sched_4bsd.c Sun Nov 15 01:54:44 2020(r367700) @@ -952,7 +952,7 @@ sched_lend_user_prio_cond(struct thread *td, u_char pr goto lend; if (td->td_user_pri != min(prio, td->td_base_user_pri)) goto lend; - if (td->td_priority >= td->td_user_pri) + if (td->td_priority != td->td_user_pri) goto lend; return; Modified: head/sys/kern/sched_ule.c == --- head/sys/kern/sched_ule.c Sat Nov 14 20:45:12 2020(r367699) +++ head/sys/kern/sched_ule.c Sun Nov 15 01:54:44 2020(r367700) @@ -1900,7 +1900,7 @@ sched_lend_user_prio_cond(struct thread *td, u_char pr goto lend; if (td->td_user_pri != min(prio, td->td_base_user_pri)) goto lend; - if (td->td_priority >= td->td_user_pri) + if (td->td_priority != td->td_user_pri) goto lend; return; ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r367692 - head/sys/sys
There are "KBI" breaking changes happening all the time and version bumps only sometimes happen. The build process already has infrastructure to rebuild all port kmods as well -- see PORTS_MODULES in make.conf(5) Perhaps it would be beneficial to add checking that the kernel is older than the to-be-loaded module and refuse otherwise. Can be overwritten by a loader.conf knob, then people who run into problems explicitly asked for it. But it's not clear to me if "older" should mean build date or revision or something else. Still, I suspect it will be mostly annoying to developers. Regardless, I have no intention to working on any of this. Maybe PORTS_MODULES is just not advertised enough. On 11/14/20, Warner Losh wrote: > true, but a version bump forces that and versions are cheap enough... > > Warner > > On Sat, Nov 14, 2020 at 1:58 PM Mateusz Guzik wrote: > >> you are expected to recompile all your kernel modules every time you >> update head >> >> On 11/14/20, Shawn Webb wrote: >> > Are there any kernel modules (in base, in ports, or out-of-both-trees) >> > that access struct ucred? >> > >> > On Sat, Nov 14, 2020 at 09:51:47PM +0100, Mateusz Guzik wrote: >> >> I don't think so, it does not change any APIs >> >> >> >> On 11/14/20, Shawn Webb wrote: >> >> > On Sat, Nov 14, 2020 at 07:20:37PM +, Mateusz Guzik wrote: >> >> >> Author: mjg >> >> >> Date: Sat Nov 14 19:20:37 2020 >> >> >> New Revision: 367692 >> >> >> URL: https://svnweb.freebsd.org/changeset/base/367692 >> >> >> >> >> >> Log: >> >> >> cred: reorder cr_audit to be closer to the lock >> >> >> >> >> >> This makes cr_uid avoid sharing. >> >> >> >> >> >> Modified: >> >> >> head/sys/sys/ucred.h >> >> >> >> >> >> Modified: head/sys/sys/ucred.h >> >> >> >> == >> >> >> --- head/sys/sys/ucred.h Sat Nov 14 19:19:27 2020(r367691) >> >> >> +++ head/sys/sys/ucred.h Sat Nov 14 19:20:37 2020(r367692) >> >> >> @@ -63,6 +63,7 @@ struct ucred { >> >> >> struct mtx cr_mtx; >> >> >> u_int cr_ref; /* (c) reference count */ >> >> >> u_int cr_users; /* (c) proc + thread using this >> cred */ >> >> >> + struct auditinfo_addr cr_audit; /* Audit properties. */ >> >> >> #define cr_startcopy cr_uid >> >> >> uid_t cr_uid; /* effective user id */ >> >> >> uid_t cr_ruid;/* real user id */ >> >> >> @@ -78,7 +79,6 @@ struct ucred { >> >> >> void*cr_pspare2[2]; /* general use 2 */ >> >> >> #define cr_endcopy cr_label >> >> >> struct label*cr_label; /* MAC label */ >> >> >> - struct auditinfo_addr cr_audit; /* Audit properties. */ >> >> >> gid_t *cr_groups; /* groups */ >> >> >> int cr_agroups; /* Available groups */ >> >> >> gid_t cr_smallgroups[XU_NGROUPS]; /* storage for small >> groups */ >> >> > >> >> > Hey Mateusz, >> >> > >> >> > Since this changes KBI, does __FreeBSD_version need bumping? >> >> > >> >> > Thanks, >> >> > >> >> > -- >> >> > Shawn Webb >> >> > Cofounder / Security Engineer >> >> > HardenedBSD >> >> > >> >> > GPG Key ID: 0xFF2E67A277F8E1FA >> >> > GPG Key Fingerprint: D206 BB45 15E0 9C49 0CF9 3633 C85B 0AF8 AB23 >> 0FB2 >> >> > >> https://git-01.md.hardenedbsd.org/HardenedBSD/pubkeys/src/branch/master/Shawn_Webb/03A4CBEBB82EA5A67D9F3853FF2E67A277F8E1FA.pub.asc >> >> > >> >> >> >> >> >> -- >> >> Mateusz Guzik >> > >> > -- >> > Shawn Webb >> > Cofounder / Security Engineer >> > HardenedBSD >> > >> > GPG Key ID: 0xFF2E67A277F8E1FA >> > GPG Key Fingerprint: D206 BB45 15E0 9C49 0CF9 3633 C85B 0AF8 AB23 0FB2 >> > >> https://git-01.md.hardenedbsd.org/HardenedBSD/pubkeys/src/branch/master/Shawn_Webb/03A4CBEBB82EA5A67D9F3853FF2E67A277F8E1FA.pub.asc >> > >> >> >> -- >> Mateusz Guzik >> > -- Mateusz Guzik ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r367692 - head/sys/sys
you are expected to recompile all your kernel modules every time you update head On 11/14/20, Shawn Webb wrote: > Are there any kernel modules (in base, in ports, or out-of-both-trees) > that access struct ucred? > > On Sat, Nov 14, 2020 at 09:51:47PM +0100, Mateusz Guzik wrote: >> I don't think so, it does not change any APIs >> >> On 11/14/20, Shawn Webb wrote: >> > On Sat, Nov 14, 2020 at 07:20:37PM +, Mateusz Guzik wrote: >> >> Author: mjg >> >> Date: Sat Nov 14 19:20:37 2020 >> >> New Revision: 367692 >> >> URL: https://svnweb.freebsd.org/changeset/base/367692 >> >> >> >> Log: >> >> cred: reorder cr_audit to be closer to the lock >> >> >> >> This makes cr_uid avoid sharing. >> >> >> >> Modified: >> >> head/sys/sys/ucred.h >> >> >> >> Modified: head/sys/sys/ucred.h >> >> == >> >> --- head/sys/sys/ucred.h Sat Nov 14 19:19:27 2020(r367691) >> >> +++ head/sys/sys/ucred.h Sat Nov 14 19:20:37 2020(r367692) >> >> @@ -63,6 +63,7 @@ struct ucred { >> >> struct mtx cr_mtx; >> >> u_int cr_ref; /* (c) reference count */ >> >> u_int cr_users; /* (c) proc + thread using this cred */ >> >> + struct auditinfo_addr cr_audit; /* Audit properties. */ >> >> #define cr_startcopy cr_uid >> >> uid_t cr_uid; /* effective user id */ >> >> uid_t cr_ruid;/* real user id */ >> >> @@ -78,7 +79,6 @@ struct ucred { >> >> void*cr_pspare2[2]; /* general use 2 */ >> >> #define cr_endcopy cr_label >> >> struct label*cr_label; /* MAC label */ >> >> - struct auditinfo_addr cr_audit; /* Audit properties. */ >> >> gid_t *cr_groups; /* groups */ >> >> int cr_agroups; /* Available groups */ >> >> gid_t cr_smallgroups[XU_NGROUPS]; /* storage for small groups */ >> > >> > Hey Mateusz, >> > >> > Since this changes KBI, does __FreeBSD_version need bumping? >> > >> > Thanks, >> > >> > -- >> > Shawn Webb >> > Cofounder / Security Engineer >> > HardenedBSD >> > >> > GPG Key ID: 0xFF2E67A277F8E1FA >> > GPG Key Fingerprint: D206 BB45 15E0 9C49 0CF9 3633 C85B 0AF8 AB23 0FB2 >> > https://git-01.md.hardenedbsd.org/HardenedBSD/pubkeys/src/branch/master/Shawn_Webb/03A4CBEBB82EA5A67D9F3853FF2E67A277F8E1FA.pub.asc >> > >> >> >> -- >> Mateusz Guzik > > -- > Shawn Webb > Cofounder / Security Engineer > HardenedBSD > > GPG Key ID: 0xFF2E67A277F8E1FA > GPG Key Fingerprint: D206 BB45 15E0 9C49 0CF9 3633 C85B 0AF8 AB23 0FB2 > https://git-01.md.hardenedbsd.org/HardenedBSD/pubkeys/src/branch/master/Shawn_Webb/03A4CBEBB82EA5A67D9F3853FF2E67A277F8E1FA.pub.asc > -- Mateusz Guzik ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r367692 - head/sys/sys
I don't think so, it does not change any APIs On 11/14/20, Shawn Webb wrote: > On Sat, Nov 14, 2020 at 07:20:37PM +0000, Mateusz Guzik wrote: >> Author: mjg >> Date: Sat Nov 14 19:20:37 2020 >> New Revision: 367692 >> URL: https://svnweb.freebsd.org/changeset/base/367692 >> >> Log: >> cred: reorder cr_audit to be closer to the lock >> >> This makes cr_uid avoid sharing. >> >> Modified: >> head/sys/sys/ucred.h >> >> Modified: head/sys/sys/ucred.h >> == >> --- head/sys/sys/ucred.h Sat Nov 14 19:19:27 2020(r367691) >> +++ head/sys/sys/ucred.h Sat Nov 14 19:20:37 2020(r367692) >> @@ -63,6 +63,7 @@ struct ucred { >> struct mtx cr_mtx; >> u_int cr_ref; /* (c) reference count */ >> u_int cr_users; /* (c) proc + thread using this cred */ >> +struct auditinfo_addr cr_audit; /* Audit properties. */ >> #define cr_startcopy cr_uid >> uid_t cr_uid; /* effective user id */ >> uid_t cr_ruid;/* real user id */ >> @@ -78,7 +79,6 @@ struct ucred { >> void*cr_pspare2[2]; /* general use 2 */ >> #define cr_endcopy cr_label >> struct label*cr_label; /* MAC label */ >> -struct auditinfo_addr cr_audit; /* Audit properties. */ >> gid_t *cr_groups; /* groups */ >> int cr_agroups; /* Available groups */ >> gid_t cr_smallgroups[XU_NGROUPS]; /* storage for small groups */ > > Hey Mateusz, > > Since this changes KBI, does __FreeBSD_version need bumping? > > Thanks, > > -- > Shawn Webb > Cofounder / Security Engineer > HardenedBSD > > GPG Key ID: 0xFF2E67A277F8E1FA > GPG Key Fingerprint: D206 BB45 15E0 9C49 0CF9 3633 C85B 0AF8 AB23 0FB2 > https://git-01.md.hardenedbsd.org/HardenedBSD/pubkeys/src/branch/master/Shawn_Webb/03A4CBEBB82EA5A67D9F3853FF2E67A277F8E1FA.pub.asc > -- Mateusz Guzik ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r367697 - head/sys/sys
Author: mjg Date: Sat Nov 14 19:56:11 2020 New Revision: 367697 URL: https://svnweb.freebsd.org/changeset/base/367697 Log: cred: annotate credbatch_process argument as unused Fixes libprocstat compilation as zfs defines _KERNEL. Modified: head/sys/sys/ucred.h Modified: head/sys/sys/ucred.h == --- head/sys/sys/ucred.hSat Nov 14 19:23:07 2020(r367696) +++ head/sys/sys/ucred.hSat Nov 14 19:56:11 2020(r367697) @@ -129,7 +129,7 @@ credbatch_prep(struct credbatch *crb) } void credbatch_add(struct credbatch *crb, struct thread *td); static inline void -credbatch_process(struct credbatch *crb) +credbatch_process(struct credbatch *crb __unused) { } ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r367696 - head/sys/contrib/openzfs/module/zfs
Author: mjg Date: Sat Nov 14 19:23:07 2020 New Revision: 367696 URL: https://svnweb.freebsd.org/changeset/base/367696 Log: zfs: disable periodic arc updates They are only there to provide less innacurate statistics for debuggers. However, this is quite heavy-weight and instead it would be better to teach debuggers how to obtain the necessary information. Modified: head/sys/contrib/openzfs/module/zfs/arc.c Modified: head/sys/contrib/openzfs/module/zfs/arc.c == --- head/sys/contrib/openzfs/module/zfs/arc.c Sat Nov 14 19:22:02 2020 (r367695) +++ head/sys/contrib/openzfs/module/zfs/arc.c Sat Nov 14 19:23:07 2020 (r367696) @@ -4791,8 +4791,10 @@ arc_evict_cb_check(void *arg, zthr_t *zthr) * arc_state_t structures can be queried directly if more * accurate information is needed. */ +#ifndef __FreeBSD__ if (arc_ksp != NULL) arc_ksp->ks_update(arc_ksp, KSTAT_READ); +#endif /* * We have to rely on arc_wait_for_eviction() to tell us when to ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r367695 - in head/sys: kern sys
Author: mjg Date: Sat Nov 14 19:22:02 2020 New Revision: 367695 URL: https://svnweb.freebsd.org/changeset/base/367695 Log: thread: batch credential freeing Modified: head/sys/kern/kern_prot.c head/sys/kern/kern_thread.c head/sys/sys/ucred.h Modified: head/sys/kern/kern_prot.c == --- head/sys/kern/kern_prot.c Sat Nov 14 19:21:46 2020(r367694) +++ head/sys/kern/kern_prot.c Sat Nov 14 19:22:02 2020(r367695) @@ -86,6 +86,7 @@ static MALLOC_DEFINE(M_CRED, "cred", "credentials"); SYSCTL_NODE(_security, OID_AUTO, bsd, CTLFLAG_RW | CTLFLAG_MPSAFE, 0, "BSD security policy"); +static void crfree_final(struct ucred *cr); static void crsetgroups_locked(struct ucred *cr, int ngrp, gid_t *groups); @@ -1902,6 +1903,31 @@ crunuse(struct thread *td) return (crold); } +static void +crunusebatch(struct ucred *cr, int users, int ref) +{ + + KASSERT(users > 0, ("%s: passed users %d not > 0 ; cred %p", + __func__, users, cr)); + mtx_lock(>cr_mtx); + KASSERT(cr->cr_users >= users, ("%s: users %d not > %d on cred %p", + __func__, cr->cr_users, users, cr)); + cr->cr_users -= users; + cr->cr_ref += ref; + cr->cr_ref -= users; + if (cr->cr_users > 0) { + mtx_unlock(>cr_mtx); + return; + } + KASSERT(cr->cr_ref >= 0, ("%s: ref %d not >= 0 on cred %p", + __func__, cr->cr_ref, cr)); + if (cr->cr_ref > 0) { + mtx_unlock(>cr_mtx); + return; + } + crfree_final(cr); +} + void crcowfree(struct thread *td) { @@ -1935,6 +1961,44 @@ crcowsync(void) } /* + * Batching. + */ +void +credbatch_add(struct credbatch *crb, struct thread *td) +{ + struct ucred *cr; + + MPASS(td->td_realucred != NULL); + MPASS(td->td_realucred == td->td_ucred); + MPASS(td->td_state == TDS_INACTIVE); + cr = td->td_realucred; + KASSERT(cr->cr_users > 0, ("%s: users %d not > 0 on cred %p", + __func__, cr->cr_users, cr)); + if (crb->cred != cr) { + if (crb->users > 0) { + MPASS(crb->cred != NULL); + crunusebatch(crb->cred, crb->users, crb->ref); + crb->users = 0; + crb->ref = 0; + } + } + crb->cred = cr; + crb->users++; + crb->ref += td->td_ucredref; + td->td_ucredref = 0; + td->td_realucred = NULL; +} + +void +credbatch_final(struct credbatch *crb) +{ + + MPASS(crb->cred != NULL); + MPASS(crb->users > 0); + crunusebatch(crb->cred, crb->users, crb->ref); +} + +/* * Allocate a zeroed cred structure. */ struct ucred * @@ -2007,6 +2071,17 @@ crfree(struct ucred *cr) mtx_unlock(>cr_mtx); return; } + crfree_final(cr); +} + +static void +crfree_final(struct ucred *cr) +{ + + KASSERT(cr->cr_users == 0, ("%s: users %d not == 0 on cred %p", + __func__, cr->cr_users, cr)); + KASSERT(cr->cr_ref == 0, ("%s: ref %d not == 0 on cred %p", + __func__, cr->cr_ref, cr)); /* * Some callers of crget(), such as nfs_statfs(), allocate a temporary * credential, but don't allocate a uidinfo structure. Modified: head/sys/kern/kern_thread.c == --- head/sys/kern/kern_thread.c Sat Nov 14 19:21:46 2020(r367694) +++ head/sys/kern/kern_thread.c Sat Nov 14 19:22:02 2020(r367695) @@ -536,6 +536,7 @@ thread_reap(void) { struct thread *itd, *ntd; struct tidbatch tidbatch; + struct credbatch credbatch; int tdcount; struct plimit *lim; int limcount; @@ -553,6 +554,7 @@ thread_reap(void) return; tidbatch_prep(); + credbatch_prep(); tdcount = 0; lim = NULL; limcount = 0; @@ -560,8 +562,7 @@ thread_reap(void) ntd = itd->td_zombie; EVENTHANDLER_DIRECT_INVOKE(thread_dtor, itd); tidbatch_add(, itd); - MPASS(itd->td_realucred != NULL); - crcowfree(itd); + credbatch_add(, itd); MPASS(itd->td_limit != NULL); if (lim != itd->td_limit) { if (limcount != 0) { @@ -573,6 +574,7 @@ thread_reap(void) limcount++; thread_free_batched(itd); tidbatch_process(); + credbatch_process(); tdcount++; if (tdcount == 32) { thread_count_sub(tdcount); @@ -582,6 +584,7 @@ thread_reap(void) } tidbatch_final(); + credbatch_final(); if (tdcount != 0) { thread_count_sub(tdcount); } Modified: head/sys/sys/ucred.h
svn commit: r367694 - in head/sys: kern sys
Author: mjg Date: Sat Nov 14 19:21:46 2020 New Revision: 367694 URL: https://svnweb.freebsd.org/changeset/base/367694 Log: thread: batch resource limit free calls Modified: head/sys/kern/kern_resource.c head/sys/kern/kern_thread.c head/sys/sys/resourcevar.h Modified: head/sys/kern/kern_resource.c == --- head/sys/kern/kern_resource.c Sat Nov 14 19:20:58 2020 (r367693) +++ head/sys/kern/kern_resource.c Sat Nov 14 19:21:46 2020 (r367694) @@ -1236,6 +1236,14 @@ lim_free(struct plimit *limp) free((void *)limp, M_PLIMIT); } +void +lim_freen(struct plimit *limp, int n) +{ + + if (refcount_releasen(>pl_refcnt, n)) + free((void *)limp, M_PLIMIT); +} + /* * Make a copy of the plimit structure. * We share these structures copy-on-write after fork. Modified: head/sys/kern/kern_thread.c == --- head/sys/kern/kern_thread.c Sat Nov 14 19:20:58 2020(r367693) +++ head/sys/kern/kern_thread.c Sat Nov 14 19:21:46 2020(r367694) @@ -537,6 +537,8 @@ thread_reap(void) struct thread *itd, *ntd; struct tidbatch tidbatch; int tdcount; + struct plimit *lim; + int limcount; /* * Reading upfront is pessimal if followed by concurrent atomic_swap, @@ -552,11 +554,23 @@ thread_reap(void) tidbatch_prep(); tdcount = 0; + lim = NULL; + limcount = 0; while (itd != NULL) { ntd = itd->td_zombie; EVENTHANDLER_DIRECT_INVOKE(thread_dtor, itd); tidbatch_add(, itd); - thread_cow_free(itd); + MPASS(itd->td_realucred != NULL); + crcowfree(itd); + MPASS(itd->td_limit != NULL); + if (lim != itd->td_limit) { + if (limcount != 0) { + lim_freen(lim, limcount); + limcount = 0; + } + } + lim = itd->td_limit; + limcount++; thread_free_batched(itd); tidbatch_process(); tdcount++; @@ -571,6 +585,8 @@ thread_reap(void) if (tdcount != 0) { thread_count_sub(tdcount); } + MPASS(limcount != 0); + lim_freen(lim, limcount); } /* Modified: head/sys/sys/resourcevar.h == --- head/sys/sys/resourcevar.h Sat Nov 14 19:20:58 2020(r367693) +++ head/sys/sys/resourcevar.h Sat Nov 14 19:21:46 2020(r367694) @@ -145,6 +145,7 @@ rlim_t lim_cur(struct thread *td, int which); rlim_t lim_cur_proc(struct proc *p, int which); voidlim_fork(struct proc *p1, struct proc *p2); voidlim_free(struct plimit *limp); +voidlim_freen(struct plimit *limp, int n); struct plimit *lim_hold(struct plimit *limp); rlim_t lim_max(struct thread *td, int which); ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r367693 - head/sys/kern
Author: mjg Date: Sat Nov 14 19:20:58 2020 New Revision: 367693 URL: https://svnweb.freebsd.org/changeset/base/367693 Log: thread: rework tid batch to use helpers Modified: head/sys/kern/kern_thread.c Modified: head/sys/kern/kern_thread.c == --- head/sys/kern/kern_thread.c Sat Nov 14 19:20:37 2020(r367692) +++ head/sys/kern/kern_thread.c Sat Nov 14 19:20:58 2020(r367693) @@ -259,6 +259,54 @@ tid_free_batch(lwpid_t *batch, int n) } /* + * Batching for thread reapping. + */ +struct tidbatch { + lwpid_t tab[16]; + int n; +}; + +static void +tidbatch_prep(struct tidbatch *tb) +{ + + tb->n = 0; +} + +static void +tidbatch_add(struct tidbatch *tb, struct thread *td) +{ + + KASSERT(tb->n < nitems(tb->tab), + ("%s: count too high %d", __func__, tb->n)); + tb->tab[tb->n] = td->td_tid; + tb->n++; +} + +static void +tidbatch_process(struct tidbatch *tb) +{ + + KASSERT(tb->n <= nitems(tb->tab), + ("%s: count too high %d", __func__, tb->n)); + if (tb->n == nitems(tb->tab)) { + tid_free_batch(tb->tab, tb->n); + tb->n = 0; + } +} + +static void +tidbatch_final(struct tidbatch *tb) +{ + + KASSERT(tb->n <= nitems(tb->tab), + ("%s: count too high %d", __func__, tb->n)); + if (tb->n != 0) { + tid_free_batch(tb->tab, tb->n); + } +} + +/* * Prepare a thread for use. */ static int @@ -487,8 +535,8 @@ void thread_reap(void) { struct thread *itd, *ntd; - lwpid_t tidbatch[16]; - int tidbatchn; + struct tidbatch tidbatch; + int tdcount; /* * Reading upfront is pessimal if followed by concurrent atomic_swap, @@ -499,24 +547,29 @@ thread_reap(void) itd = (struct thread *)atomic_swap_ptr((uintptr_t *)_zombies, (uintptr_t)NULL); - tidbatchn = 0; + if (itd == NULL) + return; + + tidbatch_prep(); + tdcount = 0; while (itd != NULL) { ntd = itd->td_zombie; - tidbatch[tidbatchn] = itd->td_tid; - tidbatchn++; + EVENTHANDLER_DIRECT_INVOKE(thread_dtor, itd); + tidbatch_add(, itd); thread_cow_free(itd); thread_free_batched(itd); - if (tidbatchn == nitems(tidbatch)) { - tid_free_batch(tidbatch, tidbatchn); - thread_count_sub(tidbatchn); - tidbatchn = 0; + tidbatch_process(); + tdcount++; + if (tdcount == 32) { + thread_count_sub(tdcount); + tdcount = 0; } itd = ntd; } - if (tidbatchn != 0) { - tid_free_batch(tidbatch, tidbatchn); - thread_count_sub(tidbatchn); + tidbatch_final(); + if (tdcount != 0) { + thread_count_sub(tdcount); } } @@ -567,7 +620,6 @@ static void thread_free_batched(struct thread *td) { - EVENTHANDLER_DIRECT_INVOKE(thread_dtor, td); lock_profile_thread_exit(td); if (td->td_cpuset) cpuset_rel(td->td_cpuset); @@ -588,6 +640,7 @@ thread_free(struct thread *td) { lwpid_t tid; + EVENTHANDLER_DIRECT_INVOKE(thread_dtor, td); tid = td->td_tid; thread_free_batched(td); tid_free(tid); ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r367692 - head/sys/sys
Author: mjg Date: Sat Nov 14 19:20:37 2020 New Revision: 367692 URL: https://svnweb.freebsd.org/changeset/base/367692 Log: cred: reorder cr_audit to be closer to the lock This makes cr_uid avoid sharing. Modified: head/sys/sys/ucred.h Modified: head/sys/sys/ucred.h == --- head/sys/sys/ucred.hSat Nov 14 19:19:27 2020(r367691) +++ head/sys/sys/ucred.hSat Nov 14 19:20:37 2020(r367692) @@ -63,6 +63,7 @@ struct ucred { struct mtx cr_mtx; u_int cr_ref; /* (c) reference count */ u_int cr_users; /* (c) proc + thread using this cred */ + struct auditinfo_addr cr_audit; /* Audit properties. */ #definecr_startcopy cr_uid uid_t cr_uid; /* effective user id */ uid_t cr_ruid;/* real user id */ @@ -78,7 +79,6 @@ struct ucred { void*cr_pspare2[2]; /* general use 2 */ #definecr_endcopy cr_label struct label*cr_label; /* MAC label */ - struct auditinfo_addr cr_audit; /* Audit properties. */ gid_t *cr_groups; /* groups */ int cr_agroups; /* Available groups */ gid_t cr_smallgroups[XU_NGROUPS]; /* storage for small groups */ ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r367691 - head/sys/kern
Author: mjg Date: Sat Nov 14 19:19:27 2020 New Revision: 367691 URL: https://svnweb.freebsd.org/changeset/base/367691 Log: thread: pad tid lock On a kernel with other changes this bumps 104-way thread creation/destruction from 0.96 mln ops/s to 1.1 mln ops/s. Modified: head/sys/kern/kern_thread.c Modified: head/sys/kern/kern_thread.c == --- head/sys/kern/kern_thread.c Sat Nov 14 19:16:39 2020(r367690) +++ head/sys/kern/kern_thread.c Sat Nov 14 19:19:27 2020(r367691) @@ -135,7 +135,7 @@ static int thread_unsuspend_one(struct thread *td, str bool boundary); static void thread_free_batched(struct thread *td); -static struct mtx tid_lock; +static __exclusive_cache_line struct mtx tid_lock; static bitstr_t *tid_bitmap; static MALLOC_DEFINE(M_TIDHASH, "tidhash", "thread hash"); ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r367650 - head/sys/kern
Author: mjg Date: Fri Nov 13 19:22:53 2020 New Revision: 367650 URL: https://svnweb.freebsd.org/changeset/base/367650 Log: malloc: retire MALLOC_PROFILE The global array has prohibitive performance impact on multicore systems. The same data (and more) can be obtained with dtrace. Reviewed by: markj Differential Revision:https://reviews.freebsd.org/D27199 Modified: head/sys/kern/kern_malloc.c Modified: head/sys/kern/kern_malloc.c == --- head/sys/kern/kern_malloc.c Fri Nov 13 19:09:21 2020(r367649) +++ head/sys/kern/kern_malloc.c Fri Nov 13 19:22:53 2020(r367650) @@ -223,12 +223,6 @@ SYSCTL_PROC(_vm_malloc, OID_AUTO, zone_sizes, */ struct mtx malloc_mtx; -#ifdef MALLOC_PROFILE -uint64_t krequests[KMEM_ZSIZE + 1]; - -static int sysctl_kern_mprof(SYSCTL_HANDLER_ARGS); -#endif - static int sysctl_kern_malloc_stats(SYSCTL_HANDLER_ARGS); #if defined(MALLOC_MAKE_FAILURES) || (MALLOC_DEBUG_MAXZONES > 1) @@ -635,9 +629,6 @@ void * size = (size & ~KMEM_ZMASK) + KMEM_ZBASE; indx = kmemsize[size >> KMEM_ZSHIFT]; zone = kmemzones[indx].kz_zone[mtp_get_subzone(mtp)]; -#ifdef MALLOC_PROFILE - krequests[size >> KMEM_ZSHIFT]++; -#endif va = uma_zalloc(zone, flags); if (va != NULL) size = zone->uz_size; @@ -673,9 +664,6 @@ malloc_domain(size_t *sizep, int *indxp, struct malloc size = (size & ~KMEM_ZMASK) + KMEM_ZBASE; indx = kmemsize[size >> KMEM_ZSHIFT]; zone = kmemzones[indx].kz_zone[mtp_get_subzone(mtp)]; -#ifdef MALLOC_PROFILE - krequests[size >> KMEM_ZSHIFT]++; -#endif va = uma_zalloc_domain(zone, NULL, domain, flags); if (va != NULL) *sizep = zone->uz_size; @@ -1495,52 +1483,3 @@ DB_SHOW_COMMAND(multizone_matches, db_show_multizone_m } #endif /* MALLOC_DEBUG_MAXZONES > 1 */ #endif /* DDB */ - -#ifdef MALLOC_PROFILE - -static int -sysctl_kern_mprof(SYSCTL_HANDLER_ARGS) -{ - struct sbuf sbuf; - uint64_t count; - uint64_t waste; - uint64_t mem; - int error; - int rsize; - int size; - int i; - - waste = 0; - mem = 0; - - error = sysctl_wire_old_buffer(req, 0); - if (error != 0) - return (error); - sbuf_new_for_sysctl(, NULL, 128, req); - sbuf_printf(, - "\n SizeRequests Real Size\n"); - for (i = 0; i < KMEM_ZSIZE; i++) { - size = i << KMEM_ZSHIFT; - rsize = kmemzones[kmemsize[i]].kz_size; - count = (long long unsigned)krequests[i]; - - sbuf_printf(, "%6d%28llu%11d\n", size, - (unsigned long long)count, rsize); - - if ((rsize * count) > (size * count)) - waste += (rsize * count) - (size * count); - mem += (rsize * count); - } - sbuf_printf(, - "\nTotal memory used:\t%30llu\nTotal Memory wasted:\t%30llu\n", - (unsigned long long)mem, (unsigned long long)waste); - error = sbuf_finish(); - sbuf_delete(); - return (error); -} - -SYSCTL_OID(_kern, OID_AUTO, mprof, -CTLTYPE_STRING | CTLFLAG_RD | CTLFLAG_NEEDGIANT, NULL, 0, -sysctl_kern_mprof, "A", -"Malloc Profiling"); -#endif /* MALLOC_PROFILE */ ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r367631 - in head/sys: kern sys
On 11/13/20, Konstantin Belousov wrote: > +static u_long vn_lock_pair_pause_cnt; > +SYSCTL_ULONG(_debug, OID_AUTO, vn_lock_pair_pause, CTLFLAG_RD, > +_lock_pair_pause_cnt, 0, > +"Count of vn_lock_pair deadlocks"); > + > +static void > +vn_lock_pair_pause(const char *wmesg) > +{ > + atomic_add_long(_lock_pair_pause_cnt, 1); > + pause(wmesg, prng32_bounded(hz / 10)); > +} > + > +/* > + * Lock pair of vnodes vp1, vp2, avoiding lock order reversal. > + * vp1_locked indicates whether vp1 is exclusively locked; if not, vp1 > + * must be unlocked. Same for vp2 and vp2_locked. One of the vnodes > + * can be NULL. > + * > + * The function returns with both vnodes exclusively locked, and > + * guarantees that it does not create lock order reversal with other > + * threads during its execution. Both vnodes could be unlocked > + * temporary (and reclaimed). > + */ > +void > +vn_lock_pair(struct vnode *vp1, bool vp1_locked, struct vnode *vp2, > +bool vp2_locked) > +{ > + int error; > + > + if (vp1 == NULL && vp2 == NULL) > + return; > + if (vp1 != NULL) { > + if (vp1_locked) > + ASSERT_VOP_ELOCKED(vp1, "vp1"); > + else > + ASSERT_VOP_UNLOCKED(vp1, "vp1"); > + } else { > + vp1_locked = true; > + } > + if (vp2 != NULL) { > + if (vp2_locked) > + ASSERT_VOP_ELOCKED(vp2, "vp2"); > + else > + ASSERT_VOP_UNLOCKED(vp2, "vp2"); > + } else { > + vp2_locked = true; > + } > + if (!vp1_locked && !vp2_locked) { > + vn_lock(vp1, LK_EXCLUSIVE | LK_RETRY); > + vp1_locked = true; > + } > + > + for (;;) { > + if (vp1_locked && vp2_locked) > + break; > + if (vp1_locked && vp2 != NULL) { > + if (vp1 != NULL) { > + error = VOP_LOCK1(vp2, LK_EXCLUSIVE | LK_NOWAIT, > + __FILE__, __LINE__); > + if (error == 0) > + break; > + VOP_UNLOCK(vp1); > + vp1_locked = false; > + vn_lock_pair_pause("vlp1"); > + } > + vn_lock(vp2, LK_EXCLUSIVE | LK_RETRY); > + vp2_locked = true; > + } > + if (vp2_locked && vp1 != NULL) { > + if (vp2 != NULL) { > + error = VOP_LOCK1(vp1, LK_EXCLUSIVE | LK_NOWAIT, > + __FILE__, __LINE__); > + if (error == 0) > + break; > + VOP_UNLOCK(vp2); > + vp2_locked = false; > + vn_lock_pair_pause("vlp2"); > + } > + vn_lock(vp1, LK_EXCLUSIVE | LK_RETRY); > + vp1_locked = true; > + } > + } > + if (vp1 != NULL) > + ASSERT_VOP_ELOCKED(vp1, "vp1 ret"); > + if (vp2 != NULL) > + ASSERT_VOP_ELOCKED(vp2, "vp2 ret"); > } > Multiple callers who get here with flipped addresses can end up failing against each other in an indefinite loop. Instead, after initial trylocking fails, the routine should establish ordering it will follow for itself, for example by sorting by address. Then pseudo-code would be: retry: vn_lock(lower, ...); if (!VOP_LOCK(higher, LK_NOWAIT ...)) { vn_unlock(lower); vn_lock(higher); vn_unlock(higher); goto retry; } Note this also eliminates the pause. -- Mateusz Guzik ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r367626 - head/sys/geom/bde
On 11/12/20, Oliver Pinter wrote: > On Thursday, November 12, 2020, Mateusz Guzik wrote: > >> Author: mjg >> Date: Thu Nov 12 20:20:57 2020 >> New Revision: 367626 >> URL: https://svnweb.freebsd.org/changeset/base/367626 >> >> Log: >> gbde: replace malloc_last_fail with a kludge >> >> This facilitates removal of malloc_last_fail without really impacting >> anything. >> >> Modified: >> head/sys/geom/bde/g_bde_work.c >> >> Modified: head/sys/geom/bde/g_bde_work.c >> >> == >> --- head/sys/geom/bde/g_bde_work.c Thu Nov 12 20:20:43 2020 >> (r367625) >> +++ head/sys/geom/bde/g_bde_work.c Thu Nov 12 20:20:57 2020 >> (r367626) >> @@ -77,6 +77,20 @@ >> #include >> #include >> >> +/* >> + * FIXME: This used to call malloc_last_fail which in practice was >> almost >> + * guaranteed to return time_uptime even in face of severe memory >> shortage. >> + * As GBDE is the only consumer the kludge below was added to facilitate >> the >> + * removal with minimial changes. The code should be fixed to respond to >> memory >> + * pressure (e.g., by using lowmem eventhandler) instead. >> + */ >> +static int >> +g_bde_malloc_last_fail(void) >> +{ >> + >> + return (time_uptime); >> +} >> + > > > Previously malloc_last_fail returned a relatively small number - if i read > the code correctly: > > -int > -malloc_last_fail(void) > -{ > - > - return (time_uptime - t_malloc_fail); > -} > - > > >> static void g_bde_delete_sector(struct g_bde_softc *wp, struct >> g_bde_sector *sp); >> static struct g_bde_sector * g_bde_new_sector(struct g_bde_work *wp, >> u_int len); >> static void g_bde_release_keysector(struct g_bde_work *wp); >> @@ -210,7 +224,7 @@ g_bde_get_keysector(struct g_bde_work *wp) >> g_trace(G_T_TOPOLOGY, "g_bde_get_keysector(%p, %jd)", wp, >> (intmax_t)offset); >> sc = wp->softc; >> >> - if (malloc_last_fail() < g_bde_ncache) >> + if (g_bde_malloc_last_fail() < g_bde_ncache) >> g_bde_purge_sector(sc, -1); > > > And in this case, the semantic change renders all of these calls from alway > true to always false expression. > t_malloc_fail value was almost guaranteed to always be 0, so there is no actual change. The hack was put in place so that gbde does not stall work on malloc. gbde itself definitely needs love. > >> >> sp = TAILQ_FIRST(>freelist); >> @@ -228,7 +242,7 @@ g_bde_get_keysector(struct g_bde_work *wp) >> if (sp->ref == 1) >> sp->owner = wp; >> } else { >> - if (malloc_last_fail() < g_bde_ncache) { >> + if (g_bde_malloc_last_fail() < g_bde_ncache) { >> TAILQ_FOREACH(sp, >freelist, list) >> if (sp->ref == 0) >> break; >> @@ -311,7 +325,7 @@ g_bde_purge_sector(struct g_bde_softc *sc, int >> fractio >> if (fraction > 0) >> n = sc->ncache / fraction + 1; >> else >> - n = g_bde_ncache - malloc_last_fail(); >> + n = g_bde_ncache - g_bde_malloc_last_fail(); >> if (n < 0) >> return; >> if (n > sc->ncache) >> ___ >> svn-src-head@freebsd.org mailing list >> https://lists.freebsd.org/mailman/listinfo/svn-src-head >> To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org" >> > -- Mateusz Guzik ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r367627 - in head/sys: kern sys
Author: mjg Date: Thu Nov 12 20:22:58 2020 New Revision: 367627 URL: https://svnweb.freebsd.org/changeset/base/367627 Log: malloc: retire malloc_last_fail The routine does not serve any practical purpose. Memory can be allocated in many other ways and most consumers pass the M_WAITOK flag, making malloc not fail in the first place. Reviewed by: markj Differential Revision:https://reviews.freebsd.org/D27143 Modified: head/sys/kern/kern_malloc.c head/sys/sys/malloc.h head/sys/sys/param.h Modified: head/sys/kern/kern_malloc.c == --- head/sys/kern/kern_malloc.c Thu Nov 12 20:20:57 2020(r367626) +++ head/sys/kern/kern_malloc.c Thu Nov 12 20:22:58 2020(r367627) @@ -231,11 +231,6 @@ static int sysctl_kern_mprof(SYSCTL_HANDLER_ARGS); static int sysctl_kern_malloc_stats(SYSCTL_HANDLER_ARGS); -/* - * time_uptime of the last malloc(9) failure (induced or real). - */ -static time_t t_malloc_fail; - #if defined(MALLOC_MAKE_FAILURES) || (MALLOC_DEBUG_MAXZONES > 1) static SYSCTL_NODE(_debug, OID_AUTO, malloc, CTLFLAG_RD | CTLFLAG_MPSAFE, 0, "Kernel malloc debugging options"); @@ -373,13 +368,6 @@ mtp_get_subzone(struct malloc_type *mtp) } #endif /* MALLOC_DEBUG_MAXZONES > 1 */ -int -malloc_last_fail(void) -{ - - return (time_uptime - t_malloc_fail); -} - /* * An allocation has succeeded -- update malloc type statistics for the * amount of bucket size. Occurs within a critical section so that the @@ -535,7 +523,6 @@ malloc_dbg(caddr_t *vap, size_t *sizep, struct malloc_ atomic_add_int(_nowait_count, 1); if ((malloc_nowait_count % malloc_failure_rate) == 0) { atomic_add_int(_failure_count, 1); - t_malloc_fail = time_uptime; *vap = NULL; return (EJUSTRETURN); } @@ -662,7 +649,6 @@ void * if (__predict_false(va == NULL)) { KASSERT((flags & M_WAITOK) == 0, ("malloc(M_WAITOK) returned NULL")); - t_malloc_fail = time_uptime; } #ifdef DEBUG_REDZONE if (va != NULL) @@ -730,7 +716,6 @@ malloc_domainset(size_t size, struct malloc_type *mtp, if (__predict_false(va == NULL)) { KASSERT((flags & M_WAITOK) == 0, ("malloc(M_WAITOK) returned NULL")); - t_malloc_fail = time_uptime; } #ifdef DEBUG_REDZONE if (va != NULL) @@ -761,7 +746,6 @@ malloc_exec(size_t size, struct malloc_type *mtp, int if (__predict_false(va == NULL)) { KASSERT((flags & M_WAITOK) == 0, ("malloc(M_WAITOK) returned NULL")); - t_malloc_fail = time_uptime; } #ifdef DEBUG_REDZONE if (va != NULL) @@ -791,7 +775,6 @@ malloc_domainset_exec(size_t size, struct malloc_type if (__predict_false(va == NULL)) { KASSERT((flags & M_WAITOK) == 0, ("malloc(M_WAITOK) returned NULL")); - t_malloc_fail = time_uptime; } #ifdef DEBUG_REDZONE if (va != NULL) Modified: head/sys/sys/malloc.h == --- head/sys/sys/malloc.h Thu Nov 12 20:20:57 2020(r367626) +++ head/sys/sys/malloc.h Thu Nov 12 20:22:58 2020(r367627) @@ -252,7 +252,6 @@ void*malloc_domainset_exec(size_t size, struct malloc struct domainset *ds, int flags) __malloc_like __result_use_check __alloc_size(1); void malloc_init(void *); -intmalloc_last_fail(void); void malloc_type_allocated(struct malloc_type *type, unsigned long size); void malloc_type_freed(struct malloc_type *type, unsigned long size); void malloc_type_list(malloc_type_list_func_t *, void *); Modified: head/sys/sys/param.h == --- head/sys/sys/param.hThu Nov 12 20:20:57 2020(r367626) +++ head/sys/sys/param.hThu Nov 12 20:22:58 2020(r367627) @@ -60,7 +60,7 @@ * in the range 5 to 9. */ #undef __FreeBSD_version -#define __FreeBSD_version 1300128 /* Master, propagated to newvers */ +#define __FreeBSD_version 1300129 /* Master, propagated to newvers */ /* * __FreeBSD_kernel__ indicates that this system uses the kernel of FreeBSD, ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r367626 - head/sys/geom/bde
Author: mjg Date: Thu Nov 12 20:20:57 2020 New Revision: 367626 URL: https://svnweb.freebsd.org/changeset/base/367626 Log: gbde: replace malloc_last_fail with a kludge This facilitates removal of malloc_last_fail without really impacting anything. Modified: head/sys/geom/bde/g_bde_work.c Modified: head/sys/geom/bde/g_bde_work.c == --- head/sys/geom/bde/g_bde_work.c Thu Nov 12 20:20:43 2020 (r367625) +++ head/sys/geom/bde/g_bde_work.c Thu Nov 12 20:20:57 2020 (r367626) @@ -77,6 +77,20 @@ #include #include +/* + * FIXME: This used to call malloc_last_fail which in practice was almost + * guaranteed to return time_uptime even in face of severe memory shortage. + * As GBDE is the only consumer the kludge below was added to facilitate the + * removal with minimial changes. The code should be fixed to respond to memory + * pressure (e.g., by using lowmem eventhandler) instead. + */ +static int +g_bde_malloc_last_fail(void) +{ + + return (time_uptime); +} + static void g_bde_delete_sector(struct g_bde_softc *wp, struct g_bde_sector *sp); static struct g_bde_sector * g_bde_new_sector(struct g_bde_work *wp, u_int len); static void g_bde_release_keysector(struct g_bde_work *wp); @@ -210,7 +224,7 @@ g_bde_get_keysector(struct g_bde_work *wp) g_trace(G_T_TOPOLOGY, "g_bde_get_keysector(%p, %jd)", wp, (intmax_t)offset); sc = wp->softc; - if (malloc_last_fail() < g_bde_ncache) + if (g_bde_malloc_last_fail() < g_bde_ncache) g_bde_purge_sector(sc, -1); sp = TAILQ_FIRST(>freelist); @@ -228,7 +242,7 @@ g_bde_get_keysector(struct g_bde_work *wp) if (sp->ref == 1) sp->owner = wp; } else { - if (malloc_last_fail() < g_bde_ncache) { + if (g_bde_malloc_last_fail() < g_bde_ncache) { TAILQ_FOREACH(sp, >freelist, list) if (sp->ref == 0) break; @@ -311,7 +325,7 @@ g_bde_purge_sector(struct g_bde_softc *sc, int fractio if (fraction > 0) n = sc->ncache / fraction + 1; else - n = g_bde_ncache - malloc_last_fail(); + n = g_bde_ncache - g_bde_malloc_last_fail(); if (n < 0) return; if (n > sc->ncache) ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r367605 - head/sys/kern
Author: mjg Date: Thu Nov 12 00:29:23 2020 New Revision: 367605 URL: https://svnweb.freebsd.org/changeset/base/367605 Log: thread: move nthread management out of tid_alloc While this adds more work single-threaded, it also enables SMP-related speed ups. Modified: head/sys/kern/kern_thread.c Modified: head/sys/kern/kern_thread.c == --- head/sys/kern/kern_thread.c Wed Nov 11 22:35:23 2020(r367604) +++ head/sys/kern/kern_thread.c Thu Nov 12 00:29:23 2020(r367605) @@ -144,7 +144,7 @@ static int maxthread; SYSCTL_INT(_kern, OID_AUTO, maxthread, CTLFLAG_RDTUN, , 0, "Maximum number of threads"); -static int nthreads; +static __exclusive_cache_line int nthreads; static LIST_HEAD(tidhashhead, thread) *tidhashtbl; static u_long tidhash; @@ -158,29 +158,52 @@ EVENTHANDLER_LIST_DEFINE(thread_dtor); EVENTHANDLER_LIST_DEFINE(thread_init); EVENTHANDLER_LIST_DEFINE(thread_fini); -static lwpid_t -tid_alloc(void) +static bool +thread_count_inc(void) { static struct timeval lastfail; static int curfail; - static lwpid_t trytid; - lwpid_t tid; + int nthreads_new; - mtx_lock(_lock); - if (nthreads + 1 >= maxthread - 100) { + thread_reap(); + + nthreads_new = atomic_fetchadd_int(, 1) + 1; + if (nthreads_new >= maxthread - 100) { if (priv_check_cred(curthread->td_ucred, PRIV_MAXPROC) != 0 || - nthreads + 1 >= maxthread) { - mtx_unlock(_lock); + nthreads_new >= maxthread) { + atomic_subtract_int(, 1); if (ppsratecheck(, , 1)) { printf("maxthread limit exceeded by uid %u " "(pid %d); consider increasing kern.maxthread\n", curthread->td_ucred->cr_ruid, curproc->p_pid); } - return (-1); + return (false); } } + return (true); +} - nthreads++; +static void +thread_count_sub(int n) +{ + + atomic_subtract_int(, n); +} + +static void +thread_count_dec(void) +{ + + thread_count_sub(1); +} + +static lwpid_t +tid_alloc(void) +{ + static lwpid_t trytid; + lwpid_t tid; + + mtx_lock(_lock); /* * It is an invariant that the bitmap is big enough to hold maxthread * IDs. If we got to this point there has to be at least one free. @@ -212,7 +235,6 @@ tid_free_locked(lwpid_t rtid) KASSERT(bit_test(tid_bitmap, tid) != 0, ("thread ID %d not allocated\n", rtid)); bit_clear(tid_bitmap, tid); - nthreads--; } static void @@ -398,6 +420,10 @@ threadinit(void) mtx_init(_lock, "TID lock", NULL, MTX_DEF); tid_bitmap = bit_alloc(maxthread, M_TIDHASH, M_WAITOK); + /* +* Handle thread0. +*/ + thread_count_inc(); tid0 = tid_alloc(); if (tid0 != THREAD0_TID) panic("tid0 %d != %d\n", tid0, THREAD0_TID); @@ -482,6 +508,7 @@ thread_reap(void) thread_free_batched(itd); if (tidbatchn == nitems(tidbatch)) { tid_free_batch(tidbatch, tidbatchn); + thread_count_sub(tidbatchn); tidbatchn = 0; } itd = ntd; @@ -489,6 +516,7 @@ thread_reap(void) if (tidbatchn != 0) { tid_free_batch(tidbatch, tidbatchn); + thread_count_sub(tidbatchn); } } @@ -501,18 +529,17 @@ thread_alloc(int pages) struct thread *td; lwpid_t tid; - thread_reap(); /* check if any zombies to get */ - - tid = tid_alloc(); - if (tid == -1) { + if (!thread_count_inc()) { return (NULL); } + tid = tid_alloc(); td = uma_zalloc(thread_zone, M_WAITOK); KASSERT(td->td_kstack == 0, ("thread_alloc got thread with kstack")); if (!vm_thread_new(td, pages)) { uma_zfree(thread_zone, td); tid_free(tid); + thread_count_dec(); return (NULL); } td->td_tid = tid; @@ -564,6 +591,7 @@ thread_free(struct thread *td) tid = td->td_tid; thread_free_batched(td); tid_free(tid); + thread_count_dec(); } void ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r367598 - head/sys/kern
Author: mjg Date: Wed Nov 11 18:45:06 2020 New Revision: 367598 URL: https://svnweb.freebsd.org/changeset/base/367598 Log: thread: batch tid_free calls in thread_reap This eliminates the highly pessimal pattern of relocking from multiple CPUs in quick succession. Note this is still globally serialized. Modified: head/sys/kern/kern_thread.c Modified: head/sys/kern/kern_thread.c == --- head/sys/kern/kern_thread.c Wed Nov 11 18:43:51 2020(r367597) +++ head/sys/kern/kern_thread.c Wed Nov 11 18:45:06 2020(r367598) @@ -133,6 +133,7 @@ static __exclusive_cache_line struct thread *thread_zo static void thread_zombie(struct thread *); static int thread_unsuspend_one(struct thread *td, struct proc *p, bool boundary); +static void thread_free_batched(struct thread *td); static struct mtx tid_lock; static bitstr_t *tid_bitmap; @@ -200,21 +201,41 @@ tid_alloc(void) } static void -tid_free(lwpid_t rtid) +tid_free_locked(lwpid_t rtid) { lwpid_t tid; + mtx_assert(_lock, MA_OWNED); KASSERT(rtid >= NO_PID, ("%s: invalid tid %d\n", __func__, rtid)); tid = rtid - NO_PID; - mtx_lock(_lock); KASSERT(bit_test(tid_bitmap, tid) != 0, ("thread ID %d not allocated\n", rtid)); bit_clear(tid_bitmap, tid); nthreads--; +} + +static void +tid_free(lwpid_t rtid) +{ + + mtx_lock(_lock); + tid_free_locked(rtid); mtx_unlock(_lock); } +static void +tid_free_batch(lwpid_t *batch, int n) +{ + int i; + + mtx_lock(_lock); + for (i = 0; i < n; i++) { + tid_free_locked(batch[i]); + } + mtx_unlock(_lock); +} + /* * Prepare a thread for use. */ @@ -440,6 +461,8 @@ void thread_reap(void) { struct thread *itd, *ntd; + lwpid_t tidbatch[16]; + int tidbatchn; /* * Reading upfront is pessimal if followed by concurrent atomic_swap, @@ -450,12 +473,23 @@ thread_reap(void) itd = (struct thread *)atomic_swap_ptr((uintptr_t *)_zombies, (uintptr_t)NULL); + tidbatchn = 0; while (itd != NULL) { ntd = itd->td_zombie; + tidbatch[tidbatchn] = itd->td_tid; + tidbatchn++; thread_cow_free(itd); - thread_free(itd); + thread_free_batched(itd); + if (tidbatchn == nitems(tidbatch)) { + tid_free_batch(tidbatch, tidbatchn); + tidbatchn = 0; + } itd = ntd; } + + if (tidbatchn != 0) { + tid_free_batch(tidbatch, tidbatchn); + } } /* @@ -502,8 +536,8 @@ thread_alloc_stack(struct thread *td, int pages) /* * Deallocate a thread. */ -void -thread_free(struct thread *td) +static void +thread_free_batched(struct thread *td) { EVENTHANDLER_DIRECT_INVOKE(thread_dtor, td); @@ -515,9 +549,21 @@ thread_free(struct thread *td) if (td->td_kstack != 0) vm_thread_dispose(td); callout_drain(>td_slpcallout); - tid_free(td->td_tid); + /* +* Freeing handled by the caller. +*/ td->td_tid = -1; uma_zfree(thread_zone, td); +} + +void +thread_free(struct thread *td) +{ + lwpid_t tid; + + tid = td->td_tid; + thread_free_batched(td); + tid_free(tid); } void ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r367597 - in head/sys: kern sys
Author: mjg Date: Wed Nov 11 18:43:51 2020 New Revision: 367597 URL: https://svnweb.freebsd.org/changeset/base/367597 Log: thread: lockless zombie list manipulation This gets rid of the most contended spinlock seen when creating/destroying threads in a loop. (modulo kstack) Tested by:alfredo (ppc64), bdragon (ppc64) Modified: head/sys/kern/kern_thread.c head/sys/sys/proc.h Modified: head/sys/kern/kern_thread.c == --- head/sys/kern/kern_thread.c Wed Nov 11 18:00:06 2020(r367596) +++ head/sys/kern/kern_thread.c Wed Nov 11 18:43:51 2020(r367597) @@ -128,9 +128,7 @@ SDT_PROBE_DEFINE(proc, , , lwp__exit); */ static uma_zone_t thread_zone; -TAILQ_HEAD(, thread) zombie_threads = TAILQ_HEAD_INITIALIZER(zombie_threads); -static struct mtx zombie_lock; -MTX_SYSINIT(zombie_lock, _lock, "zombie lock", MTX_SPIN); +static __exclusive_cache_line struct thread *thread_zombies; static void thread_zombie(struct thread *); static int thread_unsuspend_one(struct thread *td, struct proc *p, @@ -409,14 +407,20 @@ threadinit(void) /* * Place an unused thread on the zombie list. - * Use the slpq as that must be unused by now. */ void thread_zombie(struct thread *td) { - mtx_lock_spin(_lock); - TAILQ_INSERT_HEAD(_threads, td, td_slpq); - mtx_unlock_spin(_lock); + struct thread *ztd; + + ztd = atomic_load_ptr(_zombies); + for (;;) { + td->td_zombie = ztd; + if (atomic_fcmpset_rel_ptr((uintptr_t *)_zombies, + (uintptr_t *), (uintptr_t)td)) + break; + continue; + } } /* @@ -430,29 +434,27 @@ thread_stash(struct thread *td) } /* - * Reap zombie resources. + * Reap zombie threads. */ void thread_reap(void) { - struct thread *td_first, *td_next; + struct thread *itd, *ntd; /* -* Don't even bother to lock if none at this instant, -* we really don't care about the next instant. +* Reading upfront is pessimal if followed by concurrent atomic_swap, +* but most of the time the list is empty. */ - if (!TAILQ_EMPTY(_threads)) { - mtx_lock_spin(_lock); - td_first = TAILQ_FIRST(_threads); - if (td_first) - TAILQ_INIT(_threads); - mtx_unlock_spin(_lock); - while (td_first) { - td_next = TAILQ_NEXT(td_first, td_slpq); - thread_cow_free(td_first); - thread_free(td_first); - td_first = td_next; - } + if (thread_zombies == NULL) + return; + + itd = (struct thread *)atomic_swap_ptr((uintptr_t *)_zombies, + (uintptr_t)NULL); + while (itd != NULL) { + ntd = itd->td_zombie; + thread_cow_free(itd); + thread_free(itd); + itd = ntd; } } Modified: head/sys/sys/proc.h == --- head/sys/sys/proc.h Wed Nov 11 18:00:06 2020(r367596) +++ head/sys/sys/proc.h Wed Nov 11 18:43:51 2020(r367597) @@ -229,7 +229,10 @@ struct thread { struct proc *td_proc; /* (*) Associated process. */ TAILQ_ENTRY(thread) td_plist; /* (*) All threads in this proc. */ TAILQ_ENTRY(thread) td_runq;/* (t) Run queue. */ - TAILQ_ENTRY(thread) td_slpq;/* (t) Sleep queue. */ + union { + TAILQ_ENTRY(thread) td_slpq;/* (t) Sleep queue. */ + struct thread *td_zombie; /* Zombie list linkage */ + }; TAILQ_ENTRY(thread) td_lockq; /* (t) Lock queue. */ LIST_ENTRY(thread) td_hash; /* (d) Hash chain. */ struct cpuset *td_cpuset; /* (t) CPU affinity mask. */ ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r367585 - head/sys/kern
Author: mjg Date: Wed Nov 11 08:51:04 2020 New Revision: 367585 URL: https://svnweb.freebsd.org/changeset/base/367585 Log: thread: add more fine-grained tidhash locking Note this still does not scale but is enough to move it out of the way for the foreseable future. In particular a trivial benchmark spawning/killing threads stops contesting on tidhash. Modified: head/sys/kern/kern_thread.c Modified: head/sys/kern/kern_thread.c == --- head/sys/kern/kern_thread.c Wed Nov 11 08:50:04 2020(r367584) +++ head/sys/kern/kern_thread.c Wed Nov 11 08:51:04 2020(r367585) @@ -149,8 +149,10 @@ static int nthreads; static LIST_HEAD(tidhashhead, thread) *tidhashtbl; static u_long tidhash; -static struct rwlock tidhash_lock; -#defineTIDHASH(tid)([(tid) & tidhash]) +static u_long tidhashlock; +static struct rwlock *tidhashtbl_lock; +#defineTIDHASH(tid)([(tid) & tidhash]) +#defineTIDHASHLOCK(tid)(_lock[(tid) & tidhashlock]) EVENTHANDLER_LIST_DEFINE(thread_ctor); EVENTHANDLER_LIST_DEFINE(thread_dtor); @@ -355,6 +357,7 @@ extern int max_threads_per_proc; void threadinit(void) { + u_long i; lwpid_t tid0; uint32_t flags; @@ -395,7 +398,13 @@ threadinit(void) thread_ctor, thread_dtor, thread_init, thread_fini, 32 - 1, flags); tidhashtbl = hashinit(maxproc / 2, M_TIDHASH, ); - rw_init(_lock, "tidhash"); + tidhashlock = (tidhash + 1) / 64; + if (tidhashlock > 0) + tidhashlock--; + tidhashtbl_lock = malloc(sizeof(*tidhashtbl_lock) * (tidhashlock + 1), + M_TIDHASH, M_WAITOK | M_ZERO); + for (i = 0; i < tidhashlock + 1; i++) + rw_init(_lock[i], "tidhash"); } /* @@ -1351,7 +1360,7 @@ tdfind_hash(lwpid_t tid, pid_t pid, struct proc **pp, bool locked; run = 0; - rw_rlock(_lock); + rw_rlock(TIDHASHLOCK(tid)); locked = true; LIST_FOREACH(td, TIDHASH(tid), td_hash) { if (td->td_tid != tid) { @@ -1364,11 +1373,11 @@ tdfind_hash(lwpid_t tid, pid_t pid, struct proc **pp, break; } if (run > RUN_THRESH) { - if (rw_try_upgrade(_lock)) { + if (rw_try_upgrade(TIDHASHLOCK(tid))) { LIST_REMOVE(td, td_hash); LIST_INSERT_HEAD(TIDHASH(td->td_tid), td, td_hash); - rw_wunlock(_lock); + rw_wunlock(TIDHASHLOCK(tid)); locked = false; break; } @@ -1376,7 +1385,7 @@ tdfind_hash(lwpid_t tid, pid_t pid, struct proc **pp, break; } if (locked) - rw_runlock(_lock); + rw_runlock(TIDHASHLOCK(tid)); if (td == NULL) return (false); *pp = p; @@ -1421,15 +1430,16 @@ tdfind(lwpid_t tid, pid_t pid) void tidhash_add(struct thread *td) { - rw_wlock(_lock); + rw_wlock(TIDHASHLOCK(td->td_tid)); LIST_INSERT_HEAD(TIDHASH(td->td_tid), td, td_hash); - rw_wunlock(_lock); + rw_wunlock(TIDHASHLOCK(td->td_tid)); } void tidhash_remove(struct thread *td) { - rw_wlock(_lock); + + rw_wlock(TIDHASHLOCK(td->td_tid)); LIST_REMOVE(td, td_hash); - rw_wunlock(_lock); + rw_wunlock(TIDHASHLOCK(td->td_tid)); } ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r367584 - in head/sys: kern sys
Author: mjg Date: Wed Nov 11 08:50:04 2020 New Revision: 367584 URL: https://svnweb.freebsd.org/changeset/base/367584 Log: thread: rework tidhash vs proc lock interaction Apart from minor clean up this gets rid of proc unlock/lock cycle on thread exit to work around LOR against tidhash lock. Modified: head/sys/kern/init_main.c head/sys/kern/kern_kthread.c head/sys/kern/kern_thr.c head/sys/kern/kern_thread.c head/sys/sys/proc.h Modified: head/sys/kern/init_main.c == --- head/sys/kern/init_main.c Wed Nov 11 08:48:43 2020(r367583) +++ head/sys/kern/init_main.c Wed Nov 11 08:50:04 2020(r367584) @@ -497,7 +497,7 @@ proc0_init(void *dummy __unused) STAILQ_INIT(>p_ktr); p->p_nice = NZERO; td->td_tid = THREAD0_TID; - LIST_INSERT_HEAD(TIDHASH(td->td_tid), td, td_hash); + tidhash_add(td); td->td_state = TDS_RUNNING; td->td_pri_class = PRI_TIMESHARE; td->td_user_pri = PUSER; Modified: head/sys/kern/kern_kthread.c == --- head/sys/kern/kern_kthread.cWed Nov 11 08:48:43 2020 (r367583) +++ head/sys/kern/kern_kthread.cWed Nov 11 08:50:04 2020 (r367584) @@ -350,15 +350,12 @@ kthread_exit(void) * The last exiting thread in a kernel process must tear down * the whole process. */ - rw_wlock(_lock); PROC_LOCK(p); if (p->p_numthreads == 1) { PROC_UNLOCK(p); - rw_wunlock(_lock); kproc_exit(0); } - LIST_REMOVE(td, td_hash); - rw_wunlock(_lock); + tidhash_remove(td); umtx_thread_exit(td); tdsigcleanup(td); PROC_SLOCK(p); Modified: head/sys/kern/kern_thr.c == --- head/sys/kern/kern_thr.cWed Nov 11 08:48:43 2020(r367583) +++ head/sys/kern/kern_thr.cWed Nov 11 08:50:04 2020(r367584) @@ -353,14 +353,13 @@ kern_thr_exit(struct thread *td) return (0); } - p->p_pendingexits++; td->td_dbgflags |= TDB_EXIT; - if (p->p_ptevents & PTRACE_LWP) + if (p->p_ptevents & PTRACE_LWP) { + p->p_pendingexits++; ptracestop(td, SIGTRAP, NULL); - PROC_UNLOCK(p); + p->p_pendingexits--; + } tidhash_remove(td); - PROC_LOCK(p); - p->p_pendingexits--; /* * The check above should prevent all other threads from this Modified: head/sys/kern/kern_thread.c == --- head/sys/kern/kern_thread.c Wed Nov 11 08:48:43 2020(r367583) +++ head/sys/kern/kern_thread.c Wed Nov 11 08:50:04 2020(r367584) @@ -147,9 +147,10 @@ SYSCTL_INT(_kern, OID_AUTO, maxthread, CTLFLAG_RDTUN, static int nthreads; -struct tidhashhead *tidhashtbl; -u_long tidhash; -struct rwlock tidhash_lock; +static LIST_HEAD(tidhashhead, thread) *tidhashtbl; +static u_long tidhash; +static struct rwlock tidhash_lock; +#defineTIDHASH(tid)([(tid) & tidhash]) EVENTHANDLER_LIST_DEFINE(thread_ctor); EVENTHANDLER_LIST_DEFINE(thread_dtor); @@ -1329,13 +1330,65 @@ thread_single_end(struct proc *p, int mode) kick_proc0(); } -/* Locate a thread by number; return with proc lock held. */ +/* + * Locate a thread by number and return with proc lock held. + * + * thread exit establishes proc -> tidhash lock ordering, but lookup + * takes tidhash first and needs to return locked proc. + * + * The problem is worked around by relying on type-safety of both + * structures and doing the work in 2 steps: + * - tidhash-locked lookup which saves both thread and proc pointers + * - proc-locked verification that the found thread still matches + */ +static bool +tdfind_hash(lwpid_t tid, pid_t pid, struct proc **pp, struct thread **tdp) +{ +#define RUN_THRESH 16 + struct proc *p; + struct thread *td; + int run; + bool locked; + + run = 0; + rw_rlock(_lock); + locked = true; + LIST_FOREACH(td, TIDHASH(tid), td_hash) { + if (td->td_tid != tid) { + run++; + continue; + } + p = td->td_proc; + if (pid != -1 && p->p_pid != pid) { + td = NULL; + break; + } + if (run > RUN_THRESH) { + if (rw_try_upgrade(_lock)) { + LIST_REMOVE(td, td_hash); + LIST_INSERT_HEAD(TIDHASH(td->td_tid), + td, td_hash); + rw_wunlock(_lock); + locked = false; +
svn commit: r367583 - in head/sys: kern sys
Author: mjg Date: Wed Nov 11 08:48:43 2020 New Revision: 367583 URL: https://svnweb.freebsd.org/changeset/base/367583 Log: thread: fix thread0 tid allocation Startup code hardcodes the value instead of allocating it. The first spawned thread would then be a duplicate. Pointy hat: mjg Modified: head/sys/kern/init_main.c head/sys/kern/kern_thread.c head/sys/sys/proc.h Modified: head/sys/kern/init_main.c == --- head/sys/kern/init_main.c Wed Nov 11 00:43:13 2020(r367582) +++ head/sys/kern/init_main.c Wed Nov 11 08:48:43 2020(r367583) @@ -496,8 +496,7 @@ proc0_init(void *dummy __unused) p->p_klist = knlist_alloc(>p_mtx); STAILQ_INIT(>p_ktr); p->p_nice = NZERO; - /* pid_max cannot be greater than PID_MAX */ - td->td_tid = PID_MAX + 1; + td->td_tid = THREAD0_TID; LIST_INSERT_HEAD(TIDHASH(td->td_tid), td, td_hash); td->td_state = TDS_RUNNING; td->td_pri_class = PRI_TIMESHARE; Modified: head/sys/kern/kern_thread.c == --- head/sys/kern/kern_thread.c Wed Nov 11 00:43:13 2020(r367582) +++ head/sys/kern/kern_thread.c Wed Nov 11 08:48:43 2020(r367583) @@ -354,6 +354,7 @@ extern int max_threads_per_proc; void threadinit(void) { + lwpid_t tid0; uint32_t flags; /* @@ -374,6 +375,9 @@ threadinit(void) mtx_init(_lock, "TID lock", NULL, MTX_DEF); tid_bitmap = bit_alloc(maxthread, M_TIDHASH, M_WAITOK); + tid0 = tid_alloc(); + if (tid0 != THREAD0_TID) + panic("tid0 %d != %d\n", tid0, THREAD0_TID); flags = UMA_ZONE_NOFREE; #ifdef __aarch64__ Modified: head/sys/sys/proc.h == --- head/sys/sys/proc.h Wed Nov 11 00:43:13 2020(r367582) +++ head/sys/sys/proc.h Wed Nov 11 08:48:43 2020(r367583) @@ -855,6 +855,7 @@ MALLOC_DECLARE(M_SUBPROC); */ #definePID_MAX 9 #defineNO_PID 10 +#defineTHREAD0_TID NO_PID extern pid_t pid_max; #defineSESS_LEADER(p) ((p)->p_session->s_leader == (p)) ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r367580 - head/sys/kern
Author: mjg Date: Tue Nov 10 21:29:10 2020 New Revision: 367580 URL: https://svnweb.freebsd.org/changeset/base/367580 Log: thread: tidy up r367543 "locked" variable is spurious in the committed version. Modified: head/sys/kern/kern_proc.c Modified: head/sys/kern/kern_proc.c == --- head/sys/kern/kern_proc.c Tue Nov 10 21:12:32 2020(r367579) +++ head/sys/kern/kern_proc.c Tue Nov 10 21:29:10 2020(r367580) @@ -2748,7 +2748,6 @@ sysctl_kern_proc_kstack(SYSCTL_HANDLER_ARGS) struct stack *st; struct sbuf sb; struct proc *p; - bool locked; name = (int *)arg1; error = pget((pid_t)name[0], PGET_NOTINEXEC | PGET_WANTREAD, ); @@ -2789,14 +2788,12 @@ sysctl_kern_proc_kstack(SYSCTL_HANDLER_ARGS) i++; } PROC_UNLOCK(p); - locked = false; numthreads = i; for (i = 0; i < numthreads; i++) { td = tdfind(lwpidarray[i], p->p_pid); if (td == NULL) { continue; } - locked = true; bzero(kkstp, sizeof(*kkstp)); (void)sbuf_new(, kkstp->kkst_trace, sizeof(kkstp->kkst_trace), SBUF_FIXEDLEN); @@ -2810,7 +2807,6 @@ sysctl_kern_proc_kstack(SYSCTL_HANDLER_ARGS) kkstp->kkst_state = KKST_STATE_RUNNING; thread_unlock(td); PROC_UNLOCK(p); - locked = false; stack_sbuf_print(, st); sbuf_finish(); sbuf_delete(); @@ -2818,10 +2814,7 @@ sysctl_kern_proc_kstack(SYSCTL_HANDLER_ARGS) if (error) break; } - if (!locked) - PROC_LOCK(p); - _PRELE(p); - PROC_UNLOCK(p); + PRELE(p); if (lwpidarray != NULL) free(lwpidarray, M_TEMP); stack_destroy(st); ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r367572 - head/sys/kern
Author: mjg Date: Tue Nov 10 18:10:50 2020 New Revision: 367572 URL: https://svnweb.freebsd.org/changeset/base/367572 Log: Allow rtprio_thread to operate on threads of any process This in particular unbreaks rtkit. The limitation was a leftover of previous state, to quote a comment: /* * Though lwpid is unique, only current process is supported * since there is no efficient way to look up a LWP yet. */ Long since then a global tid hash was introduced to remedy the problem. Permission checks still apply. Submitted by: greg_unrelenting.technology (Greg V) Differential Revision:https://reviews.freebsd.org/D27158 Modified: head/sys/kern/kern_resource.c Modified: head/sys/kern/kern_resource.c == --- head/sys/kern/kern_resource.c Tue Nov 10 18:07:13 2020 (r367571) +++ head/sys/kern/kern_resource.c Tue Nov 10 18:10:50 2020 (r367572) @@ -315,8 +315,7 @@ sys_rtprio_thread(struct thread *td, struct rtprio_thr td1 = td; PROC_LOCK(p); } else { - /* Only look up thread in current process */ - td1 = tdfind(uap->lwpid, curproc->p_pid); + td1 = tdfind(uap->lwpid, -1); if (td1 == NULL) return (ESRCH); p = td1->td_proc; ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r367569 - head/sys/contrib/openzfs/module/zfs
Author: mjg Date: Tue Nov 10 14:23:46 2020 New Revision: 367569 URL: https://svnweb.freebsd.org/changeset/base/367569 Log: zfs: combine zio caches if possible This deduplicates 2 sets of caches using the same sizes. Memory savings fluctuate a lot, one sample result is buildworld on zfs saving ~180MB RAM in reduced page count associated with zio caches. Modified: head/sys/contrib/openzfs/module/zfs/zio.c Modified: head/sys/contrib/openzfs/module/zfs/zio.c == --- head/sys/contrib/openzfs/module/zfs/zio.c Tue Nov 10 14:21:23 2020 (r367568) +++ head/sys/contrib/openzfs/module/zfs/zio.c Tue Nov 10 14:23:46 2020 (r367569) @@ -204,6 +204,19 @@ zio_init(void) if (align != 0) { char name[36]; + if (cflags == data_cflags) { + /* +* Resulting kmem caches would be identical. +* Save memory by creating only one. +*/ + (void) snprintf(name, sizeof (name), + "zio_buf_comb_%lu", (ulong_t)size); + zio_buf_cache[c] = kmem_cache_create(name, + size, align, NULL, NULL, NULL, NULL, NULL, + cflags); + zio_data_buf_cache[c] = zio_buf_cache[c]; + continue; + } (void) snprintf(name, sizeof (name), "zio_buf_%lu", (ulong_t)size); zio_buf_cache[c] = kmem_cache_create(name, size, @@ -234,37 +247,55 @@ zio_init(void) void zio_fini(void) { - size_t c; - kmem_cache_t *last_cache = NULL; - kmem_cache_t *last_data_cache = NULL; + size_t i, j, n; + kmem_cache_t *cache; - for (c = 0; c < SPA_MAXBLOCKSIZE >> SPA_MINBLOCKSHIFT; c++) { -#ifdef _ILP32 - /* -* Cache size limited to 1M on 32-bit platforms until ARC -* buffers no longer require virtual address space. -*/ - if (((c + 1) << SPA_MINBLOCKSHIFT) > zfs_max_recordsize) - break; -#endif + n = SPA_MAXBLOCKSIZE >> SPA_MINBLOCKSHIFT; + #if defined(ZFS_DEBUG) && !defined(_KERNEL) - if (zio_buf_cache_allocs[c] != zio_buf_cache_frees[c]) + for (i = 0; i < n; i++) { + if (zio_buf_cache_allocs[i] != zio_buf_cache_frees[i]) (void) printf("zio_fini: [%d] %llu != %llu\n", - (int)((c + 1) << SPA_MINBLOCKSHIFT), - (long long unsigned)zio_buf_cache_allocs[c], - (long long unsigned)zio_buf_cache_frees[c]); + (int)((i + 1) << SPA_MINBLOCKSHIFT), + (long long unsigned)zio_buf_cache_allocs[i], + (long long unsigned)zio_buf_cache_frees[i]); + } #endif - if (zio_buf_cache[c] != last_cache) { - last_cache = zio_buf_cache[c]; - kmem_cache_destroy(zio_buf_cache[c]); + + /* +* The same kmem cache can show up multiple times in both zio_buf_cache +* and zio_data_buf_cache. Do a wasteful but trivially correct scan to +* sort it out. +*/ + for (i = 0; i < n; i++) { + cache = zio_buf_cache[i]; + if (cache == NULL) + continue; + for (j = i; j < n; j++) { + if (cache == zio_buf_cache[j]) + zio_buf_cache[j] = NULL; + if (cache == zio_data_buf_cache[j]) + zio_data_buf_cache[j] = NULL; } - zio_buf_cache[c] = NULL; + kmem_cache_destroy(cache); + } - if (zio_data_buf_cache[c] != last_data_cache) { - last_data_cache = zio_data_buf_cache[c]; - kmem_cache_destroy(zio_data_buf_cache[c]); + for (i = 0; i < n; i++) { + cache = zio_data_buf_cache[i]; + if (cache == NULL) + continue; + for (j = i; j < n; j++) { + if (cache == zio_data_buf_cache[j]) + zio_data_buf_cache[j] = NULL; } - zio_data_buf_cache[c] = NULL; + kmem_cache_destroy(cache); + } + + for (i = 0; i < n; i++) { + if (zio_buf_cache[i] != NULL) + panic("zio_fini: zio_buf_cache[%d] != NULL", (int)i); + if (zio_data_buf_cache[i] != NULL) + panic("zio_fini: zio_data_buf_cache[%d] !=
svn commit: r367568 - head/sys/contrib/openzfs/module/zfs
Author: mjg Date: Tue Nov 10 14:21:23 2020 New Revision: 367568 URL: https://svnweb.freebsd.org/changeset/base/367568 Log: zfs: g/c unused data_alloc_arena Modified: head/sys/contrib/openzfs/module/zfs/zio.c Modified: head/sys/contrib/openzfs/module/zfs/zio.c == --- head/sys/contrib/openzfs/module/zfs/zio.c Tue Nov 10 14:17:05 2020 (r367567) +++ head/sys/contrib/openzfs/module/zfs/zio.c Tue Nov 10 14:21:23 2020 (r367568) @@ -144,7 +144,6 @@ void zio_init(void) { size_t c; - vmem_t *data_alloc_arena = NULL; zio_cache = kmem_cache_create("zio_cache", sizeof (zio_t), 0, NULL, NULL, NULL, NULL, NULL, 0); @@ -213,8 +212,7 @@ zio_init(void) (void) snprintf(name, sizeof (name), "zio_data_buf_%lu", (ulong_t)size); zio_data_buf_cache[c] = kmem_cache_create(name, size, - align, NULL, NULL, NULL, NULL, - data_alloc_arena, data_cflags); + align, NULL, NULL, NULL, NULL, NULL, data_cflags); } } ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r367544 - in head/sys: kern sys
Author: mjg Date: Tue Nov 10 01:57:48 2020 New Revision: 367544 URL: https://svnweb.freebsd.org/changeset/base/367544 Log: thread: retire thread_find tdfind should be used instead. Modified: head/sys/kern/kern_thread.c head/sys/sys/proc.h Modified: head/sys/kern/kern_thread.c == --- head/sys/kern/kern_thread.c Tue Nov 10 01:57:19 2020(r367543) +++ head/sys/kern/kern_thread.c Tue Nov 10 01:57:48 2020(r367544) @@ -1325,19 +1325,6 @@ thread_single_end(struct proc *p, int mode) kick_proc0(); } -struct thread * -thread_find(struct proc *p, lwpid_t tid) -{ - struct thread *td; - - PROC_LOCK_ASSERT(p, MA_OWNED); - FOREACH_THREAD_IN_PROC(p, td) { - if (td->td_tid == tid) - break; - } - return (td); -} - /* Locate a thread by number; return with proc lock held. */ struct thread * tdfind(lwpid_t tid, pid_t pid) Modified: head/sys/sys/proc.h == --- head/sys/sys/proc.h Tue Nov 10 01:57:19 2020(r367543) +++ head/sys/sys/proc.h Tue Nov 10 01:57:48 2020(r367544) @@ -1153,7 +1153,6 @@ void thread_suspend_one(struct thread *td); void thread_unlink(struct thread *td); void thread_unsuspend(struct proc *p); void thread_wait(struct proc *p); -struct thread *thread_find(struct proc *p, lwpid_t tid); void stop_all_proc(void); void resume_all_proc(void); ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r367543 - head/sys/kern
Author: mjg Date: Tue Nov 10 01:57:19 2020 New Revision: 367543 URL: https://svnweb.freebsd.org/changeset/base/367543 Log: thread: use tdfind in sysctl_kern_proc_kstack This treads linear scans for locked lookup, but more importantly removes the only consumer of thread_find. Modified: head/sys/kern/kern_proc.c Modified: head/sys/kern/kern_proc.c == --- head/sys/kern/kern_proc.c Tue Nov 10 01:31:06 2020(r367542) +++ head/sys/kern/kern_proc.c Tue Nov 10 01:57:19 2020(r367543) @@ -2748,6 +2748,7 @@ sysctl_kern_proc_kstack(SYSCTL_HANDLER_ARGS) struct stack *st; struct sbuf sb; struct proc *p; + bool locked; name = (int *)arg1; error = pget((pid_t)name[0], PGET_NOTINEXEC | PGET_WANTREAD, ); @@ -2787,12 +2788,15 @@ sysctl_kern_proc_kstack(SYSCTL_HANDLER_ARGS) lwpidarray[i] = td->td_tid; i++; } + PROC_UNLOCK(p); + locked = false; numthreads = i; for (i = 0; i < numthreads; i++) { - td = thread_find(p, lwpidarray[i]); + td = tdfind(lwpidarray[i], p->p_pid); if (td == NULL) { continue; } + locked = true; bzero(kkstp, sizeof(*kkstp)); (void)sbuf_new(, kkstp->kkst_trace, sizeof(kkstp->kkst_trace), SBUF_FIXEDLEN); @@ -2806,14 +2810,16 @@ sysctl_kern_proc_kstack(SYSCTL_HANDLER_ARGS) kkstp->kkst_state = KKST_STATE_RUNNING; thread_unlock(td); PROC_UNLOCK(p); + locked = false; stack_sbuf_print(, st); sbuf_finish(); sbuf_delete(); error = SYSCTL_OUT(req, kkstp, sizeof(*kkstp)); - PROC_LOCK(p); if (error) break; } + if (!locked) + PROC_LOCK(p); _PRELE(p); PROC_UNLOCK(p); if (lwpidarray != NULL) ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r367542 - head/sys/kern
Author: mjg Date: Tue Nov 10 01:31:06 2020 New Revision: 367542 URL: https://svnweb.freebsd.org/changeset/base/367542 Log: threads: remove the unused TID_BUFFER_SIZE macro Modified: head/sys/kern/kern_thread.c Modified: head/sys/kern/kern_thread.c == --- head/sys/kern/kern_thread.c Tue Nov 10 01:13:58 2020(r367541) +++ head/sys/kern/kern_thread.c Tue Nov 10 01:31:06 2020(r367542) @@ -136,8 +136,6 @@ static void thread_zombie(struct thread *); static int thread_unsuspend_one(struct thread *td, struct proc *p, bool boundary); -#define TID_BUFFER_SIZE1024 - static struct mtx tid_lock; static bitstr_t *tid_bitmap; ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r367541 - head/sys/kern
Author: mjg Date: Tue Nov 10 01:13:58 2020 New Revision: 367541 URL: https://svnweb.freebsd.org/changeset/base/367541 Log: thread: adds newer bits for r367537 The committed patch was an older version. Modified: head/sys/kern/kern_thread.c Modified: head/sys/kern/kern_thread.c == --- head/sys/kern/kern_thread.c Mon Nov 9 23:38:39 2020(r367540) +++ head/sys/kern/kern_thread.c Tue Nov 10 01:13:58 2020(r367541) @@ -138,8 +138,8 @@ static int thread_unsuspend_one(struct thread *td, str #define TID_BUFFER_SIZE1024 -struct mtx tid_lock; -bitstr_t *tid_bitmap; +static struct mtx tid_lock; +static bitstr_t *tid_bitmap; static MALLOC_DEFINE(M_TIDHASH, "tidhash", "thread hash"); @@ -195,7 +195,7 @@ tid_alloc(void) KASSERT(tid != -1, ("unexpectedly ran out of IDs")); } bit_set(tid_bitmap, tid); - trytid++; + trytid = tid + 1; mtx_unlock(_lock); return (tid + NO_PID); } ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r367537 - head/sys/kern
Author: mjg Date: Mon Nov 9 23:05:28 2020 New Revision: 367537 URL: https://svnweb.freebsd.org/changeset/base/367537 Log: threads: reimplement tid allocation on top of a bitmap There are workloads with very bursty tid allocation and since unr tries very hard to have small-sized bitmaps it keeps reallocating memory. Just doing buildkernel gives almost 150k calls to free coming from unr. This also gets rid of the hack which tried to postpone TID reuse. Reviewed by: kib, markj Tested by:pho Differential Revision:https://reviews.freebsd.org/D27101 Modified: head/sys/kern/kern_thread.c Modified: head/sys/kern/kern_thread.c == --- head/sys/kern/kern_thread.c Mon Nov 9 23:04:30 2020(r367536) +++ head/sys/kern/kern_thread.c Mon Nov 9 23:05:28 2020(r367537) @@ -40,6 +40,7 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include #include #include #include @@ -138,9 +139,8 @@ static int thread_unsuspend_one(struct thread *td, str #define TID_BUFFER_SIZE1024 struct mtx tid_lock; -static struct unrhdr *tid_unrhdr; -static lwpid_t tid_buffer[TID_BUFFER_SIZE]; -static int tid_head, tid_tail; +bitstr_t *tid_bitmap; + static MALLOC_DEFINE(M_TIDHASH, "tidhash", "thread hash"); static int maxthread; @@ -163,14 +163,14 @@ tid_alloc(void) { static struct timeval lastfail; static int curfail; - int nthreads_new; + static lwpid_t trytid; lwpid_t tid; - nthreads_new = atomic_fetchadd_int(, 1) + 1; - if (nthreads_new >= maxthread - 100) { + mtx_lock(_lock); + if (nthreads + 1 >= maxthread - 100) { if (priv_check_cred(curthread->td_ucred, PRIV_MAXPROC) != 0 || - nthreads_new >= maxthread) { - atomic_subtract_int(, 1); + nthreads + 1 >= maxthread) { + mtx_unlock(_lock); if (ppsratecheck(, , 1)) { printf("maxthread limit exceeded by uid %u " "(pid %d); consider increasing kern.maxthread\n", @@ -180,36 +180,40 @@ tid_alloc(void) } } - tid = alloc_unr(tid_unrhdr); - if (tid != -1) - return (tid); - mtx_lock(_lock); - if (tid_head == tid_tail) { - mtx_unlock(_lock); - return (-1); + nthreads++; + /* +* It is an invariant that the bitmap is big enough to hold maxthread +* IDs. If we got to this point there has to be at least one free. +*/ + if (trytid >= maxthread) + trytid = 0; + bit_ffc_at(tid_bitmap, trytid, maxthread, ); + if (tid == -1) { + KASSERT(trytid != 0, ("unexpectedly ran out of IDs")); + trytid = 0; + bit_ffc_at(tid_bitmap, trytid, maxthread, ); + KASSERT(tid != -1, ("unexpectedly ran out of IDs")); } - tid = tid_buffer[tid_head]; - tid_head = (tid_head + 1) % TID_BUFFER_SIZE; + bit_set(tid_bitmap, tid); + trytid++; mtx_unlock(_lock); - return (tid); + return (tid + NO_PID); } static void -tid_free(lwpid_t tid) +tid_free(lwpid_t rtid) { - lwpid_t tmp_tid = -1; + lwpid_t tid; + KASSERT(rtid >= NO_PID, + ("%s: invalid tid %d\n", __func__, rtid)); + tid = rtid - NO_PID; mtx_lock(_lock); - if ((tid_tail + 1) % TID_BUFFER_SIZE == tid_head) { - tmp_tid = tid_buffer[tid_head]; - tid_head = (tid_head + 1) % TID_BUFFER_SIZE; - } - tid_buffer[tid_tail] = tid; - tid_tail = (tid_tail + 1) % TID_BUFFER_SIZE; + KASSERT(bit_test(tid_bitmap, tid) != 0, + ("thread ID %d not allocated\n", rtid)); + bit_clear(tid_bitmap, tid); + nthreads--; mtx_unlock(_lock); - if (tmp_tid != -1) - free_unr(tid_unrhdr, tmp_tid); - atomic_subtract_int(, 1); } /* @@ -371,12 +375,7 @@ threadinit(void) } mtx_init(_lock, "TID lock", NULL, MTX_DEF); - - /* -* pid_max cannot be greater than PID_MAX. -* leave one number for thread0. -*/ - tid_unrhdr = new_unrhdr(PID_MAX + 2, INT_MAX, _lock); + tid_bitmap = bit_alloc(maxthread, M_TIDHASH, M_WAITOK); flags = UMA_ZONE_NOFREE; #ifdef __aarch64__ ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r367536 - head/sys/kern
Author: mjg Date: Mon Nov 9 23:04:30 2020 New Revision: 367536 URL: https://svnweb.freebsd.org/changeset/base/367536 Log: threads: introduce a limit for total number The intent is to replace the current id allocation method and a known upper bound will be useful. Reviewed by: kib (previous version), markj (previous version) Tested by:pho Differential Revision:https://reviews.freebsd.org/D27100 Modified: head/sys/kern/kern_thr.c head/sys/kern/kern_thread.c Modified: head/sys/kern/kern_thr.c == --- head/sys/kern/kern_thr.cMon Nov 9 23:02:13 2020(r367535) +++ head/sys/kern/kern_thr.cMon Nov 9 23:04:30 2020(r367536) @@ -67,7 +67,7 @@ __FBSDID("$FreeBSD$"); static SYSCTL_NODE(_kern, OID_AUTO, threads, CTLFLAG_RW | CTLFLAG_MPSAFE, 0, "thread allocation"); -static int max_threads_per_proc = 1500; +int max_threads_per_proc = 1500; SYSCTL_INT(_kern_threads, OID_AUTO, max_threads_per_proc, CTLFLAG_RW, _threads_per_proc, 0, "Limit on threads per proc"); Modified: head/sys/kern/kern_thread.c == --- head/sys/kern/kern_thread.c Mon Nov 9 23:02:13 2020(r367535) +++ head/sys/kern/kern_thread.c Mon Nov 9 23:04:30 2020(r367536) @@ -59,6 +59,7 @@ __FBSDID("$FreeBSD$"); #ifdef HWPMC_HOOKS #include #endif +#include #include @@ -142,6 +143,12 @@ static lwpid_t tid_buffer[TID_BUFFER_SIZE]; static int tid_head, tid_tail; static MALLOC_DEFINE(M_TIDHASH, "tidhash", "thread hash"); +static int maxthread; +SYSCTL_INT(_kern, OID_AUTO, maxthread, CTLFLAG_RDTUN, +, 0, "Maximum number of threads"); + +static int nthreads; + struct tidhashhead *tidhashtbl; u_long tidhash; struct rwlock tidhash_lock; @@ -154,8 +161,25 @@ EVENTHANDLER_LIST_DEFINE(thread_fini); static lwpid_t tid_alloc(void) { - lwpid_t tid; + static struct timeval lastfail; + static int curfail; + int nthreads_new; + lwpid_t tid; + nthreads_new = atomic_fetchadd_int(, 1) + 1; + if (nthreads_new >= maxthread - 100) { + if (priv_check_cred(curthread->td_ucred, PRIV_MAXPROC) != 0 || + nthreads_new >= maxthread) { + atomic_subtract_int(, 1); + if (ppsratecheck(, , 1)) { + printf("maxthread limit exceeded by uid %u " + "(pid %d); consider increasing kern.maxthread\n", + curthread->td_ucred->cr_ruid, curproc->p_pid); + } + return (-1); + } + } + tid = alloc_unr(tid_unrhdr); if (tid != -1) return (tid); @@ -185,6 +209,7 @@ tid_free(lwpid_t tid) mtx_unlock(_lock); if (tmp_tid != -1) free_unr(tid_unrhdr, tmp_tid); + atomic_subtract_int(, 1); } /* @@ -199,8 +224,6 @@ thread_ctor(void *mem, int size, void *arg, int flags) td->td_state = TDS_INACTIVE; td->td_lastcpu = td->td_oncpu = NOCPU; - td->td_tid = tid_alloc(); - /* * Note that td_critnest begins life as 1 because the thread is not * running and is thereby implicitly waiting to be on the receiving @@ -208,7 +231,6 @@ thread_ctor(void *mem, int size, void *arg, int flags) */ td->td_critnest = 1; td->td_lend_user_pri = PRI_MAX; - EVENTHANDLER_DIRECT_INVOKE(thread_ctor, td); #ifdef AUDIT audit_thread_alloc(td); #endif @@ -253,9 +275,6 @@ thread_dtor(void *mem, int size, void *arg) osd_thread_exit(td); td_softdep_cleanup(td); MPASS(td->td_su == NULL); - - EVENTHANDLER_DIRECT_INVOKE(thread_dtor, td); - tid_free(td->td_tid); } /* @@ -325,6 +344,8 @@ proc_linkup(struct proc *p, struct thread *td) thread_link(td, p); } +extern int max_threads_per_proc; + /* * Initialize global thread allocation resources. */ @@ -333,6 +354,22 @@ threadinit(void) { uint32_t flags; + /* +* Place an upper limit on threads which can be allocated. +* +* Note that other factors may make the de facto limit much lower. +* +* Platform limits are somewhat arbitrary but deemed "more than good +* enough" for the foreseable future. +*/ + if (maxthread == 0) { +#ifdef _LP64 + maxthread = MIN(maxproc * max_threads_per_proc, 100); +#else + maxthread = MIN(maxproc * max_threads_per_proc, 10); +#endif + } + mtx_init(_lock, "TID lock", NULL, MTX_DEF); /* @@ -415,16 +452,25 @@ struct thread * thread_alloc(int pages) { struct thread *td; + lwpid_t tid; thread_reap(); /* check if any zombies to get */ - td = (struct thread
svn commit: r367535 - in head/sys: kern sys
Author: mjg Date: Mon Nov 9 23:02:13 2020 New Revision: 367535 URL: https://svnweb.freebsd.org/changeset/base/367535 Log: vfs: group mount per-cpu vars into one struct While here move frequently read stuff into the same cacheline. This shrinks struct mount by 64 bytes. Tested by:pho Modified: head/sys/kern/vfs_cache.c head/sys/kern/vfs_default.c head/sys/kern/vfs_mount.c head/sys/kern/vfs_subr.c head/sys/kern/vfs_vnops.c head/sys/sys/mount.h Modified: head/sys/kern/vfs_cache.c == --- head/sys/kern/vfs_cache.c Mon Nov 9 23:00:29 2020(r367534) +++ head/sys/kern/vfs_cache.c Mon Nov 9 23:02:13 2020(r367535) @@ -4249,6 +4249,7 @@ static int __noinline cache_fplookup_climb_mount(struct cache_fpl *fpl) { struct mount *mp, *prev_mp; + struct mount_pcpu *mpcpu, *prev_mpcpu; struct vnode *vp; seqc_t vp_seqc; @@ -4262,38 +4263,39 @@ cache_fplookup_climb_mount(struct cache_fpl *fpl) prev_mp = NULL; for (;;) { - if (!vfs_op_thread_enter_crit(mp)) { + if (!vfs_op_thread_enter_crit(mp, mpcpu)) { if (prev_mp != NULL) - vfs_op_thread_exit_crit(prev_mp); + vfs_op_thread_exit_crit(prev_mp, prev_mpcpu); return (cache_fpl_partial(fpl)); } if (prev_mp != NULL) - vfs_op_thread_exit_crit(prev_mp); + vfs_op_thread_exit_crit(prev_mp, prev_mpcpu); if (!vn_seqc_consistent(vp, vp_seqc)) { - vfs_op_thread_exit_crit(mp); + vfs_op_thread_exit_crit(mp, mpcpu); return (cache_fpl_partial(fpl)); } if (!cache_fplookup_mp_supported(mp)) { - vfs_op_thread_exit_crit(mp); + vfs_op_thread_exit_crit(mp, mpcpu); return (cache_fpl_partial(fpl)); } vp = atomic_load_ptr(>mnt_rootvnode); if (vp == NULL || VN_IS_DOOMED(vp)) { - vfs_op_thread_exit_crit(mp); + vfs_op_thread_exit_crit(mp, mpcpu); return (cache_fpl_partial(fpl)); } vp_seqc = vn_seqc_read_any(vp); if (seqc_in_modify(vp_seqc)) { - vfs_op_thread_exit_crit(mp); + vfs_op_thread_exit_crit(mp, mpcpu); return (cache_fpl_partial(fpl)); } prev_mp = mp; + prev_mpcpu = mpcpu; mp = atomic_load_ptr(>v_mountedhere); if (mp == NULL) break; } - vfs_op_thread_exit_crit(prev_mp); + vfs_op_thread_exit_crit(prev_mp, prev_mpcpu); fpl->tvp = vp; fpl->tvp_seqc = vp_seqc; return (0); Modified: head/sys/kern/vfs_default.c == --- head/sys/kern/vfs_default.c Mon Nov 9 23:00:29 2020(r367534) +++ head/sys/kern/vfs_default.c Mon Nov 9 23:02:13 2020(r367535) @@ -663,6 +663,7 @@ vop_stdgetwritemount(ap) } */ *ap; { struct mount *mp; + struct mount_pcpu *mpcpu; struct vnode *vp; /* @@ -680,12 +681,12 @@ vop_stdgetwritemount(ap) *(ap->a_mpp) = NULL; return (0); } - if (vfs_op_thread_enter(mp)) { + if (vfs_op_thread_enter(mp, mpcpu)) { if (mp == vp->v_mount) { - vfs_mp_count_add_pcpu(mp, ref, 1); - vfs_op_thread_exit(mp); + vfs_mp_count_add_pcpu(mpcpu, ref, 1); + vfs_op_thread_exit(mp, mpcpu); } else { - vfs_op_thread_exit(mp); + vfs_op_thread_exit(mp, mpcpu); mp = NULL; } } else { Modified: head/sys/kern/vfs_mount.c == --- head/sys/kern/vfs_mount.c Mon Nov 9 23:00:29 2020(r367534) +++ head/sys/kern/vfs_mount.c Mon Nov 9 23:02:13 2020(r367535) @@ -127,14 +127,7 @@ mount_init(void *mem, int size, int flags) mtx_init(>mnt_mtx, "struct mount mtx", NULL, MTX_DEF); mtx_init(>mnt_listmtx, "struct mount vlist mtx", NULL, MTX_DEF); lockinit(>mnt_explock, PVFS, "explock", 0, 0); - mp->mnt_thread_in_ops_pcpu = uma_zalloc_pcpu(pcpu_zone_4, - M_WAITOK | M_ZERO); - mp->mnt_ref_pcpu = uma_zalloc_pcpu(pcpu_zone_4, - M_WAITOK | M_ZERO); - mp->mnt_lockref_pcpu = uma_zalloc_pcpu(pcpu_zone_4, - M_WAITOK | M_ZERO); -
svn commit: r367534 - head/usr.bin/vmstat
Author: mjg Date: Mon Nov 9 23:00:29 2020 New Revision: 367534 URL: https://svnweb.freebsd.org/changeset/base/367534 Log: vmstat: drop the HighUse field from malloc dump It is hardwired to "-" since its introduction in 2005. Reviewed by: markj Differential Revision:https://reviews.freebsd.org/D27141 Modified: head/usr.bin/vmstat/vmstat.c Modified: head/usr.bin/vmstat/vmstat.c == --- head/usr.bin/vmstat/vmstat.cMon Nov 9 22:59:41 2020 (r367533) +++ head/usr.bin/vmstat/vmstat.cMon Nov 9 23:00:29 2020 (r367534) @@ -1433,8 +1433,8 @@ domemstat_malloc(void) } } xo_open_container("malloc-statistics"); - xo_emit("{T:/%13s} {T:/%5s} {T:/%6s} {T:/%7s} {T:/%8s} {T:Size(s)}\n", - "Type", "InUse", "MemUse", "HighUse", "Requests"); + xo_emit("{T:/%13s} {T:/%5s} {T:/%6s} {T:/%8s} {T:Size(s)}\n", + "Type", "InUse", "MemUse", "Requests"); xo_open_list("memory"); zones = memstat_malloc_zone_get_count(); for (mtp = memstat_mtl_first(mtlp); mtp != NULL; @@ -1444,10 +1444,9 @@ domemstat_malloc(void) continue; xo_open_instance("memory"); xo_emit("{k:type/%13s/%s} {:in-use/%5ju} " - "{:memory-use/%5ju}{U:K} {:high-use/%7s} " - "{:requests/%8ju} ", + "{:memory-use/%5ju}{U:K} {:requests/%8ju} ", memstat_get_name(mtp), (uintmax_t)memstat_get_count(mtp), - ((uintmax_t)memstat_get_bytes(mtp) + 1023) / 1024, "-", + ((uintmax_t)memstat_get_bytes(mtp) + 1023) / 1024, (uintmax_t)memstat_get_numallocs(mtp)); first = 1; xo_open_list("size"); ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r367533 - head/sys/kern
Author: mjg Date: Mon Nov 9 22:59:41 2020 New Revision: 367533 URL: https://svnweb.freebsd.org/changeset/base/367533 Log: malloc: provide 384 byte zone Total page count after buildworld on ZFS for 384 (if present) and 512 zones: before: 29713 after: 25946 per-zone page use: vm.uma.malloc_384.keg.domain.1.pages: 11621 vm.uma.malloc_384.keg.domain.0.pages: 11597 vm.uma.malloc_512.keg.domain.1.pages: 1280 vm.uma.malloc_512.keg.domain.0.pages: 1448 Reviewed by: markj Differential Revision:https://reviews.freebsd.org/D27145 Modified: head/sys/kern/kern_malloc.c Modified: head/sys/kern/kern_malloc.c == --- head/sys/kern/kern_malloc.c Mon Nov 9 22:58:29 2020(r367532) +++ head/sys/kern/kern_malloc.c Mon Nov 9 22:59:41 2020(r367533) @@ -163,6 +163,7 @@ struct { {64, "malloc-64", }, {128, "malloc-128", }, {256, "malloc-256", }, + {384, "malloc-384", }, {512, "malloc-512", }, {1024, "malloc-1024", }, {2048, "malloc-2048", }, ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r367532 - in head/sys: kern sys
Author: mjg Date: Mon Nov 9 22:58:29 2020 New Revision: 367532 URL: https://svnweb.freebsd.org/changeset/base/367532 Log: malloc: retire mt_stats_zone in favor of pcpu_zone_64 Reviewed by: markj, imp Differential Revision:https://reviews.freebsd.org/D27142 Modified: head/sys/kern/kern_malloc.c head/sys/sys/malloc.h Modified: head/sys/kern/kern_malloc.c == --- head/sys/kern/kern_malloc.c Mon Nov 9 22:23:25 2020(r367531) +++ head/sys/kern/kern_malloc.c Mon Nov 9 22:58:29 2020(r367532) @@ -174,11 +174,6 @@ struct { {0, NULL}, }; -/* - * Zone to allocate per-CPU storage for statistics. - */ -static uma_zone_t mt_stats_zone; - u_long vm_kmem_size; SYSCTL_ULONG(_vm, OID_AUTO, kmem_size, CTLFLAG_RDTUN, _kmem_size, 0, "Size of kernel memory"); @@ -1184,9 +1179,6 @@ mallocinit(void *dummy) if (kmem_zmax < PAGE_SIZE || kmem_zmax > KMEM_ZMAX) kmem_zmax = KMEM_ZMAX; - mt_stats_zone = uma_zcreate("mt_stats_zone", - sizeof(struct malloc_type_stats), NULL, NULL, NULL, NULL, - UMA_ALIGN_PTR, UMA_ZONE_PCPU); for (i = 0, indx = 0; kmemzones[indx].kz_size != 0; indx++) { int size = kmemzones[indx].kz_size; const char *name = kmemzones[indx].kz_name; @@ -1222,7 +1214,7 @@ malloc_init(void *data) mtp->ks_shortdesc, mtp->ks_version); mtip = >ks_mti; - mtip->mti_stats = uma_zalloc_pcpu(mt_stats_zone, M_WAITOK | M_ZERO); + mtip->mti_stats = uma_zalloc_pcpu(pcpu_zone_64, M_WAITOK | M_ZERO); mtp_set_subzone(mtp); mtx_lock(_mtx); @@ -1279,7 +1271,7 @@ malloc_uninit(void *data) temp_allocs, temp_bytes); } - uma_zfree_pcpu(mt_stats_zone, mtip->mti_stats); + uma_zfree_pcpu(pcpu_zone_64, mtip->mti_stats); } struct malloc_type * Modified: head/sys/sys/malloc.h == --- head/sys/sys/malloc.h Mon Nov 9 22:23:25 2020(r367531) +++ head/sys/sys/malloc.h Mon Nov 9 22:58:29 2020(r367532) @@ -92,6 +92,9 @@ struct malloc_type_stats { uint64_t_mts_reserved3; /* Reserved field. */ }; +_Static_assert(sizeof(struct malloc_type_stats) == 64, +"allocations come from pcpu_zone_64"); + /* * Index definitions for the mti_probes[] array. */ ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r367503 - in head/sys: kern vm
Author: mjg Date: Mon Nov 9 00:34:23 2020 New Revision: 367503 URL: https://svnweb.freebsd.org/changeset/base/367503 Log: Add more per-cpu zones. This covers powers of 2 up to 64. Example pending user is ZFS. Modified: head/sys/kern/subr_pcpu.c head/sys/vm/uma.h Modified: head/sys/kern/subr_pcpu.c == --- head/sys/kern/subr_pcpu.c Mon Nov 9 00:08:35 2020(r367502) +++ head/sys/kern/subr_pcpu.c Mon Nov 9 00:34:23 2020(r367503) @@ -133,17 +133,25 @@ SYSINIT(dpcpu, SI_SUB_KLD, SI_ORDER_FIRST, dpcpu_start /* * UMA_ZONE_PCPU zones for general kernel use. */ - uma_zone_t pcpu_zone_4; uma_zone_t pcpu_zone_8; +uma_zone_t pcpu_zone_16; +uma_zone_t pcpu_zone_32; +uma_zone_t pcpu_zone_64; static void pcpu_zones_startup(void) { - pcpu_zone_4 = uma_zcreate("pcpu-4", sizeof(uint32_t), + pcpu_zone_4 = uma_zcreate("pcpu-4", 4, NULL, NULL, NULL, NULL, UMA_ALIGN_PTR, UMA_ZONE_PCPU); - pcpu_zone_8 = uma_zcreate("pcpu-8", sizeof(uint64_t), + pcpu_zone_8 = uma_zcreate("pcpu-8", 8, + NULL, NULL, NULL, NULL, UMA_ALIGN_PTR, UMA_ZONE_PCPU); + pcpu_zone_16 = uma_zcreate("pcpu-16", 16, + NULL, NULL, NULL, NULL, UMA_ALIGN_PTR, UMA_ZONE_PCPU); + pcpu_zone_32 = uma_zcreate("pcpu-32", 32, + NULL, NULL, NULL, NULL, UMA_ALIGN_PTR, UMA_ZONE_PCPU); + pcpu_zone_64 = uma_zcreate("pcpu-64", 64, NULL, NULL, NULL, NULL, UMA_ALIGN_PTR, UMA_ZONE_PCPU); } SYSINIT(pcpu_zones, SI_SUB_COUNTER, SI_ORDER_FIRST, pcpu_zones_startup, NULL); Modified: head/sys/vm/uma.h == --- head/sys/vm/uma.h Mon Nov 9 00:08:35 2020(r367502) +++ head/sys/vm/uma.h Mon Nov 9 00:34:23 2020(r367503) @@ -668,6 +668,9 @@ size_t uma_zone_memory(uma_zone_t zone); */ extern uma_zone_t pcpu_zone_4; extern uma_zone_t pcpu_zone_8; +extern uma_zone_t pcpu_zone_16; +extern uma_zone_t pcpu_zone_32; +extern uma_zone_t pcpu_zone_64; /* * Exported statistics structures to be used by user space monitoring tools. ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r367501 - head/usr.bin/vmstat
Author: mjg Date: Mon Nov 9 00:05:45 2020 New Revision: 367501 URL: https://svnweb.freebsd.org/changeset/base/367501 Log: vmstat: remove spurious newlines when reporting zones Modified: head/usr.bin/vmstat/vmstat.c Modified: head/usr.bin/vmstat/vmstat.c == --- head/usr.bin/vmstat/vmstat.cMon Nov 9 00:05:21 2020 (r367500) +++ head/usr.bin/vmstat/vmstat.cMon Nov 9 00:05:45 2020 (r367501) @@ -1500,7 +1500,7 @@ domemstat_zone(void) } xo_open_container("memory-zone-statistics"); xo_emit("{T:/%-20s} {T:/%6s} {T:/%6s} {T:/%8s} {T:/%8s} {T:/%8s} {T:/%8s}" - "{T:/%4s} {T:/%4s}\n\n", "ITEM", "SIZE", + "{T:/%4s} {T:/%4s}\n", "ITEM", "SIZE", "LIMIT", "USED", "FREE", "REQ", "FAIL", "SLEEP", "XDOMAIN"); xo_open_list("zone"); for (mtp = memstat_mtl_first(mtlp); mtp != NULL; @@ -1526,7 +1526,6 @@ domemstat_zone(void) memstat_mtl_free(mtlp); xo_close_list("zone"); xo_close_container("memory-zone-statistics"); - xo_emit("\n"); } static void ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r367500 - head/sys/kern
Author: mjg Date: Mon Nov 9 00:05:21 2020 New Revision: 367500 URL: https://svnweb.freebsd.org/changeset/base/367500 Log: procdesc: convert the zone to a malloc type The object is 128 bytes in size. Modified: head/sys/kern/sys_procdesc.c Modified: head/sys/kern/sys_procdesc.c == --- head/sys/kern/sys_procdesc.cMon Nov 9 00:04:58 2020 (r367499) +++ head/sys/kern/sys_procdesc.cMon Nov 9 00:05:21 2020 (r367500) @@ -92,7 +92,7 @@ __FBSDID("$FreeBSD$"); FEATURE(process_descriptors, "Process Descriptors"); -static uma_zone_t procdesc_zone; +MALLOC_DEFINE(M_PROCDESC, "procdesc", "process descriptors"); static fo_poll_t procdesc_poll; static fo_kqfilter_t procdesc_kqfilter; @@ -117,22 +117,6 @@ static struct fileops procdesc_ops = { }; /* - * Initialize with VFS so that process descriptors are available along with - * other file descriptor types. As long as it runs before init(8) starts, - * there shouldn't be a problem. - */ -static void -procdesc_init(void *dummy __unused) -{ - - procdesc_zone = uma_zcreate("procdesc", sizeof(struct procdesc), - NULL, NULL, NULL, NULL, UMA_ALIGN_PTR, 0); - if (procdesc_zone == NULL) - panic("procdesc_init: procdesc_zone not initialized"); -} -SYSINIT(vfs, SI_SUB_VFS, SI_ORDER_ANY, procdesc_init, NULL); - -/* * Return a locked process given a process descriptor, or ESRCH if it has * died. */ @@ -229,7 +213,7 @@ procdesc_new(struct proc *p, int flags) { struct procdesc *pd; - pd = uma_zalloc(procdesc_zone, M_WAITOK | M_ZERO); + pd = malloc(sizeof(*pd), M_PROCDESC, M_WAITOK | M_ZERO); pd->pd_proc = p; pd->pd_pid = p->p_pid; p->p_procdesc = pd; @@ -290,7 +274,7 @@ procdesc_free(struct procdesc *pd) knlist_destroy(>pd_selinfo.si_note); PROCDESC_LOCK_DESTROY(pd); - uma_zfree(procdesc_zone, pd); + free(pd, M_PROCDESC); } } ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r367499 - head/sys/sys
Author: mjg Date: Mon Nov 9 00:04:58 2020 New Revision: 367499 URL: https://svnweb.freebsd.org/changeset/base/367499 Log: bufcache: convert bo_numoutput from long to int int is wide enough and it plugs a hole in struct vnode, taking it down from 496 to 488 bytes. Modified: head/sys/sys/bufobj.h Modified: head/sys/sys/bufobj.h == --- head/sys/sys/bufobj.h Mon Nov 9 00:04:35 2020(r367498) +++ head/sys/sys/bufobj.h Mon Nov 9 00:04:58 2020(r367499) @@ -104,7 +104,7 @@ struct bufobj { void*bo_private;/* private pointer */ struct bufv bo_clean; /* i Clean buffers */ struct bufv bo_dirty; /* i Dirty buffers */ - longbo_numoutput; /* i Writes in progress */ + int bo_numoutput; /* i Writes in progress */ u_int bo_flag;/* i Flags */ int bo_domain; /* - Clean queue affinity */ int bo_bsize; /* - Block size for i/o */ ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r367498 - in head/sys: compat/linuxkpi/common/src dev/evdev kern sys
Author: mjg Date: Mon Nov 9 00:04:35 2020 New Revision: 367498 URL: https://svnweb.freebsd.org/changeset/base/367498 Log: kqueue: save space by using only one func pointer for assertions Modified: head/sys/compat/linuxkpi/common/src/linux_compat.c head/sys/dev/evdev/uinput.c head/sys/kern/kern_event.c head/sys/kern/uipc_socket.c head/sys/kern/vfs_subr.c head/sys/sys/event.h Modified: head/sys/compat/linuxkpi/common/src/linux_compat.c == --- head/sys/compat/linuxkpi/common/src/linux_compat.c Mon Nov 9 00:01:13 2020(r367497) +++ head/sys/compat/linuxkpi/common/src/linux_compat.c Mon Nov 9 00:04:35 2020(r367498) @@ -422,26 +422,19 @@ linux_kq_unlock(void *arg) } static void -linux_kq_lock_owned(void *arg) +linux_kq_assert_lock(void *arg, int what) { #ifdef INVARIANTS spinlock_t *s = arg; - mtx_assert(>m, MA_OWNED); + if (what == LA_LOCKED) + mtx_assert(>m, MA_OWNED); + else + mtx_assert(>m, MA_NOTOWNED); #endif } static void -linux_kq_lock_unowned(void *arg) -{ -#ifdef INVARIANTS - spinlock_t *s = arg; - - mtx_assert(>m, MA_NOTOWNED); -#endif -} - -static void linux_file_kqfilter_poll(struct linux_file *, int); struct linux_file * @@ -457,8 +450,7 @@ linux_file_alloc(void) /* setup fields needed by kqueue support */ spin_lock_init(>f_kqlock); knlist_init(>f_selinfo.si_note, >f_kqlock, - linux_kq_lock, linux_kq_unlock, - linux_kq_lock_owned, linux_kq_lock_unowned); + linux_kq_lock, linux_kq_unlock, linux_kq_assert_lock); return (filp); } Modified: head/sys/dev/evdev/uinput.c == --- head/sys/dev/evdev/uinput.c Mon Nov 9 00:01:13 2020(r367497) +++ head/sys/dev/evdev/uinput.c Mon Nov 9 00:04:35 2020(r367498) @@ -145,20 +145,16 @@ uinput_knlunlock(void *arg) } static void -uinput_knl_assert_locked(void *arg) +uinput_knl_assert_lock(void *arg, int what) { - sx_assert((struct sx*)arg, SA_XLOCKED); + if (what == LA_LOCKED) + sx_assert((struct sx*)arg, SA_XLOCKED); + else + sx_assert((struct sx*)arg, SA_UNLOCKED); } static void -uinput_knl_assert_unlocked(void *arg) -{ - - sx_assert((struct sx*)arg, SA_UNLOCKED); -} - -static void uinput_ev_event(struct evdev_dev *evdev, uint16_t type, uint16_t code, int32_t value) { @@ -212,8 +208,7 @@ uinput_open(struct cdev *dev, int oflags, int devtype, sx_init(>ucs_lock, "uinput"); knlist_init(>ucs_selp.si_note, >ucs_lock, uinput_knllock, - uinput_knlunlock, uinput_knl_assert_locked, - uinput_knl_assert_unlocked); + uinput_knlunlock, uinput_knl_assert_lock); devfs_set_cdevpriv(state, uinput_dtor); return (0); Modified: head/sys/kern/kern_event.c == --- head/sys/kern/kern_event.c Mon Nov 9 00:01:13 2020(r367497) +++ head/sys/kern/kern_event.c Mon Nov 9 00:04:35 2020(r367498) @@ -305,10 +305,10 @@ kn_leave_flux(struct knote *kn) } while (0) #ifdef INVARIANTS #defineKNL_ASSERT_LOCKED(knl) do { \ - knl->kl_assert_locked((knl)->kl_lockarg); \ + knl->kl_assert_lock((knl)->kl_lockarg, LA_LOCKED); \ } while (0) #defineKNL_ASSERT_UNLOCKED(knl) do { \ - knl->kl_assert_unlocked((knl)->kl_lockarg); \ + knl->kl_assert_lock((knl)->kl_lockarg, LA_UNLOCKED);\ } while (0) #else /* !INVARIANTS */ #defineKNL_ASSERT_LOCKED(knl) do {} while(0) @@ -2375,20 +2375,16 @@ knlist_mtx_unlock(void *arg) } static void -knlist_mtx_assert_locked(void *arg) +knlist_mtx_assert_lock(void *arg, int what) { - mtx_assert((struct mtx *)arg, MA_OWNED); + if (what == LA_LOCKED) + mtx_assert((struct mtx *)arg, MA_OWNED); + else + mtx_assert((struct mtx *)arg, MA_NOTOWNED); } static void -knlist_mtx_assert_unlocked(void *arg) -{ - - mtx_assert((struct mtx *)arg, MA_NOTOWNED); -} - -static void knlist_rw_rlock(void *arg) { @@ -2403,23 +2399,19 @@ knlist_rw_runlock(void *arg) } static void -knlist_rw_assert_locked(void *arg) +knlist_rw_assert_lock(void *arg, int what) { - rw_assert((struct rwlock *)arg, RA_LOCKED); + if (what == LA_LOCKED) + rw_assert((struct rwlock *)arg, RA_LOCKED); + else + rw_assert((struct rwlock *)arg, RA_UNLOCKED); } -static void -knlist_rw_assert_unlocked(void *arg) -{ - - rw_assert((struct rwlock *)arg, RA_UNLOCKED); -} - void knlist_init(struct knlist *knl, void *lock, void (*kl_lock)(void *),
svn commit: r367454 - head/sys/contrib/openzfs/module/os/freebsd/zfs
Author: mjg Date: Sat Nov 7 16:58:38 2020 New Revision: 367454 URL: https://svnweb.freebsd.org/changeset/base/367454 Log: zfs: remove 2 assertions that teardown lock is not held They are not very useful and hard to implement with rms. This has a side effect of simplying the code. Modified: head/sys/contrib/openzfs/module/os/freebsd/zfs/zfs_vnops.c Modified: head/sys/contrib/openzfs/module/os/freebsd/zfs/zfs_vnops.c == --- head/sys/contrib/openzfs/module/os/freebsd/zfs/zfs_vnops.c Sat Nov 7 16:57:53 2020(r367453) +++ head/sys/contrib/openzfs/module/os/freebsd/zfs/zfs_vnops.c Sat Nov 7 16:58:38 2020(r367454) @@ -1451,10 +1451,6 @@ zfs_lookup_lock(vnode_t *dvp, vnode_t *vp, const char if (zfsvfs->z_replay == B_FALSE) ASSERT_VOP_LOCKED(dvp, __func__); -#ifdef DIAGNOSTIC - if ((zdp->z_pflags & ZFS_XATTR) == 0) - VERIFY(!ZFS_TEARDOWN_HELD(zfsvfs)); -#endif if (name[0] == 0 || (name[0] == '.' && name[1] == 0)) { ASSERT3P(dvp, ==, vp); @@ -6523,39 +6519,6 @@ zfs_vptocnp(struct vop_vptocnp_args *ap) return (error); } -#ifdef DIAGNOSTIC -#ifndef _SYS_SYSPROTO_H_ -struct vop_lock1_args { - struct vnode *a_vp; - int a_flags; - char *file; - int line; -}; -#endif - -static int -zfs_lock(struct vop_lock1_args *ap) -{ - vnode_t *vp; - znode_t *zp; - int err; - -#if __FreeBSD_version >= 1300064 - err = vop_lock(ap); -#else - err = vop_stdlock(ap); -#endif - if (err == 0 && (ap->a_flags & LK_NOWAIT) == 0) { - vp = ap->a_vp; - zp = vp->v_data; - if (vp->v_mount != NULL && !VN_IS_DOOMED(vp) && - zp != NULL && (zp->z_pflags & ZFS_XATTR) == 0) - VERIFY(!ZFS_TEARDOWN_HELD(zp->z_zfsvfs)); - } - return (err); -} -#endif - struct vop_vector zfs_vnodeops; struct vop_vector zfs_fifoops; struct vop_vector zfs_shareops; @@ -6606,17 +6569,9 @@ struct vop_vector zfs_vnodeops = { .vop_putpages = zfs_freebsd_putpages, .vop_vptocnp = zfs_vptocnp, #if __FreeBSD_version >= 1300064 -#ifdef DIAGNOSTIC - .vop_lock1 =zfs_lock, -#else .vop_lock1 =vop_lock, -#endif .vop_unlock = vop_unlock, .vop_islocked = vop_islocked, -#else -#ifdef DIAGNOSTIC - .vop_lock1 =zfs_lock, -#endif #endif }; VFS_VOP_VECTOR_REGISTER(zfs_vnodeops); ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r367453 - in head/sys: kern sys
Author: mjg Date: Sat Nov 7 16:57:53 2020 New Revision: 367453 URL: https://svnweb.freebsd.org/changeset/base/367453 Log: rms: several cleanups + debug read lockers handling This adds a dedicated counter updated with atomics when INVARIANTS is used. As a side effect one can reliably determine the lock is held for reading by at least one thread, but it's still not possible to find out whether curthread has the lock in said mode. This should be good enough in practice. Problem spotted by avg. Modified: head/sys/kern/kern_rmlock.c head/sys/sys/_rmlock.h head/sys/sys/rmlock.h Modified: head/sys/kern/kern_rmlock.c == --- head/sys/kern/kern_rmlock.c Sat Nov 7 16:41:59 2020(r367452) +++ head/sys/kern/kern_rmlock.c Sat Nov 7 16:57:53 2020(r367453) @@ -878,10 +878,105 @@ db_show_rm(const struct lock_object *lock) * problem at some point. The easiest way to lessen it is to provide a bitmap. */ +#define rms_int_membar() __compiler_membar() + #defineRMS_NOOWNER ((void *)0x1) #defineRMS_TRANSIENT ((void *)0x2) #defineRMS_FLAGMASK0xf +struct rmslock_pcpu { + int influx; + int readers; +}; + +_Static_assert(sizeof(struct rmslock_pcpu) == 8, "bad size"); + +/* + * Internal routines + */ +static struct rmslock_pcpu * +rms_int_pcpu(struct rmslock *rms) +{ + + CRITICAL_ASSERT(curthread); + return (zpcpu_get(rms->pcpu)); +} + +static struct rmslock_pcpu * +rms_int_remote_pcpu(struct rmslock *rms, int cpu) +{ + + return (zpcpu_get_cpu(rms->pcpu, cpu)); +} + +static void +rms_int_influx_enter(struct rmslock *rms, struct rmslock_pcpu *pcpu) +{ + + CRITICAL_ASSERT(curthread); + MPASS(pcpu->influx == 0); + pcpu->influx = 1; +} + +static void +rms_int_influx_exit(struct rmslock *rms, struct rmslock_pcpu *pcpu) +{ + + CRITICAL_ASSERT(curthread); + MPASS(pcpu->influx == 1); + pcpu->influx = 0; +} + +#ifdef INVARIANTS +static void +rms_int_debug_readers_inc(struct rmslock *rms) +{ + int old; + old = atomic_fetchadd_int(>debug_readers, 1); + KASSERT(old >= 0, ("%s: bad readers count %d\n", __func__, old)); +} + +static void +rms_int_debug_readers_dec(struct rmslock *rms) +{ + int old; + + old = atomic_fetchadd_int(>debug_readers, -1); + KASSERT(old > 0, ("%s: bad readers count %d\n", __func__, old)); +} +#else +static void +rms_int_debug_readers_inc(struct rmslock *rms) +{ +} + +static void +rms_int_debug_readers_dec(struct rmslock *rms) +{ +} +#endif + +static void +rms_int_readers_inc(struct rmslock *rms, struct rmslock_pcpu *pcpu) +{ + + CRITICAL_ASSERT(curthread); + rms_int_debug_readers_inc(rms); + pcpu->readers++; +} + +static void +rms_int_readers_dec(struct rmslock *rms, struct rmslock_pcpu *pcpu) +{ + + CRITICAL_ASSERT(curthread); + rms_int_debug_readers_dec(rms); + pcpu->readers--; +} + +/* + * Public API + */ void rms_init(struct rmslock *rms, const char *name) { @@ -889,9 +984,9 @@ rms_init(struct rmslock *rms, const char *name) rms->owner = RMS_NOOWNER; rms->writers = 0; rms->readers = 0; + rms->debug_readers = 0; mtx_init(>mtx, name, NULL, MTX_DEF | MTX_NEW); - rms->readers_pcpu = uma_zalloc_pcpu(pcpu_zone_4, M_WAITOK | M_ZERO); - rms->readers_influx = uma_zalloc_pcpu(pcpu_zone_4, M_WAITOK | M_ZERO); + rms->pcpu = uma_zalloc_pcpu(pcpu_zone_8, M_WAITOK | M_ZERO); } void @@ -901,23 +996,21 @@ rms_destroy(struct rmslock *rms) MPASS(rms->writers == 0); MPASS(rms->readers == 0); mtx_destroy(>mtx); - uma_zfree_pcpu(pcpu_zone_4, rms->readers_pcpu); - uma_zfree_pcpu(pcpu_zone_4, rms->readers_influx); + uma_zfree_pcpu(pcpu_zone_8, rms->pcpu); } static void __noinline rms_rlock_fallback(struct rmslock *rms) { - zpcpu_set_protected(rms->readers_influx, 0); + rms_int_influx_exit(rms, rms_int_pcpu(rms)); critical_exit(); mtx_lock(>mtx); - MPASS(*zpcpu_get(rms->readers_pcpu) == 0); while (rms->writers > 0) msleep(>readers, >mtx, PUSER - 1, mtx_name(>mtx), 0); critical_enter(); - zpcpu_add_protected(rms->readers_pcpu, 1); + rms_int_readers_inc(rms, rms_int_pcpu(rms)); mtx_unlock(>mtx); critical_exit(); } @@ -925,43 +1018,46 @@ rms_rlock_fallback(struct rmslock *rms) void rms_rlock(struct rmslock *rms) { + struct rmslock_pcpu *pcpu; WITNESS_WARN(WARN_GIANTOK | WARN_SLEEPOK, NULL, __func__); MPASS(atomic_load_ptr(>owner) != curthread); critical_enter(); - zpcpu_set_protected(rms->readers_influx, 1); - __compiler_membar(); + pcpu = rms_int_pcpu(rms); + rms_int_influx_enter(rms, pcpu); + rms_int_membar(); if (__predict_false(rms->writers > 0)) {
svn commit: r367438 - head/sys/kern
Author: mjg Date: Sat Nov 7 01:32:16 2020 New Revision: 367438 URL: https://svnweb.freebsd.org/changeset/base/367438 Log: malloc: tweak the version check in r367432 to include type name While here fix a whitespace problem. Modified: head/sys/kern/kern_malloc.c Modified: head/sys/kern/kern_malloc.c == --- head/sys/kern/kern_malloc.c Fri Nov 6 23:37:59 2020(r367437) +++ head/sys/kern/kern_malloc.c Sat Nov 7 01:32:16 2020(r367438) @@ -1201,7 +1201,7 @@ mallocinit(void *dummy) NULL, NULL, NULL, NULL, #endif UMA_ALIGN_PTR, UMA_ZONE_MALLOC); - } + } for (;i <= size; i+= KMEM_ZBASE) kmemsize[i >> KMEM_ZSHIFT] = indx; } @@ -1218,8 +1218,8 @@ malloc_init(void *data) mtp = data; if (mtp->ks_version != M_VERSION) - panic("malloc_init: unsupported malloc type version %lu", - mtp->ks_version); + panic("malloc_init: type %s with unsupported version %lu", + mtp->ks_shortdesc, mtp->ks_version); mtip = >ks_mti; mtip->mti_stats = uma_zalloc_pcpu(mt_stats_zone, M_WAITOK | M_ZERO); ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r367432 - in head: lib/libmemstat sys/cddl/dev/dtmalloc sys/kern sys/sys
You need to recompile all modules. On 11/7/20, Yasuhiro KIMURA wrote: > From: Mateusz Guzik > Subject: svn commit: r367432 - in head: lib/libmemstat sys/cddl/dev/dtmalloc > sys/kern sys/sys > Date: Fri, 6 Nov 2020 21:33:59 + (UTC) > >> Author: mjg >> Date: Fri Nov 6 21:33:59 2020 >> New Revision: 367432 >> URL: https://svnweb.freebsd.org/changeset/base/367432 >> >> Log: >> malloc: move malloc_type_internal into malloc_type >> >> According to code comments the original motivation was to allow for >> malloc_type_internal changes without ABI breakage. This can be >> trivially >> accomplished by providing spare fields and versioning the struct, as >> implemented in the patch below. >> >> The upshots are one less memory indirection on each alloc and >> disappearance >> of mt_zone. >> >> Reviewed by: markj >> Differential Revision: https://reviews.freebsd.org/D27104 > > With this commit kernel panic happens on amd64 as following. > > https://www.utahime.org/FreeBSD/FreeBSD.13-CURRENT.amd64.r367432.panic.png > > --- > Yasuhiro KIMURA > -- Mateusz Guzik ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r367432 - in head: lib/libmemstat sys/cddl/dev/dtmalloc sys/kern sys/sys
Author: mjg Date: Fri Nov 6 21:33:59 2020 New Revision: 367432 URL: https://svnweb.freebsd.org/changeset/base/367432 Log: malloc: move malloc_type_internal into malloc_type According to code comments the original motivation was to allow for malloc_type_internal changes without ABI breakage. This can be trivially accomplished by providing spare fields and versioning the struct, as implemented in the patch below. The upshots are one less memory indirection on each alloc and disappearance of mt_zone. Reviewed by: markj Differential Revision:https://reviews.freebsd.org/D27104 Modified: head/lib/libmemstat/memstat_malloc.c head/sys/cddl/dev/dtmalloc/dtmalloc.c head/sys/kern/kern_malloc.c head/sys/sys/malloc.h head/sys/sys/param.h Modified: head/lib/libmemstat/memstat_malloc.c == --- head/lib/libmemstat/memstat_malloc.cFri Nov 6 21:27:54 2020 (r367431) +++ head/lib/libmemstat/memstat_malloc.cFri Nov 6 21:33:59 2020 (r367432) @@ -317,7 +317,7 @@ memstat_kvm_malloc(struct memory_type_list *list, void int hint_dontsearch, j, mp_maxcpus, mp_ncpus, ret; char name[MEMTYPE_MAXNAME]; struct malloc_type_stats mts; - struct malloc_type_internal mti, *mtip; + struct malloc_type_internal *mtip; struct malloc_type type, *typep; kvm_t *kvm; @@ -372,18 +372,17 @@ memstat_kvm_malloc(struct memory_type_list *list, void list->mtl_error = ret; return (-1); } + if (type.ks_version != M_VERSION) { + warnx("type %s with unsupported version %lu; skipped", + name, type.ks_version); + continue; + } /* * Since our compile-time value for MAXCPU may differ from the * kernel's, we populate our own array. */ - mtip = type.ks_handle; - ret = kread(kvm, mtip, , sizeof(mti), 0); - if (ret != 0) { - _memstat_mtl_empty(list); - list->mtl_error = ret; - return (-1); - } + mtip = _mti; if (hint_dontsearch == 0) { mtp = memstat_mtl_find(list, ALLOCATOR_MALLOC, name); @@ -404,7 +403,7 @@ memstat_kvm_malloc(struct memory_type_list *list, void */ _memstat_mt_reset_stats(mtp, mp_maxcpus); for (j = 0; j < mp_ncpus; j++) { - ret = kread_zpcpu(kvm, (u_long)mti.mti_stats, , + ret = kread_zpcpu(kvm, (u_long)mtip->mti_stats, , sizeof(mts), j); if (ret != 0) { _memstat_mtl_empty(list); Modified: head/sys/cddl/dev/dtmalloc/dtmalloc.c == --- head/sys/cddl/dev/dtmalloc/dtmalloc.c Fri Nov 6 21:27:54 2020 (r367431) +++ head/sys/cddl/dev/dtmalloc/dtmalloc.c Fri Nov 6 21:33:59 2020 (r367432) @@ -114,7 +114,7 @@ static void dtmalloc_type_cb(struct malloc_type *mtp, void *arg __unused) { char name[DTRACE_FUNCNAMELEN]; - struct malloc_type_internal *mtip = mtp->ks_handle; + struct malloc_type_internal *mtip = >ks_mti; int i; /* Modified: head/sys/kern/kern_malloc.c == --- head/sys/kern/kern_malloc.c Fri Nov 6 21:27:54 2020(r367431) +++ head/sys/kern/kern_malloc.c Fri Nov 6 21:33:59 2020(r367432) @@ -175,14 +175,8 @@ struct { }; /* - * Zone to allocate malloc type descriptions from. For ABI reasons, memory - * types are described by a data structure passed by the declaring code, but - * the malloc(9) implementation has its own data structure describing the - * type and statistics. This permits the malloc(9)-internal data structures - * to be modified without breaking binary-compiled kernel modules that - * declare malloc types. + * Zone to allocate per-CPU storage for statistics. */ -static uma_zone_t mt_zone; static uma_zone_t mt_stats_zone; u_long vm_kmem_size; @@ -342,7 +336,7 @@ mtp_set_subzone(struct malloc_type *mtp) size_t len; u_int val; - mtip = mtp->ks_handle; + mtip = >ks_mti; desc = mtp->ks_shortdesc; if (desc == NULL || (len = strlen(desc)) == 0) val = 0; @@ -356,7 +350,7 @@ mtp_get_subzone(struct malloc_type *mtp) { struct malloc_type_internal *mtip; - mtip = mtp->ks_handle; + mtip = >ks_mti; KASSERT(mtip->mti_zone < numzones, ("mti_zone %u out of range %d", @@ -371,7 +365,7 @@ mtp_set_subzone(struct malloc_type *mtp)
svn commit: r367400 - head/sys/dev/nvme
Author: mjg Date: Thu Nov 5 21:44:58 2020 New Revision: 367400 URL: https://svnweb.freebsd.org/changeset/base/367400 Log: nvme: change namei_request_zone into a malloc type Both the size (128 bytes) and ephemeral nature of allocations make it a great fit for malloc. A dedicated zone unnecessarily avoids sharing buckets with 128-byte objects. Reviewed by: imp Differential Revision:https://reviews.freebsd.org/D27103 Modified: head/sys/dev/nvme/nvme.c head/sys/dev/nvme/nvme_private.h Modified: head/sys/dev/nvme/nvme.c == --- head/sys/dev/nvme/nvme.cThu Nov 5 21:37:24 2020(r367399) +++ head/sys/dev/nvme/nvme.cThu Nov 5 21:44:58 2020(r367400) @@ -49,7 +49,6 @@ struct nvme_consumer { struct nvme_consumer nvme_consumer[NVME_MAX_CONSUMERS]; #defineINVALID_CONSUMER_ID 0x -uma_zone_t nvme_request_zone; int32_tnvme_retry_count; MALLOC_DEFINE(M_NVME, "nvme", "nvme(4) memory allocations"); @@ -61,9 +60,6 @@ nvme_init(void) { uint32_ti; - nvme_request_zone = uma_zcreate("nvme_request", - sizeof(struct nvme_request), NULL, NULL, NULL, NULL, 0, 0); - for (i = 0; i < NVME_MAX_CONSUMERS; i++) nvme_consumer[i].id = INVALID_CONSUMER_ID; } @@ -73,7 +69,6 @@ SYSINIT(nvme_register, SI_SUB_DRIVERS, SI_ORDER_SECOND static void nvme_uninit(void) { - uma_zdestroy(nvme_request_zone); } SYSUNINIT(nvme_unregister, SI_SUB_DRIVERS, SI_ORDER_SECOND, nvme_uninit, NULL); Modified: head/sys/dev/nvme/nvme_private.h == --- head/sys/dev/nvme/nvme_private.hThu Nov 5 21:37:24 2020 (r367399) +++ head/sys/dev/nvme/nvme_private.hThu Nov 5 21:44:58 2020 (r367400) @@ -113,7 +113,6 @@ MALLOC_DECLARE(M_NVME); #define CACHE_LINE_SIZE(64) #endif -extern uma_zone_t nvme_request_zone; extern int32_t nvme_retry_count; extern boolnvme_verbose_cmd_dump; @@ -489,7 +488,7 @@ _nvme_allocate_request(nvme_cb_fn_t cb_fn, void *cb_ar { struct nvme_request *req; - req = uma_zalloc(nvme_request_zone, M_NOWAIT | M_ZERO); + req = malloc(sizeof(*req), M_NVME, M_NOWAIT | M_ZERO); if (req != NULL) { req->cb_fn = cb_fn; req->cb_arg = cb_arg; @@ -551,7 +550,7 @@ nvme_allocate_request_ccb(union ccb *ccb, nvme_cb_fn_t return (req); } -#define nvme_free_request(req) uma_zfree(nvme_request_zone, req) +#define nvme_free_request(req) free(req, M_NVME) void nvme_notify_async_consumers(struct nvme_controller *ctrlr, const struct nvme_completion *async_cpl, ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r367389 - in head/sys: kern sys
Author: mjg Date: Thu Nov 5 16:21:21 2020 New Revision: 367389 URL: https://svnweb.freebsd.org/changeset/base/367389 Log: malloc: add a helper returning size allocated for given request Sample usage: kernel modules can decide whether to stick to malloc or create their own zone. Reviewed by: markj Differential Revision:https://reviews.freebsd.org/D27097 Modified: head/sys/kern/kern_malloc.c head/sys/sys/malloc.h Modified: head/sys/kern/kern_malloc.c == --- head/sys/kern/kern_malloc.c Thu Nov 5 16:00:57 2020(r367388) +++ head/sys/kern/kern_malloc.c Thu Nov 5 16:21:21 2020(r367389) @@ -1031,6 +1031,23 @@ reallocf(void *addr, size_t size, struct malloc_type * } /* + * malloc_size: returns the number of bytes allocated for a request of the + * specified size + */ +size_t +malloc_size(size_t size) +{ + int indx; + + if (size > kmem_zmax) + return (0); + if (size & KMEM_ZMASK) + size = (size & ~KMEM_ZMASK) + KMEM_ZBASE; + indx = kmemsize[size >> KMEM_ZSHIFT]; + return (kmemzones[indx].kz_size); +} + +/* * malloc_usable_size: returns the usable size of the allocation. */ size_t Modified: head/sys/sys/malloc.h == --- head/sys/sys/malloc.h Thu Nov 5 16:00:57 2020(r367388) +++ head/sys/sys/malloc.h Thu Nov 5 16:21:21 2020(r367389) @@ -250,6 +250,7 @@ voidmalloc_type_allocated(struct malloc_type *type, u void malloc_type_freed(struct malloc_type *type, unsigned long size); void malloc_type_list(malloc_type_list_func_t *, void *); void malloc_uninit(void *); +size_t malloc_size(size_t); size_t malloc_usable_size(const void *); void *realloc(void *addr, size_t size, struct malloc_type *type, int flags) __result_use_check __alloc_size(2); ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r367384 - in head/sys: kern sys vm
Author: mjg Date: Thu Nov 5 15:08:56 2020 New Revision: 367384 URL: https://svnweb.freebsd.org/changeset/base/367384 Log: Rationalize per-cpu zones. The 2 provided zones had inconsistent naming between each other ("int" and "64") and other allocator zones (which use bytes). Follow malloc by naming them "pcpu-" + size in bytes. This is a step towards replacing ad-hoc per-cpu zones with general slabs. Modified: head/sys/kern/kern_rmlock.c head/sys/kern/subr_counter.c head/sys/kern/subr_pcpu.c head/sys/kern/vfs_mount.c head/sys/sys/param.h head/sys/vm/uma.h Modified: head/sys/kern/kern_rmlock.c == --- head/sys/kern/kern_rmlock.c Thu Nov 5 14:15:50 2020(r367383) +++ head/sys/kern/kern_rmlock.c Thu Nov 5 15:08:56 2020(r367384) @@ -890,8 +890,8 @@ rms_init(struct rmslock *rms, const char *name) rms->writers = 0; rms->readers = 0; mtx_init(>mtx, name, NULL, MTX_DEF | MTX_NEW); - rms->readers_pcpu = uma_zalloc_pcpu(pcpu_zone_int, M_WAITOK | M_ZERO); - rms->readers_influx = uma_zalloc_pcpu(pcpu_zone_int, M_WAITOK | M_ZERO); + rms->readers_pcpu = uma_zalloc_pcpu(pcpu_zone_4, M_WAITOK | M_ZERO); + rms->readers_influx = uma_zalloc_pcpu(pcpu_zone_4, M_WAITOK | M_ZERO); } void @@ -901,8 +901,8 @@ rms_destroy(struct rmslock *rms) MPASS(rms->writers == 0); MPASS(rms->readers == 0); mtx_destroy(>mtx); - uma_zfree_pcpu(pcpu_zone_int, rms->readers_pcpu); - uma_zfree_pcpu(pcpu_zone_int, rms->readers_influx); + uma_zfree_pcpu(pcpu_zone_4, rms->readers_pcpu); + uma_zfree_pcpu(pcpu_zone_4, rms->readers_influx); } static void __noinline Modified: head/sys/kern/subr_counter.c == --- head/sys/kern/subr_counter.cThu Nov 5 14:15:50 2020 (r367383) +++ head/sys/kern/subr_counter.cThu Nov 5 15:08:56 2020 (r367384) @@ -61,14 +61,14 @@ counter_u64_t counter_u64_alloc(int flags) { - return (uma_zalloc_pcpu(pcpu_zone_64, flags | M_ZERO)); + return (uma_zalloc_pcpu(pcpu_zone_8, flags | M_ZERO)); } void counter_u64_free(counter_u64_t c) { - uma_zfree_pcpu(pcpu_zone_64, c); + uma_zfree_pcpu(pcpu_zone_8, c); } int Modified: head/sys/kern/subr_pcpu.c == --- head/sys/kern/subr_pcpu.c Thu Nov 5 14:15:50 2020(r367383) +++ head/sys/kern/subr_pcpu.c Thu Nov 5 15:08:56 2020(r367384) @@ -131,21 +131,19 @@ dpcpu_startup(void *dummy __unused) SYSINIT(dpcpu, SI_SUB_KLD, SI_ORDER_FIRST, dpcpu_startup, NULL); /* - * UMA_PCPU_ZONE zones, that are available for all kernel - * consumers. Right now 64 bit zone is used for counter(9) - * and int zone is used for mount point counters. + * UMA_ZONE_PCPU zones for general kernel use. */ -uma_zone_t pcpu_zone_int; -uma_zone_t pcpu_zone_64; +uma_zone_t pcpu_zone_4; +uma_zone_t pcpu_zone_8; static void pcpu_zones_startup(void) { - pcpu_zone_int = uma_zcreate("int pcpu", sizeof(int), + pcpu_zone_4 = uma_zcreate("pcpu-4", sizeof(uint32_t), NULL, NULL, NULL, NULL, UMA_ALIGN_PTR, UMA_ZONE_PCPU); - pcpu_zone_64 = uma_zcreate("64 pcpu", sizeof(uint64_t), + pcpu_zone_8 = uma_zcreate("pcpu-8", sizeof(uint64_t), NULL, NULL, NULL, NULL, UMA_ALIGN_PTR, UMA_ZONE_PCPU); } SYSINIT(pcpu_zones, SI_SUB_COUNTER, SI_ORDER_FIRST, pcpu_zones_startup, NULL); Modified: head/sys/kern/vfs_mount.c == --- head/sys/kern/vfs_mount.c Thu Nov 5 14:15:50 2020(r367383) +++ head/sys/kern/vfs_mount.c Thu Nov 5 15:08:56 2020(r367384) @@ -127,13 +127,13 @@ mount_init(void *mem, int size, int flags) mtx_init(>mnt_mtx, "struct mount mtx", NULL, MTX_DEF); mtx_init(>mnt_listmtx, "struct mount vlist mtx", NULL, MTX_DEF); lockinit(>mnt_explock, PVFS, "explock", 0, 0); - mp->mnt_thread_in_ops_pcpu = uma_zalloc_pcpu(pcpu_zone_int, + mp->mnt_thread_in_ops_pcpu = uma_zalloc_pcpu(pcpu_zone_4, M_WAITOK | M_ZERO); - mp->mnt_ref_pcpu = uma_zalloc_pcpu(pcpu_zone_int, + mp->mnt_ref_pcpu = uma_zalloc_pcpu(pcpu_zone_4, M_WAITOK | M_ZERO); - mp->mnt_lockref_pcpu = uma_zalloc_pcpu(pcpu_zone_int, + mp->mnt_lockref_pcpu = uma_zalloc_pcpu(pcpu_zone_4, M_WAITOK | M_ZERO); - mp->mnt_writeopcount_pcpu = uma_zalloc_pcpu(pcpu_zone_int, + mp->mnt_writeopcount_pcpu = uma_zalloc_pcpu(pcpu_zone_4, M_WAITOK | M_ZERO); mp->mnt_ref = 0; mp->mnt_vfs_ops = 1; @@ -147,10 +147,10 @@ mount_fini(void *mem, int size) struct mount *mp; mp = (struct mount *)mem; - uma_zfree_pcpu(pcpu_zone_int,
svn commit: r367379 - head/sys/kern
Author: mjg Date: Thu Nov 5 12:24:37 2020 New Revision: 367379 URL: https://svnweb.freebsd.org/changeset/base/367379 Log: poll/select: change selfd_zone into a malloc type On a sample box vmstat -z shows: ITEM SIZE LIMIT USED FREE REQ 64: 64, 0, 1043784, 4367538,3698187229 selfd: 64, 0,1520, 13726,182729008 But at the same time: vm.uma.selfd.keg.domain.1.pages: 121 vm.uma.selfd.keg.domain.0.pages: 121 Thus 242 pages got pulled even though the malloc zone would likely accomodate the load without using extra memory. Modified: head/sys/kern/sys_generic.c Modified: head/sys/kern/sys_generic.c == --- head/sys/kern/sys_generic.c Thu Nov 5 12:17:50 2020(r367378) +++ head/sys/kern/sys_generic.c Thu Nov 5 12:24:37 2020(r367379) @@ -159,7 +159,7 @@ struct selfd { u_int sf_refs; }; -static uma_zone_t selfd_zone; +MALLOC_DEFINE(M_SELFD, "selfd", "selfd"); static struct mtx_pool *mtxpool_select; #ifdef __LP64__ @@ -1691,11 +1691,11 @@ selfdalloc(struct thread *td, void *cookie) stp = td->td_sel; if (stp->st_free1 == NULL) - stp->st_free1 = uma_zalloc(selfd_zone, M_WAITOK|M_ZERO); + stp->st_free1 = malloc(sizeof(*stp->st_free1), M_SELFD, M_WAITOK|M_ZERO); stp->st_free1->sf_td = stp; stp->st_free1->sf_cookie = cookie; if (stp->st_free2 == NULL) - stp->st_free2 = uma_zalloc(selfd_zone, M_WAITOK|M_ZERO); + stp->st_free2 = malloc(sizeof(*stp->st_free2), M_SELFD, M_WAITOK|M_ZERO); stp->st_free2->sf_td = stp; stp->st_free2->sf_cookie = cookie; } @@ -1713,7 +1713,7 @@ selfdfree(struct seltd *stp, struct selfd *sfp) mtx_unlock(sfp->sf_mtx); } if (refcount_release(>sf_refs)) - uma_zfree(selfd_zone, sfp); + free(sfp, M_SELFD); } /* Drain the waiters tied to all the selfd belonging the specified selinfo. */ @@ -1827,7 +1827,7 @@ doselwakeup(struct selinfo *sip, int pri) cv_broadcastpri(>st_wait, pri); mtx_unlock(>st_mtx); if (refcount_release(>sf_refs)) - uma_zfree(selfd_zone, sfp); + free(sfp, M_SELFD); } mtx_unlock(sip->si_mtx); } @@ -1888,9 +1888,9 @@ seltdfini(struct thread *td) if (stp == NULL) return; if (stp->st_free1) - uma_zfree(selfd_zone, stp->st_free1); + free(stp->st_free1, M_SELFD); if (stp->st_free2) - uma_zfree(selfd_zone, stp->st_free2); + free(stp->st_free2, M_SELFD); td->td_sel = NULL; cv_destroy(>st_wait); mtx_destroy(>st_mtx); @@ -1920,8 +1920,6 @@ static void selectinit(void *dummy __unused) { - selfd_zone = uma_zcreate("selfd", sizeof(struct selfd), NULL, NULL, - NULL, NULL, UMA_ALIGN_PTR, 0); mtxpool_select = mtx_pool_create("select mtxpool", 128, MTX_DEF); } ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"