Yunhong, in c/s 20617:283a5357d196 you modified init_frametable() to populate the frame table slightly differently for the hotplug case. I wonder why you did that, because (apart from the bug already fixed, and the off-by-one bugs I''m having a fix pending for) I fear you didn''t pay attention to the fact that using pdx_to_page() on something that doesn''t really represent the PDX for a valid page may return a value not validly usable here. Do you happen to recall what it was that caused you to do that adjustment in the first place? If you don''t, do you have an environment where you would be able to test an eventual change of mine (effectively undoing that part of said c/s)? Thanks, Jan
Jan, sorry for slow response. IIRC, the reason we do this is because in memory hotplug situation, there will be a very big hole between the address of the memory populated before hot-plug and the memory populated by hot-added memory. (i.e. the added memory started at very high-end address). So instead of setup the frame table for the whole address space, we expand the frame table dynamically after hotplug. We have the memory hotplug environment, so if you have any patch, I''m glad to test it, or have my colleagues help to test it. Thanks --jyh> -----Original Message----- > From: Jan Beulich [mailto:JBeulich@suse.com] > Sent: Saturday, December 08, 2012 12:16 AM > To: Jiang, Yunhong > Cc: xen-devel > Subject: frame table setup for memory hotplug > > Yunhong, > > in c/s 20617:283a5357d196 you modified init_frametable() to > populate the frame table slightly differently for the hotplug > case. I wonder why you did that, because (apart from the bug > already fixed, and the off-by-one bugs I''m having a fix pending > for) I fear you didn''t pay attention to the fact that using > pdx_to_page() on something that doesn''t really represent the > PDX for a valid page may return a value not validly usable here. > > Do you happen to recall what it was that caused you to do that > adjustment in the first place? If you don''t, do you have an > environment where you would be able to test an eventual > change of mine (effectively undoing that part of said c/s)? > > Thanks, Jan >
>>> On 10.12.12 at 14:32, "Jiang, Yunhong" <yunhong.jiang@intel.com> wrote: > IIRC, the reason we do this is because in memory hotplug situation, there > will be a very big hole between the address of the memory populated before > hot-plug and the memory populated by hot-added memory. (i.e. the added memory > started at very high-end address). So instead of setup the frame table for the > whole address space, we expand the frame table dynamically after hotplug. > > We have the memory hotplug environment, so if you have any patch, I''m glad > to test it, or have my colleagues help to test it.I meanwhile decided to keep the code logically the same, but testing the patch at http://lists.xen.org/archives/html/xen-devel/2012-12/msg00793.html (or the staging/normal trees once it went in/got pushed) would still be much appreciated. Thanks, Jan
That hot-plug system is quite difficult to setup. Will let you know when the test is done. --jyh> -----Original Message----- > From: Jan Beulich [mailto:JBeulich@suse.com] > Sent: Monday, December 10, 2012 9:48 PM > To: Jiang, Yunhong > Cc: xen-devel > Subject: RE: frame table setup for memory hotplug > > >>> On 10.12.12 at 14:32, "Jiang, Yunhong" <yunhong.jiang@intel.com> wrote: > > IIRC, the reason we do this is because in memory hotplug situation, there > > will be a very big hole between the address of the memory populated before > > hot-plug and the memory populated by hot-added memory. (i.e. the added > memory > > started at very high-end address). So instead of setup the frame table for the > > whole address space, we expand the frame table dynamically after hotplug. > > > > We have the memory hotplug environment, so if you have any patch, I''m glad > > to test it, or have my colleagues help to test it. > > I meanwhile decided to keep the code logically the same, but > testing the patch at > http://lists.xen.org/archives/html/xen-devel/2012-12/msg00793.html > (or the staging/normal trees once it went in/got pushed) would still be > much appreciated. > > Thanks, Jan
My colleagues helped me to test and seems it''s working. A minor question is, with your patch, we will create all frame table for all potential memory range? That''s a bit low efficient IMHO, because there will be large hole between address between pre-populated memory and the new added one. Thanks --jyh> -----Original Message----- > From: Jan Beulich [mailto:JBeulich@suse.com] > Sent: Monday, December 10, 2012 9:48 PM > To: Jiang, Yunhong > Cc: xen-devel > Subject: RE: frame table setup for memory hotplug > > >>> On 10.12.12 at 14:32, "Jiang, Yunhong" <yunhong.jiang@intel.com> wrote: > > IIRC, the reason we do this is because in memory hotplug situation, there > > will be a very big hole between the address of the memory populated before > > hot-plug and the memory populated by hot-added memory. (i.e. the added > memory > > started at very high-end address). So instead of setup the frame table for the > > whole address space, we expand the frame table dynamically after hotplug. > > > > We have the memory hotplug environment, so if you have any patch, I''m glad > > to test it, or have my colleagues help to test it. > > I meanwhile decided to keep the code logically the same, but > testing the patch at > http://lists.xen.org/archives/html/xen-devel/2012-12/msg00793.html > (or the staging/normal trees once it went in/got pushed) would still be > much appreciated. > > Thanks, Jan
>>> On 14.12.12 at 11:07, "Jiang, Yunhong" <yunhong.jiang@intel.com> wrote: > My colleagues helped me to test and seems it''s working.Thanks!> A minor question is, with your patch, we will create all frame table for all > potential memory range? That''s a bit low efficient IMHO, because there will > be large hole between address between pre-populated memory and the new added > one.No, we won''t. The only thing we will fully populate is the super page frame table (if enabled in the first place), which is better than not populating its ranges covering hotplug memory at all (as was the case before the patch); I''m certainly not against making this more efficient, but that would imo need to be done by someone being able to actually test the code paths needing modification. Jan