On Mon, 09 Mar 2009 17:06:34 -0400 Andy Walls <awalls@xxxxxxxxx> wrote: > On Mon, 2009-03-09 at 08:16 +0100, Hans Verkuil 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. > > > > Personally I think this code is overoptimization. In my view the performance > > advantage is minimal while reducing the readability of the code. > > On x86_64, I counted 3 bytes per switch case saved. For the 22 switch > cases: a total of 66 bytes saved. If compiled code size reduction is > the efficiency measure, I suspect there are probably lower hanging fruit > to be picked. > > Assuming a 32 byte I-cache line, the aforementioned savings saves about > 2 lines worth of loading from memory. Maybe it matters for platforms > with small cache sizes (embedded platforms?). You should remind that copy_[from|to]_user can sleep, in order to get data from swaped memory. It is probably not that easy to measure the performance wins done by such change, since we'll need to measure also cache hits/misses not only on L1/L2/L3 cache, but also on swap. Probably, the most significant effects will be on embedded hardware with smaller caches. 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
