Hello Baoquan, On 2013/11/26 11:53:34, kexec <kexec-bounces at lists.infradead.org> wrote: > On 11/25/13 at 01:33pm, HATAYAMA Daisuke wrote: > > (2013/11/25 11:31), Baoquan He wrote: > > >Hi HATAYAMA and Atsushi, > > > > > >I think v2 is better than v1, since update_cyclic_region can be used > > >with a more flexible calling. > > > > > >What's your opinion about this? > > > > > >On 11/23/13 at 05:29pm, Baoquan He wrote: > > > > Thanks for your patch. The bug is caused by my patch set for creating a > > whole part of 1st bitmap before entering cyclic process. > > > > I think v1 is better than v2. The update_cyclic_range() call relevant > > to this regression is somewhat special compared to other calls; it is > > the almost only call that doesn't need to perform filtering processing. > > To fix this bug, please make the patch so as not to affect the other calls, > > in order to keep change as small as possible. > > OK, if you think so. But I still think update_cyclic_region is a little > weird, its name doesn't match its functionality, this confuses code > reviewers. And it does something unnecessary somewhere. If it's > possible, I would rather take out the create_1st_bitmap_cyclic and > exclude_unnecessary_pages_cyclic, and call them explicitly where they > are really needed. Surely we can make a little change in both of them, > E.g add a parameter pfn to them, then we can also judge like > update_cyclic_region has done: > > if (is_cyclic_region(pfn)) > return TRUE; > > If you insist on v1 is a better idea, I will repost based on it, but keep > my personal opinion. I also prefer v1 because the usage of update_cyclic_region_without_exclude() is definite and understandable while v2's update_cyclic_region() is complicated. On the other hand, I agree with your opinion, so could you post a cleanup patch separately from v1 patch ? Thanks Atsushi Kumagai > Baoquan > Thanks > > > > > -- > > Thanks. > > HATAYAMA, Daisuke > > > > > > _______________________________________________ > > kexec mailing list > > kexec at lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/kexec > > _______________________________________________ > kexec mailing list > kexec at lists.infradead.org > http://lists.infradead.org/mailman/listinfo/kexec >
