From: Luis Henriques <lhenriques@suse.de>
To: David Howells <dhowells@redhat.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>,
Dominique Martinet <asmadeus@codewreck.org>,
Eric Van Hensbergen <ericvh@gmail.com>,
Latchesar Ionkov <lucho@ionkov.net>,
linux-fsdevel@vger.kernel.org,
v9fs-developer@lists.sourceforge.net
Subject: Re: What sort of inode state does ->evict_inode() expect to see? [was Re: 9p: fscache duplicate cookie]
Date: Fri, 14 May 2021 17:10:56 +0100 [thread overview]
Message-ID: <87bl9dwb1r.fsf@suse.de> (raw)
In-Reply-To: <2919958.1620828730@warthog.procyon.org.uk> (David Howells's message of "Wed, 12 May 2021 15:12:10 +0100")
David Howells <dhowells@redhat.com> writes:
> Al Viro <viro@zeniv.linux.org.uk> 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@suse.de/
>
> 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@suse.de/
>
> 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
next prev parent reply other threads:[~2021-05-14 16:09 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-06 10:03 9p: fscache duplicate cookie Luis Henriques
2021-05-06 10:45 ` Dominique Martinet
2021-05-06 12:18 ` Luis Henriques
2021-05-07 16:36 ` Luis Henriques
2021-05-08 0:47 ` Dominique Martinet
2021-05-10 10:54 ` Luis Henriques
2021-05-10 11:47 ` Luis Henriques
2021-05-11 12:44 ` David Howells
2021-05-12 10:10 ` Luis Henriques
2021-05-11 12:53 ` David Howells
2021-05-11 12:38 ` David Howells
2021-05-12 10:07 ` Luis Henriques
2021-05-12 11:04 ` David Howells
2021-05-12 11:58 ` Luis Henriques
2021-05-12 12:26 ` Dominique Martinet
2021-05-12 12:57 ` What sort of inode state does ->evict_inode() expect to see? [was Re: 9p: fscache duplicate cookie] David Howells
2021-05-12 13:45 ` Al Viro
2021-05-12 14:12 ` David Howells
2021-05-14 16:10 ` Luis Henriques [this message]
2021-05-14 21:16 ` Dominique Martinet
2021-05-17 15:56 ` Luis Henriques
2021-05-17 17:39 ` Aneesh Kumar K.V
2021-05-12 11:09 ` 9p: fscache duplicate cookie David Howells
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87bl9dwb1r.fsf@suse.de \
--to=lhenriques@suse.de \
--cc=asmadeus@codewreck.org \
--cc=dhowells@redhat.com \
--cc=ericvh@gmail.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=lucho@ionkov.net \
--cc=v9fs-developer@lists.sourceforge.net \
--cc=viro@zeniv.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.