Re: [Patch v3 1/1] media: am437x-vpfe: Fix a race condition during release

2015-07-13 Thread Hans Verkuil 
On 06/29/2015 11:18 PM, Benoit Parrot wrote:
 There was a race condition where during cleanup/release operation
 on-going streaming would cause a kernel panic because the hardware
 module was disabled prematurely with IRQ still pending.
 
 Fixes: 417d2e507edc ([media] media: platform: add VPFE capture driver 
 support for AM437X)
 Cc: sta...@vger.kernel.org # v4.0+
 Signed-off-by: Benoit Parrot bpar...@ti.com
 ---
 Changes since v2:
 - fix the stable commit reference syntax
 
  drivers/media/platform/am437x/am437x-vpfe.c | 11 +--
  1 file changed, 9 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/media/platform/am437x/am437x-vpfe.c 
 b/drivers/media/platform/am437x/am437x-vpfe.c
 index a30cc2f7e4f1..eb25c43da126 100644
 --- a/drivers/media/platform/am437x/am437x-vpfe.c
 +++ b/drivers/media/platform/am437x/am437x-vpfe.c
 @@ -1185,14 +1185,21 @@ static int vpfe_initialize_device(struct vpfe_device 
 *vpfe)
  static int vpfe_release(struct file *file)
  {
   struct vpfe_device *vpfe = video_drvdata(file);
 + bool fh_singular = v4l2_fh_is_singular_file(file);

Close, but no cigar.

The assignment to fh_singular should be moved inside the mutex. Right now
there still is a race condition between setting fh_singular and taking
the lock.

Regards,

Hans

   int ret;
  
   mutex_lock(vpfe-lock);
  
 - if (v4l2_fh_is_singular_file(file))
 - vpfe_ccdc_close(vpfe-ccdc, vpfe-pdev);
 + /* the release helper will cleanup any on-going streaming */
   ret = _vb2_fop_release(file, NULL);
  
 + /*
 +  * If this was the last open file.
 +  * Then de-initialize hw module.
 +  */
 + if (fh_singular)
 + vpfe_ccdc_close(vpfe-ccdc, vpfe-pdev);
 +
   mutex_unlock(vpfe-lock);
  
   return ret;
 

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Patch v3 1/1] media: am437x-vpfe: Fix a race condition during release

2015-07-07 Thread Lad, Prabhakar 
On Mon, Jun 29, 2015 at 10:18 PM, Benoit Parrot bpar...@ti.com wrote:
 There was a race condition where during cleanup/release operation
 on-going streaming would cause a kernel panic because the hardware
 module was disabled prematurely with IRQ still pending.

 Fixes: 417d2e507edc ([media] media: platform: add VPFE capture driver 
 support for AM437X)
 Cc: sta...@vger.kernel.org # v4.0+
 Signed-off-by: Benoit Parrot bpar...@ti.com

Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com

Cheers,
--Prabhakar Lad

 ---
 Changes since v2:
 - fix the stable commit reference syntax

  drivers/media/platform/am437x/am437x-vpfe.c | 11 +--
  1 file changed, 9 insertions(+), 2 deletions(-)

 diff --git a/drivers/media/platform/am437x/am437x-vpfe.c 
 b/drivers/media/platform/am437x/am437x-vpfe.c
 index a30cc2f7e4f1..eb25c43da126 100644
 --- a/drivers/media/platform/am437x/am437x-vpfe.c
 +++ b/drivers/media/platform/am437x/am437x-vpfe.c
 @@ -1185,14 +1185,21 @@ static int vpfe_initialize_device(struct vpfe_device 
 *vpfe)
  static int vpfe_release(struct file *file)
  {
 struct vpfe_device *vpfe = video_drvdata(file);
 +   bool fh_singular = v4l2_fh_is_singular_file(file);
 int ret;

 mutex_lock(vpfe-lock);

 -   if (v4l2_fh_is_singular_file(file))
 -   vpfe_ccdc_close(vpfe-ccdc, vpfe-pdev);
 +   /* the release helper will cleanup any on-going streaming */
 ret = _vb2_fop_release(file, NULL);

 +   /*
 +* If this was the last open file.
 +* Then de-initialize hw module.
 +*/
 +   if (fh_singular)
 +   vpfe_ccdc_close(vpfe-ccdc, vpfe-pdev);
 +
 mutex_unlock(vpfe-lock);

 return ret;
 --
 1.8.5.1

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html