Skip to contentSkip to conversation

more kdev_t-ectomy

CFT [email protected] 4 participants 5 messages Apr 20, 2003

lore
Share & tools
lore
#0 lore raw

Patchset is on ftp.linux.org.uk/pub/people/viro/T*
So far it kills kdev_t crap in tty and console layers; more to follow. The goal is to kill kdev_t completely and switch the character device side of things to real objects. Fortunately it's nowhere near the nastiness of corresponding block device work.

Please, test. Patches are against 2.5.68 and It Works Here(tm).
If there will be no screams, it goes to Linus.

Summary of patches follows:

1) tty.driver turned into a pointer
* we never modify any fields of tty.driver
* we initialize it with a copy of registered struct tty_driver
* we never use any fields that might be modified in original
* we pin the original down until we free tty
ergo, we can store a reference instead of copying the damn thing. Aside of shrinking tty_struct, it allows for much saner refcounting and a bunch of cleanups later.

2) /proc/tty/drivers converted to seq_file

3) removed fake drivers
instead of registering "drivers" for /dev/tty, /dev/vc/0, /dev/ptmx
and /dev/console (they are never looked up since tty_open() special-cases them and they should not be looked up - these devices are remapped on open) we register corresponding chrdev ranges and devfs nodes directly.
/proc/tty/drivers code updated to keep the contents unchanged

4) tty->tty_name added

	new field; initialized to <driver->name><tty index+driver->base_name>

when we allocate tty_struct. Drivers code switched to use of that beast (in debugging printks, mostly).

5) tty->tty_index added

	tty->tty_index added; we initialize it with minor(tty->device) -

tty->driver->minor_start. Majority of remaining tty->device uses had that form and are switched to use of tty->index.

6) driver->driver_name cleanup

	sanitized driver->driver_name initialization and use

7) misc tty cleanups
* generic_serial.c typo fix (->driver used instead of correct
->driver_data)
* tubio cleaned up

