Messages in this thread | | | Date | Fri, 9 Apr 2021 07:06:43 +0200 | From | Oscar Salvador <> | Subject | Re: [PATCH 04/10] mm/migrate: make migrate_pages() return nr_succeeded |
| |
On Thu, Apr 08, 2021 at 01:40:33PM -0700, Yang Shi wrote: > Thanks a lot for the example code. You didn't miss anything. At first > glance, I thought your suggestion seemed neater. Actually I > misunderstood what Dave said about "That could really have caused some > interesting problems." with multiple calls to migrate_pages(). I was > thinking about: > > unsigned long foo() > { > unsigned long *ret_succeeded; > > migrate_pages(..., ret_succeeded); > > migrate_pages(..., ret_succeeded); > > return *ret_succeeded; > }
But that would not be a problem as well. I mean I am not sure what is foo() supposed to do. I assume is supposed to return the *total* number of pages that were migrated?
Then could do something like:
unsigned long foo() { unsigned long ret_succeeded; unsigned long total_succeeded = 0;
migrate_pages(..., &ret_succeeded); total_succeeded += ret_succeeded;
migrate_pages(..., &ret_succeeded); total_succeeded += ret_succeeded;
return *total_succeeded; }
But AFAICS, you would have to do that with Wei Xu's version and with mine, no difference there.
IIUC, Dave's concern was that nr_succeeded was only set to 0 at the beginning of the function, and never reset back, which means, we would carry the sum of previous nr_succeeded instead of the nr_succeeded in that round. That would be misleading for e.g: reclaim in case we were to call migrate_pages() several times, as instead of a delta value, nr_succeeded would accumulate.
But that won't happen neither with Wei Xu's version nor with mine.
-- Oscar Salvador SUSE L3
| |