On Thu, Nov 07, 2013 at 08:31:59AM +0100, Takashi Iwai wrote:
But then we're still left with bombing out if the first half of the function ever changes. The check is there for the benefit of the register handling, the fact that it's affecting events is just a side effect rather than the intention.
Actually the check reg == w->reg must be always true for all pending items, no matter at which point. Otherwise BUG() would have hit.
Right, but that's not actually what the check is there for. The loop is both doing an event check and building up the register value to write, the check is to tell us if we're confused about the register value since we'll end up writing to the wrong register during coalescing.
That being said, maybe a cleaner way would be to run the check over the loop once, and quit the function if anything inconsistent found.
Or do two passes, one for the registers and one for the events. You could also reorder the loop to pull the events to the top and then do the check and skip between them and the value update, or simply remove the BUG().
Or just don't worry about it and leave the second loop alone since none of this is ideal - possibly the most helpful thing would be to insert a non-coalesced write for the out of place widget, though depending on why things are going wrong perhaps that'll just make things worse and we should be trying to unwind the entire transaction instead. Unwinding the entire thing is going to be the generally safest but least helpful option.
My main concern here is not to confuse the code by having the check for the register value protecting an event call, that'll be confusing later on. If you just want to fix the BUG()s do that alone, change them to WARN()s or whatever.