public inbox for linux-media@vger.kernel.org
 help / color / mirror / Atom feed
From: Florian Mickler <florian@mickler.org>
To: Mauro Carvalho Chehab <mchehab@infradead.org>
Cc: oliver@neukum.org, jwjstone@fastmail.fm,
	linux-kernel@vger.kernel.org,
	Mauro Carvalho Chehab <mchehab@redhat.com>,
	linux-media@vger.kernel.org,
	Patrick Boettcher <pboettcher@dibcom.fr>
Subject: Re: [PATCH 01/16] [media] dib0700: get rid of on-stack dma buffers
Date: Tue, 15 Mar 2011 22:14:34 +0100	[thread overview]
Message-ID: <20110315221434.7b0dc2a8@schatten.dmk.lab> (raw)
In-Reply-To: <4D7F5538.6080907@infradead.org>

On Tue, 15 Mar 2011 09:02:00 -0300
Mauro Carvalho Chehab <mchehab@infradead.org> wrote:
> 
> You're allocating a buffer for URB control messages. IMO, this is a good idea, as
> this way, allocating/freeing on each urb call is avoided. However, on most places,
> you're not using it. The better would be to just use this buffer on all
> the above places.
> 
> You should just take care of protecting such buffer with a mutex, to avoid concurrency.
> Such kind of protection is generally ok, as dvb applications generally serialize
> the calls anyway.
> 

I didn't do so already, because I had/have no overview over the big
picture operation of the dvb framework and thus feared to introduce
deadlocks or massive serializations where concurrency is needed. But if
you suggest it, I guess it should be benign. I'm wondering about the
purpose of the usb_mutex and the i2c_mutex ... 

Should I introduce new driver specific mutexes to protect the buffer or
is it possible to reuse one of the 2? 

vp702x_usb_inout_op takes the usb_mutex, 
but vp702x_usb_out_op and vp702x_usb_in_op get called without that
mutex hold. That makes me wonder what that mutex purpose is in that
driver?

Other drivers like the az6027 introduce a driver specific mutex and
also use the usb_mutex. That make conceptually (to my inexperienced
mind) more sense. (each layer does it's own locking) but the idea of
operation is not yet clear in my mind...

Can you perhaps shed some light on what the purpose of those locks is
and if it is sufficient to use the usb_mutex to serialize all
usb_control_msg calls (which would probably protect the buffer
sufficiently but may be too much if the dvb-usb framework uses that
mutex for different purposes). 

In the meantime I will respin this series using preallocated buffers and
will hopefully work out stuff that is unclear to me yet ...

Regards,
Flo



  reply	other threads:[~2011-03-15 21:14 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-06 11:16 [PATCH] [media] dib0700: get rid of on-stack dma buffers Florian Mickler
2011-03-06 12:06 ` Oliver Neukum
2011-03-06 14:38   ` Florian Mickler
2011-03-06 14:45     ` [PATCH 1/3 v2] " Florian Mickler
2011-03-06 14:45       ` [PATCH 2/3] [media] dib0700: remove unused variable Florian Mickler
2011-03-06 14:45       ` [PATCH 3/3] [media] dib0700: don't ignore errors in driver probe Florian Mickler
2011-03-06 15:06     ` [PATCH] [media] dib0700: get rid of on-stack dma buffers Oliver Neukum
2011-03-06 15:45       ` Florian Mickler
2011-03-06 16:44         ` Oliver Neukum
2011-03-06 17:47           ` [PATCH 1/2 v3] " Florian Mickler
2011-03-06 17:47             ` [PATCH 2/2] [media] dib0700: remove unused variable Florian Mickler
2011-03-06 17:57             ` [PATCH 1/2 v3] [media] dib0700: get rid of on-stack dma buffers Florian Mickler
2011-03-15  8:36               ` Florian Mickler
2011-03-15  8:43                 ` [PATCH 01/16] " Florian Mickler
2011-03-15  8:43                   ` [PATCH 02/16] [media] dib0700: remove unused variable Florian Mickler
2011-03-15  8:43                   ` [PATCH 03/16] [media] a800: get rid of on-stack dma buffers Florian Mickler
2011-03-15  8:43                   ` [PATCH 04/16] [media] vp7045: " Florian Mickler
2011-03-15  8:43                   ` [PATCH 05/16] [media] ec168: get rid of on stack " Florian Mickler
2011-03-18 16:36                     ` Antti Palosaari
2011-03-18 21:33                       ` Florian Mickler
2011-03-15  8:43                   ` [PATCH 06/16] [media] ce6230: get rid of on-stack dma buffer Florian Mickler
2011-03-18 16:36                     ` Antti Palosaari
2011-03-18 21:28                       ` Florian Mickler
2011-03-15  8:43                   ` [PATCH 07/16] [media] friio: get rid of on-stack dma buffers Florian Mickler
2011-03-15  8:43                   ` [PATCH 11/16] [media] lmedm04: correct indentation Florian Mickler
2011-03-15  8:43                   ` [PATCH 12/16] [media] lmedm04: get rid of on-stack dma buffers Florian Mickler
2011-03-15 20:54                     ` Malcolm Priestley
2011-03-15 21:46                       ` Florian Mickler
2011-03-15 12:02                   ` [PATCH 01/16] [media] dib0700: " Mauro Carvalho Chehab
2011-03-15 21:14                     ` Florian Mickler [this message]
     [not found]                   ` <1300178655-24832-9-git-send-email-florian@mickler.org>
2011-03-18 16:34                     ` [PATCH 09/16] [media] au6610: get rid of on-stack dma buffer Antti Palosaari
2011-03-18 21:27                       ` Florian Mickler
2011-03-18 21:40                         ` Antti Palosaari
2011-03-15 11:37                 ` [PATCH 1/2 v3] [media] dib0700: get rid of on-stack dma buffers Mauro Carvalho Chehab
2011-03-06 13:49 ` [PATCH] " Jack Stone
2011-03-06 14:00   ` Florian Mickler

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110315221434.7b0dc2a8@schatten.dmk.lab \
    --to=florian@mickler.org \
    --cc=jwjstone@fastmail.fm \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@infradead.org \
    --cc=mchehab@redhat.com \
    --cc=oliver@neukum.org \
    --cc=pboettcher@dibcom.fr \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox