From patchwork Sun Mar 1 15:18:00 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jean Delvare X-Patchwork-Id: 9458 Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id n21FIpNK011176 for ; Sun, 1 Mar 2009 15:18:51 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752185AbZCAPSS (ORCPT ); Sun, 1 Mar 2009 10:18:18 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1752964AbZCAPSS (ORCPT ); Sun, 1 Mar 2009 10:18:18 -0500 Received: from zone0.gcu-squad.org ([212.85.147.21]:26251 "EHLO services.gcu-squad.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752185AbZCAPSR (ORCPT ); Sun, 1 Mar 2009 10:18:17 -0500 Received: from jdelvare.pck.nerim.net ([62.212.121.182] helo=hyperion.delvare) by services.gcu-squad.org (GCU Mailer Daemon) with esmtpsa id 1LdoUc-0003Jw-3p (TLSv1:AES256-SHA:256) (envelope-from ) ; Sun, 01 Mar 2009 17:26:18 +0100 Date: Sun, 1 Mar 2009 16:18:00 +0100 From: Jean Delvare To: Trent Piepho Cc: Hans Verkuil , linux-media@vger.kernel.org, Mauro Carvalho Chehab , Martin Samuelsson Subject: Re: [PULL] http://www.linuxtv.org/hg/~hverkuil/v4l-dvb-zoran Message-ID: <20090301161800.4ce70acd@hyperion.delvare> In-Reply-To: References: <200902221858.29725.hverkuil@xs4all.nl> X-Mailer: Claws Mail 3.5.0 (GTK+ 2.14.4; x86_64-suse-linux-gnu) Mime-Version: 1.0 Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org Hi Trent, On Sun, 1 Mar 2009 06:38:03 -0800 (PST), Trent Piepho wrote: > On Sun, 22 Feb 2009, Hans Verkuil wrote: > > Please pull from http://www.linuxtv.org/hg/~hverkuil/v4l-dvb-zoran for the > > following: > > I have some questions about your changes. > (...) > > - zoran: remove broken BIGPHYS_AREA and BUZ_HIMEM code, and allow for > > kmallocs > 128 kB > > It looks like the last time the bigphys_area patch was updated was to > 2.6.11 so there probably isn't much point is supporting that anymore. But > is the highmem code broken? Trying to allocate multiple megabyte buffers > on systems without gigabytes of memory doesn't work very well. You get > nice things like this in your kernel log: > > tvtime: page allocation failure. order:8, mode:0x40d0 > [] __alloc_pages+0x29b/0x2aa > [] __slab_alloc+0x192/0x343 This can be "fixed" with the following patch: Subject: zoran: Don't frighten users with failed buffer allocation kmalloc() can fail for large video buffers. By default the kernel complains loudly about allocation failures, but we don't want to frighten the user, so ask kmalloc() to keep quiet on such failures. Signed-off-by: Jean Delvare --- linux/drivers/media/video/zoran/zoran_driver.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) Some other v4l drivers do that already. Of course the allocation still fails. But I doubt that restoring the highmem code is the right fix. For one thing, it didn't work for me when I tried it. Did it ever work for you? For another, I remember the big fat warning next to that piece of code, which said that if any other piece of kernel code tried to do the same, everything would break into pieces. So, if the allocation failures are really an issue for anyone out there, I'd rather have the zr36067 driver (optionally) pre-allocate buffers so that it stands are greater chance to get them. This should be fairly easy to implement, and safe too. I am curious if the allocation problem really bothers anyone though. I suspect that users that have a Zoran adapter on low-memory machines use the MJPEG interface (lavrec/lavplay) and not the V4L2 interface. > > - zoran: use slider flag with volume etc. controls. > > Volume control? zoran has no audio. > > > - zoran: fix field typo. > > Why not merge this patch into the patch that created the typo? It only > takes seconds if you use the features of modern SCMs. > > > - zoran: set bytesperline to 0 when using MJPEG. > > Why not merge this patch into the one that created the bug? While I tend to agree with you, I fear it is too late in this case, as Mauro as already pulled Hans' changes. --- v4l-dvb-zoran.orig/linux/drivers/media/video/zoran/zoran_driver.c 2009-02-23 09:48:49.000000000 +0100 +++ v4l-dvb-zoran/linux/drivers/media/video/zoran/zoran_driver.c 2009-02-23 12:51:21.000000000 +0100 @@ -212,7 +212,8 @@ v4l_fbuffer_alloc (struct file *file) ZR_DEVNAME(zr), i); //udelay(20); - mem = kmalloc(fh->v4l_buffers.buffer_size, GFP_KERNEL); + mem = kmalloc(fh->v4l_buffers.buffer_size, + GFP_KERNEL | __GFP_NOWARN); if (!mem) { dprintk(1, KERN_ERR