linux-kernel - Re: [GIT PULL] bcachefs fixes for 6.15-rc4

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: <rn2bojnk2h3z6xavoap6phjbib55poltxclv64xaijtikg4f5v@npknltjjnzan>
Date: Sat, 26 Apr 2025 00:49:39 -0400
From: Kent Overstreet <kent.overstreet@...ux.dev>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: linux-bcachefs@...r.kernel.org, linux-fsdevel@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [GIT PULL] bcachefs fixes for 6.15-rc4

On Fri, Apr 25, 2025 at 09:11:18PM -0700, Linus Torvalds wrote:
> On Fri, 25 Apr 2025 at 20:59, Kent Overstreet <kent.overstreet@...ux.dev> wrote:
> >
> > Yeah, Al just pointed me at generic_set_sb_d_ops().
> >
> > Which is a perverse way to hide an ops struct. Bloody hell...
> 
> Kent, it's that perverse thing EXACTLY FOR THE REASONS I TOLD YOU.

And you never noticed that the complaints I had about the dcache bits
didn't make sense and how I said it should work was how it actually does
work? Heh.

We were talking past each other because everywhere else in filesystem
land you define your ops structs, which means you can read through them,
but when the ops struct is behind a helper that can be called from
anywhere (the ext4 init code is what, 400 lines ballpark last I check) -
good luck...

> The common case will never even *look* at the dentry ops, because
> that's way too damn expensive, and the common case wants nothing at
> all to do with case insensitivity.
> 
> So guess why that odd specialized function exists?
> 
> Exactly because the dcache makes damn sure that the irrelevant CI case
> is never in the hot path. So when you set those special dentry
> operations to say that you want the CI slow-paths, the VFS layer then
> sets magic bits in the dentry itself that makes it go to there.
> 
> That way the dentry code doesn't do the extra check for "do I have
> special dentry operations for hashing on bad filesystems?" IOW, in
> order to avoid two pointer dereferences, we set one bit in the dentry
> flags instead, and now we have that information right there.

Well, that's d_set_op(), for the "flags to skip hitting the ops struct"
bits. That's perfectly sane, sensible thing to do; you could still pass
a d_ops to it that was defined within that specific filesystem.

generic_set_sb_d_ops() just hides the fact that a d_operations exists.
That's where the confusion came from, because if any of the other major
local filesystems defined one I'd have seen it.

> So yes, people who want to use case-insensitive lookups need to go to
> some extra effort, exactly because we do NOT want that garbage to
> affect the well-behaved paths.
> 
> And no, I'm not surprised that you didn't get it all right. The VFS
> layer is complicated, and the dentry cache is some of the more complex
> parts of it.

I think the more hilarious part is that CI lookups without the special
d_ops seems to actually work - passes tests and I got a report today
that the code I finished last night "works now, please don't change it".

So, yeah. Fun times.