Re: LVM / General IOCTL question

On Fri, 14 Nov 2003, Peter Jones wrote:

> > My real question is, after searching this out, I find a whole bunch of
> > ioctl translation stuff in the module independent parts of the kernel.  I
> > see some wrong assumptions in this (mainly that IOCTL numbers are not
> > globally unique)
>
> Where does it make that assumption?

In the translation table, which has an entry for each IOCTL, keyed only by
IOCTL number. If the IOCTL numbers are reused by different devices (which
they can be), then there is no way to determine which device to do the
translation for.  A unique mapping is (fd[ eg device/file/socket], IOCTL).
The IOCTL numbers may not be unique.  So for example 0x4004fe98 (used by
LVM) might be used by another device. In the normal case, there is no
problem since the file handle allows the device/file/socket driver to
handle the ioctl processing.  But the translation table can't properly
handle the case were there are two different devices that use the same
ioctl.

If the IOCTL number was unique, we would not need to pass a file
descriptor to the ioctl system call.  The unqifying components of IOCTL
are meant to minimize the damage done when the wrong device file is used.
In some hardware devices, sending random IOCTLs results in relays being
altered, and can result in damage and even fire. Fire actually happened to
us at KSR in the early 90's, though not as a result of IOCTL, but rather a
bug in a selftest routine.  That person was made a manager.

> > but wondered if someone can clue me in on what the idea is with doing
> > translations.  It seems that all that is being done is fix up structures
> > which have pointers embedded in them.  But of course, this could also be
> > done by the userspace code that uses the structures coming out of the
> > kernel, which seems (to my way of thinking anyway) a much, much better
> > place to do this.
>
> Userspace is running 32-bit, kernel is running 64-bit.  For most things,
> the pointer just goes to an integral value, not a structure, so you can
> just you can just translate the pointer and pass a value through, but some
> things need more sophisticated translation.  COMPATIBLE_IOCTL() is saying
> "you can just futz with the pointer for me", HANDLE_IOCTL() is saying "the
> args here are complicated, and we built a special function which
> translates the values", and they build a table of how to handle each
> ioctl.

Yep.

> > This seems to be such a terrible clusterf*ck, creating a lot of update
> > contention on a ioctl32.c. But of course, we'll also ignore/acknowledge
> > that sending pointers out of the kernel is probably also a clusterf*ck
> > waiting to happen.
>
> Not just a clusterfuck.  You actually _can't_ do it, because the pointers
> aren't the same as userland's.

Kernel pointers shouldn't be passed to userland. In the case of lvm's
VG_STATUS, they are in the structure, but are just ignored, and the
do_lvm_ioctl routine just fixes up the vg_uuid to come in the right place
if the pointers were 32 bits. It is after the pointers in the kernel
version of the structure.  The actual value of the pointers are not
changed. In fact, the first "32 bit pointer" is really half of the first
64 bit pointer, and the second "32 bit pointer" is the other half of the
first 64 bit pointer from the original structure.  So apparently LVM does
not use these pointers in userland. They are just cruft coming back from
the kernel code.

I can't say that all of the pointers being passed back are never used, but
of course, if they are to be used, then userland needs to get the entire
64 bit pointer, so it can pass the right value back later.  But I doubt
that they are being used, since then the userland would have to be
changed, and these 'handlers' seem to be trying to avoid that.  So I think
it is likely that in every case, these pointers are cruft.

> > However, this just seems to make it worse, rather than better.
>
> It makes it possible

It is certainly not the only solution. It is also possible to change the
kernel/userland interface structures to not contain/pass back pointers
(they are no good to anyone anyway, as they can't be trustably passed
back). Then there is no problem if sizeof(void *) changes.  This is the
cleanest solution.

The other alternative is to fix the driver code directly. There is no
reason that, for example, the lvm module/driver (lvm.c) ioctl handler
can't do what's in do_lvm_ioctl.  This would be far more portable.  Both
of these alternatives seem to be much better than this translation table
scheme.

Its not that I'm against kernel fixups of userland cruft. I'll tell you a
little story to prove that.

Back in the 90's, Sun altered solaris to abort any program that made an
unaligned access, on the theory that the user program needed to be altered
not to make unaligned accesses.  This made it a major pain to port
freeware to Solaris, since the freeware at the time was mostly written on
Vax, and thus used packed structures without concern for word alignment.
The original reference port of OSF/1 to MIPS (reference risc port)
followed this idea about unaligned accesses.

At the time, there was much religion and controversy about whether the
application should be changed, the compilers should be changed, etc.  The
zealots at Sun (and at the OSF) obviously had their point of view, which
was to dump the program that made an unaligned access. There was nothing
an end user could do about it.

Almost nothing.  I added a change to the OSF/1 MIPS port unaligned access
trap handler to fixup unaligned accesses. Basically, I just made the trap
handler decode the instruction, it had to be a load or store, and stuff or
unload the register a byte at a time, and then restarted the program at
the next instruction.  This was a lot slower, but it was more portable.
I also included (at the request of the risc zealots), a kernel printf to
say that this happened.  So now it became much easier to port things like
news and such to the OSF/1 MIPS machine.  I should add that getting this
change into the mainline was not nearly as easy as writing the trap
handler.  Lets just say that politics and negotiation has always been a
forte. ;-)

DEC then ported OSF/1 to the Alpha, and included my trap handler,
appropriately ported.  Even better, DEC altered it so the kernel printf
was conditionally called.  Before long, it seems that Sun was pressured
into adding a similer trap handler to solaris. Other risc machine OS
vendors also followed suit.  Now, word alignment is a performance issue,
but not a portability issue.

So, you see, I'm not at all against having the kernel do certain things.
But I think in this case there are two other alternatives, both of which
are better.

		--Dean



_______________________________________________
Aurora-sparc-devel mailing list
[email protected]
http://lists.auroralinux.org/mailman/listinfo/aurora-sparc-devel
Aurora FAQ: http://www.ecs.soton.ac.uk/~mas01r/aurorafaq.html