From patchwork Thu Nov 24 08:12:38 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Kara X-Patchwork-Id: 9444953 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 35F2F606DB for ; Thu, 24 Nov 2016 08:14:03 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 2A0E026E39 for ; Thu, 24 Nov 2016 08:14:03 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 1EFCD271BC; Thu, 24 Nov 2016 08:14:03 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from aserp1040.oracle.com (aserp1040.oracle.com [141.146.126.69]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 973F326E39 for ; Thu, 24 Nov 2016 08:13:59 +0000 (UTC) Received: from userv0021.oracle.com (userv0021.oracle.com [156.151.31.71]) by aserp1040.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2) with ESMTP id uAO8Dne5031901 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 24 Nov 2016 08:13:49 GMT Received: from oss.oracle.com (oss-old-reserved.oracle.com [137.254.22.2]) by userv0021.oracle.com (8.14.4/8.14.4) with ESMTP id uAO8DmZ7017307 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Thu, 24 Nov 2016 08:13:49 GMT Received: from localhost ([127.0.0.1] helo=lb-oss.oracle.com) by oss.oracle.com with esmtp (Exim 4.63) (envelope-from ) id 1c9pAC-0004Tb-OG; Thu, 24 Nov 2016 00:13:48 -0800 Received: from userv0022.oracle.com ([156.151.31.74]) by oss.oracle.com with esmtp (Exim 4.63) (envelope-from ) id 1c9p9Q-0004PZ-3L for ocfs2-devel@oss.oracle.com; Thu, 24 Nov 2016 00:13:00 -0800 Received: from aserp1020.oracle.com (aserp1020.oracle.com [141.146.126.67]) by userv0022.oracle.com (8.14.4/8.14.4) with ESMTP id uAO8CxJK008297 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Thu, 24 Nov 2016 08:12:59 GMT Received: from userp2030.oracle.com (userp2030.oracle.com [156.151.31.89]) by aserp1020.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2) with ESMTP id uAO8CwUf030969 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO) for ; Thu, 24 Nov 2016 08:12:59 GMT Received: from pps.filterd (userp2030.oracle.com [127.0.0.1]) by userp2030.oracle.com (8.16.0.17/8.16.0.17) with SMTP id uAO8BW77040909 for ; Thu, 24 Nov 2016 08:12:58 GMT Authentication-Results: oracle.com; spf=pass smtp.mailfrom=jack@suse.cz Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by userp2030.oracle.com with ESMTP id 26vwdj41uq-1 (version=TLSv1 cipher=ECDHE-RSA-AES256-SHA bits=256 verify=NOT) for ; Thu, 24 Nov 2016 08:12:58 +0000 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 2133CACA3; Thu, 24 Nov 2016 08:12:55 +0000 (UTC) Received: by quack2.suse.cz (Postfix, from userid 1000) id 7C5671E2F93; Thu, 24 Nov 2016 09:12:53 +0100 (CET) From: Jan Kara To: Date: Thu, 24 Nov 2016 09:12:38 +0100 Message-Id: <1479975162-24060-4-git-send-email-jack@suse.cz> X-Mailer: git-send-email 2.6.6 In-Reply-To: <1479975162-24060-1-git-send-email-jack@suse.cz> References: <1479975162-24060-1-git-send-email-jack@suse.cz> X-PDR: PASS X-ServerName: mx2.suse.de X-Proofpoint-SPF-Result: pass X-Proofpoint-SPF-Record: v=spf1 ip4:137.65.0.0/16 ip4:151.155.28.0/17 ip4:149.44.0.0/16 ip4:147.2.0.0/16 ip4:164.99.0.0/16 ip4:130.57.0.0/16 ip4:192.31.114.0/24 ip4:195.135.221.0/24 ip4:195.135.220.0/24 ip4:69.7.179.0/24 ip4:150.215.214.0/24 include:mailcontrol.com ~all X-Proofpoint-Virus-Version: vendor=nai engine=5800 definitions=8358 signatures=670744 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 suspectscore=1 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1609300000 definitions=main-1611240142 Cc: Jan Kara , Al Viro , ocfs2-devel@oss.oracle.com Subject: [Ocfs2-devel] [PATCH 3/7] quota: Use s_umount protection for quota operations X-BeenThere: ocfs2-devel@oss.oracle.com X-Mailman-Version: 2.1.9 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: ocfs2-devel-bounces@oss.oracle.com Errors-To: ocfs2-devel-bounces@oss.oracle.com X-Source-IP: userv0021.oracle.com [156.151.31.71] X-Virus-Scanned: ClamAV using ClamSMTP Writeback quota is protected by s_umount semaphore held for reading because every writeback must be protected by that lock (grabbed either by the generic writeback code or by quotactl handler). Getting next available ID in quota file, querying quota state, setting quota information, getting quota format are all quotactl operations protected by s_umount semaphore held for reading grabbed in quotactl handler. This also fixes lockdep splat about possible deadlock during filesystem freezing where sync_filesystem() is called with page-faults already blocked but sync_filesystem() calls into dquot_writeback_dquots() which grabs dqonoff_mutex which ranks above i_mutex (vfs_load_quota_inode() grabs i_mutex under dqonoff_mutex) which clearly ranks below page fault freeze protection (e.g. via mmap_sem dependencies). The reported problem is not a real deadlock possibility since during quota on we check whether filesystem freezing is not in progress but still it is good to have this fixed. Reported-by: Ted Tso Reported-by: Eric Whitney Signed-off-by: Jan Kara --- fs/quota/dquot.c | 39 ++++++++++----------------------------- fs/quota/quota.c | 6 +----- 2 files changed, 11 insertions(+), 34 deletions(-) diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c index 047afb966420..2a9dc3fb491c 100644 --- a/fs/quota/dquot.c +++ b/fs/quota/dquot.c @@ -617,7 +617,8 @@ int dquot_writeback_dquots(struct super_block *sb, int type) int cnt; int err, ret = 0; - mutex_lock(&dqopt->dqonoff_mutex); + WARN_ON_ONCE(!rwsem_is_locked(&sb->s_umount)); + for (cnt = 0; cnt < MAXQUOTAS; cnt++) { if (type != -1 && cnt != type) continue; @@ -653,7 +654,6 @@ int dquot_writeback_dquots(struct super_block *sb, int type) && info_dirty(&dqopt->info[cnt])) sb->dq_op->write_info(sb, cnt); dqstats_inc(DQST_SYNCS); - mutex_unlock(&dqopt->dqonoff_mutex); return ret; } @@ -683,7 +683,6 @@ int dquot_quota_sync(struct super_block *sb, int type) * Now when everything is written we can discard the pagecache so * that userspace sees the changes. */ - mutex_lock(&dqopt->dqonoff_mutex); for (cnt = 0; cnt < MAXQUOTAS; cnt++) { if (type != -1 && cnt != type) continue; @@ -693,7 +692,6 @@ int dquot_quota_sync(struct super_block *sb, int type) truncate_inode_pages(&dqopt->files[cnt]->i_data, 0); inode_unlock(dqopt->files[cnt]); } - mutex_unlock(&dqopt->dqonoff_mutex); return 0; } @@ -2050,21 +2048,13 @@ int dquot_get_next_id(struct super_block *sb, struct kqid *qid) struct quota_info *dqopt = sb_dqopt(sb); int err; - mutex_lock(&dqopt->dqonoff_mutex); - if (!sb_has_quota_active(sb, qid->type)) { - err = -ESRCH; - goto out; - } - if (!dqopt->ops[qid->type]->get_next_id) { - err = -ENOSYS; - goto out; - } + if (!sb_has_quota_active(sb, qid->type)) + return -ESRCH; + if (!dqopt->ops[qid->type]->get_next_id) + return -ENOSYS; mutex_lock(&dqopt->dqio_mutex); err = dqopt->ops[qid->type]->get_next_id(sb, qid); mutex_unlock(&dqopt->dqio_mutex); -out: - mutex_unlock(&dqopt->dqonoff_mutex); - return err; } EXPORT_SYMBOL(dquot_get_next_id); @@ -2762,7 +2752,6 @@ int dquot_get_state(struct super_block *sb, struct qc_state *state) struct quota_info *dqopt = sb_dqopt(sb); int type; - mutex_lock(&sb_dqopt(sb)->dqonoff_mutex); memset(state, 0, sizeof(*state)); for (type = 0; type < MAXQUOTAS; type++) { if (!sb_has_quota_active(sb, type)) @@ -2784,7 +2773,6 @@ int dquot_get_state(struct super_block *sb, struct qc_state *state) tstate->nextents = 1; /* We don't know... */ spin_unlock(&dq_data_lock); } - mutex_unlock(&sb_dqopt(sb)->dqonoff_mutex); return 0; } EXPORT_SYMBOL(dquot_get_state); @@ -2798,18 +2786,13 @@ int dquot_set_dqinfo(struct super_block *sb, int type, struct qc_info *ii) if ((ii->i_fieldmask & QC_WARNS_MASK) || (ii->i_fieldmask & QC_RT_SPC_TIMER)) return -EINVAL; - mutex_lock(&sb_dqopt(sb)->dqonoff_mutex); - if (!sb_has_quota_active(sb, type)) { - err = -ESRCH; - goto out; - } + if (!sb_has_quota_active(sb, type)) + return -ESRCH; mi = sb_dqopt(sb)->info + type; if (ii->i_fieldmask & QC_FLAGS) { if ((ii->i_flags & QCI_ROOT_SQUASH && - mi->dqi_format->qf_fmt_id != QFMT_VFS_OLD)) { - err = -EINVAL; - goto out; - } + mi->dqi_format->qf_fmt_id != QFMT_VFS_OLD)) + return -EINVAL; } spin_lock(&dq_data_lock); if (ii->i_fieldmask & QC_SPC_TIMER) @@ -2826,8 +2809,6 @@ int dquot_set_dqinfo(struct super_block *sb, int type, struct qc_info *ii) mark_info_dirty(sb, type); /* Force write to disk */ sb->dq_op->write_info(sb, type); -out: - mutex_unlock(&sb_dqopt(sb)->dqonoff_mutex); return err; } EXPORT_SYMBOL(dquot_set_dqinfo); diff --git a/fs/quota/quota.c b/fs/quota/quota.c index 790c61abc663..0c8041688dcf 100644 --- a/fs/quota/quota.c +++ b/fs/quota/quota.c @@ -104,13 +104,9 @@ static int quota_getfmt(struct super_block *sb, int type, void __user *addr) { __u32 fmt; - mutex_lock(&sb_dqopt(sb)->dqonoff_mutex); - if (!sb_has_quota_active(sb, type)) { - mutex_unlock(&sb_dqopt(sb)->dqonoff_mutex); + if (!sb_has_quota_active(sb, type)) return -ESRCH; - } fmt = sb_dqopt(sb)->info[type].dqi_format->qf_fmt_id; - mutex_unlock(&sb_dqopt(sb)->dqonoff_mutex); if (copy_to_user(addr, &fmt, sizeof(fmt))) return -EFAULT; return 0;