From: Jinliang Zheng <alexjlzheng@gmail.com>
To: david@fromorbit.com
Cc: alexjlzheng@gmail.com, bfoster@redhat.com, djwong@kernel.org,
linux-fsdevel@vger.kernel.org, linux-xfs@vger.kernel.org,
raven@themaw.net, rcu@vger.kernel.org
Subject: Re: About the conflict between XFS inode recycle and VFS rcu-walk
Date: Wed, 31 Jan 2024 14:35:17 +0800 [thread overview]
Message-ID: <20240131063517.1812354-1-alexjlzheng@tencent.com> (raw)
In-Reply-To: <ZXJf6C0V1znU+ngP@dread.disaster.area>
On Fri, 8 Dec 2023 11:14:32 +1100, david@fromorbit.com wrote:
> On Tue, Dec 05, 2023 at 07:38:33PM +0800, alexjlzheng@gmail.com wrote:
> > Hi, all
> >
> > I would like to ask if the conflict between xfs inode recycle and vfs rcu-walk
> > which can lead to null pointer references has been resolved?
> >
> > I browsed through emails about the following patches and their discussions:
> > - https://lore.kernel.org/linux-xfs/20220217172518.3842951-2-bfoster@redhat.com/
> > - https://lore.kernel.org/linux-xfs/20220121142454.1994916-1-bfoster@redhat.com/
> > - https://lore.kernel.org/linux-xfs/164180589176.86426.501271559065590169.stgit@mickey.themaw.net/
> >
> > And then came to the conclusion that this problem has not been solved, am I
> > right? Did I miss some patch that could solve this problem?
>
> We fixed the known problems this caused by turning off the VFS
> functionality that the rcu pathwalks kept tripping over. See commit
> 7b7820b83f23 ("xfs: don't expose internal symlink metadata buffers to
> the vfs").
Sorry for the delay.
The problem I encountered in the production environment was that during the
rcu walk process the ->get_link() pointer was NULL, which caused a crash.
As far as I know, commit 7b7820b83f23 ("xfs: don't expose internal symlink
metadata buffers to the vfs") first appeared in:
- https://lore.kernel.org/linux-fsdevel/YZvvP9RFXi3%2FjX0q@bfoster/
Does this commit solve the problem of NULL ->get_link()? And how?
>
> Apart from that issue, I'm not aware of any other issues that the
> XFS inode recycling directly exposes.
>
> > According to my understanding, the essence of this problem is that XFS reuses
> > the inode evicted by VFS, but VFS rcu-walk assumes that this will not happen.
>
> It assumes that the inode will not change identity during the RCU
> grace period after the inode has been evicted from cache. We can
> safely reinstantiate an evicted inode without waiting for an RCU
> grace period as long as it is the same inode with the same content
> and same state.
>
> Problems *may* arise when we unlink the inode, then evict it, then a
> new file is created and the old slab cache memory address is used
> for the new inode. I describe the issue here:
>
> https://lore.kernel.org/linux-xfs/20220118232547.GD59729@dread.disaster.area/
And judging from the relevant emails, the main reason why ->get_link() is set
to NULL should be the lack of synchronize_rcu() before xfs_reinit_inode() when
the inode is chosen to be reused.
However, perhaps due to performance reasons, this solution has not been merged
for a long time. How is it now?
Maybe I am missing something in the threads of mail?
Thank you very much. :)
Jinliang Zheng
>
> That said, we have exactly zero evidence that this is actually a
> problem in production systems. We did get systems tripping over the
> symlink issue, but there's no evidence that the
> unlink->close->open(O_CREAT) issues are manifesting in the wild and
> hence there hasn't been any particular urgency to address it.
>
> > Are there any recommended workarounds until an elegant and efficient solution
> > can be proposed? After all, causing a crash is extremely unacceptable in a
> > production environment.
>
> What crashes are you seeing in your production environment?
>
> -Dave.
> --
> Dave Chinner
> david@fromorbit.com
next prev parent reply other threads:[~2024-01-31 6:35 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-05 11:38 About the conflict between XFS inode recycle and VFS rcu-walk alexjlzheng
2023-12-08 0:14 ` Dave Chinner
2024-01-31 6:35 ` Jinliang Zheng [this message]
2024-01-31 19:30 ` Darrick J. Wong
2024-05-15 15:54 ` alexjlzheng
2024-05-16 4:56 ` Jinliang Zheng
2024-05-16 7:08 ` Ian Kent
2024-05-16 7:23 ` Ian Kent
2024-05-20 17:36 ` Darrick J. Wong
2024-05-21 1:35 ` Ian Kent
2024-05-21 2:13 ` Ian Kent
2024-05-26 15:04 ` Jinliang Zheng
2024-05-26 17:21 ` Paul E. McKenney
2024-05-26 23:51 ` Ian Kent
2024-05-27 0:18 ` Al Viro
2024-05-28 15:51 ` Brian Foster
2024-05-27 9:41 ` Dave Chinner
2024-05-27 13:56 ` Jinliang Zheng
2024-05-28 2:10 ` Dave Chinner
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=20240131063517.1812354-1-alexjlzheng@tencent.com \
--to=alexjlzheng@gmail.com \
--cc=bfoster@redhat.com \
--cc=david@fromorbit.com \
--cc=djwong@kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-xfs@vger.kernel.org \
--cc=raven@themaw.net \
--cc=rcu@vger.kernel.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).