Re: RFC: Remove pty(4)

On Wed, Nov 26, 2014 at 04:41:27PM -0800, Davide Italiano wrote:
> On Mon, Aug 25, 2014 at 12:37 PM, John Baldwin <jhb_at_freebsd.org> wrote:
> > On Wednesday, August 20, 2014 11:00:14 AM Davide Italiano wrote:
> >> One of my personal goals for 11 is to get rid of cloning mechanism
> >> entirely, and pty(4) is one of the few in-kernel drivers still relying
> >> on such mechanism.
Why this is good thing to do ?

> >> It's not possible, at least to my understanding, converting pty(4) to
> >> cdevpriv(9) as happened with other drivers. This is mainly because we
> >> always need a pair of devices (/dev/ptyXX and /dev/ttyXX) and
> >> userspace loops over ptyXX and after it successfully opens it tries to
> >> open the other one with the same suffix. So, having a single device is
> >> not really enough.
> >> My option, instead, is that of removing pty(4), which is nothing more
> >> than a compatibility driver, and move pmtx(4) code somewhere else.
> >> The main drawback of the removal of this is that it makes impossible
> >> to run FreeBSD <= 7 jails and SSH into them. I personally don't
> >> consider this a huge issue, in light of the fact that FreeBSD-7 has
> >> been EOL for a long time, but I would like to hear other people
> >> comments.
You don't, but other people care about ABI.

Besides older jails which you do not care about, there is significant
set of programs which were coded to use Berkley' pty directly.  Even
high-profile applications like Emacs automatically selected pty(4)
up to its previous version on FreeBSD.

> >>
> >> The code review for the proposed change can be found here:
> >> https://reviews.freebsd.org/D659
> >>
> >> If I won't get any objection I'll commit this in one week time, i.e.
> >> August 27th.
> >
> > Why not just statically create the pairs in /dev?  Use some loader tunable
> > (kern.ptymax) to set a count on the number of pre-created device pairs to
> > create and then just explicitly create them in the mod_event handler?  It
> > could default to 100 or so.
> >
> 
> Done, thank you for the suggestion, John.
> 
> root_at_maxwell:/home/davide # kldload pty
> root_at_maxwell/home/davide # sysctl -a |grep pty
> kern.tty_pty_warningcnt: 1
> kern.npty: 32
> debug.softdep.emptyjblocks: 0
> 
> root_at_maxwell:/home/davide # ls /dev/pty*
> /dev/ptyl0 /dev/ptyl2 /dev/ptyl4 /dev/ptyl6 /dev/ptyl8 /dev/ptyla
> /dev/ptylc /dev/ptyle /dev/ptylg /dev/ptyli /dev/ptylk /dev/ptylm
> /dev/ptylo /dev/ptylq /dev/ptyls /dev/ptylu
> /dev/ptyl1 /dev/ptyl3 /dev/ptyl5 /dev/ptyl7 /dev/ptyl9 /dev/ptylb
> /dev/ptyld /dev/ptylf /dev/ptylh /dev/ptylj /dev/ptyll /dev/ptyln
> /dev/ptylp /dev/ptylr /dev/ptylt /dev/ptylv
> 
> https://reviews.freebsd.org/D1238 for review.
> I hope anybody that raised concerns about the previous patch can try
> this new one.

I do not see why dev_clone event makes your so unhappy.  I object against
removal of it (and this is what you are aiming at, it seems).  It provides
useful functionality, which is not substituted by anything cdevpriv(9)
can provide.

My only hope is that you are confusing dev_clone event and a library of
clone_create(9)/clone_cleanup(9)/dev_stdclone(9) functions.  The former
is needed and cannot be replaced by cdevpriv(9).

The later is clumsy and never was used properly. My opinion is that it
is impossible to use properly. There are five uses of that in tree left,
and it seems that removing them worth cleaning of buggy by design and
undocumented KPI.

Really big and complicated target is the hand-written timeout-based (?!)
custom cloning code in snd(4).  I believe it _can_ be converted to
cdevpriv(9).