From patchwork Wed Jun 3 00:31:00 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Omar Sandoval X-Patchwork-Id: 6531751 Return-Path: X-Original-To: patchwork-linux-btrfs@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 88F68C0020 for ; Wed, 3 Jun 2015 00:31:23 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id AD7B820690 for ; Wed, 3 Jun 2015 00:31:22 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 7AB7F2068E for ; Wed, 3 Jun 2015 00:31:21 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751443AbbFCAbM (ORCPT ); Tue, 2 Jun 2015 20:31:12 -0400 Received: from mail-pa0-f51.google.com ([209.85.220.51]:33591 "EHLO mail-pa0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751360AbbFCAbK (ORCPT ); Tue, 2 Jun 2015 20:31:10 -0400 Received: by padj3 with SMTP id j3so78174882pad.0 for ; Tue, 02 Jun 2015 17:31:09 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=yYIss3EAPB2/g28R7eMJOzigjBOi7zBjwq+zySqSfYI=; b=ghZoNzBr0Xpod3VkrmzGmlv/nO6Z7YrSb8jMpKSgeY5eElOJk+uqJwN7fL8+hyNbln XM0qY0PPMwUK4fhW4iPsitP8yQOYku+dyn0451JvrowGL45Sw2WzAVWD3y4+i6ciVBlP tSY1AEObZYCLbhopG/oOOP3twof6WmuxE60+FNqFIFcLgn7hgw9HgTulmAN8ZpOWuY+L WTvk8zoFG7NFgydxYHJST3G3QTKJzqTtUeIlWvHojjGQfYkGo1nk5RXz88bGwZbAuytb o7hwg/qglyQwH0ScFoW5P9jr5IaPpyPU29e5A5jdXAXf/+OKKI8LMGX/eujPDeOf+t5P xhjQ== X-Gm-Message-State: ALoCoQmQOWLgm9TcvlNOoHARdMYTMNN9gAqZY1GibQRTrJRi2bgMg3C16usBNkKYGsyMO7inQ1sn X-Received: by 10.68.69.79 with SMTP id c15mr17083823pbu.25.1433291469547; Tue, 02 Jun 2015 17:31:09 -0700 (PDT) Received: from mew.localdomain (c-76-104-211-44.hsd1.wa.comcast.net. [76.104.211.44]) by mx.google.com with ESMTPSA id ro9sm7079578pab.30.2015.06.02.17.31.08 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Tue, 02 Jun 2015 17:31:08 -0700 (PDT) From: Omar Sandoval To: linux-btrfs@vger.kernel.org Cc: Markus Schauler , Filipe David Manana , Omar Sandoval , Subject: [PATCH v2] Btrfs: don't invalidate root dentry when subvolume deletion fails Date: Tue, 2 Jun 2015 17:31:00 -0700 Message-Id: <14b0605f7a3a50a29bdfd40161860a45c7ea7513.1433290960.git.osandov@osandov.com> X-Mailer: git-send-email 2.4.2 Sender: linux-btrfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-btrfs@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Since commit bafc9b754f75 ("vfs: More precise tests in d_invalidate"), mounted subvolumes can be deleted because d_invalidate() won't fail. However, we run into problems when we attempt to delete the default subvolume while it is mounted as the root filesystem: # btrfs subvol list / ID 257 gen 306 top level 5 path rootvol ID 267 gen 334 top level 5 path snap1 # btrfs subvol get-default / ID 267 gen 334 top level 5 path snap1 # btrfs inspect-internal rootid / 267 # mount -o subvol=/ /dev/vda1 /mnt # btrfs subvol del /mnt/snap1 Delete subvolume (no-commit): '/mnt/snap1' ERROR: cannot delete '/mnt/snap1' - Operation not permitted # findmnt / findmnt: can't read /proc/mounts: No such file or directory # ls /proc # Markus reported that this same scenario simply led to a kernel oops. This happens because in btrfs_ioctl_snap_destroy(), we call d_invalidate() before we check may_destroy_subvol(), which means that we detach the submounts and drop the dentry before erroring out. Instead, we should only invalidate the dentry once the deletion has succeeded. Additionally, the shrink_dcache_sb() isn't necessary; d_invalidate() will prune the dcache for the deleted subvolume. Cc: Fixes: bafc9b754f75 ("vfs: More precise tests in d_invalidate") Reported-by: Markus Schauler Signed-off-by: Omar Sandoval --- v1->v2: - Move d_invalidate() to only happen if the whole deletion was successful instead of after may_destroy_subvol() succeeds (pointed out by Filipe) - Remove unnecessary shrink_dcache_sb() As I mentioned in the thread for v1 (http://thread.gmane.org/gmane.comp.file-systems.btrfs/45448), I don't know why that shrink_dcache_sb() is in there. The commit that introduced it, efefb1438be2 ("Btrfs: remove negative dentry when deleting subvolumne"), doesn't explain why it was added, but maybe someone can enlighten me :) fs/btrfs/ioctl.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 1c22c6518504..6f790bcddfc1 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -2413,8 +2413,6 @@ static noinline int btrfs_ioctl_snap_destroy(struct file *file, goto out_unlock_inode; } - d_invalidate(dentry); - down_write(&root->fs_info->subvol_sem); err = may_destroy_subvol(dest); @@ -2508,7 +2506,7 @@ out_up_write: out_unlock_inode: mutex_unlock(&inode->i_mutex); if (!err) { - shrink_dcache_sb(root->fs_info->sb); + d_invalidate(dentry); btrfs_invalidate_inodes(dest); d_delete(dentry); ASSERT(dest->send_in_progress == 0);