Fix up reads.
authorRonald G. Minnich <rminnich@google.com>
Thu, 3 Jul 2014 20:13:20 +0000 (13:13 -0700)
committerRonald G. Minnich <rminnich@google.com>
Fri, 4 Jul 2014 03:27:46 +0000 (20:27 -0700)
Buffer up directory read info so we can feed it to the
kernel one kdirent at a time. There is some performance
improvement to be had but this all by itself should remove the
'missed entries' problem and it also gets rid of the ghetto hack.

You should run the latest version of github.com/rminnich/go9p
to get the advantages

Signed-off-by: Ronald G. Minnich <rminnich@google.com>
kern/drivers/dev/mnt.c
kern/include/ns.h
kern/src/ns/chan.c
kern/src/ns/convM2D.c
kern/src/ns/sysfile.c

index ac0c426..14bea93 100644 (file)
@@ -654,10 +654,17 @@ static int mntwstat(struct chan *c, uint8_t * dp, int n)
        return n;
 }
 
+/* the servers should either return units of whole directory entries
+ * OR support seeking to an arbitrary place. One or other.
+ * Both are fine, but at least one is a minimum.
+ * If the return a partial result, but more than one result,
+ * we'll return a shorter read and the next offset will be aligned
+ */
 static long mntread(struct chan *c, void *buf, long n, int64_t off)
 {
        uint8_t *p, *e;
        int nc, cache, isdir, dirlen;
+       int numdirent = 0;
 
        isdir = 0;
        cache = c->flag & CCACHE;
@@ -682,16 +689,24 @@ static long mntread(struct chan *c, void *buf, long n, int64_t off)
        }
 
        n = mntrdwr(Tread, c, buf, n, off);
+
        if (isdir) {
                for (e = &p[n]; p + BIT16SZ < e; p += dirlen) {
                        dirlen = BIT16SZ + GBIT16(p);
-                       if (p + dirlen > e)
+                       if (p + dirlen > e){
                                break;
+                       }
                        validstat(p, dirlen);
                        mntdirfix(p, c);
+                       numdirent += dirlen;
+               }
+               if (p != e) {
+                       //error(Esbadstat);
+                       /* not really. Maybe the server supports
+                        * arbitrary seek like go9p now does.
+                        */
+                       n = numdirent;
                }
-               if (p != e)
-                       error(Esbadstat);
        }
        return n;
 }
index 667971b..e32c7d1 100644 (file)
@@ -389,6 +389,9 @@ struct chan {
        struct chan *mchan;                     /* channel to mounted server */
        struct qid mqid;                        /* qid of root of mount point */
        struct cname *name;
+       /* hack for reads to try to get them right. */
+       void *buf;
+       int bufused;
 };
 
 struct cname {
index 17ec2d6..131ece5 100644 (file)
@@ -169,6 +169,7 @@ struct chan *newchan(void)
        c->mqid.vers = 0;
        c->mqid.type = 0;
        c->name = 0;
+       c->buf = NULL;
        return c;
 }
 
@@ -258,6 +259,10 @@ void chanfree(struct chan *c)
        }
 
        cnameclose(c->name);
+       if (c->buf)
+               kfree(c->buf);
+       c->buf = NULL;
+       c->bufused = 0;
 
        spin_lock(&(&chanalloc)->lock);
        c->next = chanalloc.free;
index f7fd611..dc2e81c 100644 (file)
@@ -20,8 +20,13 @@ int statcheck(uint8_t * buf, unsigned int nbuf)
 
        ebuf = buf + nbuf;
 
-       if (nbuf < STATFIXLEN || nbuf != BIT16SZ + GBIT16(buf))
+       if (nbuf < STATFIXLEN || nbuf != BIT16SZ + GBIT16(buf)){
+               printk("nbuf %d, STATFIXLEN %d ", nbuf, STATFIXLEN);
+               printk("BIT16SZ %d, GBIT16(buf) %d ", 
+                       BIT16SZ, GBIT16(buf));
+               printk("This is bad!\n");
                return -1;
+       }
 
        buf += STATFIXLEN - 4 * BIT16SZ;
 
@@ -31,8 +36,9 @@ int statcheck(uint8_t * buf, unsigned int nbuf)
                buf += BIT16SZ + GBIT16(buf);
        }
 
-       if (buf != ebuf)
+       if (buf != ebuf){
                return -1;
+       }
 
        return 0;
 }
