From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pl1-f172.google.com (mail-pl1-f172.google.com [209.85.214.172]) (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 EB45D1F1921 for ; Thu, 30 Jan 2025 21:36:34 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738272996; cv=none; b=Vk/MQ/Sp7c1C4BpTQLzAfGFLN6GU+JOK6Q26qAVwA/QEeH4ypBPqwHHTZQKp2KdkmeVTANVIkoE9KBunOj9Cp0QaUZwck+a5Rq5sgkrkP3kLxfCBm0+X+EcoiQWBoew8GITHZ9j9CzGuUy+mFA5w+a4opvqZ7zdvJ44g2hIA+hU= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738272996; c=relaxed/simple; bh=3ZdxPvAfcmQNlIJmdWCIqX/gQrdKdUCtNB+7Kj0py+c=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=QVc8P+X7KWn7SRrGo2Tx9k3G8SLLEr1viCnXXmsItP1indqXiKE+amwfvhDii2LBk6Tel8jtu0OHH4rec0Tvua5UeHExxr33KqdOsSKcFfBiyduTqixw+JiUPSMYHeeHJyovqnkym77Zlxgdnd341ipkqb6Hnp1FB2tTY3Boxt4= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=fromorbit.com; spf=pass smtp.mailfrom=fromorbit.com; dkim=pass (2048-bit key) header.d=fromorbit-com.20230601.gappssmtp.com header.i=@fromorbit-com.20230601.gappssmtp.com header.b=a0pwewO4; arc=none smtp.client-ip=209.85.214.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=fromorbit.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=fromorbit.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=fromorbit-com.20230601.gappssmtp.com header.i=@fromorbit-com.20230601.gappssmtp.com header.b="a0pwewO4" Received: by mail-pl1-f172.google.com with SMTP id d9443c01a7336-2164b1f05caso23386285ad.3 for ; Thu, 30 Jan 2025 13:36:34 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=fromorbit-com.20230601.gappssmtp.com; s=20230601; t=1738272994; x=1738877794; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=s9ijuiEZzKjbqvpEPNvRB635Y5i4hqTbnqe3HTPqnhM=; b=a0pwewO442LqYNdI0M0/AHEK+rM+WRAV5DM9N8vGOdL8A/whDNHeJP9h05BvnnXpw/ TUVG0B2mrVxtyHgNujs6JM6QngBXsRjAFkCd/4w44HRjZS/Io7MPPvJJBLZzFVrVxFWc Q9Xx1V37rmqjImPzshcE5JMfdfUexRSCpw6GHG1lEEZojLBZyKG4YXvdEieKCpEeTfz+ ZNJ4wyuW27brg5aUeFdcwbEBQu6ePm4SjPoMq4GxiwqWpliCnPwSnxk3r3/4cfYYR5jX +TSYuxNHHn0XdKbuGMHB44ZZx2IIj/Gg/5BRATzrAC6APNzHWXJwu6tv2ap/QRtM3UY9 tcpw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1738272994; x=1738877794; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=s9ijuiEZzKjbqvpEPNvRB635Y5i4hqTbnqe3HTPqnhM=; b=vYfNrr3VrG6p3DlUIOXN91TJzhjyGG3xXYmDFAYpRavPxl/uEhUb6CXv2k7uw0GuM1 1UNX7fOTflvcpbivaXEfqyzrWYRB2uNVCS7nN/Q7NNi0BsT/RItEr5JJeEpWagf/752M 5aGHLlqS14DSxAhT+nOPiUl15uvjjSsINxLQf9ck2Rq500fT2id9tWWU9oep89C3PAHD POkJp+EQAaE0/5Di9892mYi8Xw+z8SZ1Xd2aa1j4Za4Z+9OVKoVTpYGn578zMPQYQrSX YNJmq41I1a+tu7qQKUbLzY7pEBSoLgdQ4Gi+8UC08smMKSNMu5r9L/5US+28ThSfdQJ5 GOCw== X-Forwarded-Encrypted: i=1; AJvYcCX0L7If39d1geHVI/N/YjsNPML21YE4nu6wxizLYUq3uEc58m8VJ1NGDqvvskchlTtpbLVcL/M+SChLVA==@vger.kernel.org X-Gm-Message-State: AOJu0Yx+q4Pvvihmko8b+CgPyB0txDW+GhghEhqv2oSrbgSP8bJSfyV9 9KIqYnf03eZ6lY/i61zLTSodXbYs2bauvczdsiO5Aa7YbbK/pzbaa0FBChya07o= X-Gm-Gg: ASbGncvX+EsajxoIvZqv+khpBErQoDffh/mu4cZIinlZchVif+8OJNRaRiyNVUt41ka r4YuAlA1+jZEMsLm5MM/gtbvg1XYJyFwmMmvGSg/YhOH13yksP4cVq12XWx1rqAMPK1oAYHCD5x CmWkvBWvkldW0SpClJFYj3z8LUHYAX1uOZsOfM4fTtXKUytINCqMyJDehrnKp0KmKoYdJj4PA6y syI8kDIxeM+Y2ZJh3/DuNwn5LzEqa//eMEH7Er/hk9eX4Rv2DkKX2pc04v5aXE6sBwFAl3sksE3 LwjnvBKn3/QNW95wutyDt7UxwoCQmHMlC75Q7oUvzNjlVt2fN5r05hH3H/oMSZ9ZPsQ= X-Google-Smtp-Source: AGHT+IFi7KP/uFWpdWAegIG7HhkzsEsibzJqO2iNuwK1YAkhLz6wsiYT3eWBWRTxK1zOZlc8uBTEzg== X-Received: by 2002:a05:6a21:4598:b0:1e1:e2d8:fd1d with SMTP id adf61e73a8af0-1ed7a6b1784mr14087254637.33.1738272994219; Thu, 30 Jan 2025 13:36:34 -0800 (PST) Received: from dread.disaster.area (pa49-186-89-135.pa.vic.optusnet.com.au. [49.186.89.135]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-72fe6a1cbc8sm1991253b3a.177.2025.01.30.13.36.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 30 Jan 2025 13:36:33 -0800 (PST) Received: from dave by dread.disaster.area with local (Exim 4.98) (envelope-from ) id 1tdcD8-0000000CZwg-00Ha; Fri, 31 Jan 2025 08:36:30 +1100 Date: Fri, 31 Jan 2025 08:36:29 +1100 From: Dave Chinner To: Dave Hansen Cc: Kent Overstreet , Dave Hansen , linux-kernel@vger.kernel.org, Linus Torvalds , Ted Ts'o , Christian Brauner , "Darrick J. Wong" , Matthew Wilcox , Al Viro , linux-fsdevel@vger.kernel.org, almaz.alexandrovich@paragon-software.com, ntfs3@lists.linux.dev, miklos@szeredi.hu, linux-bcachefs@vger.kernel.org, clm@fb.com, josef@toxicpanda.com, dsterba@suse.com, linux-btrfs@vger.kernel.org, dhowells@redhat.com, jlayton@kernel.org, netfs@lists.linux.dev Subject: Re: [PATCH 0/7] Move prefaulting into write slow paths Message-ID: References: <20250129181749.C229F6F3@davehans-spike.ostc.intel.com> Precedence: bulk X-Mailing-List: linux-btrfs@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: On Thu, Jan 30, 2025 at 08:04:49AM -0800, Dave Hansen wrote: > On 1/29/25 23:44, Kent Overstreet wrote: > > On Wed, Jan 29, 2025 at 10:17:49AM -0800, Dave Hansen wrote: > >> tl;dr: The VFS and several filesystems have some suspect prefaulting > >> code. It is unnecessarily slow for the common case where a write's > >> source buffer is resident and does not need to be faulted in. > >> > >> Move these "prefaulting" operations to slow paths where they ensure > >> forward progress but they do not slow down the fast paths. This > >> optimizes the fast path to touch userspace once instead of twice. > >> > >> Also update somewhat dubious comments about the need for prefaulting. > >> > >> This has been very lightly tested. I have not tested any of the fs/ > >> code explicitly. > > > > Q: what is preventing us from posting code to the list that's been > > properly tested? > > > > I just got another bcachefs patch series that blew up immediately when I > > threw it at my CI. > > > > This is getting _utterly ridiculous_. That's a bit of an over-reaction, Kent. IMO, the developers and/or maintainers of each filesystem have some responsibility to test changes like this themselves as part of their review process. That's what you have just done, Kent. Good work! However, it is not OK to rant about how the proposed change failed because it was not exhaustively tested on every filesytem before it was posted. I agree with Dave - it is difficult for someone to test widepsread changes in code outside their specific expertise. In many cases, the test infrastructure just doesn't exist or, if it does, requires specialised knowledge and tools to run. In such cases, we have to acknowledge that best effort testing is about as good as we can do without overly burdening the author of such a change. In these cases, it is best left to the maintainer of that subsystem to exhaustively test the change to their subsystem.... Indeed, this is the whole point of extensive post-merge integration testing (e.g. the testing that gets run on linux-next -every day-). It reduces the burden on individuals proposing changes created by requiring exhaustive testing before review by amortising the cost of that exhaustive testing over many peer reviewed changes.... > In this case, I started with a single patch for generic code that I knew > I could test. In fact, I even had the 9-year-old binary sitting on my > test box. > > Dave Chinner suggested that I take the generic pattern go look a _bit_ > more widely in the tree for a similar pattern. That search paid off, I > think. But I ended up touching corners of the tree I don't know well and > don't have test cases for. Many thanks for doing the search, identifying all the places where this pattern existed and trying to address them, Dave. > For bcachefs specifically, how should we move forward? If you're happy > with the concept, would you prefer that I do some manual bcachefs > testing? Or leave a branch sitting there for a week and pray the robots > test it? The public automated test robots are horribly unreliable with their coverage of proposed changes. Hence my comment above about the subsystem maintainers bearing some responsibility to test the code as part of their review process.... -Dave. -- Dave Chinner david@fromorbit.com