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]
Date:	Mon, 22 Jun 2009 10:16:26 +0100
From:	Mel Gorman <mel@....ul.ie>
To:	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>
Cc:	Jiri Slaby <jirislaby@...il.com>,
	Maxim Levitsky <maximlevitsky@...il.com>,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org,
	Andrew Morton <akpm@...ux-foundation.org>,
	Lee Schermerhorn <Lee.Schermerhorn@...com>,
	Christoph Lameter <cl@...ux-foundation.org>,
	Pekka Enberg <penberg@...helsinki.fi>
Subject: Re: BUG: Bad page state [was: Strange oopses in 2.6.30]

On Mon, Jun 22, 2009 at 11:39:53AM +0900, KOSAKI Motohiro wrote:
> (cc to Mel and some reviewer)
> 
> > Flags are:
> > 0000000000400000 -- __PG_MLOCKED
> > 800000000050000c -- my page flags
> >         3650000c -- Maxim's page flags
> > 0000000000693ce1 -- my PAGE_FLAGS_CHECK_AT_FREE
> 
> I guess commit da456f14d (page allocator: do not disable interrupts in
> free_page_mlock()) is a bit wrong.
> 
> current code is:
> -------------------------------------------------------------
> static void free_hot_cold_page(struct page *page, int cold)
> {
> (snip)
>         int clearMlocked = PageMlocked(page);
> (snip)
>         if (free_pages_check(page))
>                 return;
> (snip)
>         local_irq_save(flags);
>         if (unlikely(clearMlocked))
>                 free_page_mlock(page);
> -------------------------------------------------------------
> 
> Oh well, we remove PG_Mlocked *after* free_pages_check().
> Then, it makes false-positive warning.
> 
> Sorry, my review was also wrong. I think reverting this patch is better ;)
> 

I think a revert is way overkill. The intention of the patch is sound -
reducing the number of times interrupts are disabled. Having pages
with the PG_locked bit is now somewhat of an expected situation. I'd
prefer to go with either

1. Unconditionally clearing the bit with TestClearPageLocked as the
   patch already posted does
2. Removing PG_locked from the free_pages_check()
3. Unlocking the pages as we go when an mlocked VMA is being torn town

The patch that addresses 1 seemed ok to me. What do you think?

-- 
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab
--
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/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