lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ncewetc7vpijallpvplti4ham7m3dmver6jkymabdedid4iedb@muodfjq2io3m>
Date: Thu, 30 Jan 2025 20:06:36 -0500
From: Kent Overstreet <kent.overstreet@...ux.dev>
To: Dave Chinner <david@...morbit.com>
Cc: Dave Hansen <dave.hansen@...el.com>, 
	Dave Hansen <dave.hansen@...ux.intel.com>, linux-kernel@...r.kernel.org, 
	Linus Torvalds <torvalds@...ux-foundation.org>, Ted Ts'o <tytso@....edu>, Christian Brauner <brauner@...nel.org>, 
	"Darrick J. Wong" <djwong@...nel.org>, Matthew Wilcox <willy@...radead.org>, 
	Al Viro <viro@...iv.linux.org.uk>, linux-fsdevel@...r.kernel.org, 
	almaz.alexandrovich@...agon-software.com, ntfs3@...ts.linux.dev, miklos@...redi.hu, 
	linux-bcachefs@...r.kernel.org, clm@...com, josef@...icpanda.com, dsterba@...e.com, 
	linux-btrfs@...r.kernel.org, dhowells@...hat.com, jlayton@...nel.org, netfs@...ts.linux.dev
Subject: Re: [PATCH 0/7] Move prefaulting into write slow paths

On Fri, Jan 31, 2025 at 08:36:29AM +1100, Dave Chinner wrote:
> 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.

This is just tone policing.

> 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.

No, it exists - I built it.

> 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....

I keep having the same conversations in code review:
"Did you test this?"
"No really, did you test this?"
"See that error path you modified, that our automated tests definitely
don't cover? You need to test that manually".

Developers don't think enough about testing - and if the excuse is that
the tools suck, then we need to address that. I'm not going to babysit
and do a bunch of manual work - I spend quite enough of my day staring
at test dashboards already, thank you very much.

> > 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....

AFAIK the public test robots don't run xfstests at all.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