[Click] ToUserDevice IOCTL

[Joonwoo Park]

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 <joonwpark81 at gmail.com> wrote:
> 
> > From: Joonwoo Park <joonwpark81 at gmail.com>
> > Subject: Re: [Click] click Digest, Vol 92, Issue 4
> > To: "Bobby Longpocket" <bobbylongpocket at yahoo.com>
> > Cc: click at pdos.csail.mit.edu
> > Date: Wednesday, February 2, 2011, 11:20 AM
> > Bobby,
> > 
> > On Wed, Feb 2, 2011 at 11:04 AM, Joonwoo Park <joonwpark81 at gmail.com>
> > 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
> > 
> 
> 
>