Hi Bobby, Hmm.. I think it depends on the terminology of 'valid'. I just checked code. I think flip->private_data's stored value is valid only to the GETELEM macro's perspective. If multiple threads race with shared filp->private_data, without having critical section, we cannot prevent situation like below. thr 1) enter to set private_data's LSB to 0 thr 2) enter and get private_data's LSB and exit. (it's not 0 yet) thr 1) set private_data's LSB to 0 finally and exit
Now although thr1 set LSB to 0 earlier than thr2, thr2 read non-0 LSB. I doubt that thr2 read *valid* data at this point. Moreover this behaviour wouldn't happen prior to this patch under the BKL. BUT, at anyway. I confirmed that filp->private_data is not pointing shared data. So I'll remove unnecessary mutex. Thanks! Joonwoo On Wed, Feb 02, 2011 at 01:03:39PM -0800, Bobby Longpocket wrote: > Hi Joonwoo, > > I think it doesn't matter whether the filp is shared by multiple threads or > not. The mutex is only protecting the assignment, and the assignment can > differ only in the least-significant bit. No matter how many threads call at > once, the stored value will be valid. There's a race condition as to which > thread wins, but that's true even with the mutex. > > Regards, > BBL > > > --- On Wed, 2/2/11, Joonwoo Park <[email protected]> wrote: > > > From: Joonwoo Park <[email protected]> > > Subject: Re: [Click] click Digest, Vol 92, Issue 4 > > To: "Bobby Longpocket" <[email protected]> > > Cc: [email protected] > > Date: Wednesday, February 2, 2011, 11:20 AM > > Bobby, > > > > On Wed, Feb 2, 2011 at 11:04 AM, Joonwoo Park <[email protected]> > > wrote: > > >> In ToUserDevice: It looks like the existing > > IOCTL isn't doing anything that requires locking, so you > > should be able to leave out the mutex. > > > > > > I don't think so... filp->private_data is not > > atomic on every platform. > > > > > > > I have to double check tonight (I'm not accessing click > > source code at > > work) but I think it's possible I was wrong. > > I was thinking that filp->private_data is shared between > > different > > userspace open. But it's probably not true. > > I'll have look again. > > > > Thanks! > > Joonwoo > > > > > _______________________________________________ click mailing list [email protected] https://amsterdam.lcs.mit.edu/mailman/listinfo/click
