From owner-freebsd-fs@freebsd.org  Thu Mar 10 11:38:45 2016
Return-Path: <owner-freebsd-fs@freebsd.org>
Delivered-To: freebsd-fs@mailman.ysv.freebsd.org
Received: from mx1.freebsd.org (mx1.freebsd.org
 [IPv6:2001:1900:2254:206a::19:1])
 by mailman.ysv.freebsd.org (Postfix) with ESMTP id B1A82ACBF7B
 for <freebsd-fs@mailman.ysv.freebsd.org>; Thu, 10 Mar 2016 11:38:45 +0000 (UTC)
 (envelope-from torek@elf.torek.net)
Received: from elf.torek.net (mail.torek.net [96.90.199.121])
 (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits))
 (Client did not present a certificate)
 by mx1.freebsd.org (Postfix) with ESMTPS id 9A7B6F58
 for <freebsd-fs@freebsd.org>; Thu, 10 Mar 2016 11:38:45 +0000 (UTC)
 (envelope-from torek@elf.torek.net)
Received: from elf.torek.net (localhost [127.0.0.1])
 by elf.torek.net (8.14.9/8.14.9) with ESMTP id u2ABcif2048012;
 Thu, 10 Mar 2016 03:38:44 -0800 (PST)
 (envelope-from torek@elf.torek.net)
Received: (from torek@localhost)
 by elf.torek.net (8.14.9/8.14.9/Submit) id u2ABcihi048011;
 Thu, 10 Mar 2016 03:38:44 -0800 (PST) (envelope-from torek)
Date: Thu, 10 Mar 2016 03:38:44 -0800 (PST)
From: Chris Torek <torek@elf.torek.net>
Message-Id: <201603101138.u2ABcihi048011@elf.torek.net>
To: kostikbel@gmail.com
Subject: Re: quotactl bug: vfs_busy never unbusy-es
Cc: freebsd-fs@freebsd.org
In-Reply-To: <20160310091433.GS67250@kib.kiev.ua>
X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.4.3
 (elf.torek.net [127.0.0.1]); Thu, 10 Mar 2016 03:38:44 -0800 (PST)
X-BeenThere: freebsd-fs@freebsd.org
X-Mailman-Version: 2.1.21
Precedence: list
List-Id: Filesystems <freebsd-fs.freebsd.org>
List-Unsubscribe: <https://lists.freebsd.org/mailman/options/freebsd-fs>,
 <mailto:freebsd-fs-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/freebsd-fs/>
List-Post: <mailto:freebsd-fs@freebsd.org>
List-Help: <mailto:freebsd-fs-request@freebsd.org?subject=help>
List-Subscribe: <https://lists.freebsd.org/mailman/listinfo/freebsd-fs>,
 <mailto:freebsd-fs-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Thu, 10 Mar 2016 11:38:45 -0000

[A nicer change breaks KPI - yes, alas.]

>Why not do just the following ?

That fixes the main bug, although pass-through file systems
(nullfs and unionfs) are still wrong: they don't vfs_busy their
target mount points when passing the op through, nor unbusy
their "mp" arguments when the sub-command is Q_QUOTAON.

We could perhaps have a little subroutine in the VFS code that
does the vfs_busy(mp)-then-call sequence, so that these two need
not repeat it.

>With regard to the ufs/ufs/quota.h pollution of the VFS code, this
>is ugly, I agree. I wanted to move the ufs quota code into vfs layer
>for very long time.

This would be good (though zfs still has its own special
quota code; hooking that up is what started me down this path,
and hooking that up still looks difficult...).

>diff --git a/sys/kern/vfs_default.c b/sys/kern/vfs_default.c
>index a7977bf..26e45f4 100644
>--- a/sys/kern/vfs_default.c
>+++ b/sys/kern/vfs_default.c
>@@ -66,6 +66,8 @@ __FBSDID("$FreeBSD$");
> #include <vm/vm_pager.h>
> #include <vm/vnode_pager.h>
> 
>+#include <ufs/ufs/quota.h>
>+
> static int	vop_nolookup(struct vop_lookup_args *);
> static int	vop_norename(struct vop_rename_args *);
> static int	vop_nostrategy(struct vop_strategy_args *);
>@@ -1190,6 +1192,8 @@ vfs_stdquotactl (mp, cmds, uid, arg)
> 	void *arg;
> {
> 
>+	if ((cmds >> SUBCMDSHIFT) == Q_QUOTAON)
>+		vfs_unbusy(mp);
> 	return (EOPNOTSUPP);
> }

That, in fact, is what I started with, before I investigated
further and found the nullfs and unionfs pattern violation.

It's certainly worth doing as a start, though.

Chris