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=-0.8 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_PASS 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 310C3C282C4 for ; Tue, 12 Feb 2019 14:19:14 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id CC6C220838 for ; Tue, 12 Feb 2019 14:19:13 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="key not found in DNS" (0-bit key) header.d=szeredi.hu header.i=@szeredi.hu header.b="LlkNtnAi" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730187AbfBLOTN (ORCPT ); Tue, 12 Feb 2019 09:19:13 -0500 Received: from mail-it1-f194.google.com ([209.85.166.194]:55432 "EHLO mail-it1-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730183AbfBLOTM (ORCPT ); Tue, 12 Feb 2019 09:19:12 -0500 Received: by mail-it1-f194.google.com with SMTP id o131so4194067itc.5 for ; Tue, 12 Feb 2019 06:19:11 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=szeredi.hu; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=H8DIinfz2LflwhAKrpZk1YkKL6zlEiyHEolwga3PuUQ=; b=LlkNtnAiozIEuX0c46N1+4hvU68CNp1I1zTgLzMf4u/Tj80LutWdTzi4GqXCwx9uIt dK4yhZ/Gq3dWrjEhGYUE/GQMSTzzicFCmxw1FBTfCu150yROMh09Hdl29QVerhIYxbp4 a85ZK3wn77c3SSdCNa3Skx/UQkxGGG/3zSsfU= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=H8DIinfz2LflwhAKrpZk1YkKL6zlEiyHEolwga3PuUQ=; b=eZhyFDVKZmmYLq2J1hB035SVR490/5tlmtubYvlobSNYiiKEIA1h5GdC36R/7OVvyd ILSib5d4kSdexazJsz1LAtulsLegmquXDaS+RSi83LnB8JVnapMahHq36D1ONAaIZ9Ge qJqVF4ydRTF7lh7taNgcO3Iwcu3NaXxecltnP3bECoMSpMqwmIxdFtl6+fB+pj/ZbBvJ k33WYaVvWe23r+a60Pe3GPOuofYaLbL6JncWpH/MCe1oL4wzC4g/R5xGN+B5c99mXVM2 ChqXhLhFqUE73PjRS/B39DfhBX/DusEYoq+FUTTzPxH3f4/ObxFRN5khe8ivYaC1GEvt WGQg== X-Gm-Message-State: AHQUAuaiw5gPIqAIUsVTF2SIgP0rCL5Bj4cnfp7foFPF+DRglvupdF8H IinOX78IaD8UymKBM4wMsLduI1ZYnkspYf4JLLJc1g== X-Google-Smtp-Source: AHgI3IbOz80sLMMdfcXmz6SREdYgl5hSQNBDkcDne+a2NG+rVnqiyyAlIpnx00InPCGQOnrZKDv1lTYK4XkPZfUxfJQ= X-Received: by 2002:a5d:8248:: with SMTP id n8mr2089840ioo.246.1549981151502; Tue, 12 Feb 2019 06:19:11 -0800 (PST) MIME-Version: 1.0 References: <000000000000701c3305818e4814@google.com> <20190212111402.GU19029@quack2.suse.cz> In-Reply-To: From: Miklos Szeredi Date: Tue, 12 Feb 2019 15:19:00 +0100 Message-ID: Subject: Re: possible deadlock in pipe_lock (2) To: Amir Goldstein Cc: Jan Kara , linux-fsdevel , linux-kernel , syzkaller-bugs@googlegroups.com, Al Viro , syzbot , overlayfs Content-Type: text/plain; charset="UTF-8" Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org On Tue, Feb 12, 2019 at 2:39 PM 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. sb_start_write() calls cannot be nested (for the same reason two shared locks may be part of a deadlock), so this would mean having to make sure that none of the code that overlay calls does sb_start_write() itself. Which means quite a bit of vfs api churn and the associated pain... > 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. > > Not sure if my assumption is correct that this would be ok > w.r.t locking rules? Yes, using trylock is always deadlock free. Thanks, Miklos