Industrial device driver uio/uio_*

Industrial device driver uio/uio_*

[Alan Cox]

This passed me by while I was away so I only saw it to review in mm6. It
looks like it has a few problems...


uio_lseek locking ?

uio_read uses f->f_pos which it should never do
ditto uio_write ...

The uio_read/write functions can race seek or other read/write

The uio_read/write functions do signed maths checks on unsigned types
(size_t) so the count check fails.

Partially completed I/O returns -EFAULT, should return the length
transferred OK

The *ppos adjustment means you can get f_pos to interestingly unsafe
values. Generally speaking do

	loff_t pos = *ppos;

	do stuff with pos

	pos += movement;
	*ppos = pos;


If idev->virtaddr is an mmio object you can't use copy_from/to_user on
it

uio_event is based on sizeof(int) so makes 32bit compat code insanely
hard

event_poll is wrong - poll methods shouldn't error just return "ready"

idev->event_listener appears to have no locking versus the irq handler
and the like.

Anyone appears to be able to select any process as the task to signal or
out of range or negative values

I've not begun to look at uio_base.c in detail but it seems a lot of
code to do very little. That said I can see the problem it is trying to
solve, I'm just not sure it helps as is.

Alan

Re: Industrial device driver uio/uio_*

[Jan Engelhardt]

>Subject: Industrial device driver uio/uio_*

Hm has this now been named uio? iio may have seem strange to some, but 
uio also resembles BSD/Solaris (uio_copyin, uio_copyout, uiomove, etc.)


Jan Engelhardt
-- 

Re: Industrial device driver uio/uio_*

[Greg KH]

On Mon, Sep 11, 2006 at 09:23:23PM +0200, Jan Engelhardt wrote:
> 
> >Subject: Industrial device driver uio/uio_*
> 
> Hm has this now been named uio? iio may have seem strange to some, but 
> uio also resembles BSD/Solaris (uio_copyin, uio_copyout, uiomove, etc.)

Yes, I am aware of uio, we have include/linux/uio.h :)

Do you have a better name for this code?  If you don't like uio, I'm
open to new suggestions.

thanks,

greg k-h

Re: Industrial device driver uio/uio_*

[Jan Engelhardt]

>> Hm has this now been named uio? iio may have seem strange to some, but 
>> uio also resembles BSD/Solaris (uio_copyin, uio_copyout, uiomove, etc.)
>
>Yes, I am aware of uio, we have include/linux/uio.h :)
>
>Do you have a better name for this code?  If you don't like uio, I'm
>open to new suggestions.

