svn commit: r238907 - projects/calloutng/sys/kern

[John Baldwin]

On Thursday, September 13, 2012 10:38:54 am Attilio Rao wrote:
> On Thu, Sep 13, 2012 at 2:10 PM, John Baldwin <jhb at freebsd.org> wrote:
> > On Wednesday, September 12, 2012 9:36:58 pm Attilio Rao wrote:
> >> On Thu, Aug 2, 2012 at 10:07 PM, John Baldwin <jhb at freebsd.org> wrote:
> >> > On Thursday, August 02, 2012 4:56:03 pm Attilio Rao wrote:
> >> >> On 7/30/12, John Baldwin <jhb at freebsd.org> wrote:
> >> >> > --- //depot/projects/smpng/sys/kern/kern_rmlock.c   2012-03-25
> >> >> > 18:45:29.000000000 0000
> >> >> > +++ //depot/user/jhb/lock/kern/kern_rmlock.c        2012-06-18 21:20:58.000000000
> >> >> > 0000
> >> >> > @@ -70,6 +70,9 @@
> >> >> >  }
> >> >> >
> >> >> >  static void        assert_rm(const struct lock_object *lock, int what);
> >> >> > +#ifdef DDB
> >> >> > +static void        db_show_rm(const struct lock_object *lock);
> >> >> > +#endif
> >> >> >  static void        lock_rm(struct lock_object *lock, int how);
> >> >> >  #ifdef KDTRACE_HOOKS
> >> >> >  static int owner_rm(const struct lock_object *lock, struct thread
> >> >> > **owner);
> >> >>
> >> >> While here, did you consider also:
> >> >> - Abstracting compiler_memory_barrier() into a MI, compiler dependent function?
> >> >> - Fix rm_queue with DCPU possibly
> >> >
> >> > Mostly I just wanted to fill in missing functionality and fixup the
> >> > RM_SLEEPABLE bits a bit.
> >>
> >> So what do you think about the following patch? If you agree I will
> >> send to pho@ for testing in a batch with other patches.
> >
> > It's not super clear to me that having it be static vs dynamic is all that
> > big of a deal.  However, your approach in general is better, and it certainly
> > should have been using PCPU_GET() for the curcpu case all along rather than
> > inlining pcpu_find().
> 
> You mean what is the performance difference between static vs dynamic?
> Or you mean, why we want such patch at all?
> In the former question there is a further indirection (pc_dynamic
> access), for the latter question the patched code avoids namespace
> pollution at all and makes the code more readable.

More why we want it.  I think most of your readability fixes would work just
as well if it remained static and we used PCPU_GET().  However, I think your
changes are fine.

FYI, much of subr_rmlock.c goes out of its way to optimize for performance
(such as inlining critical_enter(), critical_exit(), and pcpu_find()), so
adding the new indirection goes against the grain of that.

-- 
John Baldwin