linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alan Cox <alan@lxorguk.ukuu.org.uk>
To: Adam Jackson <ajax@redhat.com>
Cc: linux-kernel@vger.kernel.org, arnd@arndb.de, gregkh@linuxfoundation.org
Subject: Re: [PATCH 1/2] char/mem: Add /dev/io (v2)
Date: Wed, 8 Feb 2012 14:32:38 +0000	[thread overview]
Message-ID: <20120208143238.0003287e@pyramind.ukuu.org.uk> (raw)
In-Reply-To: <4F3280C7.1030401@redhat.com>

> > They are both equally insane and have effectively identical security
> > semntics. Continuing to use iopl is both faster and avoids adding a kernel
> > API however. Even better it's x86 specific so that piece of manure
> > doesn't escape onto other platforms without the legacy vesa mess.
> 
> Every point in this paragraph is at best misleading, if not outright wrong.

Really - exactly where is it wrong. None of your claims below seems
valid. I think my statement is accurate even after your response.
> 
> iopl does not have identical security semantics to ioperm.  iopl lets my 

I didn't mention ioperm. I said /dev/ports

> X server disable interrupts.  /dev/io would not, and would let me add a 

Yes it would - you can issue I/O accesses to the interrupt controller. So
let me repeat that - the two are the same thing and equally insane.

> Your definition of "faster" is spurious.  VBE calls are not a 

You are not the only user of iopl, and they are faster. You may not
*care* about the speed difference but the fact is they are faster.

> performance path and system call overhead is negligible compared to I/O 
> serialization.  If anything /dev/io can be _faster_ in the mainline case 
> because we'll no longer need to handle the ioperm bitmask on every 
> context switch.

See before - I never mentioned ioperm.

> The current patch set results in a net gain of zero kernel interfaces, 
> once /dev/port is put down in a year.  I will admit that the current 

People will be shipping code using it for years.

> /dev/port is probably not a meaningfully used API, seeing how it's been 
> broken since literally kernel 0.10.  But it's there and enabled by 
> default.  I would like it to work, please.

So why hasn't it changed - because nobody has had a problem with it.

> Invoking architecture-specificity is spurious.  Competent architectures 
> have a usable framebuffer interface from the firmware, so vesa never 
> comes up.

Exactly - so they don't need another port interface.
 
> Basically what I'm hearing here is "don't bother doing this well since 
> you already have a way you're doing it badly".  That's crap.  I've 
> written the code to do it well.  I've signed up for the support cost. 
> Can I please have something good instead of something bad?  I sort of 
> thought "good" was the whole point of what we're doing here.

I'm all for doing it well but that isn't doing it well. Its adding
another turd which is logically and functionally equivalent to the
existing one. Two turds is not usually better than one unless you are
running a methane digestion plant.

iopl is functionally equivalent to your io port code. It's already in the
kernel, it already works and it's already maintained.

To be useful a port interface really needs to aware of hotplug, device
mappings and the like. For VESA that's going to be pretty horrible both
because of the uncontrolled nature of the interface and because of the
vga "magic" both for ports and routing.

An interface where you could do something like

	open a port channel
	bind device X to port channel  (imports its I/O ranges)
	bind portrange A-B to port channel (for nasties like the vga
				ports)

read/write in 1/2/4/(8 ?) byte chunks meaningfully

	close

and which properly handled device hotplug - ie if you bind to device X, X
is unplugged then the next read/write errors, or at least the address
space is not freed until it isn't in the port range space.

Now that would be an improvement, but it seems to be a fair amount of
work.

Alan




  reply	other threads:[~2012-02-08 14:31 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-07 14:11 [PATCH 1/2] char/mem: Add /dev/io Adam Jackson
2012-02-07 14:11 ` [PATCH 2/2] char/mem: Schedule /dev/port for removal Adam Jackson
2012-02-07 23:39 ` [PATCH 1/2] char/mem: Add /dev/io (v2) Adam Jackson
2012-02-08  0:17   ` Alan Cox
2012-02-08  0:38     ` Adam Jackson
2012-02-08  9:26       ` Alan Cox
2012-02-08 14:03         ` Adam Jackson
2012-02-08 14:32           ` Alan Cox [this message]
2012-02-08 23:16             ` Adam Jackson
2012-02-09 11:27               ` Alan Cox

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120208143238.0003287e@pyramind.ukuu.org.uk \
    --to=alan@lxorguk.ukuu.org.uk \
    --cc=ajax@redhat.com \
    --cc=arnd@arndb.de \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).