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

[Stefan Richter <stefanr@s5r6.in-berlin.de>]

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
-=====-==-=- =-== -=-==
http://arcgraph.de/sr/