tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: fcntl(F_GETPATH) support



In article <4e7e49e0-9c71-1f21-22fc-8ed54590a686%gmx.com@localhost>,
Kamil Rytarowski  <n54%gmx.com@localhost> wrote:
>-=-=-=-=-=-
>-=-=-=-=-=-
>
>On 15.09.2019 20:03, Christos Zoulas wrote:
>> I think it is quite reliable because all the file descriptors would be
>recently
>> opened and therefore be in the cache.  One would need to DoS the cache
>> cause eviction. If that turns out to be false, we can make the namecache
>> reliable, or withdraw it.
>
>As discussed with Mateusz Guzik offlist, the dentry approach looks
>reliable and as a way to go.
>
>It changes fd -> vnode -> inode to fd -> dentry -> vnode.
>
>We could switch from catching program name on exec to proper pathname
>resolution with KERN_PROC_PATHNAME.
>
>Certain programs require always correct path to be resolved from
>hardlinks, not the last one from the cache. This used to affect LLVM.
>
>There is also a hole in the current namecache implementation as it
>misses entry for newly created files (as informed by Mateusz). Example:
>
>#include <sys/types.h>
>#include <err.h>
>#include <fcntl.h>
>#include <stdio.h>
>#include <unistd.h>
>
>int
>main(void)
>{
>        char buf[1024];
>        int fd;
>
>        fd = open("/tmp/test", O_RDWR|O_CREAT|O_EXCL,0600);
>        if (fd == -1)
>                err(1, "open");
>        if (fcntl(fd, F_GETPATH, buf) < 0)
>                err(1, "fcntl");
>        printf("[%s]\n", buf);
>}
>
>For the time being, the current code is still an improvement over the
>past state.

Well, I did some experiments by adding a cache_enter call in ufs inode
creation. This fixes the above case, and makes the cache more logically
consistent i.e. we usually create files we are going to access, so we
might as well cache them (consider a build scenario and object files,
or even binaries that will get installed). This is a one line change
per filesystem:

Index: ufs_vnops.c
===================================================================
RCS file: /cvsroot/src/sys/ufs/ufs/ufs_vnops.c,v
retrieving revision 1.247
diff -u -p -u -r1.247 ufs_vnops.c
--- ufs_vnops.c 1 Jul 2019 00:57:06 -0000       1.247
+++ ufs_vnops.c 17 Sep 2019 20:32:48 -0000
@@ -1909,6 +1909,7 @@ ufs_makeinode(struct vattr *vap, struct 
        if (error)
                goto bad;
        *vpp = tvp;
+       cache_enter(dvp, *vpp, cnp->cn_nameptr, cnp->cn_namelen, cnp->cn_flags);
        return (0);
 
  bad:

The above change does not seem to affect build times...

no cache
build.sh started:    Mon Sep 16 13:11:04 EDT 2019
build.sh ended:      Mon Sep 16 15:27:41 EDT 2019
2:16:37

cache
build.sh started:    Mon Sep 16 16:02:18 EDT 2019
build.sh ended:      Mon Sep 16 18:18:23 EDT 2019
2:16:15

I don't know what made the builds slower the second day... We should
investigate why builds on a freshly booted machines are faster!

cache
build.sh started:    Tue Sep 17 11:05:35 EDT 2019
build.sh ended:      Tue Sep 17 13:37:33 EDT 2019
2:31:58

no cache
build.sh started:    Tue Sep 17 14:01:14 EDT 2019
build.sh ended:      Tue Sep 17 16:29:17 EDT 2019
2:28:03

I am not sure though if we should change the current behavior just to make
F_GETPATH better? Opinions?

christos



Home | Main Index | Thread Index | Old Index