Messages in this thread | | | Date | Mon, 23 Jun 2008 22:11:52 +0200 | From | Marcin Slusarz <> | Subject | Re: [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; }
| |