Messages in this thread | | | Date | Thu, 18 Mar 2021 12:36:52 +0100 | From | Michal Hocko <> | Subject | Re: [PATCH v5 2/5] mm,compaction: Let isolate_migratepages_{range,block} return error codes |
| |
On Thu 18-03-21 12:10:14, Vlastimil Babka wrote: > On 3/18/21 11:22 AM, Michal Hocko wrote: [...] > > E.g. something like the following > > diff --git a/mm/internal.h b/mm/internal.h > > index 1432feec62df..6c5a9066adf0 100644 > > --- a/mm/internal.h > > +++ b/mm/internal.h > > @@ -225,7 +225,13 @@ struct compact_control { > > unsigned int nr_freepages; /* Number of isolated free pages */ > > unsigned int nr_migratepages; /* Number of pages to migrate */ > > unsigned long free_pfn; /* isolate_freepages search base */ > > - unsigned long migrate_pfn; /* isolate_migratepages search base */ > > + unsigned long migrate_pfn; /* Acts as an in/out parameter to page > > + * isolation. > > + * isolate_migratepages uses it as a search base. > > + * isolate_migratepages_block will update the > > + * value the next pfn after the last isolated > > + * one. > > + */ > > Fair enough. I would even stop pretending we might cram something useful in the > rest of the line, and move all the comments to blocks before the variables. > There might be more of them that would deserve more thorough description.
Yeah, makes sense. I am not a fan of the above form of documentation. Btw. maybe renaming the field would be even better, both from the intention and review all existing users. I would go with pfn_iter or something that wouldn't make it sound like migration specific.
-- Michal Hocko SUSE Labs
| |