Re: [PULL] http://www.linuxtv.org/hg/~hverkuil/v4l-dvb

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tuesday 10 March 2009 00:50:41 Mauro Carvalho Chehab wrote:
> On Mon, 9 Mar 2009 08:16:53 +0100
>
> Hans Verkuil <hverkuil@xxxxxxxxx> wrote:
> > On Monday 09 March 2009 02:07:33 Trent Piepho wrote:
> > > On Sun, 8 Mar 2009, Hans Verkuil wrote:
> > > > The last one fixes an ivtv regression caused by this change:
> > > >
> > > > changeset:   10811:0a0eba8e64d5
> > > > user:        Trent Piepho <xyzzy@xxxxxxxxxxxxx>
> > > > date:        Tue Mar 03 20:21:02 2009 -0800
> > > > summary:     videodev: only copy needed part of RW ioctl's
> > > > parameter
> > > >
> > > > The fix is simple: switch on the full ioctl command instead of just
> > > > the NR field.
> > > >
> > > > Thanks to Martin Dauskardt for doing the bisect and tracing the
> > > > breakage to this change.
> > >
> > > Switching on the whole ioctl makes the switch statement a lot less
> > > efficient.  I'd rather just put a if (_IOC_TYPE(cmd) != 'V') return
> > > 0; in there.  That should fix the non-v4l2 ioctls, right?
> >
> > That was my first thought as well, but I realized that this is one area
> > where you really do not want to take any risk.
>
> IMO, it is better to use Trent's way, and reduce the number of places
> that do memset(0).
>
> > Personally I think this code is overoptimization. In my view the
> > performance advantage is minimal while reducing the readability of the
> > code.
>
> In general, it is a good idea to avoid copying a data that won't be used
> from userspace. I liked the optimizations done. Let's just fix what's
> broken and test a lot to avoid causing a kernel regression.

I strongly suspect that the extra function call and tests involved takes 
longer than the initial copy_from_user and memset. Perhaps with the 
exception of G_FMT, which is really the only one that has a non-trivial 
amount of data to copy. None of the commands involved are high-performance 
commands (well, we haven't any of those anyway), nor is a tight inner-loop 
involved. I'll see if I can run some benchmarks, but I remain convinced 
that this change has no benefit.

Regards,

	Hans

-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux