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 16356C27C53 for ; Fri, 7 Jun 2024 04:56:39 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 9B10A6B0098; Fri, 7 Jun 2024 00:56:38 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 961906B009B; Fri, 7 Jun 2024 00:56:38 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 803616B009C; Fri, 7 Jun 2024 00:56:38 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 61C966B0098 for ; Fri, 7 Jun 2024 00:56:38 -0400 (EDT) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id D8CA8161102 for ; Fri, 7 Jun 2024 04:56:37 +0000 (UTC) X-FDA: 82202882034.24.8C73FE0 Received: from mail-ed1-f50.google.com (mail-ed1-f50.google.com [209.85.208.50]) by imf05.hostedemail.com (Postfix) with ESMTP id E50D4100006 for ; Fri, 7 Jun 2024 04:56:35 +0000 (UTC) Authentication-Results: imf05.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b="QC/Dmx4i"; spf=pass (imf05.hostedemail.com: domain of yosryahmed@google.com designates 209.85.208.50 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1717736196; a=rsa-sha256; cv=none; b=wDV6cHd+NhGUzmEqW+dOVTtFRNfKc4VGEf3xiAGiWgXetErcaXDiQHNKCf9+QP6f/vkHcB rI63eV5hjoRiBK5smCy6Gqohtz4+p/e+OXlqFPjM5XIaLqWQEHFR7GYCYvhJyu52oLGxkZ zX6nOIUp8ba5hlVQXvAOO1ccjjtPLMs= ARC-Authentication-Results: i=1; imf05.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b="QC/Dmx4i"; spf=pass (imf05.hostedemail.com: domain of yosryahmed@google.com designates 209.85.208.50 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1717736196; 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=22jJUg56o3o2LRfw08GcS6cO34XlfeFsCoCXxwR36XQ=; b=D4EQw0tZMKYZWNjiFX4Yg4I0T2YnZJRenHhFfGdkMwqiMaNSfPix7eglSI13o9id6rtVyL KOHDtbwKHFoKyJSPTBU8+4k44QPZ6g6B5YYh5tTjJSoFJBoaCIFRLZ6JsMX2r7ktfefiK1 lpnrdzzKsPSCfep6eNEA5J5fT2IaHVg= Received: by mail-ed1-f50.google.com with SMTP id 4fb4d7f45d1cf-57a1fe63a96so2067197a12.0 for ; Thu, 06 Jun 2024 21:56:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1717736194; x=1718340994; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=22jJUg56o3o2LRfw08GcS6cO34XlfeFsCoCXxwR36XQ=; b=QC/Dmx4iml8KrP3BDnySynuvo9gme2npeVkt5vXsIZbyBEyfB3sozdGOgfEvy3W9Je cZE7clGWTbO588GikavvvmK5TpdsWuDsRCmKvkVMdDdSyZF1rr/bRkcKITVQtmakeWxj CN/Z8oqeN37ssaueBjX/i3JTH0LSQTjHX48RVeNtbUOPT0yJCKaC9Ozu+3N1Yxuj1Yyr 2UpwG59qPyb+yGeNanQCp3X5K8EE5qwxRsoqJab45J2gOqtOM/QdDYZTeboLz8kKGCd/ AAXok9z7WzDAVAfyD3n525SLnTh4ejiYnP1/M4hWXzTO34C2R5TVtS/Qb4n+WEYExZDi H0RA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1717736194; x=1718340994; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=22jJUg56o3o2LRfw08GcS6cO34XlfeFsCoCXxwR36XQ=; b=BEiBBBeUUHJ/fIN3NafpVrhcvRkhj4EVJdlOSWfYylIQoabTYh15WrgjvfpTKuHokN euHx5waLxBBtrcHhFMyChivKC0Dc8j7XIb9e1SfVpmP/Sevq2RFnHs77ioBn8ajzoq3B AR7aetUF7B/Hqr1l9ULK/cS7YZtv4MO1UUwMyae+r7h/LjLvRznYEVHpbh3B2XK+Y2GG MHe2QAiLLHMu4BxFBamXmRqC5xdWH31qKzmzBjNpaXsBmc8RBtVEX7kWeVU/YjkYEGoz VCnc4L2FhycPBGhZd8kmQQo/Na7yRTVwmMW5/Vz7VH+J5RPCs4DwwfExzMg3Jm3Jau3y qivg== X-Forwarded-Encrypted: i=1; AJvYcCVx59f9I7xbl8j8QNq08xPqAQKez2JsW84cPj62N8ojMtsMAijr16kU/30lQ7JbUOwYOJIr65NQg+2v804o5bISce4= X-Gm-Message-State: AOJu0YzLZe9Rn9qN/GMLmI8lqyR98l7KksSBpsfa591b+b08R9OEYhOB SMpwY7D4CI2qkdBsttwL7K9lEIztOqitLDuhkF0TI1hIbrV0odLV2cwNzfn5dkDLOExfNbW/Vnu HgzNZ/PqasBKYdFsFlFJtFc2eHYb+LbCwgtvz X-Google-Smtp-Source: AGHT+IGv2nlZeHssEN9DHKwmokSQQGRw0Hv+ylewt2D2lkdahoDhm3urF9JZ4ZWjk4uZ0m/tXnH9Nw+rdMO6kvitdO0= X-Received: by 2002:a17:906:6953:b0:a6c:73a0:7dc3 with SMTP id a640c23a62f3a-a6cd5612064mr93148766b.9.1717736194065; Thu, 06 Jun 2024 21:56:34 -0700 (PDT) MIME-Version: 1.0 References: <20240606210840.1650341-1-yosryahmed@google.com> <871q598qy0.fsf@yhuang6-desk2.ccr.corp.intel.com> In-Reply-To: <871q598qy0.fsf@yhuang6-desk2.ccr.corp.intel.com> From: Yosry Ahmed Date: Thu, 6 Jun 2024 21:55:55 -0700 Message-ID: Subject: Re: [PATCH] mm: swap: remove 'synchronous' argument to swap_read_folio() To: "Huang, Ying" Cc: Andrew Morton , linux-mm@kvack.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: E50D4100006 X-Rspam-User: X-Rspamd-Server: rspam12 X-Stat-Signature: nbzjajkzasch38erjrraqhmosejtdpmh X-HE-Tag: 1717736195-169070 X-HE-Meta: U2FsdGVkX19jxRA0LvmgNMK/UraGY7+wLCHWg6hkah2sZwEYK1aye2u/82m+GAC32LJQ2ljWcjrCEbZ6/H04+SFXRxbSq9+dRfTxVbdor3ekrWyOxSCCCWzSCIgTc6vtkoWnxu9hwZp62g1Sy5RVo7wWz+Vw3YLt9k3GnNiG/zIjxgZrnwyALeGA/LrzC6rASFH5mIdQilzHWvRRz94VM7h5F3CfNY40AAOQ1yKqZlYHT0UDvXVWJRrB4Uel6sVX2Fa/vrP9q0vnkd/WAWDLsj8J6zgqK9iz9cBchdprjKNYwe3XkqRXjjdEhAUPOQO/ONT8l61IC1P30SEOiRz9trrLQ02AXSUuqKNZIb6ei7iSia7KDSIoYVGpCePfL79iT1wgrg7H7YTDpvbYxTpBcICXEHz1iRyidrKRWefO8rXSCljbADGLRA8CAtywUrl99GEH2md1hQzjCy9oBkuV7JPg9W06hpG9aZEsSIJYW8nC3UMCEDShH4OD5Bi0oEzf1i4A8apNw1z26vMvN9YSFGS76oI2qIrfwr7ANBjKxFQf+fOEI7h63HGz8fU013rtZn3Hbol/LFGB53cU7S2GOc9H5Z1ouyQ4p6CQ+1EVvUjgxgcZF6B60Fvdi3v5w5Ilj1Oo4rXO3UR+zpRXHlL/TaNGr3lV8HhCPBa4UAIz9HIOi8KK310n0oWTONdh4VNKdQURUZAkurNnH9HEKgcsG9aGlhWxc5fdwfTcsoVAqzrLkGK0pnpoKf3NF18xd5jVUnTGWOurYJuBishSXDi1sUK85CyDdeZi5u6noLJZTafKgWxRFskn/A0Q6PiVVj7aqHTSTo6aDPSd5UX8TbyfRTNhnRvootboN+pR2Y65m66BhChxsI0PzoN92slvDrOOYjh9FgsA0Q0MvUsmHV1JQsiXggY5slTLfV4PwdcEf/zejxF9iSqD0VUKsraQi2nUjB2xGn1vKX7iGwFBMIV OCr3g4io rEs6K35iIgZPIWifte+NNJl/EjkiUOybyUKUQhETW//2iqGwNRW8hOWjp8HrlvEh9roKa4nR3B2ufRi1H0oWhJ2SDwCewxcgwRjCSlsuuq44QyrUb5/wdBYQMbqSY/aCAcEi5cIxDkycnZHLQbCfSuDnCtJSQwaIStexe30m7lqjUtslRpeuSpMvUBHlAzAJ/3ClpWGuy1DNMru9gZtbJnvLPsVOUQSEc+JB0TXycG3OZvCD/5xQtsh1SAoCTWYMrohZNeoehQxB4jFLFyBsL3yHm+yxmhnTZnMh72b+9FRmpKV06O54HeOgxeuMcj8XtU2yp 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 Thu, Jun 6, 2024 at 6:48=E2=80=AFPM Huang, Ying w= rote: > > Yosry Ahmed writes: > > > swap_read_folio() reads the folio synchronously if synchronous is passe= d > > as true or if SWP_SYNCHRONOUS_IO is set in swap_info_struct. The only > > caller that passes synchronous=3Dtrue is in do_swap_page() in the > > SWP_SYNCHRONOUS_IO case. > > > > Hence, the argument is redundant, it is only set to true when the swap > > read would have been synchronous anyway. Remove it. > > > > Signed-off-by: Yosry Ahmed > > LGTM, Thanks! > > Reviewed-by: "Huang, Ying" Thanks! > > And, there's some history information in commit b243dcbf2f13 ("swap: > remove remnants of polling from read_swap_cache_async"). > > " > Commit [1] introduced IO polling support duding swapin to reduce swap= read > latency for block devices that can be polled. However later commit [= 2] > removed polling support. Therefore it seems safe to remove do_poll > parameter in read_swap_cache_async and always call swap_readpage with > synchronous=3Dfalse waiting for IO completion in folio_lock_or_retry. > > [1] commit 23955622ff8d ("swap: add block io poll in swapin path") > [2] commit 9650b453a3d4 ("block: ignore RWF_HIPRI hint for sync dio") > " > > IMO, it will help people to understand the code change to add some > history information as above, or refer to the commit. I sent a v2 with some history in the commit log. > > -- > Best Regards, > Huang, Ying > > > --- > > mm/memory.c | 2 +- > > mm/page_io.c | 6 +++--- > > mm/swap.h | 6 ++---- > > mm/swap_state.c | 10 +++++----- > > 4 files changed, 11 insertions(+), 13 deletions(-) > > > > diff --git a/mm/memory.c b/mm/memory.c > > index db91304882312..2b3ef08e8bb7d 100644 > > --- a/mm/memory.c > > +++ b/mm/memory.c > > @@ -4113,7 +4113,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) > > > > /* To provide entry to swap_read_folio() = */ > > folio->swap =3D entry; > > - swap_read_folio(folio, true, NULL); > > + swap_read_folio(folio, NULL); > > folio->private =3D NULL; > > } > > } else { > > diff --git a/mm/page_io.c b/mm/page_io.c > > index 41e8d738c6d28..f1a9cfab6e748 100644 > > --- a/mm/page_io.c > > +++ b/mm/page_io.c > > @@ -493,10 +493,10 @@ static void swap_read_folio_bdev_async(struct fol= io *folio, > > submit_bio(bio); > > } > > > > -void swap_read_folio(struct folio *folio, bool synchronous, > > - struct swap_iocb **plug) > > +void swap_read_folio(struct folio *folio, struct swap_iocb **plug) > > { > > struct swap_info_struct *sis =3D swp_swap_info(folio->swap); > > + bool synchronous =3D sis->flags & SWP_SYNCHRONOUS_IO; > > bool workingset =3D folio_test_workingset(folio); > > unsigned long pflags; > > bool in_thrashing; > > @@ -521,7 +521,7 @@ void swap_read_folio(struct folio *folio, bool sync= hronous, > > folio_unlock(folio); > > } else if (data_race(sis->flags & SWP_FS_OPS)) { > > swap_read_folio_fs(folio, plug); > > - } else if (synchronous || (sis->flags & SWP_SYNCHRONOUS_IO)) { > > + } else if (synchronous) { > > swap_read_folio_bdev_sync(folio, sis); > > } else { > > swap_read_folio_bdev_async(folio, sis); > > diff --git a/mm/swap.h b/mm/swap.h > > index 2c0e96272d498..baa1fa946b347 100644 > > --- a/mm/swap.h > > +++ b/mm/swap.h > > @@ -11,8 +11,7 @@ struct mempolicy; > > /* linux/mm/page_io.c */ > > int sio_pool_init(void); > > struct swap_iocb; > > -void swap_read_folio(struct folio *folio, bool do_poll, > > - struct swap_iocb **plug); > > +void swap_read_folio(struct folio *folio, struct swap_iocb **plug); > > void __swap_read_unplug(struct swap_iocb *plug); > > static inline void swap_read_unplug(struct swap_iocb *plug) > > { > > @@ -83,8 +82,7 @@ static inline unsigned int folio_swap_flags(struct fo= lio *folio) > > } > > #else /* CONFIG_SWAP */ > > struct swap_iocb; > > -static inline void swap_read_folio(struct folio *folio, bool do_poll, > > - struct swap_iocb **plug) > > +static inline void swap_read_folio(struct folio *folio, struct swap_io= cb **plug) > > { > > } > > static inline void swap_write_unplug(struct swap_iocb *sio) > > diff --git a/mm/swap_state.c b/mm/swap_state.c > > index 0803eedeabe3d..994723cef8217 100644 > > --- a/mm/swap_state.c > > +++ b/mm/swap_state.c > > @@ -567,7 +567,7 @@ struct folio *read_swap_cache_async(swp_entry_t ent= ry, gfp_t gfp_mask, > > mpol_cond_put(mpol); > > > > if (page_allocated) > > - swap_read_folio(folio, false, plug); > > + swap_read_folio(folio, plug); > > return folio; > > } > > > > @@ -684,7 +684,7 @@ struct folio *swap_cluster_readahead(swp_entry_t en= try, gfp_t gfp_mask, > > if (!folio) > > continue; > > if (page_allocated) { > > - swap_read_folio(folio, false, &splug); > > + swap_read_folio(folio, &splug); > > if (offset !=3D entry_offset) { > > folio_set_readahead(folio); > > count_vm_event(SWAP_RA); > > @@ -701,7 +701,7 @@ struct folio *swap_cluster_readahead(swp_entry_t en= try, gfp_t gfp_mask, > > &page_allocated, false); > > if (unlikely(page_allocated)) { > > zswap_folio_swapin(folio); > > - swap_read_folio(folio, false, NULL); > > + swap_read_folio(folio, NULL); > > } > > return folio; > > } > > @@ -834,7 +834,7 @@ static struct folio *swap_vma_readahead(swp_entry_t= targ_entry, gfp_t gfp_mask, > > if (!folio) > > continue; > > if (page_allocated) { > > - swap_read_folio(folio, false, &splug); > > + swap_read_folio(folio, &splug); > > if (addr !=3D vmf->address) { > > folio_set_readahead(folio); > > count_vm_event(SWAP_RA); > > @@ -853,7 +853,7 @@ static struct folio *swap_vma_readahead(swp_entry_t= targ_entry, gfp_t gfp_mask, > > &page_allocated, false); > > if (unlikely(page_allocated)) { > > zswap_folio_swapin(folio); > > - swap_read_folio(folio, false, NULL); > > + swap_read_folio(folio, NULL); > > } > > return folio; > > }