On 11/27/2018 02:06 PM, Peter Xu wrote:
On Thu, Nov 15, 2018 at 06:08:00PM +0800, Wei Wang wrote:
Again, is it possible to resize during migration?

So I think the check is fine, but uncertain about the comment.

Yes, resize would not happen with the current implementation.
But heard it could just be a temporal implementation. Probably
we could improve the comment like this:

"
Though the implementation might not support ram resize currently,
this could happen in theory with future updates. So the check here
handles the case that RAMBLOCK is resized after the free page hint is
reported.
"


And shall we print something if that happened?  We can use
error_report_once(), and squashing the above assert:

   if (!block || offset > block->used_length) {
     /* should never happen, but if it happens we ignore the hints and warn */
     error_report_once("...");
     return;
   }

What do you think?

Sounds good.


+
+        if (len <= block->used_length - offset) {
+            used_len = len;
+        } else {
+            used_len = block->used_length - offset;
+            addr += used_len;
Maybe moving this line into the for() could be a bit better?

   for (; len > 0; len -= used_len, addr += used_len) {


Yes, I think it looks better, thanks.

Best,
Wei


Reply via email to