David Howells <dhowells@xxxxxxxxxx> writes: > Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote: > >> > We're seeing cases where fscache is reporting cookie collisions that appears >> > to be due to ->evict_inode() running parallel with a new inode for the same >> > filesystem object getting set up. >> >> Huh? Details, please. What we are guaranteed is that iget{,5}_locked() et.al. >> on the same object will either prevent the call of ->evict_inode() (if they >> manage to grab the sucker before I_FREEING is set) or will wait until after >> ->evict_inode() returns. > > See the trace from Luis in: > > https://lore.kernel.org/linux-fsdevel/87fsysyxh9.fsf@xxxxxxx/ > > It appears that process 20591 manages to set up a new inode that has the same > key parameters as the one process 20585 is tearing down. > > 0000000097476aaa is the cookie pointer used by the old inode. > 0000000011fa06b1 is the cookie pointer used by the new inode. > 000000003080d900 is the cookie pointer for the parent superblock. > > The fscache_acquire traceline emission is caused by one of: > > (*) v9fs_qid_iget() or v9fs_qid_iget_dotl() calling > v9fs_cache_inode_get_cookie(). > > (*) v9fs_file_open*(O_RDONLY) or v9fs_vfs_atomic_open*(O_RDONLY) calling > v9fs_cache_inode_set_cookie(). > > (*) v9fs_cache_inode_reset_cookie(), which appears unused. > > The fscache_relinquish traceline emission is caused by one of: > > (*) v9fs_file_open(O_RDWR/O_WRONLY) or v9fs_vfs_atomic_open(O_RDWR/O_WRONLY) > calling v9fs_cache_inode_set_cookie(). > > (*) v9fs_evict_inode() calling v9fs_cache_inode_put_cookie(). > > (*) v9fs_cache_inode_reset_cookie(), which appears unused. > > From the backtrace in: > > https://lore.kernel.org/linux-fsdevel/87czu45gcs.fsf@xxxxxxx/ > > the acquisition is being triggered in v9fs_vfs_atomic_open_dotl(), so it seems > v9fs_qid_iget_dotl() already happened - which *should* have created the > cookie. So, from our last chat on IRC, we have the following happening: v9fs_vfs_atomic_open_dotl v9fs_vfs_lookup v9fs_get_new_inode_from_fid v9fs_inode_from_fid_dotl v9fs_qid_iget_dotl At this point, iget5_locked() gets called with the test function set to v9fs_test_new_inode_dotl(), which *always* returns 0. It's still not clear to me why commit ed80fcfac256 ("fs/9p: Always ask new inode in create") has introduced this behavior but even if that's not correct, we still have a race regarding cookies handling, right? I'm still seeing: CPU0 CPU1 v9fs_drop_inode ... v9fs_evict_inode /* atomic_open */ v9fs_cache_inode_get_cookie <= COLLISION fscache_relinquish So, the question remains: would it be possible to do the relinquish earlier (->drop_inode)? Or is 9p really shooting itself in the foot by forcing iget5_locked() to always create a new inode here? Cheers, -- Luis