Skip to contentSkip to conversation

Fix, refactor, and optimize xnlock services, v2

PATCH In review Jan Kiszka 2 participants 5 messages Mar 2, 2008 – Mar 28, 2008

lore
Share & tools
lore

Jan Kiszka

author
#0 lore raw

cover Fix, refactor, and optimize xnlock services, v2

Hi,

here comes version 2 of my xnlock rework. Changes since the first release:

- Moved xnlock_spin out-of-line instead uninlining the irqsave/restore services
- Beautified xnlock_dbg interface (now only static-inlines)
- Split-up basic xnlock_dbg refactoring from whitespace fixes (interdiff is your friend)
- Added support for xnlock debugging on UP
- Dropped barrier patch (already merged)

The text size reduction of this series (for CONFIG_SMP, without debugging (CONFIG_DEBUG_INFO was on, though), and without enabling ticket locks):

Before:

   text    data     bss     dec     hex filename
  75371    2184     308   77863   13027 kernel/xenomai/skins/native/xeno_native.o
  27407    2256    1112   30775    7837 kernel/xenomai/skins/rtdm/xeno_rtdm.o
  96813    1472  160224  258509   3f1cd kernel/xenomai/skins/posix/xeno_posix.o
 107476    5608  457140  570224   8b370 kernel/xenomai/nucleus/xeno_nucleus.o

After:

   text    data     bss     dec     hex filename
  71751    2184     308   74243   12203 kernel/xenomai/skins/native/xeno_native.o
  26570    2192    1112   29874    74b2 kernel/xenomai/skins/rtdm/xeno_rtdm.o
  91107    1392  160224  252723   3db33 kernel/xenomai/skins/posix/xeno_posix.o
 102860    5496  454580  562936   896f8 kernel/xenomai/nucleus/xeno_nucleus.o

To estimate the latency reduction of this series (specifically due to patch 3 and 4), I ran latency -p100 over several hours on a widely isolated 1.6 GHz core (only Linux timer and rescheduling IRQs) under cache calibrator load. The results:

RTH|-----lat min|-----lat avg|-----lat max|-overrun|----lat best|---lat worst Before:
RTS| 0.457| 1.450| 28.156| 0| 04:00:52/04:00:52
After:
RTS| 0.450| 1.453| 23.283| 0| 06:37:19/06:37:19

I'm sure this gain is far to much for this changes, ie. we are missing some constellation in the second run that caused the 28 us peak in the first run. However, the tendency matches other experiments I did with more IRQ load on the target core. Any attempt to reproduce my results is welcome!

Note that the above numbers do not yet include the gain from my ipipe-head optimizations I posted a few days ago!

Jan

#1 lore raw
Quoted 7 lines · Jan Kiszka

Hi,

here comes version 2 of my xnlock rework. Changes since the first release:

- Moved xnlock_spin out-of-line instead uninlining the irqsave/restore services

The size changes are not as dramatic as with the first version. Did you have any chance to test the latency with your first version?

Quoted 1 line

- Beautified xnlock_dbg interface (now only static-inlines)

Well, I am still not convinced, I still find the debugging version harder to read. Is there a problem with implementing the two versions separately with a unique #ifdef?

Signature · 1 line
					    Gilles Chanteperdrix.
author
#2 lore raw
Quoted 11 lines · Gilles Chanteperdrix
Quoted 7 lines · Jan Kiszka

Hi,

here comes version 2 of my xnlock rework. Changes since the first release:

- Moved xnlock_spin out-of-line instead uninlining the irqsave/restore services

The size changes are not as dramatic as with the first version. Did you have any chance to test the latency with your first version?

Not under the same conditions (isolated core). In essence, the gain was less significant (~1 us). I came to the conclusion that this might be due to some loss related to moving xnlock_put_irqrestore out-of-line (I think now that function was too small for this).

Instead of further digging into this I decided to address the root of the issue, ie. bringing down the text size of the pipeline-head services. So you now have to apply the ipipe patch to achieve both a text size reduction (almost as good as the original one) AND a code path length reduction (more important!). Actually, I haven't even done a full benchmark with this patch applied as well yet, but it is not that interesting as the runtime advantage is obvious (simply less ops).

Quoted 6 lines
Quoted 2 lines

- Beautified xnlock_dbg interface (now only static-inlines)

Well, I am still not convinced, I still find the debugging version harder to read. Is there a problem with implementing the two versions separately with a unique #ifdef?

Well, everything is possible. But we would start to duplicate LOC again, namely both the lock implementations and the debugging services. Just look at the final system.h and pod.h after applying all patches.

I could try this, but only if it is _really_ desired. I'm convinced it is the wrong way because I consider the readability and consistency of the lock implementation more important than its debug code.

Jan

#3 lore raw
Quoted 6 lines · Jan Kiszka
Quoted 4 lines · Gilles Chanteperdrix

Well, I am still not convinced, I still find the debugging version harder to read. Is there a problem with implementing the two versions separately with a unique #ifdef?

Quoted 7 lines

Well, everything is possible. But we would start to duplicate LOC again, namely both the lock implementations and the debugging services. Just look at the final system.h and pod.h after applying all patches.

I could try this, but only if it is _really_ desired. I'm convinced it is the wrong way because I consider the readability and consistency of the lock implementation more important than its debug code.

Well, it may happen that we have to debug the debug code. But Ok, I agree with your changes.

Signature · 1 line
                                               Gilles Chanteperdrix
author
#4 lore raw

Philippe,

I noticed that you merged patch 1-4. What are the plans for (or concerns about) the ticket xnlock and the UP debugging changes?

Jan