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
