On Mon, Oct 28, 2024 at 08:30:39AM -1000, Linus Torvalds wrote: > On Mon, 28 Oct 2024 at 01:17, Christian Brauner <brauner@xxxxxxxxxx> wrote: > > > > Thanks for catching this. So what I did is: > > You had better remove the __randomize_layout from 'struct file' too, > otherwise your patch is entirely broken. Yeah, it has actively been baffling me why this ever made it to struct file in the first place. > > We should damn well remove it anyway, the whole struct randomization > is just a bad joke. Nobody sane enables it, afaik. But for your patch > in particular, it's now an active bug. I'm aware and so I did end up just initializing things manually instead of the proposed patch. We do always call init_file() and we do initialize a bunch of field in there already anyway. So might just as well do the rest of the fields. > > Also, I wonder if we would be better off with f_count _away_ from the > other fields we touch, because the file count ref always ends up > making it cpu-local, so no shared caching behavior. We had that > reported for the inode contents. > > So any threaded use of the same file will end up bouncing not just the > refcount, but also won't be caching some of the useful info at the > beginning of the file, that is basically read-only and could be shared > across CPUs. Yes, I'm aware of that and I commented on that in the commit message that we will likely end up reordering fields to prevent such false sharing.
