From 5db6e492eeaac9ae9ef11ad06299c9d987fcbfda Mon Sep 17 00:00:00 2001 From: rwatson Date: Thu, 29 Sep 2005 18:40:36 +0000 Subject: [PATCH] Merge subr_prof.c:1.119, 1.120, 1.121, nfs_socket.c:1.130, rpcclnt.c:1.14 from HEAD to RELENG_6: Acquire Giant in uprintf() and tprintf() due to the non-MPSAFEty of the tty code invoked from these functions. In two cases, during timeout handling in NFS-related RPC client code, acquire Giant in the caller before other mutexes the caller might hold, in order to avoid lock order reversals with Giant (a recursive acquire is not a reversal as it won't ever wait). Correct age-old comments about uprintf()/tprintf() sleeping: they will never sleep. Much useful feedback from: bde Approved by: re (scottl) --- sys/kern/subr_prf.c | 21 +++++++++++++-------- sys/nfsclient/nfs_socket.c | 14 +++++++++++++- sys/rpc/rpcclnt.c | 9 ++++++++- 3 files changed, 34 insertions(+), 10 deletions(-) diff --git a/sys/kern/subr_prf.c b/sys/kern/subr_prf.c index 1aea4ddc28de..446f80d4c749 100644 --- a/sys/kern/subr_prf.c +++ b/sys/kern/subr_prf.c @@ -117,8 +117,6 @@ tablefull(const char *tab) /* * Uprintf prints to the controlling terminal for the current process. - * It may block if the tty queue is overfull. No message is printed if - * the queue does not clear in a reasonable time. */ int uprintf(const char *fmt, ...) @@ -132,29 +130,34 @@ uprintf(const char *fmt, ...) if (td == NULL || td == PCPU_GET(idlethread)) return (0); + mtx_lock(&Giant); p = td->td_proc; PROC_LOCK(p); if ((p->p_flag & P_CONTROLT) == 0) { PROC_UNLOCK(p); - return (0); + retval = 0; + goto out; } SESS_LOCK(p->p_session); pca.tty = p->p_session->s_ttyp; SESS_UNLOCK(p->p_session); PROC_UNLOCK(p); - if (pca.tty == NULL) - return (0); + if (pca.tty == NULL) { + retval = 0; + goto out; + } pca.flags = TOTTY; va_start(ap, fmt); retval = kvprintf(fmt, putchar, &pca, 10, ap); va_end(ap); - +out: + mtx_unlock(&Giant); return (retval); } /* - * tprintf prints on the controlling terminal associated - * with the given session, possibly to the log as well. + * tprintf prints on the controlling terminal associated with the given + * session, possibly to the log as well. */ void tprintf(struct proc *p, int pri, const char *fmt, ...) @@ -165,6 +168,7 @@ tprintf(struct proc *p, int pri, const char *fmt, ...) struct putchar_arg pca; struct session *sess = NULL; + mtx_lock(&Giant); if (pri != -1) flags |= TOLOG; if (p != NULL) { @@ -192,6 +196,7 @@ tprintf(struct proc *p, int pri, const char *fmt, ...) if (sess != NULL) SESSRELE(sess); msgbuftrigger = 1; + mtx_unlock(&Giant); } /* diff --git a/sys/nfsclient/nfs_socket.c b/sys/nfsclient/nfs_socket.c index 8f919daeab71..7b9499966e65 100644 --- a/sys/nfsclient/nfs_socket.c +++ b/sys/nfsclient/nfs_socket.c @@ -1048,8 +1048,11 @@ nfs_request(struct vnode *vp, struct mbuf *mrest, int procnum, * If there was a successful reply and a tprintf msg. * tprintf a response. */ - if (!error) + if (!error) { + mtx_lock(&Giant); nfs_up(rep, nmp, rep->r_td, "is alive again", NFSSTA_TIMEO); + mtx_unlock(&Giant); + } mrep = rep->r_mrep; md = rep->r_md; dpos = rep->r_dpos; @@ -1178,6 +1181,7 @@ nfs_timer(void *arg) getmicrouptime(&now); s = splnet(); + mtx_lock(&Giant); /* nfs_down -> tprintf */ mtx_lock(&nfs_reqq_mtx); TAILQ_FOREACH(rep, &nfs_reqq, r_chain) { nmp = rep->r_nmp; @@ -1281,6 +1285,7 @@ nfs_timer(void *arg) } } mtx_unlock(&nfs_reqq_mtx); + mtx_unlock(&Giant); /* nfs_down -> tprintf */ splx(s); callout_reset(&nfs_callout, nfs_ticks, nfs_timer, NULL); } @@ -1612,6 +1617,8 @@ nfs_msg(struct thread *td, const char *server, const char *msg, int error) { struct proc *p; + GIANT_REQUIRED; /* tprintf */ + p = td ? td->td_proc : NULL; if (error) { tprintf(p, LOG_INFO, "nfs server %s: %s, error %d\n", server, @@ -1631,6 +1638,8 @@ nfs_down(rep, nmp, td, msg, error, flags) int error, flags; { + GIANT_REQUIRED; /* nfs_msg */ + if (nmp == NULL) return; if ((flags & NFSSTA_TIMEO) && !(nmp->nm_state & NFSSTA_TIMEO)) { @@ -1658,6 +1667,9 @@ nfs_up(rep, nmp, td, msg, flags) const char *msg; int flags; { + + GIANT_REQUIRED; /* nfs_msg */ + if (nmp == NULL) return; if ((rep == NULL) || (rep->r_flags & R_TPRINTFMSG) != 0) diff --git a/sys/rpc/rpcclnt.c b/sys/rpc/rpcclnt.c index 3c32e5cfac17..698c956cd6d8 100644 --- a/sys/rpc/rpcclnt.c +++ b/sys/rpc/rpcclnt.c @@ -1254,9 +1254,12 @@ rpcclnt_request(rpc, mrest, procnum, td, cred, reply) * If there was a successful reply and a tprintf msg. tprintf a * response. */ - if (!error && (task->r_flags & R_TPRINTFMSG)) + if (!error && (task->r_flags & R_TPRINTFMSG)) { + mtx_lock(&Giant); rpcclnt_msg(task->r_td, rpc->rc_prog->prog_name, "is alive again"); + mtx_unlock(&Giant); + } /* free request header (leaving mrest) */ mheadend->m_next = NULL; @@ -1390,6 +1393,7 @@ rpcclnt_timer(arg) #else s = splnet(); #endif + mtx_lock(&Giant); /* rpc_msg -> tprintf */ TAILQ_FOREACH(rep, &rpctask_q, r_chain) { rpc = rep->r_rpcclnt; if (rep->r_mrep || (rep->r_flags & R_SOFTTERM)) @@ -1474,6 +1478,7 @@ rpcclnt_timer(arg) } } } + mtx_unlock(&Giant); /* rpc_msg -> tprintf */ splx(s); #ifdef __OpenBSD__ @@ -1775,6 +1780,8 @@ rpcclnt_msg(p, server, msg) tprintf_close(tpr); RPC_RETURN(0); #else + GIANT_REQUIRED; + tprintf(p ? p->td_proc : NULL, LOG_INFO, "nfs server %s: %s\n", server, msg); RPC_RETURN(0);