From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-io1-f53.google.com (mail-io1-f53.google.com [209.85.166.53]) (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 E0AAF13D508 for ; Thu, 2 Jan 2025 20:12:29 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.166.53 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1735848752; cv=none; b=LSbdu5ti98bSnonO/gjYGbBmFhDeAY83dI4JoTzwbanvhA1EgCGB7/ghGBpd5dQvGGZ6DjQEA7ZtMPgdui/vfkcX6vphLnHVNWijamZbqoK3NzIcd+51qU9RfjA/KLq/26rdtS0YSMcuM8fFh9GGxumODH8kf9k5CGyPGd2cxIs= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1735848752; c=relaxed/simple; bh=k1BiPny32Vdcy39cfGCcTfQtxejWNWV0QmLJ9stcBX0=; h=Message-ID:Date:MIME-Version:Subject:From:To:Cc:References: In-Reply-To:Content-Type; b=VhJFplx/U7F43NXQEhv9aZ4PfjzXOsfV8e2gXEJbUGoV4MPGjdRvp65uGVBQOjuVCc+8y8yMtApvAje108hXDznqFxsDc7BOInCh91FVDNydbJE7ttaKF292DkRAfRBMDqxAVJTOKIoJwrM7R4KEk49hyBvFNut+6h9mqkqVhNE= 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=pfO1ILJT; arc=none smtp.client-ip=209.85.166.53 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="pfO1ILJT" Received: by mail-io1-f53.google.com with SMTP id ca18e2360f4ac-844ee43460aso937838239f.1 for ; Thu, 02 Jan 2025 12:12:29 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel-dk.20230601.gappssmtp.com; s=20230601; t=1735848749; x=1736453549; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:content-language:references :cc:to:from:subject:user-agent:mime-version:date:message-id:from:to :cc:subject:date:message-id:reply-to; bh=v2UPZHRZRRh2ttfhUjF6HZBoXRk72xT/3w5rhEqqtEw=; b=pfO1ILJTL0fzKmMHxcVibOheO0IPhmRZD9vBOvvgkv+u723eU8PauZla7u0iNM6qZh LOjIDB7xTYz+32JaoB4QY41nj5/yc5saS6kfVNCAmCls80qVM4kDPZfLyP01UhJFXsJC o8qx1qgRq9909z+kBifvYyd8SWIlEpyKEfQPKSWazcb29K3ArZUQhD8yxNK7KkAQPUyj Rr9TBydCY4ghOXq+G1X4CmM4kJpOH7je2A0MK1M5OG4IuE44h07Dyf3xhTYND17BctUV YvOOrK/7gpByV3TTBESZsBALT/Rye655MCmojCJS7mmoFansXHdk+T11eDnsAORcB51P VQHA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1735848749; x=1736453549; h=content-transfer-encoding:in-reply-to:content-language:references :cc:to:from:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=v2UPZHRZRRh2ttfhUjF6HZBoXRk72xT/3w5rhEqqtEw=; b=gYS146yedXrlV5Kgzt8oY/yZG/675Y6QFeQtw/LWXY7GjuzLvPguLrNMRvVFR1pUSA xN8JogcuB+VYqNNsRfjoisW3VYpZpPQ+9sOOVnpHRqoa7iWpNh6ApV0vkNdZkKVOefKH UpNT0zg+yKfQdkY6SdzBTT+Xl5EBeFya5CFulwI5Rm4GN4i4FtOyfjvk5nk58TWi3eEr ztwkanfEhc9Sn6xvtHbMAdHIanb1jvcS1vHsHx/VgFa/Q7FZ9nChK5tSM34yzJ4QaD5z 9s1Amj/E4LdAWmDGELSwyE1emyzXYVF2jRxXdbSjctrG38zIZffn+OMRW9Tlg0kRCAYb 9Efw== X-Forwarded-Encrypted: i=1; AJvYcCVgYwbnkQV/cpBavWcAvoVu3F4+jH7BQ/5PT5CAvHm4DL1rw1XXRtaekc9XrFuD5k0Vc4XiUh1/eGB3TdY=@vger.kernel.org X-Gm-Message-State: AOJu0YxFOmefQmO4GRf+QmW32zidDHsyT+sFsjRhkcxs4++a/u1KegtZ Upz7RZiSvDeu93/aayGR2zFpAiV/pgf/ixjeFtx2bBgeLKer7nsEjm/UlHJ8mm4= X-Gm-Gg: ASbGncvnJXbgrdmwh3/o4r3ajtiVxBBWb+FQ1iuUTWWLf1uQMAYhpUa9cHiYAlnxDXv RZfBfqVOku2au6ASmd84auwDo58CE/Ptw0otX1ruQQsgItFdfh8T2/IMo5pLtXVzYe4Ryk6k/U6 iLARWpoScpf/JwIL1EKBujvLoNmfjEgfN8nexDo4W141vHbu6ybBc98Bp90RbdHdl1K/EepGAiE PCOdX73S16rP4WVv2OxpEO5DMspVBspcCOVfHmAfUUdhAZ0//E1hw== X-Google-Smtp-Source: AGHT+IEYlb99o+N5pvNX82iwMSNOKAxLdsrEbVqa3Z0i9O3bTDsJaWg1lIWWOB2JxeZcRAFbxYbMjw== X-Received: by 2002:a05:6602:14d5:b0:83a:b500:3513 with SMTP id ca18e2360f4ac-8499e4ee9a6mr4443687939f.8.1735848748939; Thu, 02 Jan 2025 12:12:28 -0800 (PST) Received: from [192.168.1.150] ([198.8.77.157]) by smtp.gmail.com with ESMTPSA id 8926c6da1cb9f-4e68bf64f29sm7341845173.43.2025.01.02.12.12.27 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 02 Jan 2025 12:12:28 -0800 (PST) Message-ID: Date: Thu, 2 Jan 2025 13:12:27 -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 From: Jens Axboe 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> <5cb98ddb-744a-4fc8-b793-9dbe56e16f35@kernel.dk> Content-Language: en-US In-Reply-To: <5cb98ddb-744a-4fc8-b793-9dbe56e16f35@kernel.dk> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit On 12/20/24 9:28 AM, Jens Axboe wrote: > 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. Any further concerns with this? Trying to nudge this patchset forward... It's not like there's a lot of time left for 6.14. -- Jens Axboe