Re: [PATCH 00/17] [RFC] AFS: Implement OpenAFS pioctls(version)s

[David Howells <dhowells@redhat.com>]

Linus Torvalds <torvalds@linux-foundation.org> wrote:

> > Ummm...  I'm not sure I completely agree.  If you've managed to open, say,
> > "/afs", where's the race with mount/umount?
> 
> Well, if you mean that you're going to have a new system call that then 
> passes in both the 'fd' from that /afs open, _and_ the pathname you want 
> to work on, then sure.

No.  I meant open + ioctl.

> But if you do that new system call, then what's the point again? You're 
> back to pinfo() anyway.

Ummm...  pinfo()?  Did you mean pioctl()?

> No. It's because it's another _typeless_ multiplexor.

What do you mean by 'typeless'?  Even the master syscall mux is typeless,
depending on how you look at it; either that, or it's the superposition of a
multiplicity of types selected by an arbitrary number.

Short of doing something like an XML or ASN1 structured interface, we aren't
going to get that, and do we really want to go down that path?

The difference between the syscall mux and a filesystem's ioctl/pioctl mux is
that the both need to check on their arguments.

> Look at ioctl. It's a F*CKING DISASTER. Look at all the compat crap, and 
> at the ioctl numbers that mean different things for different file types, 
> and all the random sizing crap. You fixed the random sizing crap (at least 
> it has well-defined "input" and "output" areas), and that's an 
> improvement, but it's still just random numbers with no semantics.

Well, to be fair, I didn't fix it.  That's the way pioctl() was defined before
I got to deal with it.  Compat code is not necessary beyond the outermost VFS
layer because you have to carefully structure your input and output blobs, and
pointers and CPU-dependent tyeps are not allowed therein.  It even uses XDR
encoding in some circumstances (VIOCSETTOK2 and VIOCGETTOK2), so some pioctls
will even work on a mixed-endian machine.  Now if it only used XDR for all...

>  - learn from your mistake, and not do another f*cking disaster that just 
>    takes a pathname instead of a fd. Do something else, that actually has 
>    semantics and has a well-defined input and output buffer.

That sounds like you want all the pioctl functions promoted to syscalls.
Emulation through ioctl does not gain this; nor does going through xattrs -
that's just a way of doing ioctls with textual command names instead of
numbers.

> And guess which one "pioctl()" is. Just take a wild stab at it.

Well, I'd say it's more intelligent that open+ioctl...

> > 	fd = open("/the/target/file", O_SUPPRESS | (nofollow?O_NOFOLLOW:0));
> > 	ioctl(fd, cmd, &args);
> > 	close(fd);
> 
> Yes, I think that would be better.

But it isn't sufficient to address all the cases - in which case it's
pointless.

David