From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id F2BCCC433F5 for ; Thu, 28 Oct 2021 12:54:39 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 88114610CB for ; Thu, 28 Oct 2021 12:54:39 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 88114610CB Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=infradead.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id E2A1A6B0071; Thu, 28 Oct 2021 08:54:38 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id DDA53940008; Thu, 28 Oct 2021 08:54:38 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C7B87940007; Thu, 28 Oct 2021 08:54:38 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0200.hostedemail.com [216.40.44.200]) by kanga.kvack.org (Postfix) with ESMTP id A48B36B0071 for ; Thu, 28 Oct 2021 08:54:38 -0400 (EDT) Received: from smtpin16.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 3101E181F5FF2 for ; Thu, 28 Oct 2021 12:54:38 +0000 (UTC) X-FDA: 78745840236.16.0214E0D Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf05.hostedemail.com (Postfix) with ESMTP id 264B7508A7BA for ; Thu, 28 Oct 2021 12:54:25 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=slRCADyo2v0NSIPRzWQzYkzJFR0ro6bdeFFKDUq24Og=; b=FXJ8aQxJgrllNidoCpLPrXQtLA ZcyqiJwDvLt5B5XxzeFKOiAbHXW2TUCyVDYht0zeZZU1qUTqnYLMOJNAh7d2L+hGtLw8KRjGYQYlu 7tzcMQJs1NtlYXk4HXQS/2IbXIaTvViEJY9pziIAJ2VhDfEQSnsk2QJGAwX4X0pfzQFLgzNBFnRl7 6fEo2xA1viJs8r/FFSshBHtmwj1myfaVdZpub7g7OIXLlGbXac9n076hVY74ceg7CUoMbjYCB3+oO arrNLJV4uc/1Xyc7mIwxcIzzU5UQ3zJQZu5BlNepcWvbw8LYgN7917pi9wsWgVGefJ1IIEFuRcZEr H1e7ELPA==; Received: from willy by casper.infradead.org with local (Exim 4.94.2 #2 (Red Hat Linux)) id 1mg4uQ-000m9Q-BA; Thu, 28 Oct 2021 12:53:45 +0000 Date: Thu, 28 Oct 2021 13:53:30 +0100 From: Matthew Wilcox To: Ning Zhang Cc: linux-mm@kvack.org, Andrew Morton , Johannes Weiner , Michal Hocko , Vladimir Davydov , Yu Zhao Subject: Re: [RFC 1/6] mm, thp: introduce thp zero subpages reclaim Message-ID: References: <1635422215-99394-1-git-send-email-ningzhang@linux.alibaba.com> <1635422215-99394-2-git-send-email-ningzhang@linux.alibaba.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1635422215-99394-2-git-send-email-ningzhang@linux.alibaba.com> X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: 264B7508A7BA X-Stat-Signature: 133qiqxpe6ykbahkbe581dwzzdkynqin Authentication-Results: imf05.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=FXJ8aQxJ; dmarc=none; spf=none (imf05.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org X-HE-Tag: 1635425665-430151 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Thu, Oct 28, 2021 at 07:56:50PM +0800, Ning Zhang wrote: > +++ b/include/linux/huge_mm.h > @@ -185,6 +185,15 @@ unsigned long thp_get_unmapped_area(struct file *filp, unsigned long addr, > void free_transhuge_page(struct page *page); > bool is_transparent_hugepage(struct page *page); > > +#ifdef CONFIG_MEMCG > +int zsr_get_hpage(struct hpage_reclaim *hr_queue, struct page **reclaim_page); > +unsigned long zsr_reclaim_hpage(struct lruvec *lruvec, struct page *page); > +static inline struct list_head *hpage_reclaim_list(struct page *page) > +{ > + return &page[3].hpage_reclaim_list; > +} > +#endif I don't think any of this needs to be under an ifdef. That goes for a lot of your other additions to header files. > @@ -1110,6 +1121,10 @@ unsigned long mem_cgroup_soft_limit_reclaim(pg_data_t *pgdat, int order, > gfp_t gfp_mask, > unsigned long *total_scanned); > > +#ifdef CONFIG_TRANSPARENT_HUGEPAGE > +void del_hpage_from_queue(struct page *page); > +#endif That name is too generic. Also, to avoid ifdefs in code, it should be: #ifdef CONFIG_TRANSPARENT_HUGEPAGE void del_hpage_from_queue(struct page *page); #else static inline void del_hpage_from_queue(struct page *page) { } #endif > @@ -159,6 +159,12 @@ struct page { > /* For both global and memcg */ > struct list_head deferred_list; > }; > + struct { /* Third tail page of compound page */ > + unsigned long _compound_pad_2; > + unsigned long _compound_pad_3; > + /* For zero subpages reclaim */ > + struct list_head hpage_reclaim_list; Why do you need _compound_pad_3 here? > +++ b/include/linux/mmzone.h > @@ -787,6 +787,12 @@ struct deferred_split { > struct list_head split_queue; > unsigned long split_queue_len; > }; > + > +struct hpage_reclaim { > + spinlock_t reclaim_queue_lock; > + struct list_head reclaim_queue; > + unsigned long reclaim_queue_len; > +}; Have you considered using an XArray instead of a linked list? > +static bool hpage_estimate_zero(struct page *page) > +{ > + unsigned int i, maybe_zero_pages = 0, offset = 0; > + void *addr; > + > +#define BYTES_PER_LONG (BITS_PER_LONG / BITS_PER_BYTE) BYTES_PER_LONG is simply sizeof(long). Also, I'd check the entire cacheline rather than just one word; it's essentially free. > +#ifdef CONFIG_MMU > +#define ZSR_PG_MLOCK(flag) (1UL << flag) > +#else > +#define ZSR_PG_MLOCK(flag) 0 > +#endif Or use __PG_MLOCKED ? > +#ifdef CONFIG_ARCH_USES_PG_UNCACHED > +#define ZSR_PG_UNCACHED(flag) (1UL << flag) > +#else > +#define ZSR_PG_UNCACHED(flag) 0 > +#endif Define __PG_UNCACHED in page-flags.h? > +#ifdef CONFIG_MEMORY_FAILURE > +#define ZSR_PG_HWPOISON(flag) (1UL << flag) > +#else > +#define ZSR_PG_HWPOISON(flag) 0 > +#endif __PG_HWPOISON > +#define hr_queue_list_to_page(head) \ > + compound_head(list_entry((head)->prev, struct page,\ > + hpage_reclaim_list)) I think you're better off subtracting 3*sizeof(struct page) than loading from compound_head. > +#ifdef CONFIG_TRANSPARENT_HUGEPAGE > +/* Need the page lock if the page is not a newly allocated page. */ > +static void add_hpage_to_queue(struct page *page, struct mem_cgroup *memcg) > +{ > + struct hpage_reclaim *hr_queue; > + unsigned long flags; > + > + if (READ_ONCE(memcg->thp_reclaim) == THP_RECLAIM_DISABLE) > + return; > + > + page = compound_head(page); Why do you think the caller might be passing in a tail page here?