Re: [PATCH 2/10] ps3: AV Settings Driver

[Andrew Morton <akpm@linux-foundation.org>]

On Thu, 8 Feb 2007 21:30:14 +0100 Christoph Hellwig <hch@lst.de> wrote:

> On Thu, Feb 08, 2007 at 02:58:31PM +0100, Geert Uytterhoeven wrote:
> > Add the PS3 AV Settings Driver.
> > 
> > The AV Settings driver is used to control Audio and Video settings.
> > It communicates with the policy manager through the virtual uart.
> 
> please don't use kernel_thread for new code, but rather kthread_create/
> ktread_run.  And check the possible errors it can return :)

yup.

Plus basically every kernel thread needs a try_to_freeze().

> Using the kthread infrastructure also means you'll have to redo your
> synchronisation scheme as it doesn't work very well with this use
> of semaphores.  But that's a good thing as these useages are not
> easily parseable for most kernel hackers despite beeing tought in
> OS101 :)

Does it actually use sempahores in their counting mode?  If not, mutexes
are preferred.

ps3av_do_pkt() allocates 512 bytes on stack which might be a bit excessive
if you're using a 4k stack.  Especially if that function or a callee does a
__GFP_FS memory allocation.