From patchwork Wed Jul 17 09:09:46 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Behrens X-Patchwork-Id: 2828498 Return-Path: X-Original-To: patchwork-linux-btrfs@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id D4AAA9F9CA for ; Wed, 17 Jul 2013 09:09:54 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id D59882012C for ; Wed, 17 Jul 2013 09:09:53 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 3C8F12011E for ; Wed, 17 Jul 2013 09:09:52 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752352Ab3GQJJt (ORCPT ); Wed, 17 Jul 2013 05:09:49 -0400 Received: from mo-p00-ob.rzone.de ([81.169.146.160]:10444 "EHLO mo-p00-ob.rzone.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752155Ab3GQJJs (ORCPT ); Wed, 17 Jul 2013 05:09:48 -0400 X-RZG-AUTH: :IGUKYFjleetgZuRbHZjp6Ve7NzeE1efWuTR/wV06y353QgIuD5+acdRFtJ8MDHZp4u74mTVB5dOWYQ== X-RZG-CLASS-ID: mo00 Received: from [172.24.1.80] (yian-ho01.nir.cronon.net [192.166.201.94]) by smtp.strato.de (josoe mo4) (RZmta 31.29 AUTH) with ESMTPA id h03168p6H8qBuM ; Wed, 17 Jul 2013 11:09:45 +0200 (CEST) Message-ID: <51E65F5A.6050501@giantdisaster.de> Date: Wed, 17 Jul 2013 11:09:46 +0200 From: Stefan Behrens User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/20130620 Thunderbird/17.0.7 MIME-Version: 1.0 To: Anand Jain CC: linux-btrfs Subject: Re: [bug] label cli hangs until balance is completed References: <51E655D1.5020309@oracle.com> In-Reply-To: <51E655D1.5020309@oracle.com> Sender: linux-btrfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-btrfs@vger.kernel.org X-Spam-Status: No, score=-7.3 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=ham 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 On Wed, 17 Jul 2013 16:29:05 +0800, Anand Jain wrote: > 'btrfs fi label /btrfs' will hang until balance is completed. > (and probably even set label would hang). This is because we > are trying to hold volume_mutex lock which balance will hold > during its tenure. > > ------- > static int btrfs_ioctl_get_fslabel(struct file *file, void __user *arg) > :: > mutex_lock(&root->fs_info->volume_mutex); > ret = copy_to_user(arg, label, len); > mutex_unlock(&root->fs_info->volume_mutex); > -------- > > I doubt if get label would need such a heavy weight lock? > Do we have any other lock which could fit better here ? > Any comments ? Just use the uuid_mutex instead of the volume_mutex. In addition to the btrfs_ioctl_get_fslabel() and btrfs_ioctl_set_fslabel() functions, only btrfs_scan_one_device() accesses the label. And btrfs_scan_one_device() holds the uuid_mutex, not the volume_mutex. You can do the same. And since cwillu's commit 1332a002b, the uuid_mutex is not hold for a long time anymore. And while you change it, please move the mutex_lock() so that it includes also the strnlen() access to the label, like this: mutex_unlock(&root->fs_info->volume_mutex); --- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 2177bea..30a8126 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -4043,15 +4043,16 @@ static int btrfs_ioctl_get_fslabel(struct file *file, vo { struct btrfs_root *root = BTRFS_I(fdentry(file)->d_inode)->root; const char *label = root->fs_info->super_copy->label; - size_t len = strnlen(label, BTRFS_LABEL_SIZE); + size_t len; int ret; + mutex_lock(&root->fs_info->volume_mutex); + len = strnlen(label, BTRFS_LABEL_SIZE); if (len == BTRFS_LABEL_SIZE) { pr_warn("btrfs: label is too long, return the first %zu bytes\n" --len); } - mutex_lock(&root->fs_info->volume_mutex); ret = copy_to_user(arg, label, len);