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
| ||
|
Date: Tue, 28 May 2013 15:28:12 +0300 (EEST) From: "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com> To: Dave Hansen <dave@...1.net> Cc: "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>, Andrea Arcangeli <aarcange@...hat.com>, Andrew Morton <akpm@...ux-foundation.org>, Al Viro <viro@...iv.linux.org.uk>, Hugh Dickins <hughd@...gle.com>, Wu Fengguang <fengguang.wu@...el.com>, Jan Kara <jack@...e.cz>, Mel Gorman <mgorman@...e.de>, linux-mm@...ck.org, Andi Kleen <ak@...ux.intel.com>, Matthew Wilcox <matthew.r.wilcox@...el.com>, "Kirill A. Shutemov" <kirill@...temov.name>, Hillf Danton <dhillf@...il.com>, linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org Subject: Re: [PATCHv4 14/39] thp, mm: rewrite delete_from_page_cache() to support huge pages Dave Hansen wrote: > On 05/11/2013 06:23 PM, Kirill A. Shutemov wrote: > > From: "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com> > > > > As with add_to_page_cache_locked() we handle HPAGE_CACHE_NR pages a > > time. > > > > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@...ux.intel.com> > > --- > > mm/filemap.c | 31 +++++++++++++++++++++++++------ > > 1 file changed, 25 insertions(+), 6 deletions(-) > > > > diff --git a/mm/filemap.c b/mm/filemap.c > > index b0c7c8c..657ce82 100644 > > --- a/mm/filemap.c > > +++ b/mm/filemap.c > > @@ -115,6 +115,9 @@ > > void __delete_from_page_cache(struct page *page) > > { > > struct address_space *mapping = page->mapping; > > + bool thp = PageTransHuge(page) && > > + IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE_PAGECACHE); > > + int nr; > > Is that check for the config option really necessary? How would we get > a page with PageTransHuge() set without it being enabled? I'll drop it and use hpagecache_nr_page() instead. > I like to rewrite your code. :) It's nice. Thanks. > Which reminds me... Why do we handle their reference counts differently? :) > > It seems like we could easily put a for loop in delete_from_page_cache() > that will release their reference counts along with the head page. > Wouldn't that make the code less special-cased for tail pages? delete_from_page_cache() is not the only user of __delete_from_page_cache()... It seems I did it wrong in add_to_page_cache_locked(). We shouldn't take references on tail pages there, only one on head. On split it will be distributed properly. > > /* Leave page->index set: truncation lookup relies upon it */ > > - mapping->nrpages--; > > - __dec_zone_page_state(page, NR_FILE_PAGES); > > + mapping->nrpages -= nr; > > + __mod_zone_page_state(page_zone(page), NR_FILE_PAGES, -nr); > > if (PageSwapBacked(page)) > > - __dec_zone_page_state(page, NR_SHMEM); > > + __mod_zone_page_state(page_zone(page), NR_SHMEM, -nr); > > BUG_ON(page_mapped(page)); > > Man, we suck: > > __dec_zone_page_state() > and > __mod_zone_page_state() > > take a differently-typed first argument. <sigh> > > Would there be any good to making __dec_zone_page_state() check to see > if the page we passed in _is_ a compound page, and adjusting its > behaviour accordingly? Yeah, it would be better but I think it outside the scope of the patchset. Probably, later. -- Kirill A. Shutemov -- 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