On 2024/10/24 17:08, Vlastimil Babka wrote: > 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 >> >> Will update, Thanks. Best regards, Ridong
