svn commit: r262914 - in head: sbin/devd sys/kern sys/sys

Warner Losh imp at bsdimp.com
Fri Mar 7 23:35:54 UTC 2014


On Mar 7, 2014, at 4:30 PM, Alan Somers <asomers at FreeBSD.org> wrote:

> Author: asomers
> Date: Fri Mar  7 23:30:48 2014
> New Revision: 262914
> URL: http://svnweb.freebsd.org/changeset/base/262914
> 
> Log:
>  sbin/devd/devd.8
>  sbin/devd/devd.cc
>  	Add a -q flag to devd that will suppress syslog logging at
>  	LOG_NOTICE or below.

This also changes the logging from debug to no_daemon…  Was that intentional?

>  Requested by:	ian@ and imp@
>  MFC after:	3 weeks
>  Sponsored by:	Spectra Logic Corporation
> 
> Modified:
>  head/sbin/devd/devd.8
>  head/sbin/devd/devd.cc
>  head/sys/kern/uipc_usrreq.c
>  head/sys/sys/sockbuf.h
>  head/sys/sys/unpcb.h

I don’t think the sys files should have been committed in the same commit, since those are clearly unrelated.

Warner

> Modified: head/sbin/devd/devd.8
> ==============================================================================
> --- head/sbin/devd/devd.8	Fri Mar  7 23:26:14 2014	(r262913)
> +++ head/sbin/devd/devd.8	Fri Mar  7 23:30:48 2014	(r262914)
> @@ -33,7 +33,7 @@
> .Nd "device state change daemon"
> .Sh SYNOPSIS
> .Nm
> -.Op Fl dn
> +.Op Fl dnq
> .Op Fl f Ar file
> .Op Fl l Ar num
> .Sh DESCRIPTION
> @@ -63,6 +63,8 @@ The default connection limit is 10.
> .It Fl n
> Do not process all pending events before becoming a daemon.
> Instead, call daemon right away.
> +.It Fl q
> +Quiet mode.  Only log messages at priority LOG_WARNING or above.
> .El
> .Sh IMPLEMENTATION NOTES
> The
> 
> Modified: head/sbin/devd/devd.cc
> ==============================================================================
> --- head/sbin/devd/devd.cc	Fri Mar  7 23:26:14 2014	(r262913)
> +++ head/sbin/devd/devd.cc	Fri Mar  7 23:30:48 2014	(r262914)
> @@ -129,8 +129,9 @@ static const char detach = '-';
> 
> static struct pidfh *pfh;
> 
> -int dflag;
> -int nflag;
> +static int no_daemon = 0;
> +static int daemonize_quick = 0;
> +static int quiet_mode = 0;
> static unsigned total_events = 0;
> static volatile sig_atomic_t got_siginfo = 0;
> static volatile sig_atomic_t romeo_must_die = 0;
> @@ -291,7 +292,7 @@ match::do_match(config &c)
> 	 * can consume excessive amounts of systime inside of connect().  Only
> 	 * log when we're in -d mode.
> 	 */
> -	if (dflag) {
> +	if (no_daemon) {
> 		devdlog(LOG_DEBUG, "Testing %s=%s against %s, invert=%d\n",
> 		    _var.c_str(), value.c_str(), _re.c_str(), _inv);
> 	}
> @@ -401,7 +402,7 @@ var_list::set_variable(const string &var
> 	 * can consume excessive amounts of systime inside of connect().  Only
> 	 * log when we're in -d mode.
> 	 */
> -	if (dflag)
> +	if (no_daemon)
> 		devdlog(LOG_DEBUG, "setting %s=%s\n", var.c_str(), val.c_str());
> 	_vars[var] = val;
> }
> @@ -945,7 +946,7 @@ event_loop(void)
> 	accepting = 1;
> 	max_fd = max(fd, server_fd) + 1;
> 	while (!romeo_must_die) {
> -		if (!once && !dflag && !nflag) {
> +		if (!once && !no_daemon && !daemonize_quick) {
> 			// Check to see if we have any events pending.
> 			tv.tv_sec = 0;
> 			tv.tv_usec = 0;
> @@ -1139,9 +1140,9 @@ devdlog(int priority, const char* fmt, .
> 	va_list argp;
> 
> 	va_start(argp, fmt);
> -	if (dflag)
> +	if (no_daemon)
> 		vfprintf(stderr, fmt, argp);
> -	else
> +	else if ((! quiet_mode) || (priority <= LOG_WARNING))
> 		vsyslog(priority, fmt, argp);
> 	va_end(argp);
> }
> @@ -1149,7 +1150,7 @@ devdlog(int priority, const char* fmt, .
> static void
> usage()
> {
> -	fprintf(stderr, "usage: %s [-dn] [-l connlimit] [-f file]\n",
> +	fprintf(stderr, "usage: %s [-dnq] [-l connlimit] [-f file]\n",
> 	    getprogname());
> 	exit(1);
> }
> @@ -1179,10 +1180,10 @@ main(int argc, char **argv)
> 	int ch;
> 
> 	check_devd_enabled();
> -	while ((ch = getopt(argc, argv, "df:l:n")) != -1) {
> +	while ((ch = getopt(argc, argv, "df:l:nq")) != -1) {
> 		switch (ch) {
> 		case 'd':
> -			dflag++;
> +			no_daemon = 1;
> 			break;
> 		case 'f':
> 			configfile = optarg;
> @@ -1191,7 +1192,10 @@ main(int argc, char **argv)
> 			max_clients = MAX(1, strtoul(optarg, NULL, 0));
> 			break;
> 		case 'n':
> -			nflag++;
> +			daemonize_quick = 1;
> +			break;
> +		case 'q':
> +			quiet_mode = 1;
> 			break;
> 		default:
> 			usage();
> @@ -1199,7 +1203,7 @@ main(int argc, char **argv)
> 	}
> 
> 	cfg.parse();
> -	if (!dflag && nflag) {
> +	if (!no_daemon && daemonize_quick) {
> 		cfg.open_pidfile();
> 		daemon(0, 0);
> 		cfg.write_pidfile();
> 
> Modified: head/sys/kern/uipc_usrreq.c
> ==============================================================================
> --- head/sys/kern/uipc_usrreq.c	Fri Mar  7 23:26:14 2014	(r262913)
> +++ head/sys/kern/uipc_usrreq.c	Fri Mar  7 23:30:48 2014	(r262914)
> @@ -51,7 +51,6 @@
>  *
>  * TODO:
>  *	RDM
> - *	distinguish datagram size limits from flow control limits in SEQPACKET
>  *	rethink name space problems
>  *	need a proper out-of-band
>  */
> @@ -789,7 +788,6 @@ uipc_rcvd(struct socket *so, int flags)
> 	struct unpcb *unp, *unp2;
> 	struct socket *so2;
> 	u_int mbcnt, sbcc;
> -	u_long newhiwat;
> 
> 	unp = sotounpcb(so);
> 	KASSERT(unp != NULL, ("uipc_rcvd: unp == NULL"));
> @@ -811,6 +809,15 @@ uipc_rcvd(struct socket *so, int flags)
> 	mbcnt = so->so_rcv.sb_mbcnt;
> 	sbcc = so->so_rcv.sb_cc;
> 	SOCKBUF_UNLOCK(&so->so_rcv);
> +	/*
> +	 * There is a benign race condition at this point.  If we're planning to
> +	 * clear SB_STOP, but uipc_send is called on the connected socket at
> +	 * this instant, it might add data to the sockbuf and set SB_STOP.  Then
> +	 * we would erroneously clear SB_STOP below, even though the sockbuf is
> +	 * full.  The race is benign because the only ill effect is to allow the
> +	 * sockbuf to exceed its size limit, and the size limits are not
> +	 * strictly guaranteed anyway.
> +	 */
> 	UNP_PCB_LOCK(unp);
> 	unp2 = unp->unp_conn;
> 	if (unp2 == NULL) {
> @@ -819,13 +826,9 @@ uipc_rcvd(struct socket *so, int flags)
> 	}
> 	so2 = unp2->unp_socket;
> 	SOCKBUF_LOCK(&so2->so_snd);
> -	so2->so_snd.sb_mbmax += unp->unp_mbcnt - mbcnt;
> -	newhiwat = so2->so_snd.sb_hiwat + unp->unp_cc - sbcc;
> -	(void)chgsbsize(so2->so_cred->cr_uidinfo, &so2->so_snd.sb_hiwat,
> -	    newhiwat, RLIM_INFINITY);
> +	if (sbcc < so2->so_snd.sb_hiwat && mbcnt < so2->so_snd.sb_mbmax)
> +		so2->so_snd.sb_flags &= ~SB_STOP;
> 	sowwakeup_locked(so2);
> -	unp->unp_mbcnt = mbcnt;
> -	unp->unp_cc = sbcc;
> 	UNP_PCB_UNLOCK(unp);
> 	return (0);
> }
> @@ -836,8 +839,7 @@ uipc_send(struct socket *so, int flags, 
> {
> 	struct unpcb *unp, *unp2;
> 	struct socket *so2;
> -	u_int mbcnt_delta, sbcc;
> -	u_int newhiwat;
> +	u_int mbcnt, sbcc;
> 	int error = 0;
> 
> 	unp = sotounpcb(so);
> @@ -991,27 +993,21 @@ uipc_send(struct socket *so, int flags, 
> 			}
> 		}
> 
> -		/*
> -		 * XXXRW: While fine for SOCK_STREAM, this conflates maximum
> -		 * datagram size and back-pressure for SOCK_SEQPACKET, which
> -		 * can lead to undesired return of EMSGSIZE on send instead
> -		 * of more desirable blocking.
> -		 */
> -		mbcnt_delta = so2->so_rcv.sb_mbcnt - unp2->unp_mbcnt;
> -		unp2->unp_mbcnt = so2->so_rcv.sb_mbcnt;
> +		mbcnt = so2->so_rcv.sb_mbcnt;
> 		sbcc = so2->so_rcv.sb_cc;
> 		sorwakeup_locked(so2);
> 
> +		/* 
> +		 * The PCB lock on unp2 protects the SB_STOP flag.  Without it,
> +		 * it would be possible for uipc_rcvd to be called at this
> +		 * point, drain the receiving sockbuf, clear SB_STOP, and then
> +		 * we would set SB_STOP below.  That could lead to an empty
> +		 * sockbuf having SB_STOP set
> +		 */
> 		SOCKBUF_LOCK(&so->so_snd);
> -		if ((int)so->so_snd.sb_hiwat >= (int)(sbcc - unp2->unp_cc))
> -			newhiwat = so->so_snd.sb_hiwat - (sbcc - unp2->unp_cc);
> -		else
> -			newhiwat = 0;
> -		(void)chgsbsize(so->so_cred->cr_uidinfo, &so->so_snd.sb_hiwat,
> -		    newhiwat, RLIM_INFINITY);
> -		so->so_snd.sb_mbmax -= mbcnt_delta;
> +		if (sbcc >= so->so_snd.sb_hiwat || mbcnt >= so->so_snd.sb_mbmax)
> +			so->so_snd.sb_flags |= SB_STOP;
> 		SOCKBUF_UNLOCK(&so->so_snd);
> -		unp2->unp_cc = sbcc;
> 		UNP_PCB_UNLOCK(unp2);
> 		m = NULL;
> 		break;
> @@ -1049,27 +1045,18 @@ release:
> static int
> uipc_sense(struct socket *so, struct stat *sb)
> {
> -	struct unpcb *unp, *unp2;
> -	struct socket *so2;
> +	struct unpcb *unp;
> 
> 	unp = sotounpcb(so);
> 	KASSERT(unp != NULL, ("uipc_sense: unp == NULL"));
> 
> 	sb->st_blksize = so->so_snd.sb_hiwat;
> -	UNP_LINK_RLOCK();
> 	UNP_PCB_LOCK(unp);
> -	unp2 = unp->unp_conn;
> -	if ((so->so_type == SOCK_STREAM || so->so_type == SOCK_SEQPACKET) &&
> -	    unp2 != NULL) {
> -		so2 = unp2->unp_socket;
> -		sb->st_blksize += so2->so_rcv.sb_cc;
> -	}
> 	sb->st_dev = NODEV;
> 	if (unp->unp_ino == 0)
> 		unp->unp_ino = (++unp_ino == 0) ? ++unp_ino : unp_ino;
> 	sb->st_ino = unp->unp_ino;
> 	UNP_PCB_UNLOCK(unp);
> -	UNP_LINK_RUNLOCK();
> 	return (0);
> }
> 
> @@ -2497,8 +2484,7 @@ DB_SHOW_COMMAND(unpcb, db_show_unpcb)
> 	/* XXXRW: Would be nice to print the full address, if any. */
> 	db_printf("unp_addr: %p\n", unp->unp_addr);
> 
> -	db_printf("unp_cc: %d   unp_mbcnt: %d   unp_gencnt: %llu\n",
> -	    unp->unp_cc, unp->unp_mbcnt,
> +	db_printf("unp_gencnt: %llu\n",
> 	    (unsigned long long)unp->unp_gencnt);
> 
> 	db_printf("unp_flags: %x (", unp->unp_flags);
> 
> Modified: head/sys/sys/sockbuf.h
> ==============================================================================
> --- head/sys/sys/sockbuf.h	Fri Mar  7 23:26:14 2014	(r262913)
> +++ head/sys/sys/sockbuf.h	Fri Mar  7 23:30:48 2014	(r262914)
> @@ -52,6 +52,7 @@
> #define	SB_NOCOALESCE	0x200		/* don't coalesce new data into existing mbufs */
> #define	SB_IN_TOE	0x400		/* socket buffer is in the middle of an operation */
> #define	SB_AUTOSIZE	0x800		/* automatically size socket buffer */
> +#define	SB_STOP		0x1000		/* backpressure indicator */
> 
> #define	SBS_CANTSENDMORE	0x0010	/* can't send more data to peer */
> #define	SBS_CANTRCVMORE		0x0020	/* can't receive more data from peer */
> @@ -168,9 +169,19 @@ void	sbunlock(struct sockbuf *sb);
>  * still be negative (cc > hiwat or mbcnt > mbmax).  Should detect
>  * overflow and return 0.  Should use "lmin" but it doesn't exist now.
>  */
> -#define	sbspace(sb) \
> -    ((long) imin((int)((sb)->sb_hiwat - (sb)->sb_cc), \
> -	 (int)((sb)->sb_mbmax - (sb)->sb_mbcnt)))
> +static __inline
> +long
> +sbspace(struct sockbuf *sb)
> +{
> +	long bleft;
> +	long mleft;
> +
> +	if (sb->sb_flags & SB_STOP)
> +		return(0);
> +	bleft = sb->sb_hiwat - sb->sb_cc;
> +	mleft = sb->sb_mbmax - sb->sb_mbcnt;
> +	return((bleft < mleft) ? bleft : mleft);
> +}
> 
> /* adjust counters in sb reflecting allocation of m */
> #define	sballoc(sb, m) { \
> 
> Modified: head/sys/sys/unpcb.h
> ==============================================================================
> --- head/sys/sys/unpcb.h	Fri Mar  7 23:26:14 2014	(r262913)
> +++ head/sys/sys/unpcb.h	Fri Mar  7 23:30:48 2014	(r262914)
> @@ -74,8 +74,8 @@ struct unpcb {
> 	struct	unp_head unp_refs;	/* referencing socket linked list */
> 	LIST_ENTRY(unpcb) unp_reflink;	/* link in unp_refs list */
> 	struct	sockaddr_un *unp_addr;	/* bound address of socket */
> -	int	unp_cc;			/* copy of rcv.sb_cc */
> -	int	unp_mbcnt;		/* copy of rcv.sb_mbcnt */
> +	int	reserved1;
> +	int	reserved2;
> 	unp_gen_t unp_gencnt;		/* generation count of this instance */
> 	short	unp_flags;		/* flags */
> 	short	unp_gcflag;		/* Garbage collector flags. */
> 



More information about the svn-src-head mailing list