svn commit: r241889 - in user/andre/tcp_workqueue/sys: arm/arm cddl/compat/opensolaris/kern cddl/contrib/opensolaris/uts/common/dtrace cddl/contrib/opensolaris/uts/common/fs/zfs ddb dev/acpica dev/...

[Attilio Rao]

On Thu, Oct 25, 2012 at 10:03 PM, Andre Oppermann <andre at freebsd.org> wrote:
> On 25.10.2012 18:21, Attilio Rao wrote:
>>
>> On Thu, Oct 25, 2012 at 5:15 PM, Andre Oppermann <andre at freebsd.org>
>> wrote:
>>>
>>> On 25.10.2012 05:14, Attilio Rao wrote:
>>>>
>>>>
>>>> On Wed, Oct 24, 2012 at 9:13 PM, Attilio Rao <attilio at freebsd.org>
>>>> wrote:
>>>> I think I've had a better idea for this.
>>>> In our locking scheme we already rely on the fact that lock_object
>>>> will always be present in all our locking primitives and that it will
>>>> be the first object of every locking primitives. This is an assumption
>>>> we must live with in order to correctly implement lock classes. I
>>>> think we can use the same concept in order to use the same KPI for the
>>>> 2 different structures (struct mtx and struct mtx_unshare) and keep
>>>> the compile time ability to find stupid bugs.
>>>
>>>
>>>
>>> I think we're completely overdoing it.  On amd64 the size difference
>>> of 64B cache line aligning and padding all mutex, sx and rw_lock
>>> structures adds the tiny amount of 16K on a GENERIC kernel of 19MB.
>>> That is a 0.009% increase in size.  Of course dynamically allocated
>>> memory that includes a mutex grows a tiny bit at well.
>>>
>>> Hence I propose to unconditionally slap __aligned(CACHE_LINE_SIZE) into
>>> all locking structures and be done with it.  As an added benefit we
>>> don't have to worry about individual micro-optimizations on a case by
>>> case basis.
>>
>>
>> Did you see my (and also Jeff) objection to your proposal about this?
>> You are deliberating ignoring this?
>
>
> Well, I'm allowed to have a different opinion, am I?  I'm not ignoring
> your objection in the sense as I'm not trying to commit any of this to
> HEAD while it is disputed.
>
> Mind you this whole conversation was started because I was trying to solve
> a problem with unfortunate cache line sharing for global mutexes in the
> kernel .bss section on my *personal* svn branch.

Andre,
I'm sorry if you felt I was being harsh or confrontative. This was
really not my intention and I apologize.

Said that, I fail in seeing a proper technical discussion on your side
on how what I propose is "overdoing it" or how do you plan to address
the concerns people are raising with your proposal of bumping all lock
sizes indiscriminately.

However, here is the first half of the patch I'd like to see in:
http:///www.freebsd.org/~attilio/mtx_decoupled.patch

This is just the part to give the ability to crunch different
structures to the mtx KPI. Please note that from the users perspective
the mtx KPI remains absolutely the same, so there is theoretically no
KPI discontinuity, the support is absolutely transparent.
It is not yet throughfully tested, because I first want to hear
opinions also on nits.
For example:
- Do we need to implement mtxlock2mtx() only in kern_mutex.c? I'd say
yes, as a general rule, but as long as this functionality relies on
the ABI maybe it is better to leave it in _mutex.h as done in the
patch.
- MTX_SYSINIT() doesn't work for hypotetical mtx_unshare right now so
it will need to have its own version. Should I add it now? Should I
add it if and only if there is a real need? (I suspect this is likely
because several external mutexes, that we want to convert, may use it)
- others that came to your mind

Thanks,
Attilio


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