Date: Mon, 19 Jul 2010 10:43:27 +0300 From: Kostik Belousov <kostikbel@gmail.com> To: Rick Macklem <rmacklem@uoguelph.ca> Cc: freebsd-fs@freebsd.org Subject: Re: fix for remove for NFS through nullfs Message-ID: <20100719074327.GV2381@deviant.kiev.zoral.com.ua> In-Reply-To: <Pine.GSO.4.63.1007182231590.21767@muncher.cs.uoguelph.ca> References: <Pine.GSO.4.63.1007182231590.21767@muncher.cs.uoguelph.ca>
next in thread | previous in thread | raw e-mail | index | archive | help
--1lE8Wy7Exphh2Vpg
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Sun, Jul 18, 2010 at 10:40:16PM -0400, Rick Macklem wrote:
> Mikolaj Golub submitted the attached patch that fixes a problem w.r.t.
> a nullfs mounted NFS mount point for remove. The problem is that,
> without this patch, NFS does not see that a file is still open
> (v_usecount > 1) when removed and removes it instead of silly renaming
> it. This patch increments the v_usecount of the lower level vnode
> during the remove call, so that silly rename works. kib@ has noted
> that this may be "racy" and result in silly rename happening when it
> isn't required but, imho, that is less of a problem than it never
> working. (I have tested it a bit for NFS and UFS and it seems to
> work for those file systems under a nullfs mount.)
Rather, I said "please commit, but note (in commit message or better
in comment) that ..."
>=20
> Why I am posting is that I am wondering if anyone knows of a file
> system type where this extra v_usecount on the vnode at the time of remove
> will/might cause problems?
>=20
> Thanks in advance for looking at this, rick
> --- submitted patch for nullfs ---
> --- fs/nullfs/null_vnops.c.sav 2010-07-18 19:33:00.000000000 -0400
> +++ fs/nullfs/null_vnops.c 2010-07-18 19:35:25.000000000 -0400
> @@ -499,6 +499,29 @@
> }
>=20
> /*
> + * Increasing refcount of lower vnode is needed at least for the case
> + * when lower FS is NFS to do sillyrename if the file is in use.
> + */
> +static int
> +null_remove(struct vop_remove_args *ap)
> +{
> + int retval;
> + struct vnode *lvp;
> + boolean_t vreleit;
> +
> + if (ap->a_vp->v_usecount > 1) {
> + lvp =3D NULLVPTOLOWERVP(ap->a_vp);
> + VREF(lvp);
> + vreleit =3D TRUE;
> + } else
> + vreleit =3D FALSE;
> + retval =3D null_bypass(&ap->a_gen);
> + if (vreleit)
> + vrele(lvp);
> + return (retval);
> +}
> +
> +/*
> * We handle this to eliminate null FS to lower FS
> * file moving. Don't know why we don't allow this,
> * possibly we should.
> @@ -809,6 +832,7 @@
> .vop_open =3D null_open,
> .vop_print =3D null_print,
> .vop_reclaim =3D null_reclaim,
> + .vop_remove =3D null_remove,
> .vop_rename =3D null_rename,
> .vop_setattr =3D null_setattr,
> .vop_strategy =3D VOP_EOPNOTSUPP,
> _______________________________________________
> freebsd-fs@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-fs
> To unsubscribe, send any mail to "freebsd-fs-unsubscribe@freebsd.org"
--1lE8Wy7Exphh2Vpg
Content-Type: application/pgp-signature
Content-Disposition: inline
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (FreeBSD)
iEYEARECAAYFAkxEAh4ACgkQC3+MBN1Mb4gkpwCg2Z5SusfxE9TDvg2sZ8urYpVI
gEwAoIMKZyKEmMEHUZRJYeYuFE6cPMKt
=+JA4
-----END PGP SIGNATURE-----
--1lE8Wy7Exphh2Vpg--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20100719074327.GV2381>