From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.5 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_PASS,URIBL_BLOCKED,USER_AGENT_MUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 988ACC282CE for ; Tue, 12 Feb 2019 16:11:12 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 74A8621901 for ; Tue, 12 Feb 2019 16:11:12 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730898AbfBLQLL (ORCPT ); Tue, 12 Feb 2019 11:11:11 -0500 Received: from mx2.suse.de ([195.135.220.15]:60162 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1730461AbfBLQLK (ORCPT ); Tue, 12 Feb 2019 11:11:10 -0500 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 4E297B02F; Tue, 12 Feb 2019 16:11:08 +0000 (UTC) Received: by quack2.suse.cz (Postfix, from userid 1000) id C4E0D1E09C5; Tue, 12 Feb 2019 17:11:07 +0100 (CET) Date: Tue, 12 Feb 2019 17:11:07 +0100 From: Jan Kara To: Amir Goldstein Cc: Jan Kara , Miklos Szeredi , linux-fsdevel , linux-kernel , syzkaller-bugs@googlegroups.com, Al Viro , syzbot , overlayfs Subject: Re: possible deadlock in pipe_lock (2) Message-ID: <20190212161107.GB19076@quack2.suse.cz> References: <000000000000701c3305818e4814@google.com> <20190212111402.GU19029@quack2.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 SUSE Labs, CR