Re: [PATCH v6] Add generic exponentially weighted moving average (EWMA) function

Stefan Richter
2010-11-11T18:19:08Z

Bruno Randolf wrote:
> On Thu November 11 2010 13:02:31 Andrew Morton wrote:
>> So perhaps it would be best to merge this via the wireless tree, so
>> everything exists in one place and it can be fed into linux-next and
>> into mainline in an orderly fashion.
>>
>> If that sounds like a plan then I can send this patch in John's
>> direction. Which means that if he merges it into mainline without also
>> merging any of your patches which _use_ this function then we still end
>> up with unused code in mainline, but at least that way it wasn't my fault
>> ;)
>>
>> Let me know your thoughts?
>
> Sounds like a good plan to me. But let's hear Johns opinion.
>
> I have a patch or two using this function, which I hope will get merged into
> wireless-testing soon after, but obviously it's up to John. There are more
> places in the wireless part which could make use of this function (right now
> each have their own EWMA implementation), and I'll try to get them all
> updated, but finally it's the individual maintainers, who decide if they want
> to make that change or not, I guess.

It seems totally clear-cut to me that code like this is submitted together
with at least one call site, and it is submitted through the tree in which
that call site is maintained.

>> (And it's a bit sad that the function will exist in the base vmlinux
>> even for people who don't ever use it, but that's a problem which we
>> don't really have a good solution for).
>
> Hmm, should I switch it back to all inlines?

Add a hidden Kconfig variable for it which is SELECTed by those Kconfig
prompts that require it? That's the good solution that we use for a number of
similar library functions.
$ cat lib/Makefile

> On the other hand, if a generic EWMA function is useful at all I guess it
> could be used in many places all over the kernel. Unfortunately I don't have
> time and knowledge to find all these places and update them to use the generic
> code.

Don't worry. There are people who specialize in this sort of activity.

Two remarks on your patch:

You use WARN_ON in lib/average.c. You should include <linux/bug.h>.

Why do ewma_init() and ewma_add() return their first argument? They look to
me like they can be straight-forward void functions.
--
Stefan Richter
-=====-==-=- =-== -=-==