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 2AC25C54E5D for ; Wed, 13 Mar 2024 03:23:30 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 81985940008; Tue, 12 Mar 2024 23:23:29 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 7CA016B02ED; Tue, 12 Mar 2024 23:23:29 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6B927940008; Tue, 12 Mar 2024 23:23:29 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 5C7B46B02EC for ; Tue, 12 Mar 2024 23:23:29 -0400 (EDT) Received: from smtpin02.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 0227BA1224 for ; Wed, 13 Mar 2024 03:23:28 +0000 (UTC) X-FDA: 81890570538.02.4604BEC Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf17.hostedemail.com (Postfix) with ESMTP id 731C74000E for ; Wed, 13 Mar 2024 03:23:27 +0000 (UTC) Authentication-Results: imf17.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=Ainxmob+; spf=none (imf17.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1710300207; 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=UhjKn9B4rdk20apnL6IkDxxDxn7j7FLTZ6GH+QAWF24=; b=8j0IAvT3NgYpkPfFJaFIc1aipFvXykFiwFk0OpAfX2zDhFCOFiYo+oyv8lRGYQolEOqbFX uQVhRQikki08H40dF/tr3PLNypThbBcAbCG8rlJwk17PCO0k9Rqbn+GpsskBx7UVmMiNvM v1kL5r7kvDp7ltzsKRSS7AfIZbwnSQE= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1710300207; a=rsa-sha256; cv=none; b=XdWP7cq+eW5anzJOVOCWMXaGsT7H8cxqfYXqkBndR04Iau2bmRBLCWLfCoArOVFRQjZ2wJ 8D4NZ1tZCRw73byvcy+KANuh19G/uX9pbH2HbrRxTiEUgD08t0K2njlrDgYCTDys2hrNCz KihY4SX02UnSyLF9CeF7SgGQ1vlKrGU= ARC-Authentication-Results: i=1; imf17.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=Ainxmob+; spf=none (imf17.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org; dmarc=none DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Transfer-Encoding: Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date: Sender:Reply-To:Content-ID:Content-Description; bh=UhjKn9B4rdk20apnL6IkDxxDxn7j7FLTZ6GH+QAWF24=; b=Ainxmob+GX048SRSWjx3dm21AO HaL3TqnBufHYu7PMXBjK5Ns1JqhirE4LK3HTu3uWQNn4veWKtApwb8tDWyRj3jvTbcBawSB8MUmmC SSADuM9fwFdaHrl4P55zqlTh4BSH7IB3xFO2aafV89YJxO0gm2qJSy4uqhRGYQfjTNR6U8A6VJEbJ nH73aA8A4iUNye46K6dQ8Hl59HTfUUWVXnxg8Vo6BqLd/HaoWt6bURc1tG4fuebaVWqnP8DuSYeKG OFry6jmuxmNw3mlcKRTh+fY2Yy64EyqYH3u1mWX9kxyTnKoBLrQQM1tUMyBOaAXMcQnTLSssBMXi3 F0i7YqWA==; Received: from willy by casper.infradead.org with local (Exim 4.97.1 #2 (Red Hat Linux)) id 1rkFD4-00000004VzR-41CM; Wed, 13 Mar 2024 03:23:19 +0000 Date: Wed, 13 Mar 2024 03:23:18 +0000 From: Matthew Wilcox To: Jane Chu Cc: Andrew Morton , ruansy.fnst@fujitsu.com, linux-mm@kvack.org, Miaohe Lin , Naoya Horiguchi , Dan Williams Subject: Re: [PATCH 1/8] mm/memory-failure: Remove fsdax_pgoff argument from __add_to_kill Message-ID: References: <20240229212036.2160900-1-willy@infradead.org> <20240229212036.2160900-2-willy@infradead.org> <35c987d6-e78e-41df-a157-cc764245fa30@oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <35c987d6-e78e-41df-a157-cc764245fa30@oracle.com> X-Rspamd-Queue-Id: 731C74000E X-Rspam-User: X-Stat-Signature: t3a3rxnhibogf4ygfsyunkzrs8iwia6t X-Rspamd-Server: rspam03 X-HE-Tag: 1710300207-42949 X-HE-Meta: U2FsdGVkX18A7d+TN2dDC8Gge4jSHx/6K98zSgVQyvZH0iQAsguE1kqa7AS/d8X9gn3ZRVX9jKBQ7/ZuRRG0NthHHIm1yo3S7QyrBb2uVtsNwObXsadfHMXKODxPvghl3lghiSpbziZIJvVIsv1N7y2PBjmc9TBF/CpBN0C7wUtRnKAYqAloj8pCgw5XswXYhKtgMl+eR+dltP7i9aT7WGWUSgTZwO1qRI+p9Yaaeyf9YLG9mBnjsINZ2IxfFqbh6G6ewd+go+C0gMo8LRqQ3glvhTgoJYgIsYrpwtTFlHOqWN1Q3wM1zGX5QrHJqlgEQdfEwR+oKx7NKfhmbTehe+KKNqBmpIgqtBzJnIvnV7lkcwl/H4G6l2tHeivtcXnh1+IQ7wA8ngVj/KSBGGLeWQvXVcCWfQIiviHWR5b7R3PmVK5BTgpvgyLMCOqKF9L9MVb8SRht0YfGYy9g8SzeGqKiZ7aJPOZxTEhLV/nBuSO79uMAc2TsDMn9EISoWf4L+H0GYirFCufhOUetQr1Pecip40HphlFplOreHM7PmKOfcnz6xlYsAoJdtezDan9CKJFULseKiCrvzObS2rA/mOGA//tl1TU9LjMwD/X0lxXhGu2W/zCQsNeTJAsaBu1699nC/wsP6jQzc42H6+RupRcFQa+k6iIyu72WhhPbfPiAEl8Cxmt5fEySW9KULlm83czIG8oKxSVHppeTtxaS8cHvuSlIhOCdviAziZvlC3PWUqRlCE+vpgUh36f9/IMD4jVTqDcEp7Q9WhN05bEKYEW/xjcGDhPYU2N3/qsYAOmK3yc3i1ssNMi0kaQxFXRilTO3PfxPlB8eMxpJJ8zqs92CRSdkCjOi2EqTlnAHkTTm/r35PlUk0WEwBZrr5ru2 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 Tue, Mar 12, 2024 at 07:07:10PM -0700, Jane Chu wrote: > On 2/29/2024 1:20 PM, Matthew Wilcox (Oracle) wrote: > > > Unify the KSM and DAX codepaths by calculating the addr in > > add_to_kill_fsdax() instead of telling __add_to_kill() to calculate it. > > > > Signed-off-by: Matthew Wilcox (Oracle) > > Cc: Dan Williams > > --- > > mm/memory-failure.c | 27 +++++++++------------------ > > 1 file changed, 9 insertions(+), 18 deletions(-) > > > > diff --git a/mm/memory-failure.c b/mm/memory-failure.c > > index 9349948f1abf..9356227a50bb 100644 > > --- a/mm/memory-failure.c > > +++ b/mm/memory-failure.c > > @@ -416,21 +416,13 @@ static unsigned long dev_pagemap_mapping_shift(struct vm_area_struct *vma, > > * not much we can do. We just print a message and ignore otherwise. > > */ > > -#define FSDAX_INVALID_PGOFF ULONG_MAX > > - > > /* > > * Schedule a process for later kill. > > * Uses GFP_ATOMIC allocations to avoid potential recursions in the VM. > > - * > > - * Note: @fsdax_pgoff is used only when @p is a fsdax page and a > > - * filesystem with a memory failure handler has claimed the > > - * memory_failure event. In all other cases, page->index and > > - * page->mapping are sufficient for mapping the page back to its > > - * corresponding user virtual address. > > */ > > static void __add_to_kill(struct task_struct *tsk, struct page *p, > > struct vm_area_struct *vma, struct list_head *to_kill, > > - unsigned long ksm_addr, pgoff_t fsdax_pgoff) > > + unsigned long addr) > > { > > struct to_kill *tk; > > @@ -440,12 +432,10 @@ static void __add_to_kill(struct task_struct *tsk, struct page *p, > > return; > > } > > - tk->addr = ksm_addr ? ksm_addr : page_address_in_vma(p, vma); > > - if (is_zone_device_page(p)) { > > - if (fsdax_pgoff != FSDAX_INVALID_PGOFF) > > - tk->addr = vma_pgoff_address(fsdax_pgoff, 1, vma); > > + tk->addr = addr ? addr : page_address_in_vma(p, vma); > > + if (is_zone_device_page(p)) > > tk->size_shift = dev_pagemap_mapping_shift(vma, tk->addr); > > Not sure about the simplification.  The fsdax special calculation was added by > > commit c36e2024957120566efd99395b5c8cc95b5175c1 > Author: Shiyang Ruan > Date:   Fri Jun 3 13:37:29 2022 +0800 > >     mm: introduce mf_dax_kill_procs() for fsdax case > >     This new function is a variant of mf_generic_kill_procs that accepts a >     file, offset pair instead of a struct to support multiple files sharing a >     DAX mapping.  It is intended to be called by the file systems as part of >     the memory_failure handler after the file system performed a reverse >     mapping from the storage address to the file and file offset. > >     Link: https://lkml.kernel.org/r/20220603053738.1218681-6-ruansy.fnst@fujitsu.co > m > [..] > > @@ -354,9 +357,15 @@ static void add_to_kill(struct task_struct *tsk, struct page *p, > >         } > >         tk->addr = page_address_in_vma(p, vma); > > -       if (is_zone_device_page(p)) > > -               tk->size_shift = dev_pagemap_mapping_shift(p, vma); > > -       else > > +       if (is_zone_device_page(p)) { > > +               /* > > +                * Since page->mapping is not used for fsdax, we need > > +                * calculate the address based on the vma. > > +                */ > > +               if (p->pgmap->type == MEMORY_DEVICE_FS_DAX) > > +                       tk->addr = vma_pgoff_address(fsdax_pgoff, 1, vma); > > +               tk->size_shift = dev_pagemap_mapping_shift(vma, tk->addr); > > +       } else > >                 tk->size_shift = page_shift(compound_head(p)); > > Looks like it was to deal away with this check > > unsignedlongpage_address_in_vma (structpage > *page > ,structvm_area_struct > *vma > ) > { [..] > > }elseif(vma ->vm_file > ->f_mapping > !=folio > ->mapping > ){ > return-EFAULT ; > } > > But, by providing nr=1 in vma_pgoff_address(fsdax_pgoff, 1, vma), it also avoided dealing with > > wrap around, which perhaps doesn't matter? perhaps noone runs fsdax on 32-bit machine? I don't understand what you're saying is wrong with this patch. All I'm doing (apart from the renaming of ksm_addr to addr) is moving the vma_pgoff_address() call from __add_to_kill() to its caller. Is there some reason this doesn't work?