From: Jan Kara <jack@suse.cz>
To: Amir Goldstein <amir73il@gmail.com>
Cc: Jan Kara <jack@suse.cz>, Miklos Szeredi <miklos@szeredi.hu>,
linux-fsdevel <linux-fsdevel@vger.kernel.org>,
linux-kernel <linux-kernel@vger.kernel.org>,
syzkaller-bugs@googlegroups.com,
Al Viro <viro@zeniv.linux.org.uk>,
syzbot <syzbot+31d8b84465a7cbfd8515@syzkaller.appspotmail.com>,
overlayfs <linux-unionfs@vger.kernel.org>
Subject: Re: possible deadlock in pipe_lock (2)
Date: Tue, 12 Feb 2019 17:11:07 +0100 [thread overview]
Message-ID: <20190212161107.GB19076@quack2.suse.cz> (raw)
In-Reply-To: <CAOQ4uxh_o53RimG-yKPYkN9sszQhwjQbmHw9pHc4kJU5MywMNA@mail.gmail.com>
On Tue 12-02-19 15:39:38, Amir Goldstein wrote:
> > > My other thought is that perhaps sb_start_write() should invoke
> > > s_ops->start_write() so that overlay can do the freeze protection on
> > > the upper early.
> >
> > So my understanding of overlayfs is pretty basic so I'm sorry if I miss
> > something. If I'm right, we have three superblocks here: ovl, upper, lower.
> > Now 'lower' is read-only so for freezing purposes we can just forget about
> > it. 'upper' is where the real changes are going into and 'ovl' is a wrapper
> > virtual superblock that handles merging of 'lower' and 'upper'. Correct so
> > far?
>
> Yes.
>
> >
> > And the problem seems to be that when you acquire freeze protection for the
> > 'ovl' superblock, you in fact want to acquire freeze protection for the
> > 'upper' (as 'ovl' is just virtual and has no disk state to protect). So I
>
> There are use case for freezing ovl (i.e. ovl snapshots) but it is not
> implemented
> at the moment.
>
> Overlayfs already gets upper freeze protection internally before any
> modification
> to upper.
> The problem that locking order of upper freeze is currently under overlay
> inode mutex. And that brings a problem with the above pipe case.
>
> > agree that a callback to allow overlayfs to acquire freeze protection on
> > 'upper' right away would be one solution. Or we could make s_writers a
> > pointer and redirect ovl->s_writers to upper->s_writers. Then VFS should do
> > the right thing from the start unless overlayfs calls back into operations
> > on 'upper' that will try to acquire the freeze protection again. Thoughts?
>
> Overlayfs definitely calls into operations on upper and upper certainly
> acquires several levels of s_writers itself.
>
> The problem with the proposal to change locking order to
> ovl freeze -> upper freeze -> ovl inode -> upper inode
> is that for some non-write operations (e.g. lookup, readdir)
> overlay may end up updating xattrs on upper, so will need
> to take upper freeze after ovl inode lock without ovl freeze
> being called by vfs.
>
> I suggested that we may use upper freeze trylock in those
> cases and skip xattr update if trylock fails.
Yes, that's what VFS does as well e.g. for atime updates. In fact I don't
see other sensible possibility since blocking read operation on frozen
filesystem is surprising to the user.
> Not sure if my assumption is correct that this would be ok
> w.r.t locking rules?
It should be fine AFAICT.
> Not sure if we can get away with trylock in all the cases that
> we need to modify upper.
I don't know overlayfs enough to be able to tell :).
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
prev parent reply other threads:[~2019-02-12 16:11 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-10 18:23 possible deadlock in pipe_lock (2) syzbot
2019-02-11 7:38 ` Amir Goldstein
2019-02-11 12:06 ` Miklos Szeredi
2019-02-11 12:37 ` Miklos Szeredi
2019-02-11 13:08 ` Amir Goldstein
2019-02-11 14:33 ` Miklos Szeredi
2019-02-11 15:06 ` Amir Goldstein
2019-02-11 15:40 ` Miklos Szeredi
2019-02-11 17:04 ` Amir Goldstein
2019-02-12 11:14 ` Jan Kara
2019-02-12 13:39 ` Amir Goldstein
2019-02-12 14:19 ` Miklos Szeredi
2019-02-12 16:11 ` Jan Kara [this message]
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=20190212161107.GB19076@quack2.suse.cz \
--to=jack@suse.cz \
--cc=amir73il@gmail.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-unionfs@vger.kernel.org \
--cc=miklos@szeredi.hu \
--cc=syzbot+31d8b84465a7cbfd8515@syzkaller.appspotmail.com \
--cc=syzkaller-bugs@googlegroups.com \
--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 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 NNTP newsgroup(s).