linux-kernel - Re: [PATCH] memory_failure: remove redundant check for the PG

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150729091725.GA1256@hori1.linux.bs1.fc.nec.co.jp>
Date:	Wed, 29 Jul 2015 09:17:32 +0000
From:	Naoya Horiguchi <n-horiguchi@...jp.nec.com>
To:	Wang Xiaoqiang <wangxq10@....edu.cn>
CC:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-mm@...ck.org" <linux-mm@...ck.org>
Subject: Re: [PATCH] memory_failure: remove redundant check for the
 PG_HWPoison flag of 'hpage'

# CC:ed linux-mm

Hi Xiaoqiang,

On Wed, Jul 29, 2015 at 03:52:46PM +0800, Wang Xiaoqiang wrote:
> Hi,
> 
> I find a little problem in the memory_failure function in
> mm/memory-failure.c . Please check it.
> 
> memory_failure: remove redundant check for the PG_HWPoison flag of
> `hpage'.
> 
> Since we have check the PG_HWPoison flag by `PageHWPoison' before,
> so the later check by `TestSetPageHWPoison' must return true, there
> is no need to check again!

I'm afraid that this TestSetPageHWPoison is not redundant, because this code
serializes the concurrent memory error events over the same hugetlb page
(, where 'p' indicates the 4kB error page and 'hpage' indicates the head page.)

When an error hits a hugetlb page, set_page_hwpoison_huge_page() sets
PageHWPoison flags over all subpages of the hugetlb page in the ascending
order of pfn. So if we don't have this TestSet, memory error handler can
run more than once on concurrent errors when the 1st memory error hits
(for example) the 100th subpage and the 2nd memory error hits (for example)
the 50th subpage.

Thanks,
Naoya Horiguchi

> Signed-off-by: Wang Xiaoqiang <wangxq10@....edu.cn>
> ---
>  mm/memory-failure.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index 1cf7f29..7794fd8 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -1115,7 +1115,7 @@ int memory_failure(unsigned long pfn, int trapno, int flags)
>  			lock_page(hpage);
>  			if (PageHWPoison(hpage)) {
>  				if ((hwpoison_filter(p) && TestClearPageHWPoison(p))
> -				    || (p != hpage && TestSetPageHWPoison(hpage))) {
> +				    || p != hpage) {
>  					atomic_long_sub(nr_pages, &num_poisoned_pages);
>  					unlock_page(hpage);
>  					return 0;
> -- 
> 1.7.10.4
> 
> 
> 
> --
> thx!
> Wang Xiaoqiang
> --
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/