linux-kernel - Re: [RFC PATCH v2 4/8] media: videobuf2: introduce VIDEOBUF2

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 23 Dec 2011 17:51:11 +0800
From:	Ming Lei <ming.lei@...onical.com>
To:	Marek Szyprowski <m.szyprowski@...sung.com>
Cc:	Mauro Carvalho Chehab <mchehab@...radead.org>,
	Tony Lindgren <tony@...mide.com>,
	Sylwester Nawrocki <snjw23@...il.com>,
	Alan Cox <alan@...rguk.ukuu.org.uk>,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	linux-media@...r.kernel.org, Pawel Osciak <p.osciak@...il.com>
Subject: Re: [RFC PATCH v2 4/8] media: videobuf2: introduce VIDEOBUF2_PAGE memops

Hi,

On Fri, Dec 23, 2011 at 5:34 PM, Marek Szyprowski
<m.szyprowski@...sung.com> wrote:

>> For example, on ARM, there is very limited kernel virtual address space reserved
>> for DMA coherent buffer mapping, the default size is about 2M if I
>> don't remember mistakenly.
>
> It can be easily increased for particular boards, there is no problem with this.

It is not easily to increase it because there is very limited space reserved for
this purpose, see Documentation/arm/memory.txt. Also looks like it is
not configurable.

>
>> > I understand that there might be some speed issues with coherent (uncached)
>> > userspace mappings, but I would solve it in completely different way. The interface
>>
>> Also there is poor performance inside kernel space, see [1]
>
> Your driver doesn't access video data inside kernel space, so this is also not an issue.

Why not introduce it so that other drivers(include face detection) can
benefit with it? :-)

>> >
>> > Your current implementation also abuses the design and api of videobuf2 memory
>> > allocators. If the allocator needs to return a custom structure to the driver
>>
>> I think returning vaddr is enough.
>>
>> > you should use cookie method. vaddr is intended to provide only a pointer to
>> > kernel virtual mapping, but you pass a struct page * there.
>>
>> No, __get_free_pages returns virtual address instead of 'struct page *'.
>
> Then you MUST use cookie for it. vaddr method should return kernel virtual address
> to the buffer video data. Some parts of videobuf2 relies on this - it is used by file
> io emulator (read(), write() calls) and mmap equivalent for non-mmu systems.
>
> Manual casting in the driver is also a bad idea, that's why there are helper functions

I don't see any casts are needed. The dma address can be got from vaddr with
dma_map_* easily in drivers, see the usage on patch 8/8(media: video: introduce
omap4 face detection module driver).

> defined for both dma_contig and dma_sg allocators: vb2_dma_contig_plane_dma_addr() and
> vb2_dma_sg_plane_desc().

These two helpers are not needed and won't be provided by VIDEOBUF2_PAGE memops.

thanks,
--
Ming Lei
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/