Re: MPSAFE TTY schedule

On Fri, 4 Jul 2008, Ed Schouten wrote:

>>>> cy(4), digi(4), rp(4), rc(4), si(4).
>>>
>>> Who actually owns one of these devices? If you do, please contact me. If
>>> I didn't make myself clear enough: I *am* willing to (assist in
>>> porting|port) these drivers.

I have 24 ports on cy devices, but don't use them except for testing.

>> I have access to a Digi Xem boards at work and have poked around
>> inside the digi(4) code in the past.  My difficulty is that the cards
>> are all in use and upgrading to a FreeBSD-current that doesn't support
>> them and then porting the driver is probably not an option (whereas
>> converting it from using shims to access the TTY layer to doing so
>> directly would probably be acceptable - because I can get the board
>> going again in a hurry if needed).
>
> The problem with the old TTY layer, is that drivers tend to access the
> internals of the TTY structure very often. A good example of this is the
> clists, where TTY drivers tamper around inside the clist and cblock
> structures. There is not much room to implement a compatibility layer
> there.

This is a very bad example.  Clist accesses are, or should be, a non-
problem.  No (non-broken) tty drivers access the internals of clists
directly, except for read-only accesses to the character count (c_cc),
which isn't clist-specific.  All of them use the old KPI functions
(putc/getc/b_to_q/q_to_b, etc) for accessing the tty queues, and the
implementation of the tty queues can be changed to anything without
any KPI changes except possibly to the spelling of c_cc.

Non-drivers like slip and ppp have slightly more clist-specific
knowledge, but again their interface is limited mainly to the KPI
(clist_alloc_cblocks, ...) and a read-only character count (cfreecount).
if_sl.c still has has a lot of comments about its knowledge of clists,
but these barely apply since its implementation only depends on an
adequate buffering mechanism for characters (not sure if the characters
need to be quotable).

Driver-specific locking for clists is even less of a problem.  No
driver-specific locking is needed for the calls, since they are locked
(using spl or Giant) in clist internals.  The direct accesses to c_cc
should be locked in the same way, but missing locking for these is
almost harmless since the accesses are read-only and reading a stale
value is usually harmless.

Internal locking at each entry point of the KPI encourages unlocked
accesses to c_cc, since c_cc becomes volatile immediately after releasing
the internal lock.  In practice, things like t_oproc() routines use
higher-level locking so that there is no race and the internal locking
in getc/q_to_b is bogus:

xxxstart:
 	/*
 	 * Lock whole loop.  Without this, getc()'s access to c_cc would
 	 * give the same race as our direct access after getc() unlocks.
 	 * With this, getc()'s locking is just a waste of time (it's
 	 * recursive so that this isn't fatal, so the waste of time
 	 * hopefully isn't very large, but this requires recursive locking
 	 * to be used all over, so there are time and robustness costs all
 	 * over).
 	 *
 	spltty();		/* Or Giant in bad drivers. */
 	/*
 	 * Non-Giant locking at a higher level than here might be OK, or
 	 * might not, depending on whether the driver wants very fine-grained
 	 * locking.  If this is done, then we can remove all these fine-
 	 * grained spl lock calls in drivers instead of replacing them by
 	 * a tty lock.  Meanwhile, the spls serve as placeholders to remind
 	 * us where to put the tty locks.
 	 */
 	while (tp->t_outq.c_cc != 0 &&	/* XXX efficiency hack. */
 	    (ch = getc(&tp->t_outq)) != -1)
 		move_ch_to_driver_buffer(...)
 	splx();

A possibly better way to handle this is to accept losing races on the
read-only variable but ensure that the driver is woken up without much
delay if the variable changes.  This already happens in most or all
cases, since changing c_cc requires action to process the new state

The number of KPI and c_cc accesses is also very small.  In most drivers
it consists of a whole 1 getc or q_to_b call in t_oproc() and a whole
1 c_cc read to avoid this call.  A few drivers implement a bulk input
routine that bypasses t_rint() in the TS_CAN_BYPASS_L_RINT case.  This
requires 1 b_to_q call and 1 c_cc read to implement flow control.  This
should be in the tty layer.  It doesn't break the clist layering but
it breaks the tty layering.  slip and ppp make much heavier use of the
KPI by count of the number of calls (about 5 putc's, 2 unputc's....
each).

By churning the KPI, you create a lot of work.

Bruce