On Fri, 30 Dec 2016, Takashi Iwai <tiwai@xxxxxxx> wrote: > On Wed, 21 Dec 2016 11:38:54 +0100, > Ioan-Adrian Ratiu wrote: >> >> >> > Please take the time to fully analyze my patch and let's have a >> >> > discussion based on it, not reject it outright and replace it with >> >> > a quick and dirty delay hack. >> >> >> >> OK. I'll deliberately check it again so that I have no overlooks. I >> >> with this thread also catch the other developers enough helpful to >> >> you. (I just eventually caught your patch in LKML and not developer >> >> for this category of devices.) >> > >> > Sorry for the late reply, as I've been (still) off and had bad net >> > connections. >> > >> > About your fix: Sakamoto-san is right, it's no good way to fix this >> > kind or problem. The easiest option right now is just to revert my >> > previous fix, as it obviously introduces another regression. The >> > correct fix will be taken after that. >> > >> > I'm going to prepare a revert patch and ask Linus to take it before >> > rc1. >> >> I agree with reverting the initial commit decision because my problem >> disappears with it. >> >> But can you please state a reason for why my patch is "no good way to >> fix"? Being too intrusive is not a good reason. > > "Being too intrusive" is the exact reason why it's not good as a > "regression fix" like this case. The logic you've implemented in the > patch itself looks good (although the code introduces a bug, the > unbalance of snd_usb_*lock_shutdown()). The only point I couldn't > take it is that it's rather a fundamental change, not a quick fix for > a regression. > > What's the worst case scenario in a regression fix? It's when a fix > introduces yet another regression. It'd be much worse if the new > regression is deeper. The complicated logical change has a potential > risk of such. Thus an intrusive change is not always good as a > "regression fix". > > In general, if the change were trivial, it's obviously OK to take as a > regression fix -- where the logic is also trivial in most cases, too. > But when the fix becomes complex, we need to rethink. Especially when > the original buggy commit is small, reverting it is often better as a > clear cut. > > Think in that way: you're addressing a deeper problem that was > revealed accidentally by my previous bad fix. Writing the change as > if it were merely a regression fix gives the wrong understanding to > readers :) Yes, this makes sense. Thank you. > > That said, I'd appreciate if you respin the fix again with a > combination of my previous fix and brush up the code a bit as a whole, > and document it not as a regression fix but as a complete fix of the > existing races. I can write it in my side quickly, but it'd be almost > in the same form as you posted, I suppose. I'll find some time to do a rebase either tomorrow (31 Dec) or until 2 Jan at the latest. Ionel > > > thanks, > > Takashi _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
