Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 18 Apr 2011 16:22:37 -0400 (EDT)
From:      Rick Macklem <rmacklem@uoguelph.ca>
To:        John Baldwin <jhb@freebsd.org>
Cc:        freebsd-hackers@freebsd.org
Subject:   Re: SMP question w.r.t. reading kernel variables
Message-ID:  <857645844.237335.1303158157618.JavaMail.root@erie.cs.uoguelph.ca>
In-Reply-To: <201104180831.33796.jhb@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
> On Sunday, April 17, 2011 3:49:48 pm Rick Macklem wrote:
> > Hi,
> >
> > I should know the answer to this, but... When reading a global
> > kernel
> > variable, where its modifications are protected by a mutex, is it
> > necessary to get the mutex lock to just read its value?
> >
> > For example:
> > A if ((mp->mnt_kern_flag & MNTK_UNMOUNTF) != 0)
> >           return (EPERM);
> > versus
> > B MNT_ILOCK(mp);
> >      if ((mp->mnt_kern_flag & MNTK_UNMOUNTF) != 0) {
> >           MNT_IUNLOCK(mp);
> >           return (EPERM);
> >      }
> >      MNT_IUNLOCK(mp);
> >
> > My hunch is that B is necessary if you need an up-to-date value
> > for the variable (mp->mnt_kern_flag in this case).
> >
> > Is that correct?
> 
> You already have good followups from Attilio and Kostik, but one thing
> to keep
> in mind is that if a simple read is part of a larger "atomic
> operation" then
> it may still need a lock. In this case Kostik points out that another
> lock
> prevents updates to mnt_kern_flag so that this is safe. However, if
> not for
> that you would need to consider the case that another thread sets the
> flag on
> the next instruction. Even the B case above might still have that
> problem
> since you drop the lock right after checking it and the rest of the
> function
> is implicitly assuming the flag is never set perhaps (or it needs to
> handle
> the case that the flag might become set in the future while
> MNT_ILOCK() is
> dropped).
> 
> One way you can make that code handle that race is by holding
> MNT_ILOCK()
> around the entire function, but that approach is often only suitable
> for a
> simple routine.
> 
All of this makes sense. What I was concerned about was memory cache
consistency and whet (if anything) has to be done to make sure a thread
doesn't see a stale cached value for the memory location.

Here's a generic example of what I was thinking of:
(assume x is a global int and y is a local int on the thread's stack)
- time proceeds down the screen
thread X on CPU 0                    thread Y on CPU 1
x = 0;
                                     x = 0; /* 0 for x's location in CPU 1's memory cache */
x = 1;
                                     y = x;
--> now, is "y" guaranteed to be 1 or can it get the stale cached 0 value?
    if not, what needs to be done to guarantee it?

For the original example, I am fine so long as the bit is seen as set after dounmount()
has set it.

Also, I see cases of:
     mtx_lock(&np);
     np->n_attrstamp = 0;
     mtx_unlock(&np);
in the regular NFS client. Why is the assignment mutex locked? (I had assumed
it was related to the above memory caching issue, but now I'm not so sure.)

Thanks a lot for all the good responses, rick
ps: I guess it comes down to whether or not "atomic" includes ensuring memory
    cache consistency. I'll admit I assumed "atomic" meant that the memory
    access or modify couldn't be interleaved with one done to the same location
    by another CPU, but not memory cache consistency.



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?857645844.237335.1303158157618.JavaMail.root>