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 55E4FC4345F for ; Wed, 24 Apr 2024 02:56:38 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A7A966B01B3; Tue, 23 Apr 2024 22:56:37 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id A2AE76B01F0; Tue, 23 Apr 2024 22:56:37 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8CAB26B01F1; Tue, 23 Apr 2024 22:56:37 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 6EF436B01B3 for ; Tue, 23 Apr 2024 22:56:37 -0400 (EDT) Received: from smtpin27.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id E771640116 for ; Wed, 24 Apr 2024 02:56:36 +0000 (UTC) X-FDA: 82042912392.27.6BF12C4 Received: from mail-lj1-f177.google.com (mail-lj1-f177.google.com [209.85.208.177]) by imf07.hostedemail.com (Postfix) with ESMTP id 1E4234000D for ; Wed, 24 Apr 2024 02:56:34 +0000 (UTC) Authentication-Results: imf07.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=XbnARx4p; spf=pass (imf07.hostedemail.com: domain of ryncsn@gmail.com designates 209.85.208.177 as permitted sender) smtp.mailfrom=ryncsn@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1713927395; 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=1Ed5BXb+3yDcaHzMwmV//gTO7icCBen+OQNCtXVGBFI=; b=wTvEExqYlZ7hHrzz6VR9LiT3AlRdYaOah1asyNo9JzG2/IQ+puKLmIhm/43Mnb8qEYDVi8 lDxL1sQ/gE0fA6JHL0kxq+ZS9ogMcDVR/+QVpBuWudbLbynuqkHiVa3ZdYI8NTXqR/Efxb Q8VWleTQiKNDfxCndsMtEOEGpkjDFCw= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1713927395; a=rsa-sha256; cv=none; b=DQWZO5lMmNY8et7OPjuTwZn//cdzJKwS3QYzgTuLynJo4wqnMvmAwrIIlQGtAKk2o636FW Oxp1oil/1JFQSn16nOSoNV9QE6ZYiTihB5/hqb288atwfa37GsRGOcLkmCqzfHPmXYoxbx LwZp9o+n94+xT5weH2NppLgYFCR4yv0= ARC-Authentication-Results: i=1; imf07.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=XbnARx4p; spf=pass (imf07.hostedemail.com: domain of ryncsn@gmail.com designates 209.85.208.177 as permitted sender) smtp.mailfrom=ryncsn@gmail.com; dmarc=pass (policy=none) header.from=gmail.com Received: by mail-lj1-f177.google.com with SMTP id 38308e7fff4ca-2dae975d0dcso4869771fa.1 for ; Tue, 23 Apr 2024 19:56:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1713927393; x=1714532193; 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=1Ed5BXb+3yDcaHzMwmV//gTO7icCBen+OQNCtXVGBFI=; b=XbnARx4pTj4LqknjsyDWTDmMPO0sHmm4C6JDbULixAWeENfgtayvRPNc74OIs0Z0Ku b5c8mmkJh+6628PG8XH+Fyc34xymLABP0pKr+zrpNQTOhAYS9rvQdsuH079Iux5KgiYd k7lY3f00YmcWFWpDh39ixAtVAsCHxbwRrQG1TetQDGvfrHw0O3sqJlm2bYEW4U0/XTvY CMtdCKKXSNSYcsqW+oGp+f+MfeW7is8guEp/J/qs+mVPn4g0jkWjzFh5d5u6aE3A6xIn QjhqwfZWtJNkPJy+ekoPp7On16cQm6A1Dj8lK30bVDKSwfUxhlIeqhOztrIA22H6dQP9 gtbw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713927393; x=1714532193; 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=1Ed5BXb+3yDcaHzMwmV//gTO7icCBen+OQNCtXVGBFI=; b=owDZZsycZVwsNNzOP8aA2q4BOZ5JBl+YZidBkj6AEMfs+H3q3ubgu2Iy789Pj23kWg FfFr9gdwOLWLn+7Eb8j11baRQYLJFSiovTYVxSAC0fIDd7V8FLJsPjBSjpHip3BifSGf tGCMem9hf6HNc3GYHRH0JnGIEFoCMU+UNCVzkeaQzZT0qX9cR151xC+epxUXxQLGBB6M M77h22+4IygvgWONalXZ13XrhbOqLUil63Dyuoak85CjmKJRHfgi1wtSakJyFuKGnX4Q tltSXPIe4O7u+VQxQIWOeaohjVoe/BPZ3cFtjuLiQU4JbwsKUe48jmYGp9H1jJ0SUWCO FntQ== X-Gm-Message-State: AOJu0YxVqwwrHsUxOujPYUQblzyFl8cGuM6/DxsFNtef9ENwlQoujIyu q3oTED5R1Q/8CnfE3eWqyCCJjS2hWmbNlewfRCgKuLx3RGCAb3xouptZ13Vu6zxy7SZW/bPP2pa xb5j93qrH93lWih5XAObADIJF5rU= X-Google-Smtp-Source: AGHT+IFVKtxLp5hgNBcE7pLqHamYU9crgavsCYxLiK2bSQMn+2ebKbKDEkz9yDM+sacn5LQH4J36EjHpzfhSrPlgcCw= X-Received: by 2002:a2e:854e:0:b0:2de:2f6e:6375 with SMTP id u14-20020a2e854e000000b002de2f6e6375mr418505ljj.4.1713927393077; Tue, 23 Apr 2024 19:56:33 -0700 (PDT) MIME-Version: 1.0 References: <20240423170339.54131-1-ryncsn@gmail.com> <20240423170339.54131-8-ryncsn@gmail.com> <87sezbsdwf.fsf@yhuang6-desk2.ccr.corp.intel.com> In-Reply-To: <87sezbsdwf.fsf@yhuang6-desk2.ccr.corp.intel.com> From: Kairui Song Date: Wed, 24 Apr 2024 10:56:16 +0800 Message-ID: Subject: Re: [PATCH v2 7/8] mm: drop page_index/page_file_offset and convert swap helpers to use folio To: "Huang, Ying" Cc: linux-mm@kvack.org, Andrew Morton , Matthew Wilcox , Chris Li , Barry Song , Ryan Roberts , Neil Brown , Minchan Kim , Hugh Dickins , David Hildenbrand , Yosry Ahmed , linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, linux-nfs@vger.kernel.org, Trond Myklebust , Anna Schumaker , linux-afs@lists.infradead.org, David Howells , Marc Dionne Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Stat-Signature: ey4seumi9pmjc7x9pfk9xcqjapx66hje X-Rspamd-Queue-Id: 1E4234000D X-Rspamd-Server: rspam06 X-Rspam-User: X-HE-Tag: 1713927394-152563 X-HE-Meta: U2FsdGVkX18qpBsbs9d9Uwkba482QYQGsA7w33i4NUq7FW0AA5mU059xjakRGb4wpVqmezzHA6L/ov8GdxHhZ0j54xrWfp6XqbIzTqloNCyR90LM+Ks67zQ4Ub3uNuHZC2YXSXldXXf3ifQh6lBtCf+hRvDMouvZ1aLdo0SRNdn+q0InVUKy9VkptWxVWjfW10K1364apOSBSjQyVzjLaH8d8ex6Pj35i4bn9s2eJfZqRC5sIDbZnBdgL4PnsD0Wbloa/yxYVd7cp6DGDESXjwsSXrl9eHkPBjlO4e11WUfBU07ajahBjUAwZ0RnySUketgwEx9d8KeGtOkcu55znsK2549OKnF9nQr0d20aAz9wDOESqD2D7Fu1tNpoYhTq5pyZmL2o1JfWnlkcDxCPpQJ9eN9wVil5VBNDUqXEoJ+QOj+Mb7dVxgOLTSnNN0p2XyS3msobFtJEv6xDdSBR/sgqMqB2Evp7OK9qYTdHHdWVyUBNkb1OH/Foyd+sCPpysGNMl5qxWEIMMnZuGmOrNwHbMHQLuM98FjnJcENLUv3Wuu8aJA7kWZBGwvlYk8qXCMcxSYzvxG9sA1BRvP1lrmT66oNb0n05vUSlX0xPiIHNK2DZo159uaYxPMh/+81QUo2Dgt0PAY3ATD5JrVKhZYCFkhBYvuLrIHD4JZEZR5MsB9R657+6aeAaq8xbe10vLlyffpLVqL4GdOm6F8cgiEalKlt9drhOZ1fqo3Fh+cKegOI2bT/sgA2nC2k78hbIy7fYWIVidYhHpPWWL0/k7vgjB+QAd4JgWRrGuXq+aH53NCg76iUkSE1rF7BjnuhSZF8/yFNZ5QkaH2MQvH9bk9hsNeky4AMp7ej0Sr3OeZstWfOd5Xa0oPiuuegFaIojvjpazVcyvbfOHqvv5sV66PqLXrjbb8GJ53ttc/Rma7jTadFMuuOm7qKntVffnvjkahM1gRXH4Yw3nvRqml1 5JVU+KUd OmGFT3kB13kD55NDK9NlrWY8KjYdwJYEVbzV9ue1HHc5ae+y+vbkpCFDeSNXfR5E6P0K7xgxDu4G1QNLGri+QNkOyjIeFJOgfRwv+2JAkyHR5GltA1PmsHn2cLRRvpLwRpvMAnbdkQdJ35ke7N6xAZvqpLyN3HTSpuRPJjb/BHOfLJw+4OEHytNBao5sVLrlb2CCSM3T0NQJ9jV6YQy28pIYEoi1G3/MCFuBVZJtP4vdX4PAWI5e0iha+y9EKrXUZvGHEyw6Mpj6y81BQ3J/+Vq6DSgLDuKROJs0dA802BjRKj1D1Ia6tuoOh05wTLjHWvVRoTC8ngsi90rkKCb7sLhxTj0GfhYXmM50+qrYJMkb+MagCV6uw6p7/G8zB2NR5cen6/MFc5hIoF1thHzMxvxeXVk4sJlRxppknH5rgU7Me2I8yWGRTRKC2lBPl947BodbS1DHi4e3dG1ff2LP0vnn75xZ3b31OpDqXSKHXVnlkyRzTULka9wUu0/XyWnYenHSPnB61zezh3CfTDJyJgz8Eaw== 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, Apr 24, 2024 at 10:19=E2=80=AFAM Huang, Ying = wrote: > > Kairui Song writes: > > > From: Kairui Song > > > > There are four helpers for retrieving the page index within address > > space, or offset within mapped file: > > > > - page_index > > - page_file_offset > > - folio_index (equivalence of page_index) > > - folio_file_pos (equivalence of page_file_offset) > > > > And they are only needed for mixed usage of swap cache & page cache (eg= . > > migration, huge memory split). Else users can just use folio->index or > > folio_pos. > > > > This commit drops page_index and page_file_offset as we have eliminated > > all users, and converts folio_index and folio_file_pos (they were simpl= y > > wrappers of page_index and page_file_offset, and implemented in a not > > very clean way) to use folio internally. > > > > After this commit, there will be only two helpers for users that may > > encounter mixed usage of swap cache and page cache: > > > > - folio_index (calls __folio_swap_cache_index for swap cache folio) > > - folio_file_pos (calls __folio_swap_dev_pos for swap cache folio) > > > > The index in swap cache and offset in swap device are still basically > > the same thing, but will be different in following commits. > > > > Signed-off-by: Kairui Song > > --- > > include/linux/mm.h | 13 ------------- > > include/linux/pagemap.h | 19 +++++++++---------- > > mm/swapfile.c | 13 +++++++++---- > > 3 files changed, 18 insertions(+), 27 deletions(-) > > > > diff --git a/include/linux/mm.h b/include/linux/mm.h > > index 0436b919f1c7..797480e76c9c 100644 > > --- a/include/linux/mm.h > > +++ b/include/linux/mm.h > > @@ -2245,19 +2245,6 @@ static inline void *folio_address(const struct f= olio *folio) > > return page_address(&folio->page); > > } > > > > -extern pgoff_t __page_file_index(struct page *page); > > - > > -/* > > - * Return the pagecache index of the passed page. Regular pagecache p= ages > > - * use ->index whereas swapcache pages use swp_offset(->private) > > - */ > > -static inline pgoff_t page_index(struct page *page) > > -{ > > - if (unlikely(PageSwapCache(page))) > > - return __page_file_index(page); > > - return page->index; > > -} > > - > > /* > > * Return true only if the page has been allocated with > > * ALLOC_NO_WATERMARKS and the low watermark was not > > diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h > > index 2df35e65557d..a7d025571ee6 100644 > > --- a/include/linux/pagemap.h > > +++ b/include/linux/pagemap.h > > @@ -780,7 +780,7 @@ static inline struct page *grab_cache_page_nowait(s= truct address_space *mapping, > > mapping_gfp_mask(mapping)); > > } > > > > -#define swapcache_index(folio) __page_file_index(&(folio)->page) > > +extern pgoff_t __folio_swap_cache_index(struct folio *folio); > > > > /** > > * folio_index - File index of a folio. > > @@ -795,9 +795,9 @@ static inline struct page *grab_cache_page_nowait(s= truct address_space *mapping, > > */ > > static inline pgoff_t folio_index(struct folio *folio) > > { > > - if (unlikely(folio_test_swapcache(folio))) > > - return swapcache_index(folio); > > - return folio->index; > > + if (unlikely(folio_test_swapcache(folio))) > > + return __folio_swap_cache_index(folio); > > + return folio->index; > > } > > > > /** > > @@ -920,11 +920,6 @@ static inline loff_t page_offset(struct page *page= ) > > return ((loff_t)page->index) << PAGE_SHIFT; > > } > > > > -static inline loff_t page_file_offset(struct page *page) > > -{ > > - return ((loff_t)page_index(page)) << PAGE_SHIFT; > > -} > > - > > /** > > * folio_pos - Returns the byte position of this folio in its file. > > * @folio: The folio. > > @@ -934,6 +929,8 @@ static inline loff_t folio_pos(struct folio *folio) > > return page_offset(&folio->page); > > } > > > > +extern loff_t __folio_swap_dev_pos(struct folio *folio); > > + > > /** > > * folio_file_pos - Returns the byte position of this folio in its fil= e. > > * @folio: The folio. > > @@ -943,7 +940,9 @@ static inline loff_t folio_pos(struct folio *folio) > > */ > > static inline loff_t folio_file_pos(struct folio *folio) > > { > > - return page_file_offset(&folio->page); > > + if (unlikely(folio_test_swapcache(folio))) > > + return __folio_swap_dev_pos(folio); > > + return ((loff_t)folio->index << PAGE_SHIFT); > > This still looks confusing for me. The function returns the byte > position of the folio in its file. But we returns the swap device > position of the folio. Thanks for the comment. This doesn't look too confusing to me, __folio_swap_dev_pos -> swap_dev_pos also returns the byte position of the folio in swap device. If we agree swap device is kind of equivalent to the page cache file here, it shouldn't be too hard to understand. > > Tried to search folio_file_pos() usage. The 2 usage in page_io.c is > swap specific, we can use swap_dev_pos() directly. The 2 usage in page_io.c is already converted to use swap_dev_pos directly in this series (patch 6/8). > > There are also other file system users (NFS and AFS) of > folio_file_pos(), I don't know why they need to work with swap > cache. Cced file system maintainers for help. > Thanks, I'm not very sure if we can just drop folio_file_pos and convert all users to use folio_pos directly. Swap cache mapping shouldn't be exposed to fs, but I'm not confident enough that this is a safe move. It looks OK to do so just by examining NFS code, but let's wait for feedback from FS people.