lkml.org 
[lkml]   [2008]   [Jun]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] Fix open/close race in saa7134
On Mon, Jun 23, 2008 at 12:53:48PM -0700, Arjan van de Ven wrote:
> On Mon, 23 Jun 2008 21:22:03 +0200
> Marcin Slusarz <marcin.slusarz@gmail.com> wrote:
>
> >
> > If dev->empress_users could be > 1 then ok - it could break, but it
> > can only be 1 or 0. If it's 1 you won't open the device. If it's 0
> > you won't reach ts_close.
> >
> > If you still see the race, please show me the sequence, because I
> > don't (of course when decrementing is the last operation of ts_close).
>
> a decrement in C, without locking, is NOT atomic.

I know about it. But in this code, 2 threads cannot modify empress_users!
This variable should be named empress_used_now or something like that.

Look:

static int ts_open(struct inode *inode, struct file *file)
{
(...)
err = -EBUSY;
if (!mutex_trylock(&dev->empress_tsq.vb_lock))
goto done;
if (dev->empress_users) <-------------
goto done_up;

/* Unmute audio */
saa_writeb(SAA7134_AUDIO_MUTE_CTRL,
saa_readb(SAA7134_AUDIO_MUTE_CTRL) & ~(1 << 6));

dev->empress_users++; <------ this should be "dev->empress_users = 1;"
file->private_data = dev;
err = 0;

done_up:
mutex_unlock(&dev->empress_tsq.vb_lock);
done:
return err;
}

static int ts_release(struct inode *inode, struct file *file)
{
(...)
dev->empress_users--; <------------ this should be "dev->empress_users = 0;"
return 0;
}


\
 
 \ /
  Last update: 2008-06-23 22:15    [W:0.251 / U:0.452 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site