Date: Wed, 14 Jul 2004 10:25:31 -0400 (EDT) From: Robert Watson <rwatson@freebsd.org> To: Gleb Smirnoff <glebius@freebsd.org> Cc: current@freebsd.org Subject: Re: Some netgraph node global locking patches Message-ID: <Pine.NEB.3.96L.1040714101817.83353G-100000@fledge.watson.org> In-Reply-To: <20040714130120.GA7897@cell.sick.ru>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, 14 Jul 2004, Gleb Smirnoff wrote: > On Wed, Jul 14, 2004 at 12:30:40AM -0400, Robert Watson wrote: > R> //depot/vendor/freebsd/src/sys/netgraph/ng_eiface.c > R> //depot/vendor/freebsd/src/sys/netgraph/ng_fec.c > R> //depot/vendor/freebsd/src/sys/netgraph/ng_iface.c > > Well, these three are quite straightforward and identical. Look fine. I was somewhat hoping someone would actually give them a try and demonstrate that practice matches the theory. :-) > R> //depot/vendor/freebsd/src/sys/netgraph/ng_ppp.c > > Is there any hidden obstacles for merging qsort_r() from libc to > libkern? It will help to remove this ugly hack. My recollection of the quicksort algorithm is extremely vague, but I recall that it involves recursion, and recursion is generally bad in the kernel due to stack depth. > R> //depot/vendor/freebsd/src/sys/netgraph/ng_pppoe.c > > Agreed with comment. Anyway, I'm planning to move this configuration > trigger to private data. sysctl's are not very elegant way to set > configaration of netgraph node. Moreover, I can imagine setup when you > need to serve non-standard PPPoE only on one interface, and normal PPPoE > on other interfaces: for example two different networks merged to use > one AC. This sounds good. I've chatted some with Julian about locking down access to private data, but I've not had a chance to really digest and think through it yet. Is this something you're interested in looking at? :-) > R> //depot/vendor/freebsd/src/sys/netgraph/ng_tty.c > R> +/* > R> + * XXXRW: ngt_unit is protected by ng_tty_mtx. ngt_ldisc is constant once > R> + * ng_tty is initialized. ngt_nodeop_ok is untouched, and might want to be a > R> + * sleep lock in the future? > R> + */ > > One question: are any locks held when linesw callbacks (ngt_open, > ngt_close, etc..) are called? Hmm. That is an interesting problem indeed. The tty code currently requires Giant, although Poul-Henning is doing cleanup that will hopefully lead to locking at some point. In the SLIP code, I currently use a taskqueue to defer calls into the tty code so that Giant is not grabbed at arbitrary points in the SLIP processing, which may be called into while holding protocol layer locks. It looks like a lot of the tty access in ng_tty.c is done from the device node calls associated with the tty subsystem, and therefore Giant will already be held (which is fine, although not 100% desirable). The problematic bit is the call to ngt_start() in ngt_rcvdata(), and synchronizing the mbuf queue in the node between various consumers. We could very easily use the same trick here I use in SLIP by deferring ngt_start() to a Giant-holding task queue. We could add a 'struct task' to the node softc and just use that, for example. Robert N M Watson FreeBSD Core Team, TrustedBSD Projects robert@fledge.watson.org Principal Research Scientist, McAfee Research
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.NEB.3.96L.1040714101817.83353G-100000>