From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-il1-f171.google.com (mail-il1-f171.google.com [209.85.166.171]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D29CD219A77 for ; Fri, 20 Dec 2024 16:28:48 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.166.171 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1734712130; cv=none; b=OB7e0wdRQTpDwx0tQ6jevlDBHaPe2uapBk6MKqab+xpRRkEjT44aFybs3gjpaZ0KkMpyhNdGA1jZrL9WS2yHBave41SWxICRe7lRPN9rDDNEhw4XkUjUj5FqD+6FcW2sTzHoNhCBDx/UYC59barPefffEsLTS8IG+TiqIessfjI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1734712130; c=relaxed/simple; bh=wErxbGiQwQOV0/BMRhEpLT1uL19MMH7WFbB/s76KBCA=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=MBTEnQt2nJ4EyflR8DCWW3c8U1yFZQpg1f6+nAHbs+KgQ1PnWGM4pk5KR5fx0OwdVUDAgm/8dZhdCAgCZeP9BS3dBSn7F5gymVnSBnmo3BAHujRTdWpFFuZGivzTu0Llu4HxU+4UkRVdbll9QPVnkPBk2DeKMafhmMxIfbHJH9E= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.dk; spf=pass smtp.mailfrom=kernel.dk; dkim=pass (2048-bit key) header.d=kernel-dk.20230601.gappssmtp.com header.i=@kernel-dk.20230601.gappssmtp.com header.b=1/tkvL8E; arc=none smtp.client-ip=209.85.166.171 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.dk Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=kernel.dk Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel-dk.20230601.gappssmtp.com header.i=@kernel-dk.20230601.gappssmtp.com header.b="1/tkvL8E" Received: by mail-il1-f171.google.com with SMTP id e9e14a558f8ab-3a9cb667783so14981275ab.1 for ; Fri, 20 Dec 2024 08:28:48 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel-dk.20230601.gappssmtp.com; s=20230601; t=1734712128; x=1735316928; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=lTrx6c6/GBb7W3eg60/efo35JCfQ6mibVMPttQjcE6k=; b=1/tkvL8ESt0o2h81srIFXlTtmTzQe3UjG6/ntassfo2XmUpzL1Pl7A3bUvqeAKTpMt GFIhPzUaL3xgKimUzoHNJz4MCRR+6Dwthbegtvh9XlsuX7azH39HuEfOztTto4AOrgkC LGRHdj2vPNisI/Ab6Tb2xPM9pHXEbX8b7raeyQXnIus85qzbS/PsCDPW5tbCQetI0CBy dXgtpmtJCB8T+/1DklY+rKgX/fESyZW1KM9CkarGWarOMiegjf2Ce+QwapVuLIrdooiI NNbABjwtsBt40V3ly/YM2GU5u9zGk+ylaVSyFIqbkxomjPkAv48ONamtT9jNIJdhRSTS q3wg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1734712128; x=1735316928; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=lTrx6c6/GBb7W3eg60/efo35JCfQ6mibVMPttQjcE6k=; b=gTT91HRKKUnDTv6KAbaNU2VkxE6FAHc7HXbZcfExlfG/v+u80NwnQaamks1sP8zHWh Nxj6lDrQ32g/LBL8iQTW7qFFT6Bm1qMGtDKWXD9zPWDYF0Q3b9fBE8VF3EJbpwkL/3PJ aqRHvLYNegOa1bqQ1Hush9ehCIKCYpi35tCO5R4K4YyytD/hGQbERIkVD3plzdcXyrm5 7Idcw7BMn9P6zLssoutct6xGVx4VhyJ51pe/Ux8hgdvArpSaL/Pj4/o2roCHuxTLz3ro 2jQH6PtU57Qsqg2pklymDgWvwfy49fOjFplUww+l8hedVEDHw0dchIbZRtwYYU2TIX7x 6A0Q== X-Forwarded-Encrypted: i=1; AJvYcCVmE3tV9UOhXAjyyZ0F+2f66ayQix+NJRoejknbISFVsPeDJI5bNdQLhN2Mq43CcQ9hU5TJL+shka6TARg=@vger.kernel.org X-Gm-Message-State: AOJu0YzePd57SqZlS+Z8PvenID4vhGh8OHfMFA1Rw0o3fV2JX5BoGcCP eDOHMNZE8JeU/+xCYyJ1wkH4Xogpiryv5ouZ8WURlwXYjRZlyOdfFaN8Zkr5Dr8= X-Gm-Gg: ASbGncsKZ0q923VVFcFJtteB0+SpK82xQW1sImc6/d+9UnythdY/Y8zXLJ1nFOjrOag I3p5b0w8sp1p36UAKtJGE5bHdAkQ9PdUcu8O3gO9X8EpPQla4WJwuOKQrLhJ2smvs51B/r86DjN NJH+800+ge/UqgvnqE1qQmfd5xQmiupzwHPZwkujaJoU9XAEjRBtn7OGcCCGDuUQ0auUe+CpIKE EFvNpcBIBy2LbcWS2UQTD1wOFb1MzXXWCj6myevbsTOyHSy8c27 X-Google-Smtp-Source: AGHT+IE946EUyT5HojL8viAlIugGNg08DKK/RMd+Co2dM+HvmIIHoy9fivtm1RgK4iLbzGr2AWIINw== X-Received: by 2002:a05:6e02:16c8:b0:3a7:d84c:f2b0 with SMTP id e9e14a558f8ab-3c2d277f25cmr42718565ab.8.1734712127998; Fri, 20 Dec 2024 08:28:47 -0800 (PST) Received: from [192.168.1.116] ([96.43.243.2]) by smtp.gmail.com with ESMTPSA id 8926c6da1cb9f-4e68bf66f47sm868587173.40.2024.12.20.08.28.46 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 20 Dec 2024 08:28:46 -0800 (PST) Message-ID: <5cb98ddb-744a-4fc8-b793-9dbe56e16f35@kernel.dk> Date: Fri, 20 Dec 2024 09:28:46 -0700 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 06/12] mm/truncate: add folio_unmap_invalidate() helper To: Matthew Wilcox Cc: linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, hannes@cmpxchg.org, clm@meta.com, linux-kernel@vger.kernel.org, kirill@shutemov.name, bfoster@redhat.com References: <20241220154831.1086649-1-axboe@kernel.dk> <20241220154831.1086649-7-axboe@kernel.dk> Content-Language: en-US From: Jens Axboe In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit On 12/20/24 9:21 AM, Matthew Wilcox wrote: > On Fri, Dec 20, 2024 at 08:47:44AM -0700, Jens Axboe wrote: >> +int folio_unmap_invalidate(struct address_space *mapping, struct folio *folio, >> + gfp_t gfp) >> { >> - if (folio->mapping != mapping) >> - return 0; >> + int ret; >> + >> + VM_BUG_ON_FOLIO(!folio_test_locked(folio), folio); >> >> - if (!filemap_release_folio(folio, GFP_KERNEL)) >> + if (folio_test_dirty(folio)) >> return 0; >> + if (folio_mapped(folio)) >> + unmap_mapping_folio(folio); >> + BUG_ON(folio_mapped(folio)); >> + >> + ret = folio_launder(mapping, folio); >> + if (ret) >> + return ret; >> + if (folio->mapping != mapping) >> + return -EBUSY; > > The position of this test confuses me. Usually we want to test > folio->mapping early on, since if the folio is no longer part of this > file, we want to stop doing things to it, rather than go to the trouble > of unmapping it. Also, why do we want to return -EBUSY in this case? > If the folio is no longer part of this file, it has been successfully > removed from this file, right? It's simply doing what the code did before. I do agree the mapping check is a bit odd at that point, but that's how invalidate_inode_pages2_range() and folio_launder() was setup. We can certainly clean that up after the merge of these helpers, but I didn't want to introduce any potential changes with this merge. -EBUSY was the return from a 0 return from those two helpers before. -- Jens Axboe