On Fri, 15 Nov 2024, Chuck Lever wrote: > > As I said before, I've failed to find any file system getattr method > that explicitly takes the inode's semaphore around a > generic_fillattr() call. My understanding is that these methods > assume that their caller handles appropriate serialization. > Therefore, taking the inode semaphore at all in shmem_getattr() > seems to me to be the wrong approach entirely. > > The point of reverting immediately is that any fix can't possibly > get the review and testing it deserves three days before v6.12 > becomes final. Also, it's not clear what the rush to fix the > KCSAN splat is; according to the Fixes: tag, this issue has been > present for years without causing overt problems. It doesn't feel > like this change belongs in an -rc in the first place. > > Please revert d949d1d14fa2, then let's discuss a proper fix in a > separate thread. Thanks! Thanks so much for reporting this issue, Chuck: just in time. I agree abso-lutely with you: I was just preparing a revert, when I saw that akpm is already on it: great, thanks Andrew. I was not very keen to see that locking added, just to silence a syzbot sanitizer splat: added where there has never been any practical problem (and the result of any stat immediately stale anyway). I was hoping we might get a performance regression reported, but a hang serves better! If there's a "data_race"-like annotation that can be added to silence the sanitizer, okay. But more locking? I don't think so. Hugh
