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

[Attilio Rao]

On Wed, Oct 10, 2012 at 5:49 PM, Attilio Rao <attilio at freebsd.org> wrote:
> On Tue, Sep 18, 2012 at 1:13 AM, Attilio Rao <attilio at freebsd.org> wrote:
>> On Thu, Aug 2, 2012 at 9:56 PM, Attilio Rao <attilio at freebsd.org> wrote:
>>> On 7/30/12, John Baldwin <jhb at freebsd.org> wrote:
>>
>> [ trimm ]
>>
>>>> --- //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?
>>
>> So what do you think about this patch? (Please double-check the GIT log).
>
> The real reason why I wanted an abstract __compiler_membar() was to
> cleanly fix a bug that I think affects sched_pin()/sched_unpin() right
> now.

Now that sched_pin()/sched_unpin() are fixed I would like to introduce
this new patch, making critical_enter()/critical_exit() inline:
http://www.freebsd.org/~attilio/inline_critical.patch

The concept is pretty simple: simple add/dec for critical_enter, exit
are inlined, the rest is in an "hard path". Debugging enables the hard
paths by default (really I think that only KTR may be due here, but I
thought that in case of INVARIANTS this was also wanted, so I added
the check also for that case).

flo@ has stress-tested the patch already and he may be starting
serious benchmarks soon.
The effectiveness of this patch is to determine and it brings again
the question: better an hard function or the usage of compiler membars
to avoid issues? I think that this patch should be in only if we can
absolutely prove a measurable performance bump, otherwise I will just
add a note to critical_enter()/critical_exit() explaining why they
should not be inlined at all.

Opinions?

Thanks,
Attilio


-- 
Peace can only be achieved by understanding - A. Einstein