From owner-svn-src-stable-11@freebsd.org Wed Nov 8 12:13:27 2017 Return-Path: Delivered-To: svn-src-stable-11@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 17060E51678; Wed, 8 Nov 2017 12:13:27 +0000 (UTC) (envelope-from kib@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id E283F77B5E; Wed, 8 Nov 2017 12:13:26 +0000 (UTC) (envelope-from kib@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id vA8CDPZY048446; Wed, 8 Nov 2017 12:13:25 GMT (envelope-from kib@FreeBSD.org) Received: (from kib@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id vA8CDPdA048443; Wed, 8 Nov 2017 12:13:25 GMT (envelope-from kib@FreeBSD.org) Message-Id: <201711081213.vA8CDPdA048443@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: kib set sender to kib@FreeBSD.org using -f From: Konstantin Belousov Date: Wed, 8 Nov 2017 12:13:25 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-11@freebsd.org Subject: svn commit: r325551 - in stable/11/sys: dev/hwpmc sys X-SVN-Group: stable-11 X-SVN-Commit-Author: kib X-SVN-Commit-Paths: in stable/11/sys: dev/hwpmc sys X-SVN-Commit-Revision: 325551 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-stable-11@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for only the 11-stable src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 08 Nov 2017 12:13:27 -0000 Author: kib Date: Wed Nov 8 12:13:25 2017 New Revision: 325551 URL: https://svnweb.freebsd.org/changeset/base/325551 Log: MFC r325277: Do not run pmclog_configure_log() without pmc_sx protection. Modified: stable/11/sys/dev/hwpmc/hwpmc_logging.c stable/11/sys/dev/hwpmc/hwpmc_mod.c stable/11/sys/sys/pmclog.h Directory Properties: stable/11/ (props changed) Modified: stable/11/sys/dev/hwpmc/hwpmc_logging.c ============================================================================== --- stable/11/sys/dev/hwpmc/hwpmc_logging.c Wed Nov 8 12:11:54 2017 (r325550) +++ stable/11/sys/dev/hwpmc/hwpmc_logging.c Wed Nov 8 12:13:25 2017 (r325551) @@ -239,6 +239,54 @@ pmclog_get_buffer(struct pmc_owner *po) return (plb ? 0 : ENOMEM); } +struct pmclog_proc_init_args { + struct proc *kthr; + struct pmc_owner *po; + bool exit; + bool acted; +}; + +int +pmclog_proc_create(struct thread *td, void **handlep) +{ + struct pmclog_proc_init_args *ia; + int error; + + ia = malloc(sizeof(*ia), M_TEMP, M_WAITOK | M_ZERO); + error = kproc_create(pmclog_loop, ia, &ia->kthr, + RFHIGHPID, 0, "hwpmc: proc(%d)", td->td_proc->p_pid); + if (error == 0) + *handlep = ia; + return (error); +} + +void +pmclog_proc_ignite(void *handle, struct pmc_owner *po) +{ + struct pmclog_proc_init_args *ia; + + ia = handle; + mtx_lock(&pmc_kthread_mtx); + MPASS(!ia->acted); + MPASS(ia->po == NULL); + MPASS(!ia->exit); + MPASS(ia->kthr != NULL); + if (po == NULL) { + ia->exit = true; + } else { + ia->po = po; + KASSERT(po->po_kthread == NULL, + ("[pmclog,%d] po=%p kthread (%p) already present", + __LINE__, po, po->po_kthread)); + po->po_kthread = ia->kthr; + } + wakeup(ia); + while (!ia->acted) + msleep(ia, &pmc_kthread_mtx, PWAIT, "pmclogw", 0); + mtx_unlock(&pmc_kthread_mtx); + free(ia, M_TEMP); +} + /* * Log handler loop. * @@ -248,7 +296,7 @@ pmclog_get_buffer(struct pmc_owner *po) static void pmclog_loop(void *arg) { - int error; + struct pmclog_proc_init_args *ia; struct pmc_owner *po; struct pmclog_buffer *lb; struct proc *p; @@ -259,15 +307,34 @@ pmclog_loop(void *arg) struct uio auio; struct iovec aiov; size_t nbytes; + int error; - po = (struct pmc_owner *) arg; - p = po->po_owner; td = curthread; SIGEMPTYSET(unb); SIGADDSET(unb, SIGHUP); (void)kern_sigprocmask(td, SIG_UNBLOCK, &unb, NULL, 0); + ia = arg; + MPASS(ia->kthr == curproc); + MPASS(!ia->acted); + mtx_lock(&pmc_kthread_mtx); + while (ia->po == NULL && !ia->exit) + msleep(ia, &pmc_kthread_mtx, PWAIT, "pmclogi", 0); + if (ia->exit) { + ia->acted = true; + wakeup(ia); + mtx_unlock(&pmc_kthread_mtx); + kproc_exit(0); + } + MPASS(ia->po != NULL); + po = ia->po; + ia->acted = true; + wakeup(ia); + mtx_unlock(&pmc_kthread_mtx); + ia = NULL; + + p = po->po_owner; mycred = td->td_ucred; PROC_LOCK(p); @@ -572,15 +639,11 @@ pmclog_stop_kthread(struct pmc_owner *po) int pmclog_configure_log(struct pmc_mdep *md, struct pmc_owner *po, int logfd) { - int error; struct proc *p; cap_rights_t rights; - /* - * As long as it is possible to get a LOR between pmc_sx lock and - * proctree/allproc sx locks used for adding a new process, assure - * the former is not held here. - */ - sx_assert(&pmc_sx, SA_UNLOCKED); + int error; + + sx_assert(&pmc_sx, SA_XLOCKED); PMCDBG2(LOG,CFG,1, "config po=%p logfd=%d", po, logfd); p = po->po_owner; @@ -589,9 +652,6 @@ pmclog_configure_log(struct pmc_mdep *md, struct pmc_o if (po->po_flags & PMC_PO_OWNS_LOGFILE) return (EBUSY); - KASSERT(po->po_kthread == NULL, - ("[pmclog,%d] po=%p kthread (%p) already present", __LINE__, po, - po->po_kthread)); KASSERT(po->po_file == NULL, ("[pmclog,%d] po=%p file (%p) already present", __LINE__, po, po->po_file)); @@ -604,10 +664,6 @@ pmclog_configure_log(struct pmc_mdep *md, struct pmc_o /* mark process as owning a log file */ po->po_flags |= PMC_PO_OWNS_LOGFILE; - error = kproc_create(pmclog_loop, po, &po->po_kthread, - RFHIGHPID, 0, "hwpmc: proc(%d)", p->p_pid); - if (error) - goto error; /* mark process as using HWPMCs */ PROC_LOCK(p); @@ -624,10 +680,6 @@ pmclog_configure_log(struct pmc_mdep *md, struct pmc_o return (0); error: - /* shutdown the thread */ - if (po->po_kthread) - pmclog_stop_kthread(po); - KASSERT(po->po_kthread == NULL, ("[pmclog,%d] po=%p kthread not " "stopped", __LINE__, po)); Modified: stable/11/sys/dev/hwpmc/hwpmc_mod.c ============================================================================== --- stable/11/sys/dev/hwpmc/hwpmc_mod.c Wed Nov 8 12:11:54 2017 (r325550) +++ stable/11/sys/dev/hwpmc/hwpmc_mod.c Wed Nov 8 12:13:25 2017 (r325551) @@ -2854,20 +2854,31 @@ static const char *pmc_op_to_name[] = { static int pmc_syscall_handler(struct thread *td, void *syscall_args) { - int error, is_sx_downgraded, is_sx_locked, op; + int error, is_sx_downgraded, op; struct pmc_syscall_args *c; + void *pmclog_proc_handle; void *arg; - PMC_GET_SX_XLOCK(ENOSYS); - - is_sx_downgraded = 0; - is_sx_locked = 1; - - c = (struct pmc_syscall_args *) syscall_args; - + c = (struct pmc_syscall_args *)syscall_args; op = c->pmop_code; arg = c->pmop_data; + if (op == PMC_OP_CONFIGURELOG) { + /* + * We cannot create the logging process inside + * pmclog_configure_log() because there is a LOR + * between pmc_sx and process structure locks. + * Instead, pre-create the process and ignite the loop + * if everything is fine, otherwise direct the process + * to exit. + */ + error = pmclog_proc_create(td, &pmclog_proc_handle); + if (error != 0) + goto done_syscall; + } + PMC_GET_SX_XLOCK(ENOSYS); + is_sx_downgraded = 0; + PMCDBG3(MOD,PMS,1, "syscall op=%d \"%s\" arg=%p", op, pmc_op_to_name[op], arg); @@ -2890,15 +2901,16 @@ pmc_syscall_handler(struct thread *td, void *syscall_a struct pmc_owner *po; struct pmc_op_configurelog cl; - sx_assert(&pmc_sx, SX_XLOCKED); - - if ((error = copyin(arg, &cl, sizeof(cl))) != 0) + if ((error = copyin(arg, &cl, sizeof(cl))) != 0) { + pmclog_proc_ignite(pmclog_proc_handle, NULL); break; + } /* mark this process as owning a log file */ p = td->td_proc; if ((po = pmc_find_owner_descriptor(p)) == NULL) if ((po = pmc_allocate_owner_descriptor(p)) == NULL) { + pmclog_proc_ignite(pmclog_proc_handle, NULL); error = ENOMEM; break; } @@ -2910,10 +2922,11 @@ pmc_syscall_handler(struct thread *td, void *syscall_a * de-configure it. */ if (cl.pm_logfd >= 0) { - sx_xunlock(&pmc_sx); - is_sx_locked = 0; error = pmclog_configure_log(md, po, cl.pm_logfd); + pmclog_proc_ignite(pmclog_proc_handle, error == 0 ? + po : NULL); } else if (po->po_flags & PMC_PO_OWNS_LOGFILE) { + pmclog_proc_ignite(pmclog_proc_handle, NULL); pmclog_process_closelog(po); error = pmclog_close(po); if (error == 0) { @@ -2923,11 +2936,10 @@ pmc_syscall_handler(struct thread *td, void *syscall_a pmc_stop(pm); error = pmclog_deconfigure_log(po); } - } else + } else { + pmclog_proc_ignite(pmclog_proc_handle, NULL); error = EINVAL; - - if (error) - break; + } } break; @@ -4022,13 +4034,11 @@ pmc_syscall_handler(struct thread *td, void *syscall_a break; } - if (is_sx_locked != 0) { - if (is_sx_downgraded) - sx_sunlock(&pmc_sx); - else - sx_xunlock(&pmc_sx); - } - + if (is_sx_downgraded) + sx_sunlock(&pmc_sx); + else + sx_xunlock(&pmc_sx); +done_syscall: if (error) atomic_add_int(&pmc_stats.pm_syscall_errors, 1); Modified: stable/11/sys/sys/pmclog.h ============================================================================== --- stable/11/sys/sys/pmclog.h Wed Nov 8 12:11:54 2017 (r325550) +++ stable/11/sys/sys/pmclog.h Wed Nov 8 12:13:25 2017 (r325551) @@ -260,6 +260,8 @@ int pmclog_deconfigure_log(struct pmc_owner *_po); int pmclog_flush(struct pmc_owner *_po); int pmclog_close(struct pmc_owner *_po); void pmclog_initialize(void); +int pmclog_proc_create(struct thread *td, void **handlep); +void pmclog_proc_ignite(void *handle, struct pmc_owner *po); void pmclog_process_callchain(struct pmc *_pm, struct pmc_sample *_ps); void pmclog_process_closelog(struct pmc_owner *po); void pmclog_process_dropnotify(struct pmc_owner *po);