- I was fine with iio
- udio, for userspace-driven IO (my favorite should iio not make it)
- idd, since it's called "Industrial device driver"
  (don't go about calling it iddqd)

more artistic ones:
- iofu, for IO From Userspace, or, on 2nd thought, simply IO-fu (as in Kung-fu)
- something else that's not related at all? /methinks of squid,
  which, from the name, does not suggest it would be a web proxy.


Jan Engelhardt
-- 

Re: Industrial device driver uio/uio_*

[Alan Cox]

Ar Llu, 2006-09-11 am 18:22 +0100, ysgrifennodd Alan Cox:
> This passed me by while I was away so I only saw it to review in mm6. It
> looks like it has a few problems...
> 

And nothing has happened since [Thats Sept 11th] even on the simple bugs
although the authors were sent a copy. At this point can the linutronix
guys either let me know if they just never got the list of flaws or can
we pull the drivers/uio stuff from -mm until its rather better ?

I would just NAK it but want to be sure the guys saw the list of
problems

Alan

Re: Industrial device driver uio/uio_*

[Andrew Morton]

On Wed, 04 Oct 2006 19:59:54 +0100
Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:

> Ar Llu, 2006-09-11 am 18:22 +0100, ysgrifennodd Alan Cox:
> > This passed me by while I was away so I only saw it to review in mm6. It
> > looks like it has a few problems...
> > 
> 
> And nothing has happened since [Thats Sept 11th] even on the simple bugs
> although the authors were sent a copy. At this point can the linutronix
> guys either let me know if they just never got the list of flaws or can
> we pull the drivers/uio stuff from -mm until its rather better ?
> 
> I would just NAK it but want to be sure the guys saw the list of
> problems
> 

cc's added.

Thomas has been a bit tied up with timers and interrupts of late.

Re: Industrial device driver uio/uio_*

[Thomas Gleixner]

On Wed, 2006-10-04 at 12:18 -0700, Andrew Morton wrote:
> On Wed, 04 Oct 2006 19:59:54 +0100
> > I would just NAK it but want to be sure the guys saw the list of
> > problems
> > 
> 
> cc's added.
> 
> Thomas has been a bit tied up with timers and interrupts of late.

Yup. fork(tglx) still returns -ETOOMANYINSTANCES.

I have no objections, if you pull it from -mm for now. The list of flaws
is accepted and we'll work on this in foreseeable time, _IF_ there is
some basic consensus about the idea itself not being fundamentaly wrong.

	tglx

Re: Industrial device driver uio/uio_*

[Alan Cox]

Ar Mer, 2006-10-04 am 21:32 +0200, ysgrifennodd Thomas Gleixner:
> I have no objections, if you pull it from -mm for now. The list of flaws
> is accepted and we'll work on this in foreseeable time, _IF_ there is
> some basic consensus about the idea itself not being fundamentaly wrong.

The device stuff seems ok, the class stuff seems to me overconvoluted
but I'd rather someone who knew the class stuff well (Greg thats you)
made that decision.

Alan

Re: Industrial device driver uio/uio_*

[Paul Mundt]

On Wed, Oct 04, 2006 at 09:32:25PM +0200, Thomas Gleixner wrote:
> On Wed, 2006-10-04 at 12:18 -0700, Andrew Morton wrote:
> > On Wed, 04 Oct 2006 19:59:54 +0100
> > > I would just NAK it but want to be sure the guys saw the list of
> > > problems
> > > 
> > 
> > cc's added.
> > 
> > Thomas has been a bit tied up with timers and interrupts of late.
> 
> Yup. fork(tglx) still returns -ETOOMANYINSTANCES.
> 
> I have no objections, if you pull it from -mm for now. The list of flaws
> is accepted and we'll work on this in foreseeable time, _IF_ there is
> some basic consensus about the idea itself not being fundamentaly wrong.
> 
I've got a few cycles I can throw at this, it's a problem space we
(Renesas) are interested in too..

Alan, is your Sept. 11 list the extent of your issues with the current
code, or was there more that you sent off-list?

I'll toss up a quick git tree for this so we don't lose anything, and
the fixes can trickle in to -mm that way, or it can just be pulled and
added back later.

Re: Industrial device driver uio/uio_*

[Alan Cox]

Ar Iau, 2006-10-05 am 17:12 +0900, ysgrifennodd Paul Mundt:
> Alan, is your Sept. 11 list the extent of your issues with the current
> code, or was there more that you sent off-list?

Its the current state from the initial pass

Alan

Re: Industrial device driver uio/uio_*

[Greg KH]

On Thu, Oct 05, 2006 at 05:12:47PM +0900, Paul Mundt wrote:
> On Wed, Oct 04, 2006 at 09:32:25PM +0200, Thomas Gleixner wrote:
> > On Wed, 2006-10-04 at 12:18 -0700, Andrew Morton wrote:
> > > On Wed, 04 Oct 2006 19:59:54 +0100
> > > > I would just NAK it but want to be sure the guys saw the list of
> > > > problems
> > > > 
> > > 
> > > cc's added.
> > > 
> > > Thomas has been a bit tied up with timers and interrupts of late.
> > 
> > Yup. fork(tglx) still returns -ETOOMANYINSTANCES.
> > 
> > I have no objections, if you pull it from -mm for now. The list of flaws
> > is accepted and we'll work on this in foreseeable time, _IF_ there is
> > some basic consensus about the idea itself not being fundamentaly wrong.
> > 
> I've got a few cycles I can throw at this, it's a problem space we
> (Renesas) are interested in too..
> 
> Alan, is your Sept. 11 list the extent of your issues with the current
> code, or was there more that you sent off-list?
> 
> I'll toss up a quick git tree for this so we don't lose anything, and
> the fixes can trickle in to -mm that way, or it can just be pulled and
> added back later.

Please just send me patches and I'll update the main version that I
have, which will get pulled into -mm.

And yes, I need to look at this some more too.  I was considering just
stripping it down to the bare essencials right now, the interrupt
handling stuff, and then slowly adding pieces back when people asked for
them.

Unless people need all of the different options right now?

thanks,


greg k-h