index 94f477d..10500b6 100644 (file)
 
 static int growfd(struct fgrp *f, int fd)
 {
+       
        int n;
        struct chan **nfd, **ofd;
 
-       if (fd < f->nfd)
+       if (fd < f->nfd) {
                return 0;
+       }
        n = f->nfd + DELTAFD;
        if (n > MAXNFD)
                n = MAXNFD;
-       if (fd >= n)
+       if (fd >= n) {
                return -1;
+       }
        nfd = kzmalloc(n * sizeof(struct chan *), 0);
-       if (nfd == NULL)
+       if (nfd == NULL) {
                return -1;
+       }
        ofd = f->fd;
        memmove(nfd, ofd, f->nfd * sizeof(struct chan *));
        f->fd = nfd;
@@ -38,6 +42,7 @@ static int growfd(struct fgrp *f, int fd)
 
 int newfd(struct chan *c)
 {
+       
        int i;
        struct fgrp *f = current->fgrp;
 
@@ -81,6 +86,7 @@ int newfd(struct chan *c)
 
 struct chan *fdtochan(struct fgrp *f, int fd, int mode, int chkmnt, int iref)
 {
+       
        struct chan *c;
 
        c = 0;
@@ -104,8 +110,9 @@ struct chan *fdtochan(struct fgrp *f, int fd, int mode, int chkmnt, int iref)
                error(Ebadusefd);
        }
 
-       if (mode < 0 || c->mode == ORDWR)
+       if (mode < 0 || c->mode == ORDWR) {
                return c;
+       }
 
        if ((mode & OTRUNC) && IS_RDONLY(c->mode)) {
                if (iref)
@@ -171,13 +178,15 @@ int openmode(uint32_t omode)
 #endif
        /* no error checking (we have a shitload of flags anyway), and we return the
         * basic access modes (RD/WR/ETC) */
-       if (omode == O_EXEC)
-               return O_RDONLY;
+       if (omode == O_EXEC) {
+       return O_RDONLY;
+       }
        return omode & O_ACCMODE;
 }
 
 void fdclose(struct fgrp *f, int fd)
 {
+       
        int i;
        struct chan *c;
 
@@ -723,12 +732,8 @@ static long rread(int fd, void *va, long n, int64_t * offp)
 
        /* dirty dirent hack */
        void *real_va = va;
-       void *buf_for_M = 0;
-       size_t buf_sz = 0;
-       long real_n = n;
 
        if (waserror()) {
-               kfree(buf_for_M);
                poperror();
                return -1;
        }
@@ -744,33 +749,30 @@ static long rread(int fd, void *va, long n, int64_t * offp)
 
        dir = c->qid.type & QTDIR;
 
-       /* dirty kdirent hack: userspace is expecting kdirents, but all of 9ns
-        * produces Ms.  i'm assuming we're only being asked to read a single
-        * dirent, which is usually the case for calls like readdir() (which just
-        * calls read for a single dirent). */
-       if (dir)
+       /* kdirent hack: userspace is expecting kdirents, but all of 9ns
+        * produces Ms.  Just save up what we don't use and append the
+        * new stuff later. Allocate 2048 bytes for that purpose. 
+        */
+       if (dir) {
                assert(n >= sizeof(struct kdirent));
-       buf_sz = 2 * MIN_M_BUF_SZ - 1;
        /* We need to read exactly one dirent and avoid reading too much from the
-        * underlying dev, so that subsequent reads don't miss any dirents.  So we
-        * start small, and if our buffer is too small (e.g. for long named
-        * dirents), we increase by a minumum amount.  This way, we'll succeed on
-        * the next invocation, but we won't have enough room for more than one
-        * entry. */
+        * underlying dev, so that subsequent reads don't miss any dirents. 
+        * Buffer 2*MAXPATH, to be safe. Read off what you can. Return one
+        * kdirent at a time. If you don't have enough, read more. If you run out,
+        * you're done.
+        */
+       if (!c->buf){
+               c->buf=kmalloc(2048, KMALLOC_WAIT);
+               c->bufused = 0;
+       }
+       }
        while (waserror()) {
-               /* FYI: this scheme doesn't work with mounts */
-               if (!dir || strcmp(current_errstr(), Eshort))
-                       nexterror();
-               buf_sz += MIN_M_BUF_SZ;
-               poperror();
+               printk("Well, sysread of a dir sucks.%s \n", current_errstr());
+               nexterror();
        }
        if (dir) {
-               if (!buf_for_M)
-                       buf_for_M = kmalloc(buf_sz, KMALLOC_WAIT);
-               else
-                       buf_for_M = krealloc(buf_for_M, buf_sz, KMALLOC_WAIT);
-               va = buf_for_M;
-               n = buf_sz;
+               va = c->buf + c->bufused;
+               n = 2048 - c->bufused;
        }
 
        /* this is the normal plan9 read */
@@ -802,8 +804,12 @@ static long rread(int fd, void *va, long n, int64_t * offp)
 
        /* dirty kdirent hack */
        if (dir) {
-               convM2kdirent(buf_for_M, buf_sz, real_va, 0);
-               kfree(buf_for_M);
+               int amt;
+               c->bufused = c->bufused + n;
+               amt = convM2kdirent(c->buf, c->bufused, real_va, 0);
+               c->bufused -= amt;
+               memmove(c->buf, c->buf + amt, c->bufused);
+               n = amt;
        }
        poperror();     /* matching our while(waserror) */
 
@@ -927,6 +933,7 @@ int64_t sysseek(int fd, int64_t off, int whence)
 
 void validstat(uint8_t * s, int n)
 {
+       
        int m;
        char buf[64];
 
@@ -942,8 +949,9 @@ void validstat(uint8_t * s, int n)
         */
        m = GBIT16(s);
        s += BIT16SZ;
-       if (m + 1 > sizeof buf)
+       if (m + 1 > sizeof buf) {
                return;
+       }
        memmove(buf, s, m);
        buf[m] = '\0';
        /* name could be '/' */
@@ -977,6 +985,7 @@ int sysfstat(int fd, uint8_t *buf, int n)
 
 int sysfstatakaros(int fd, struct kstat *ks)
 {
+       
        int n = 4096;
        uint8_t *buf;
        buf = kmalloc(n, KMALLOC_WAIT);
@@ -1015,6 +1024,7 @@ int sysstat(char *path, uint8_t *buf, int n)
 
 int sysstatakaros(char *path, struct kstat *ks)
 {
+       
        int n = 4096;
        uint8_t *buf;
        buf = kmalloc(n, KMALLOC_WAIT);
@@ -1218,6 +1228,7 @@ struct dir *sysdirfstat(int fd)
 
 int sysdirwstat(char *name, struct dir *dir)
 {
+       
        uint8_t *buf;
        int r;
 
@@ -1231,6 +1242,7 @@ int sysdirwstat(char *name, struct dir *dir)
 
 int sysdirfwstat(int fd, struct dir *dir)
 {
+       
        uint8_t *buf;
        int r;
 
@@ -1244,12 +1256,14 @@ int sysdirfwstat(int fd, struct dir *dir)
 
 static long dirpackage(uint8_t * buf, long ts, struct kdirent **d)
 {
+       
        char *s;
        long ss, i, n, nn, m = 0;
 
        *d = NULL;
-       if (ts <= 0)
+       if (ts <= 0) {
                return ts;
+       }
 
        /*
         * first find number of all stats, check they look like stats, & size all associated strings
@@ -1348,6 +1362,7 @@ int sysiounit(int fd)
  *   never happen with the current code). */
 void close_9ns_files(struct proc *p, bool only_cloexec)
 {
+       
        struct fgrp *f = p->fgrp;
 
        spin_lock(&f->lock);
@@ -1373,6 +1388,7 @@ void close_9ns_files(struct proc *p, bool only_cloexec)
 
 void print_chaninfo(struct chan *c)
 {
+       
        char buf[64] = { 0 };
        bool has_dev = c->type != -1;
        if (has_dev && !devtab[c->type].chaninfo) {
@@ -1391,6 +1407,7 @@ void print_chaninfo(struct chan *c)
 
 void print_9ns_files(struct proc *p)
 {
+       
        struct fgrp *f = p->fgrp;
        spin_lock(&f->lock);
        printk("9ns files for proc %d:\n", p->pid);
@@ -1408,6 +1425,7 @@ void print_9ns_files(struct proc *p)
  * copying the fgrp, and sharing the pgrp. */
 int plan9setup(struct proc *new_proc, struct proc *parent, int flags)
 {
+       
        struct proc *old_current;
        struct kref *new_dot_ref;
        ERRSTACK(1);