[Click] ToUserDevice IOCTL

[Bobby Longpocket]

Joonwoo,

I think you're talking about a general principle of memory coherency here.

You can assume that there is some unit for which a memory operation is atomic with respect to other threads.  Multithreading would be a nightmare if that were not the case.

BBL

--- On Wed, 2/2/11, Joonwoo Park <joonwpark81 at gmail.com> wrote:

> From: Joonwoo Park <joonwpark81 at gmail.com>
> Subject: Re: ToUserDevice IOCTL
> To: "Bobby Longpocket" <bobbylongpocket at yahoo.com>
> Cc: click at pdos.csail.mit.edu
> Date: Wednesday, February 2, 2011, 2:20 PM
> Actually, I think even reading MSBs
> is not fine either if
> filp->private_data is shared.
> The thing is assigning private_data is not atomic operation
> so don't
> know what can temporarily exist while assignment  is
> going even though
> MSBs are not changing.  It's totally up to how opcode
> works to
> specific platform.
> So in that chase I think the other thread can read invalid
> MSBs.
> 
> Regards,
> Joonwoo
> 
> On Wed, Feb 2, 2011 at 1:53 PM, Joonwoo Park <joonwpark81 at gmail.com>
> wrote:
> > 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
> >> >
> >>
> >>
> >>
> >
>