Messages in this thread | | | Date | Thu, 22 Sep 2005 15:17:51 +0200 | From | Eric Dumazet <> | Subject | Re: [PATCH 1/3] netfilter : 3 patches to boost ip_tables performance |
| |
Harald Welte a écrit : > On Wed, Sep 21, 2005 at 11:29:13PM +0200, Eric Dumazet wrote: > >>Patch 1/3 >> >>1) No more one rwlock_t protecting the 'curtain' > > > I have no problem with this change "per se", but with the > implementation. > > As of now, we live without any ugly #ifdef CONFIG_SMP / #endif sections > in the code - and if possible, I would continue this good tradition. > > For example the get_counters() function. Wouldn't all the smp specific > code (for_each_cpu(), ...) be #defined to nothing anyway?
Well... not exactly, but you are right only the first loop (SET_COUNTER) will really do something. The if (cpu == curcpu) will be true but the compiler wont know that, cpu and curcpu are still C variables.
> > And if we really need the #ifdef's, I would appreciate if those > sectionas are as small as possible. in get_counters() the section can > definitely be smaller, rather than basically having the whole function > body separate for smp and non-smp cases.
get_counters() is not critical, so I agree with you we can stick the general version (not the UP optimized one)
> > Also, how much would we loose in runtime performance if we were using a > "rwlock_t *" even in the UP case?. I mean, it's just one more pointer > dereference of something that is expected to be in cache anyway, isn't > it? This gets rid of another huge set of #ifdefs that make the code > unreadable and prone to errors being introduced later on. >
Well, in UP case, the rwlock_t is a nulldef.
I was inspired by another use of percpu data in include/linux/genhd.h #ifdef CONFIG_SMP struct disk_stats *dkstats; #else struct disk_stats dkstats; #endif
But if you dislike this, we can use pointer for all cases.
Eric - 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/
| |