Messages in this thread | | | From | Muchun Song <> | Date | Fri, 11 Dec 2020 22:08:43 +0800 | Subject | Re: [External] Re: [PATCH v8 07/12] mm/hugetlb: Set the PageHWPoison to the raw error page |
| |
On Fri, Dec 11, 2020 at 9:36 PM Oscar Salvador <osalvador@suse.de> wrote: > > On Thu, Dec 10, 2020 at 11:55:21AM +0800, Muchun Song wrote: > > +static inline void subpage_hwpoison_deliver(struct hstate *h, struct page *head) > > +{ > > + struct page *page = head; > > + > > + if (!free_vmemmap_pages_per_hpage(h)) > > + return; > > + > > + if (PageHWPoison(head)) > > + page = head + page_private(head + 4); > > + > > + /* > > + * Move PageHWPoison flag from head page to the raw error page, > > + * which makes any subpages rather than the error page reusable. > > + */ > > + if (page != head) { > > + SetPageHWPoison(page); > > + ClearPageHWPoison(head); > > + } > > +} > > I would make the names coherent. > I am not definitely goot at names, but something like: > hwpoison_subpage_{foo,bar} looks better.
It's better than mine. Thank you.
> > Also, could not subpage_hwpoison_deliver be rewritten like: > > static inline void subpage_hwpoison_deliver(struct hstate *h, struct page *head) > { > struct page *page; > > if (!PageHWPoison(head) || !free_vmemmap_pages_per_hpage(h)) > return; > > page = head + page_private(head + 4); > /* > * Move PageHWPoison flag from head page to the raw error page, > * which makes any subpages rather than the error page reusable. > */ > if (page != head) { > SetPageHWPoison(page); > ClearPageHWPoison(head); > } > } > > I think it is better code-wise.
Will do. Thank you.
> > > + * Move PageHWPoison flag from head page to the raw error page, > > + * which makes any subpages rather than the error page reusable. > > + */ > > + if (page != head) { > > + SetPageHWPoison(page); > > + ClearPageHWPoison(head); > > + } > > I would put this in an else-if above: > > if (free_vmemmap_pages_per_hpage(h)) { > set_page_private(head + 4, page - head); > return; > } else if (page != head) { > SetPageHWPoison(page); > ClearPageHWPoison(head); > } > > or will we lose the optimization in case free_vmemmap_pages_per_hpage gets compiled out? >
Either is OK. The compiler will help us optimize the code when free_vmemmap_pages_per_hpage always returns false.
Thanks for your suggestions. :-)
> > -- > Oscar Salvador > SUSE L3
-- Yours, Muchun
| |