8) rio cleanups and fixes
* drivers/char/rio/* supports up to 4 boards, each with up to 128
lines. It used to share termios for 1st/3rd and 2nd/4th boards, Fixed.
* cleanups and kdev_t removals - we pass tty instead of tty->device
in a couple of helper functions and instead of comparisons on major(tty->device) we check where does tty->driver point to.

9) tty_io.c::init_dev() change
Preparations to cleanup:
* call of get_tty_driver() moved from init_dev() to its callers
* instead of kdev_t dev we pass struct tty_struct *driver and int index

10) tty->device switched to dev_t

	There are very few uses of tty->device left by now; most of

them actually want dev_t (process accounting, proc/<pid>/stat, several ioctls, slip.c logics, etc.) and the rest will go away shortly.

11) Big ranges for tty_driver
* we allow tty_driver to cover more than 256 devices
* pty.c cleaned up - now we only one driver for UNIX98 masters and
only one driver for UNIX98 slaves, so a lot of ugliness can be killed.
* get_tty_driver() became an analog of get_gendisk() - it does
a lookup by device number and gives (pointer to tty_driver,index).
* registration/unregistration of tty_driver updated
* /proc/tty/drivers code updated (now one structure can be responsible
for several lines)

The next two patches deal with interaction between tty and console.

12) uart_console_device() helper
console drivers that come from drivers/serial switched to common
->device() method.

13) console->device() change

	instead of returning kdev_t console->device() returns tty_driver and

index. Caller updated.

14) fbdev->node kdev_t removal

	All places that use fbdev->node are interested only in one thing -

number of framebuffer (i.e. minor(fbdev->node)). The place setting that beast finds an unused number and sets ->node to mk_kdev(FB_MAJOR, number). Obvious cleanup: stop messing with kdev_t and just store what we really want to store - an integer.

15) ppc boot partition cleanup
gratitious uses of kdev_t when we are actually dealing with
device numbers (as visible by userland) removed.

#1 lore raw

On Sun, Apr 20, 2003, [email protected] wrote:

[lots of useful stuff].

Happy Easter!

Concerning kdev_t vs dev_t, probably I said this before, but just to be sure:

Long ago the purpose of kdev_t was to become a pointer. Roughly speaking we got that pointer, only it is called struct gendisk * today.

Today the purpose of kdev_t is to be a form of dev_t: taking the minor of a kdev_t is just taking the lower 32 bits, no tests, no branches; taking the minor of a dev_t requires tests and branches - more code, slower code.

So, the interface with filesystems and with userspace has dev_t. For kernel-internal numbers kdev_t is better than dev_t.

Of course it may be possible to avoid kernel-internal numbers altogether. Sometimes that is an improvement, sometimes not. Pointers are more complicated than numbers - they point at something that must be allocated and freed and reference counted. A number is like a pointer without the reference counting.

Andries

#2 lore raw

[cc fixed - my apologies]

Quoted 8 lines · Andries Brouwer

So, the interface with filesystems and with userspace has dev_t. For kernel-internal numbers kdev_t is better than dev_t.

Of course it may be possible to avoid kernel-internal numbers altogether. Sometimes that is an improvement, sometimes not. Pointers are more complicated than numbers - they point at something that must be allocated and freed and reference counted. A number is like a pointer without the reference counting.

Sigh... And what, pray tell, do we do with these numbers? That's
right, at some point(s) we use them to obtain <drumroll> pointers to driver-supplied objects. And that's where the lack of refcounting, locking, etc. bites you.

Let's sort that mess out for good. Papering over this stuff won't do us any good and will only bring more kludgy interfaces. $DEITY witness, we already have enough of those.

It's not about pointers vs. numbers - we certainly have enough cases when we really deal with the latter. However, I'd rather have clear separation between "32bit value presented to/by userland to identify device node" and "value in range 0--7, representing the number of channel on a multiport card FooLink-8X". The latter makes perfect sense for a driver. As does "structure that represents given instance of FooLink-8X". The former belongs to interaction with userland and using it outside of that context is a kludge. Dangerous kludge in case if it masks the aforementioned complications. It also breeds all sorts of ugliness in the code - see the crap around reassigning ->f_op and problems with clean implementation of revoke(2) analogs for instance. Or a buttload of fun induced by (completely artificial) separation into major and minor - see the mess around UNIX98 ptys implementation, etc.

By now all uses of mk_kdev()/major()/minor()/MAJOR()/MINOR() in the drivers are either trivially removable or represent very real problems. And it's not that there was a lot of them - in my current tree there's ~85 instances of kdev_t in the source. And only one of them (->i_rdev) is widely used - ~500 instances, most of them go away as soon as CIDR patch gets merged. The rest is part noise, part real bugs that need to be fixed anyway (~40--80 of those).

#3 lore raw
In reply to Andries Brouwer
Followup to:  <[email protected]>
By author:    Andries Brouwer <[email protected]>
In newsgroup: linux.dev.kernel
Quoted 7 lines

Of course it may be possible to avoid kernel-internal numbers altogether. Sometimes that is an improvement, sometimes not. Pointers are more complicated than numbers - they point at something that must be allocated and freed and reference counted. A number is like a pointer without the reference counting.

I guess the question is: is there any point to have three forms -- with necessary conversions between them -- or is it simpler to have two forms and just use the more awkward dev_t form everywhere? The only use for dev_t's in the kernel should be getting them from or shipping them off to userspace at some point, so it might be just as easily to do the conversion directly -- macroized, of course.

We do need a dev32_t for NFSv2 et al, though.

-hpa

Signature · 3 lines
<[email protected]> at work, <[email protected]> in private!
"Unix gives you enough rope to shoot yourself in the foot."
Architectures needed: ia64 m68k mips64 ppc ppc64 s390 s390x sh v850 x86-64
#4 lore raw
In reply to [email protected]
Quoted 5 lines · On Sun, Apr 20, 2003 at 02:31:43PM +0100…

Patchset is on ftp.linux.org.uk/pub/people/viro/T*
So far it kills kdev_t crap in tty and console layers; more to follow. The goal is to kill kdev_t completely and switch the character device side of things to real objects. Fortunately it's nowhere near the nastiness of corresponding block device work.

Applied cleanly to 2.5.68-cvs, compiled and booted fine. Running it now on IBM Thinkpad R-30, debian unstable. X works fine, emacs works, anything special to test?

Signature · 2 lines
Muli Ben-Yehuda
http://www.mulix.org