Messages in this thread | | | Date | Thu, 06 Jul 2006 18:55:06 +0200 | From | Martin Peschke <> | Subject | Re: [Patch] statistics infrastructure - update 9 |
| |
Andi Kleen wrote: > Martin Peschke <mp3@de.ibm.com> writes: >> { >> -#ifdef CONFIG_STATISTICS >> unsigned long flags; >> local_irq_save(flags); >> _statistic_add_as(type, stat, i, value, incr); >> local_irq_restore(flags); > > > Is there a particular reason you can't use local_t with cpu_local_*? > It would be faster on many architectures than local_irq_save/restore > > -Andi
Good question. Btw. - faster by what order of magnitude? local_irq_save/restore seems to be fine for kernel/profile.c
Reason 1: cpu_local_* uses __get_cpu_var, which conflicts with struct statistic being embedded into struct xyz that is allocated whenever the client needs it.
I could try to use local_t in conjunction with local_add etc. (as seen in include/linux/dmaengine.h in 2.6.17-mm6). Does this also yield a performance gain worth consideration?
Reason 2: Then, the other use of local_irq_save/restore is to avoid races regarding statistics being switched on or off, and it's underlying buffers being released:
void _statistic_add(struct statistic *stat, int i, s64 value, u64 incr) { if (stat[i].state == STATISTIC_STATE_ON) stat[i].add(&stat[i], value, incr); }
void statistic_add(struct statistic *stat, int i, s64 value, u64 incr) { unsigned long flags; local_irq_save(flags); _statistic_add(stat, i, value, incr); local_irq_restore(flags); }
static int statistic_stop(struct statistic *stat) { stat->stopped = timestamp_clock(); stat->state = STATISTIC_STATE_OFF; /* ensures that all CPUs have ceased updating statistics */ smp_mb(); on_each_cpu(_statistic_barrier, NULL, 0, 1); return 0; }
So, removing local_irq_save/restore would require statistics to be switched on and their buffers being available all the time. That is, buffers holding counters etc. can't be allocated at run time - what if allocation fails? (Should I leave this issue to clients?).
All the buffers for per-cpu counters etc. would need to be embedded into the client's struct xyz. There would be no way that users could change, for example, the number of buckets of a histogram.
Everything would be fixed. Which might be fine for some purposes; particularly for a plain counter which will only be used as a counter, and which will never be inflated to a histogram or whatever.
In short, I could try to add a per-cpu array of local_t's to struct statistic and write up another statistic_add()-variant, which would be limited to aggregating data into a counter using local_add(), without doing local_irq_save/restore, and without checking whether data gathering has been turned on. Which would resemble Christoph Lameter's light-weight VM counters to some degree. With the downside of struct statistic being inflated for everyone else.
Reason 3: local_add() & friends won't suffice for some algorithms:
void statistic_add_util(struct statistic *stat, s64 value, u64 incr) { /*...snip...*/ if (unlikely(value < util->min)) util->min = value; if (unlikely(value > util->max)) util->max = value; }
static void _statistic_add_sparse(struct statistic_sparse_list *slist, s64 value, u64 incr) { struct list_head *head = &slist->entry_lh; struct statistic_entry_sparse *entry;
list_for_each_entry(entry, head, list) { if (likely(entry->value == value)) { entry->hits += incr; statistic_add_sparse_sort(head, entry); return; } } if (unlikely(statistic_add_sparse_new(slist, value, incr))) slist->hits_missed += incr; }
Reason 4: The alleged overhead of local_irq_save/restore (as compared to atomic operations) might be less significant for clients updating a bunch of statistics in one go:
unsigned long flags;
local_irq_save(flags); _statistic_inc(dev->stat, MYSTAT_SIZE, size); _statistic_inc(dev->stat, MYSTAT_LATENCY, latency); _statistic_inc(dev->stat, MYSTAT_RESULT, result); local_irq_restore(flags);
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |