On Wed, 5 Dec 2012 17:14:33 -0500
Naoya Horiguchi <n-horiguchi@xxxxxxxxxxxxx> wrote:
> Hi Tony,
>
> On Wed, Dec 05, 2012 at 10:04:50PM +0000, Luck, Tony wrote:
> > if (PageHWPoison(page) && !(flags & TTU_IGNORE_HWPOISON)) {
> > - if (PageAnon(page))
> > + if (PageHuge(page))
> > + ;
> > + else if (PageAnon(page))
> > dec_mm_counter(mm, MM_ANONPAGES);
> > else
> > dec_mm_counter(mm, MM_FILEPAGES);
> >
> > This style minimizes the "diff" ... but wouldn't it be nicer to say:
> >
> > if (!PageHuge(page)) {
> > old code in here
> > }
> >
>
> I think this need more lines in diff because old code should be
> indented without any logical change.
I do agree with Tony on this. While it is nice to keep the diff
looking simple, it is more important that the resulting code be clean
and idiomatic.
--- a/mm/rmap.c~hwpoison-hugetlbfs-fix-rss-counter-warning-fix
+++ a/mm/rmap.c
@@ -1249,14 +1249,14 @@ int try_to_unmap_one(struct page *page,
update_hiwater_rss(mm);
if (PageHWPoison(page) && !(flags & TTU_IGNORE_HWPOISON)) {
- if (PageHuge(page))
- ;
- else if (PageAnon(page))
- dec_mm_counter(mm, MM_ANONPAGES);
- else
- dec_mm_counter(mm, MM_FILEPAGES);
- set_pte_at(mm, address, pte,
- swp_entry_to_pte(make_hwpoison_entry(page)));
+ if (!PageHuge(page)) {
+ if (PageAnon(page))
+ dec_mm_counter(mm, MM_ANONPAGES);
+ else
+ dec_mm_counter(mm, MM_FILEPAGES);
+ set_pte_at(mm, address, pte,
+ swp_entry_to_pte(make_hwpoison_entry(page)));
+ }
} else if (PageAnon(page)) {
swp_entry_t entry = { .val = page_private(page) };
_
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>