On Fri, Nov 15, 2024 at 05:31:38PM -0800, Hugh Dickins wrote: > 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. Thanks to you both for jumping on this! > 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. IMHO the KCSAN splat suggests there is a missing inode_lock/unlock pair /somewhere/. Just not in shmem_getattr(). Earlier in this thread, Jeongjun reported: > ... I found that this data-race mainly occurs when vfs_statx_path > does not acquire the inode_lock ... That sounds to me like a better place to address this; or at least that is a good place to start looking for the problem. -- Chuck Lever
