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 kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id DADD4CF3189 for ; Wed, 2 Oct 2024 01:58:08 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 247646B009F; Tue, 1 Oct 2024 21:58:08 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 1F7E06B0157; Tue, 1 Oct 2024 21:58:08 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0246E44016B; Tue, 1 Oct 2024 21:58:07 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id C56A0440168 for ; Tue, 1 Oct 2024 21:58:07 -0400 (EDT) Received: from smtpin16.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 60BBDA0434 for ; Wed, 2 Oct 2024 01:58:07 +0000 (UTC) X-FDA: 82627001814.16.62A4BDC Received: from mail-pl1-f169.google.com (mail-pl1-f169.google.com [209.85.214.169]) by imf11.hostedemail.com (Postfix) with ESMTP id 7E3784000C for ; Wed, 2 Oct 2024 01:58:05 +0000 (UTC) Authentication-Results: imf11.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=WGX2A6Dy; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf11.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.214.169 as permitted sender) smtp.mailfrom=21cnbao@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1727834246; a=rsa-sha256; cv=none; b=J5k0G0EUBL804GT+GuMQtz+fAuSrMRmaP75htob99vu6uXVTzbuar4PKaifzB1jDU/vzMx LZOQdH7W4Kj4hfS2iydU7Cy4kbGQypLHjQ/TJkZv+m8gg+ffTDJ8BbuNi9qD7XTCixco/p zrsTq5vwc3D0vfEcQyKRHIQTz9tTwnQ= ARC-Authentication-Results: i=1; imf11.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=WGX2A6Dy; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf11.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.214.169 as permitted sender) smtp.mailfrom=21cnbao@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1727834246; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=JMNrBVjF1bL7gF47dMfu/RtOoCikJdmHRhx3A5ecLfU=; b=ijlq9pYh5Azxn4LS1VFut8dQGP2ScKrfdn5uOA7UzlnjAMhktvAHogGwyGiz60uBJE71P3 pUQX/jruL5QGC0dBB2UcXTXB7exI8vgAh0uDHqezWn/L+bBjje9T6bZHP49uRlzxdcJCda xaQ/VXCg0VKcELraCvftJ7js3W7W13Q= Received: by mail-pl1-f169.google.com with SMTP id d9443c01a7336-20b9b35c7c3so25512265ad.3 for ; Tue, 01 Oct 2024 18:58:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1727834284; x=1728439084; darn=kvack.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=JMNrBVjF1bL7gF47dMfu/RtOoCikJdmHRhx3A5ecLfU=; b=WGX2A6Dy8tJKO1mYFfvTmHFVkCZG31u1/Nb+100kAPl4D0SdPIMAmpA0oJToz2XaRF Whpp7kc/xTapHW1QGHcADW6XHEKhrM76lXzljH1daTuzfJxlSYbifzOCh6g+V584SbXl O55lfghefR8onz2doT0lSb8sUtkscyXbL0R8su1jN9UZRitfh9eozlHoWNDNS39DrFwu 5eOS4OZ7XAQiZuuBIfooc9ojkKS3FsjINAxB8QMMb4Got+iPrSrxbPz7zKquZaBgravq yB2pgpcslF2C+1dUJH6E/+WnTfTIAytlSRYpfyq5EHC0wcrlPVn1QsGId7GIvJ2SY/BZ /Jzg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1727834284; x=1728439084; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=JMNrBVjF1bL7gF47dMfu/RtOoCikJdmHRhx3A5ecLfU=; b=ELndu43qjrdzQ86GcFO5YpN1c19sVqQ/VGUG+i2yBlpykW/kjCrV9FOkf5/GW+XeQe 6Tz2XFJcVvc3gKsnH5vXQY4wmSs1OQ6TN4dOhbjBI78MfreS8VfTaiqtroxbM3tEUBwe WuB0kaMhfGPgjDeAa5WL2AjwtJDEG8ngrHRbhO9qbl3PyBEgZbvZ2pMf5Id7KPZbZPuG 7qiPb0x69tYpW7In1tGwOIM0l+XYOjhn4pevdO22jZXlKK1FC9R9nNTtXtsHTjU5qGvc BtFwHv9Zc2FZxMnMClegQfIKyryHLr3unLJE4g4wjnkKJ/+YEwhA7SFwnoyxtzIWcTQp pNEQ== X-Forwarded-Encrypted: i=1; AJvYcCXrInuQc3Mxard1ZoQxvjNrXvz4UCP0F2G8dKtAw8urOc+f7VNDkw4NWoY4saYf6EucSeO5LwR41A==@kvack.org X-Gm-Message-State: AOJu0Yydx7EmhTVtKH09NGlP1A+MkSbY0897WGg0D/kNeTL7ANABs/2K Y1IHa9EosToWQXxocx9agjdeLnVUSo33xZOY7KdodVG9ojrYsILU X-Google-Smtp-Source: AGHT+IHEPTXLdFX1RXr6rSHoKJ/P6VUtNQy6DXVv+93mJMWUOlBRvZA0EwWhFAqjUm7Ei/iyqrtLvQ== X-Received: by 2002:a17:902:d512:b0:207:16b9:808c with SMTP id d9443c01a7336-20bc59c3906mr22949365ad.1.1727834283921; Tue, 01 Oct 2024 18:58:03 -0700 (PDT) Received: from localhost.localdomain ([103.135.144.4]) by smtp.gmail.com with ESMTPSA id 41be03b00d2f7-7e6db2c68absm9061493a12.42.2024.10.01.18.57.57 (version=TLS1_3 cipher=TLS_CHACHA20_POLY1305_SHA256 bits=256/256); Tue, 01 Oct 2024 18:58:03 -0700 (PDT) From: Barry Song <21cnbao@gmail.com> To: ying.huang@intel.com Cc: 21cnbao@gmail.com, akpm@linux-foundation.org, chrisl@kernel.org, david@redhat.com, hannes@cmpxchg.org, hughd@google.com, kaleshsingh@google.com, kasong@tencent.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org, liyangouwen1@oppo.com, mhocko@suse.com, minchan@kernel.org, sj@kernel.org, stable@vger.kernel.org, surenb@google.com, v-songbaohua@oppo.com, willy@infradead.org, yosryahmed@google.com, yuzhao@google.com Subject: Re: [PATCH] mm: avoid unconditional one-tick sleep when swapcache_prepare fails Date: Wed, 2 Oct 2024 09:57:54 +0800 Message-Id: <20241002015754.969-1-21cnbao@gmail.com> X-Mailer: git-send-email 2.39.3 (Apple Git-146) In-Reply-To: <87y137nxqs.fsf@yhuang6-desk2.ccr.corp.intel.com> References: <87y137nxqs.fsf@yhuang6-desk2.ccr.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspam-User: X-Rspamd-Queue-Id: 7E3784000C X-Rspamd-Server: rspam01 X-Stat-Signature: wrcyo1uo8r1iu9in3d8xjx4inszqmucc X-HE-Tag: 1727834285-85265 X-HE-Meta: U2FsdGVkX1/invusUsOVLbI8an681+EGDDnU+6oLeBvCXQYResLPtez5UzJZAY9jlVQ8SDlSsM4EMsZMoK9uXT7BTyvA+s2YCX0rxS3n1m5iwTrvuEDzVkL3n0u2yUzlRLcD6nmcqsWFHR91gZ8/3qNG9XrJ/T83ZpkEsL+urbKTVr0McJrf7+SyDIVu5EIp5tjoawh6w61eckRt3mDROkGquZDHpkg/pME5W6j1IdexGx3f90GC5Bob1kzKfb3vi4UQcQeV0JVgD+14RVAavsWcLewQhKDahN/g7shGk9rJtO+eKku9kYLFEP8/Q6M8q5nf3spxaQJmSsB1i5JCos0HSA4hGIaYwGSJFVHtDRWfhL6sRWFEOkVlYkpp7RntDLJSFJnI7enzW/7d85ZnMLPd+LxSfF2dARoCMv3iRCQl1r1g3rXJTY1MOhWhi0GbwyK6UdlqfvLgP9QPfHqqrGIhmb0ukwhPqcD+MrmJdlss9/GxpXvbCklKdN0FwDg6wqq8PsWaXb8oR7GuKXLxw++bLdX2YUD4pF48vJlqEwLK9esGQhTuMGt+ewy/UkrTbErOh/TYg1YgwMt1+8olqX29Z6cx6e6fmBl/qHdSulGpwopWJh7lCZWwszHzZUlobZ1yBpZTwmgOtQSV1EwfJZVfznMkFCN9KLVbeEpD8qOo1wK7P31mzKm4rDef2/cUXy4CCMCAb1fInovi3pXlU+wvWMlWReCZx1y7ijUyIBGKZLvKGTYr5ihMYTm4p3q8wbwi8FRz+X+jSOCefzm7AyY92CbylsANqs2b6JsXI6wN47TGTeu9ANzADAg2QRhTCRiw2jipaRUFDKk2TBOCate2Qz3Pk7oDs5AgDTURUwoT22Rxwj/rXPvJXzJVRDJXFoC1OaWCytCxA1JP+eaDd/pmAPk2icrsxW30/bCwK+TqXpNLwD3nUjpornfHIqTTFhq7onpDROgP6SPdYig 7IAMNfdb uNj92/n/AuHn575FXmwmMjfaDYRXPLkrPmPGzhMIhN33f/QsCWae49SwxcC9ReCw48oH75gckpg4wtPlhkKPeUh7Wisjnf7ZR14i22doVLsg8WRcnarYg3sh9pvUeEixh266ANHP5yqoUbQQXitzeVORb8/kEZ6bl36ugPkwSySiyWZsxPqI4uK1uu69ZJY5HLV5DGvFmGHcNpLsdUCxHtHapW5eQZkwO5sJDGQN6Kh78e/Wi7rzI1+lVDDZSxPJ6pDbZDNqPiK1Rk4UihOtWXDg7PScm+uPZ2Ty8l0kScT2PddYzKzt9OGLFNUv2rDOuU1no0blbb+RNcFrDUTyJjbXEDgG2Vhshm2H+4ObJWBTFUOJiFgw3M0Lhlt5MFmnvly3GQpv8POKJvbvmC3E7s1LDGkhK6jW0nTELELfPg90FYvG55RYX5MdhMDSXE6IUwN71pnYTDqJkeTeCt9+gg+H+TfpkP9+PDWNB0PZO2f5lWqaqeFksg+RdD/0VyKuksp25Z+DDRtjg10JohykHtiAVz+/xC3XJvrhRUupHV1oR+VZ9bD+ArLZfvH9ZXeMDGw5JK/04ruw1znueftRuVUEViQ== 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: List-Subscribe: List-Unsubscribe: On Wed, Oct 2, 2024 at 8:43 AM Huang, Ying wrote: > > Barry Song <21cnbao@gmail.com> writes: > > > On Tue, Oct 1, 2024 at 7:43 AM Huang, Ying wrote: > >> > >> Barry Song <21cnbao@gmail.com> writes: > >> > >> > On Sun, Sep 29, 2024 at 3:43 PM Huang, Ying wrote: > >> >> > >> >> Hi, Barry, > >> >> > >> >> Barry Song <21cnbao@gmail.com> writes: > >> >> > >> >> > From: Barry Song > >> >> > > >> >> > Commit 13ddaf26be32 ("mm/swap: fix race when skipping swapcache") > >> >> > introduced an unconditional one-tick sleep when `swapcache_prepare()` > >> >> > fails, which has led to reports of UI stuttering on latency-sensitive > >> >> > Android devices. To address this, we can use a waitqueue to wake up > >> >> > tasks that fail `swapcache_prepare()` sooner, instead of always > >> >> > sleeping for a full tick. While tasks may occasionally be woken by an > >> >> > unrelated `do_swap_page()`, this method is preferable to two scenarios: > >> >> > rapid re-entry into page faults, which can cause livelocks, and > >> >> > multiple millisecond sleeps, which visibly degrade user experience. > >> >> > >> >> In general, I think that this works.  Why not extend the solution to > >> >> cover schedule_timeout_uninterruptible() in __read_swap_cache_async() > >> >> too?  We can call wake_up() when we clear SWAP_HAS_CACHE.  To avoid > >> > > >> > Hi Ying, > >> > Thanks for your comments. > >> > I feel extending the solution to __read_swap_cache_async() should be done > >> > in a separate patch. On phones, I've never encountered any issues reported > >> > on that path, so it might be better suited for an optimization rather than a > >> > hotfix? > >> > >> Yes.  It's fine to do that in another patch as optimization. > > > > Ok. I'll prepare a separate patch for optimizing that path. > > Thanks! > > >> > >> >> overhead to call wake_up() when there's no task waiting, we can use an > >> >> atomic to count waiting tasks. > >> > > >> > I'm not sure it's worth adding the complexity, as wake_up() on an empty > >> > waitqueue should have a very low cost on its own? > >> > >> wake_up() needs to call spin_lock_irqsave() unconditionally on a global > >> shared lock.  On systems with many CPUs (such servers), this may cause > >> severe lock contention.  Even the cache ping-pong may hurt performance > >> much. > > > > I understand that cache synchronization was a significant issue before > > qspinlock, but it seems to be less of a concern after its implementation. > > Unfortunately, qspinlock cannot eliminate cache ping-pong issue, as > discussed in the following thread. > > https://lore.kernel.org/lkml/20220510192708.GQ76023@worktop.programming.kicks-ass.net/ > > > However, using a global atomic variable would still trigger cache broadcasts, > > correct? > > We can only change the atomic variable to non-zero when > swapcache_prepare() returns non-zero, and call wake_up() when the atomic > variable is non-zero.  Because swapcache_prepare() returns 0 most times, > the atomic variable is 0 most times.  If we don't change the value of > atomic variable, cache ping-pong will not be triggered. yes. this can be implemented by adding another atomic variable. > > Hi, Kairui, > > Do you have some test cases to test parallel zram swap-in?  If so, that > can be used to verify whether cache ping-pong is an issue and whether it > can be fixed via a global atomic variable. > Yes, Kairui please run a test on your machine with lots of cores before and after adding a global atomic variable as suggested by Ying. I am sorry I don't have a server machine. if it turns out you find cache ping-pong can be an issue, another approach would be a waitqueue hash: diff --git a/mm/memory.c b/mm/memory.c index 2366578015ad..aae0e532d8b6 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -4192,6 +4192,23 @@ static struct folio *alloc_swap_folio(struct vm_fault *vmf) } #endif /* CONFIG_TRANSPARENT_HUGEPAGE */ +/* + * Alleviating the 'thundering herd' phenomenon using a waitqueue hash + * when multiple do_swap_page() operations occur simultaneously. + */ +#define SWAPCACHE_WAIT_TABLE_BITS 5 +#define SWAPCACHE_WAIT_TABLE_SIZE (1 << SWAPCACHE_WAIT_TABLE_BITS) +static wait_queue_head_t swapcache_wqs[SWAPCACHE_WAIT_TABLE_SIZE]; + +static int __init swapcache_wqs_init(void) +{ + for (int i = 0; i < SWAPCACHE_WAIT_TABLE_SIZE; i++) + init_waitqueue_head(&swapcache_wqs[i]); + + return 0; +} +late_initcall(swapcache_wqs_init); + /* * We enter with non-exclusive mmap_lock (to exclude vma changes, * but allow concurrent faults), and pte mapped but not yet locked. @@ -4204,6 +4221,8 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) { struct vm_area_struct *vma = vmf->vma; struct folio *swapcache, *folio = NULL; + DECLARE_WAITQUEUE(wait, current); + wait_queue_head_t *swapcache_wq; struct page *page; struct swap_info_struct *si = NULL; rmap_t rmap_flags = RMAP_NONE; @@ -4297,12 +4316,16 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) * undetectable as pte_same() returns true due * to entry reuse. */ + swapcache_wq = &swapcache_wqs[hash_long(vmf->address & PMD_MASK, + SWAPCACHE_WAIT_TABLE_BITS)]; if (swapcache_prepare(entry, nr_pages)) { /* * Relax a bit to prevent rapid * repeated page faults. */ + add_wait_queue(swapcache_wq, &wait); schedule_timeout_uninterruptible(1); + remove_wait_queue(swapcache_wq, &wait); goto out_page; } need_clear_cache = true; @@ -4609,8 +4632,10 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) pte_unmap_unlock(vmf->pte, vmf->ptl); out: /* Clear the swap cache pin for direct swapin after PTL unlock */ - if (need_clear_cache) + if (need_clear_cache) { swapcache_clear(si, entry, nr_pages); + wake_up(swapcache_wq); + } if (si) put_swap_device(si); return ret; @@ -4625,8 +4650,10 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) folio_unlock(swapcache); folio_put(swapcache); } - if (need_clear_cache) + if (need_clear_cache) { swapcache_clear(si, entry, nr_pages); + wake_up(swapcache_wq); + } if (si) put_swap_device(si); return ret; -- 2.34.1 > -- > Best Regards, > Huang, Ying Thanks Barry