On 10/24/24 03:26, Chen Ridong wrote: >>>> For example, if we use his v1 proposal, we should do >>>> the cleanups again for info. But for goto-based >>>> version, we just add another label to do the >>>> cleanups and go to the new label for failure case. goto-based fix is what I insisted on. I copied my previous suggested fix here to clarify my opinion. >>> >>> Again, info is a loop-iteration-local variable, v1 fix making it truly local >>> is the way to go. If there are further cleanups added in the loop itself in >>> the future, they could hopefully keep being local to the loop as well. >>> Cleanup of info outside the loop iteration is breaking its real scope. >> >> +1 to that. >> >> I don't think it's such a big deal and both versions are ok, but I strongly >> prefer the original version (without introduction of a new label). >> >> Please, feel free to use >> Acked-by: Roman Gushchin <roman.gushchin@xxxxxxxxx> >> with the original version. >> >> Thanks! > > I agree with Roman. > Hello, Andrew and Dave, Do you have any opinions? > > The original version: > https://lore.kernel.org/linux-kernel/4184c61f-80f7-4adc-8929-c29f959cb8df@xxxxxxxxxx/ Hi, Can you resend the v1 as v3, but also move the declaration of "struct shrinker_info *info;" inside the for_each_node() block? Also you can add all the acks collected for that version and mine too: Acked-by: Vlastimil Babka <vbabka@xxxxxxx> Thanks. > Best regards, > Ridong > >
