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

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

 



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.

Cheers,
Mauro
--
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