Date: Sat, 8 May 2004 04:36:16 +0200 From: Pawel Jakub Dawidek <pjd@FreeBSD.org> To: Marc Olzheim <marcolz@stack.nl> Cc: freebsd-current@lists.freebsd.org Subject: Re: Unified getcwd() implementation Message-ID: <20040508023616.GC24376@darkness.comp.waw.pl> In-Reply-To: <20040507092235.GA61837@stack.nl> References: <20040507092235.GA61837@stack.nl>
next in thread | previous in thread | raw e-mail | index | archive | help
--ccWoVK0blSAgmhl2 Content-Type: text/plain; charset=iso-8859-2 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, May 07, 2004 at 11:22:35AM +0200, Marc Olzheim wrote: +> Hi, +>=20 +> (Re: http://lists.freebsd.org/pipermail/freebsd-arch/2003-August/001152.= html) +>=20 +> > Yes, it's quite an old patch, and much has happened since it was writt= en. +>=20 +> Mostly some fine-grained locking was introduced. +>=20 +> I hope I got everything covered. Here Bruce's patch reworked, that works +> for me. (even over NFS ;-)) +>=20 +> Anyone care to share their view on it ? I'm all for it. It will be great to see getcwd() which just work and not sometimes work. Of course it will be cool to use it in kernel as well. Some comments: +> + +> + rvp =3D fdp->fd_rdir; +> + if (rvp =3D=3D NULL) +> + rvp =3D rootvnode; +> + VREF(rvp); I'm not sure here. When fd_rdir could not be set? I hope it is always set for chrooted/jailed processes. +> + error =3D getcwd_impl(lvp, rvp, &bp, bufp, td); +> + if (!error) { +> + numcwdfound++; +> + if (bufseg =3D=3D UIO_SYSSPACE) +> + bcopy(bp, buf, strlen(bp) + 1); +> + else +> + error =3D copyout(bp, buf, strlen(bp) + 1); +> + } else { +> +#if DIAGNOSTIC +> + printf("getcwd: error %d\n", error); +> +#endif +> + } Could we move #if DIAGNOSTIC before '} else {'? And please, use #ifdef instead of #if. +> + CACHE_LOCK(); +> + ncp =3D TAILQ_FIRST(&lvp->v_cache_dst); +> +#if DIAGNOSTIC +> + /* XXX simulate cache failure every 10 lookups */ +> + if ((numcwdcalls % 10) =3D=3D 0) [...] +> + CACHE_UNLOCK(); +> + numcwdfail2++; +> +#if DIAGNOSTIC +> + printf("getcwd: using scandir\n"); +> +#endif +> + error =3D getcwd_scandir(&lvp, &uvp, &bp, bufp, td); +> + if (error) { +> +#if DIAGNOSTIC +> + printf("getcwd_scandir returned %d\n", error); +> +#endif +> + goto out; +> + } +> +#if DIAGNOSTIC +> + if (lvp !=3D NULL) +> + panic("getcwd: oops, forgot to null lvp"); +> +#endif +> + lvp =3D uvp; +> + uvp =3D NULL; +> } +> +#if DIAGNOSTIC +> + if (bufp && (bp <=3D bufp)) +> + panic("getcwd: oops, went back too far"); s/#if/#ifdef/ And instead of those if+panic I would prefer KASSERT(9), but it is for INVARIANTS... I haven't checked/tested this patch, but it looks very promising, thank you! --=20 Pawel Jakub Dawidek http://www.FreeBSD.org pjd@FreeBSD.org http://garage.freebsd.pl FreeBSD committer Am I Evil? Yes, I Am! --ccWoVK0blSAgmhl2 Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.2.4 (FreeBSD) iD8DBQFAnEegForvXbEpPzQRAt8iAKDVM2rLBjh5HXQshhSXe+n1Wfr9RgCeOkoo NrEn3FdhuqKh0ioV9t3GbAU= =STPO -----END PGP SIGNATURE----- --ccWoVK0blSAgmhl2--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20040508023616.GC24376>