Messages in this thread | | | Subject | Re: [patch 5/6] statistics infrastructure | From | Andi Kleen <> | Date | 14 Dec 2005 19:38:19 +0100 |
| |
Martin Peschke <mp3@de.ibm.com> writes: > + */ > +static inline u64 statistic_add(struct statistic *stat, s64 value, u64 incr) > +{ > + unsigned long flags; > + int retval; > + > + if (stat->on != STATISTIC_DEF_ON) > + return 0; > + > + statistic_lock(stat->interface, flags); > + retval = stat->add(stat, value, incr); > + statistic_unlock(stat->interface, flags);
Locks and indirect function calls?
It seems very wrong to me to make such heavy weight statistic functions. Most likely you will disturb the performance whatever is being counted badly.
Take a look at many other subsystems - they do per CPU counters etc. to make this all fast.
But it's still unclear why it would need such an heavyweight infrastructure. Normally it's not that bad to reimplemented on the fly. Maybe some common code can be refactored out of that, but probably not too much.
[... lots of other code snipped ... ]
Looks all very very overdesigned to me. How about you just start with a minimum specification and describe what you want to do? And then start with a *simple* base implementation. If it's really needed it will grow later anyways, but starting off with such a complex monstrosity is just wrong.
-Andi - 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/
| |