Tay Ray Chuan <rctay89@xxxxxxxxx> writes:
> This patch ensures that the same fill function is called once so to
> prevent any possible issues.
>
> Nevertheless, calling a fill function repeatedly in
> ''fill_active_slots'' will not lead to any obvious change in existing
> behavior, though performance might be affected.
>
> ''add_fill_action'' checks if the function to be added has already been
> added. Allocation of memory for the list ''fill_chain*'' is postponed
> until the check passes, unlike previously.
Could you care to explain the following a bit better?
- what "possible issues" you are addressing;
- what changes in the behaviour that are not "obvious" we would be
suffering from, if we apply this patch;
- in what situation the performance _might_ be affected, in what way and
to what extent.
If the patch author does not have clear answers to these questions, how
can others decide if it is worth reading the patch to judge if it is worth
applying?
In other words, I'd expect you to explain the issues like this:
add_fill_function() adds the same fill function twice on the fill_cfg
list; this causes THIS and THAT breakages when the fill function is
called twice.
Ignore add_fill_function() when fill_cfg list already has the function
registered on it to fix this issue. Note however that the new code may
behave very inefficiently under this situation:
- XYZZY happens, then
- FROTZ happens, and then
- NITFOL happens.
In such a case we end up doing FROTZ repeatedly, and ...; we might
want to later optimize this, but a correctly working code is more
important than efficient code that works most of the time but silently
breaks in some cases.
We need to iterate over the existing entries in fill_cfg list to find
duplicates, which may look like an overhead, but the existing function
already needed to do so to queue the new entry at the end anyway, so
this is nothing new.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
