Buffer management in the zr36067 driver

Hi Trent, Ronald,

In a different thread some times ago, Trent Piepho wrote:
> I was able to change the driver's kmalloc() limit from 128k to 1024k, and it
> appeared to be able to allocate the larger memory region fine.  This is an
> old driver, and the rules wrt large kmalloc()s may have changed since the
> 128k limit was put in.

The xfs code uses a 128 kB limit, too. From LDD3, about kmalloc:
"If your code is to be completely portable, it cannot count on being
able to allocate anything larger than 128 KB."

But maybe we should be using __get_free_pages instead:
"The maximum allowed value for order is 10 or 11 (corresponding to 1024
or 2048 pages) depending on the architecture. The chances of an
order-10 allocation succeeding on anything other than a freshly booted
system with a lot of memory are small, however."

The maximum order seems to be 10 on x86_64. This is 4 MB. Even if we
can't get that, being able to get 512 kB or 1 MB would be a significant
improvement over the current situation. Maybe we can even get 2 or 4 MB
if we preallocate the buffers on driver load. Arguably, this can be
seen as a waste of kernel memory, so maybe this should be an option
rather than the default.

I believe that the driver should not hard-code a memory limit value.
Instead, it should always try kmalloc (or __get_free_pages), and it
should fallback to other methods (get_high_mem, bigphys_area) only if
kmalloc fails. What do you think?

Other related points I'd like to discuss:

Without CONFIG_BIGPHYS_AREA (default), the driver uses get_high_mem()
to allocate large buffers, which looks like a gross, totally unsafe
hack to me. And it doesn't work for me at least, the largest amount I
manage to get is 32 kB. Does this really work for anyone? I'm surprised
it was even accepted into the kernel tree. Shouldn't we remove this
from the driver?

Other v4l drivers (bttv, cx88) use a helper module called video-buf.
The header comment says it implements non-contiguous, PCI-DMA-able
buffers for video drivers. Couldn't this be used in the zr36067 driver
as well?

Thanks,
-- 
Jean Delvare

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/