]> Zhao Yanbai Git Server - minix.git/commitdiff
VFS/PFS: remove notion of position in pipes
authorThomas Veerman <thomas@minix3.org>
Tue, 11 Dec 2012 19:46:09 +0000 (19:46 +0000)
committerThomas Veerman <thomas@minix3.org>
Fri, 11 Jan 2013 09:18:35 +0000 (09:18 +0000)
Because pipes have no file position. VFS maintained (file) offsets into a
buffer internal to PFS and stored them in vnodes for simplicity, mixing
the responsibilities of filp and vnode objects.

With this patch PFS ignores the position field in REQ_READ and REQ_WRITE
requests making VFS' job a lot simpler.

servers/pfs/read.c
servers/vfs/open.c
servers/vfs/pipe.c
servers/vfs/proto.h
servers/vfs/read.c
servers/vfs/select.c
servers/vfs/vnode.h

index 691c0fddce5b749494d811ccf73befae44d57e9d..d80647b58d2059c272ee230bb29fe39b3300b8c8 100644 (file)
@@ -32,23 +32,34 @@ int fs_readwrite(message *fs_m_in, message *fs_m_out)
   /* Get the values from the request message */
   rw_flag = (fs_m_in->m_type == REQ_READ ? READING : WRITING);
   gid = (cp_grant_id_t) fs_m_in->REQ_GRANT;
-  position = fs_m_in->REQ_SEEK_POS_LO;
   nrbytes = (unsigned) fs_m_in->REQ_NBYTES;
 
   /* We can't read beyond the max file position */
   if (nrbytes > MAX_FILE_POS) return(EFBIG);
 
-  if (rw_flag == WRITING) {
-         /* Check in advance to see if file will grow too big. */
-         /* Casting nrbytes to signed is safe, because it's guaranteed not to
-            be beyond max signed value (i.e., MAX_FILE_POS). */
-         if (position > PIPE_BUF - (signed) nrbytes) return(EFBIG);
-  }
-
   /* Mark inode in use */
   if ((get_inode(rip->i_dev, rip->i_num)) == NULL) return(err_code);
   if ((bp = get_block(rip->i_dev, rip->i_num)) == NULL) return(err_code);
 
+  if (rw_flag == WRITING) {
+       /* Check in advance to see if file will grow too big. */
+       /* Casting nrbytes to signed is safe, because it's guaranteed not to
+        * be beyond max signed value (i.e., MAX_FILE_POS).
+        */
+       position = rip->i_size;
+       if (position > PIPE_BUF - (signed) nrbytes) {
+               put_inode(rip);
+               put_block(rip->i_dev, rip->i_num);
+               return(EFBIG);
+       }
+  } else {
+       position = bp->b_bytes;
+       if (nrbytes > rip->i_size - position) {
+               /* There aren't that many bytes to read */
+               nrbytes = rip->i_size - position;
+       }
+  }
+
   if (rw_flag == READING) {
        /* Copy a chunk from the block buffer to user space. */
        r = sys_safecopyto(VFS_PROC_NR, gid, (vir_bytes) 0,
@@ -78,10 +89,12 @@ int fs_readwrite(message *fs_m_in, message *fs_m_out)
        }
   }
 
-  bp->b_bytes = position;
+  if (rw_flag == READING)
+       bp->b_bytes = position;
   if (rw_flag == READING) rip->i_update |= ATIME;
   if (rw_flag == WRITING) rip->i_update |= CTIME | MTIME;
   fs_m_out->RES_NBYTES = (size_t) cum_io;
+
   put_inode(rip);
   put_block(rip->i_dev, rip->i_num);
 
index 0bc7690ff7e2cc3ed5e92da17b40381884d9c838..cc386f6a8d7a9a0b47f24e3905b5c0dc7771a188 100644 (file)
@@ -248,8 +248,6 @@ int common_open(char path[PATH_MAX], int oflags, mode_t omode)
                        r = map_vnode(vp, PFS_PROC_NR);
                        if (r == OK) {
                                if (vp->v_ref_count == 1) {
-                                       vp->v_pipe_rd_pos = 0;
-                                       vp->v_pipe_wr_pos = 0;
                                        if (vp->v_size != 0)
                                                r = truncate_vnode(vp, 0);
                                }
index 494f229dfc3b2f16716de1bcfbb9db186de6f4aa..0ad1c471b0c6c749c04bbc3000cdd5953fd48c1e 100644 (file)
@@ -108,8 +108,6 @@ int do_pipe()
   vp->v_inode_nr = res.inode_nr;
   vp->v_mapinode_nr = res.inode_nr;
   vp->v_mode = res.fmode;
-  vp->v_pipe_rd_pos= 0;
-  vp->v_pipe_wr_pos= 0;
   vp->v_fs_count = 1;
   vp->v_mapfs_count = 1;
   vp->v_ref_count = 1;
@@ -175,13 +173,13 @@ endpoint_t map_to_fs_e;
 /*===========================================================================*
  *                             pipe_check                                   *
  *===========================================================================*/
-int pipe_check(vp, rw_flag, oflags, bytes, position, notouch)
-register struct vnode *vp;     /* the inode of the pipe */
-int rw_flag;                   /* READING or WRITING */
-int oflags;                    /* flags set by open or fcntl */
-register int bytes;            /* bytes to be read or written (all chunks) */
-u64_t position;                        /* current file position */
-int notouch;                   /* check only */
+int pipe_check(
+struct vnode *vp,      /* the inode of the pipe */
+int rw_flag,           /* READING or WRITING */
+int oflags,            /* flags set by open or fcntl */
+int bytes,             /* bytes to be read or written (all chunks) */
+int notouch            /* check only */
+)
 {
 /* Pipes are a little different.  If a process reads from an empty pipe for
  * which a writer still exists, suspend the reader.  If the pipe is empty
@@ -191,13 +189,15 @@ int notouch;                      /* check only */
   off_t pos;
   int r = OK;
 
-  if (ex64hi(position) != 0)
-       panic("pipe_check: position too large in pipe");
-  pos = ex64lo(position);
+  /* Reads start at the beginning; writes append to pipes */
+  if (rw_flag == READING)
+       pos = 0;
+  else
+       pos = vp->v_size;
 
   /* If reading, check for empty pipe. */
   if (rw_flag == READING) {
-       if (pos >= vp->v_size) {
+       if (vp->v_size == 0) {
                /* Process is reading from an empty pipe. */
                if (find_filp(vp, W_BIT) != NULL) {
                        /* Writer exists */
index 4b1f2f220705f207adc9a45a708cac36f904873c..b98d7420d52d27a0ad4d44a0d05c2f96fade741f 100644 (file)
@@ -187,7 +187,7 @@ int do_pipe(void);
 int map_vnode(struct vnode *vp, endpoint_t fs_e);
 void unpause(endpoint_t proc_e);
 int pipe_check(struct vnode *vp, int rw_flag, int oflags, int bytes,
-       u64_t position, int notouch);
+       int notouch);
 void release(struct vnode *vp, int op, int count);
 void revive(endpoint_t proc_e, int returned);
 void suspend(int why);
index 59cbd2b7aa84a9926d4a93c3fc1a51d893315da4..65722ab41066dbdbe35696b60b9b90d2c5a81031 100644 (file)
@@ -264,7 +264,7 @@ size_t req_size;
   int r, oflags, partial_pipe = 0;
   size_t size, cum_io, cum_io_incr;
   struct vnode *vp;
-  u64_t position, new_pos;
+  u64_t  position, new_pos;
 
   /* Must make sure we're operating on locked filp and vnode */
   assert(tll_locked_by_me(&f->filp_vno->v_lock));
@@ -272,12 +272,12 @@ size_t req_size;
 
   oflags = f->filp_flags;
   vp = f->filp_vno;
-  position = cvu64((rw_flag == READING) ? vp->v_pipe_rd_pos :
-                                                       vp->v_pipe_wr_pos);
+  position = cvu64(0); /* Not actually used */
+
   /* fp->fp_cum_io_partial is only nonzero when doing partial writes */
   cum_io = fp->fp_cum_io_partial;
 
-  r = pipe_check(vp, rw_flag, oflags, req_size, position, 0);
+  r = pipe_check(vp, rw_flag, oflags, req_size, 0);
   if (r <= 0) {
        if (r == SUSPEND) pipe_suspend(f, buf, req_size);
        return(r);
@@ -287,16 +287,8 @@ size_t req_size;
   if (size < req_size) partial_pipe = 1;
 
   /* Truncate read request at size. */
-  if((rw_flag == READING) &&
-       cmp64ul(add64ul(position, size), vp->v_size) > 0) {
-       /* Position always should fit in an off_t (LONG_MAX). */
-       off_t pos32;
-
-       assert(cmp64ul(position, LONG_MAX) <= 0);
-       pos32 = cv64ul(position);
-       assert(pos32 >= 0);
-       assert(pos32 <= LONG_MAX);
-       size = vp->v_size - pos32;
+  if (rw_flag == READING && size > vp->v_size) {
+       size = vp->v_size;
   }
 
   if (vp->v_mapfs_e == 0)
@@ -309,7 +301,6 @@ size_t req_size;
        if (ex64hi(new_pos))
                panic("rw_pipe: bad new pos");
 
-       position = new_pos;
        cum_io += cum_io_incr;
        buf += cum_io_incr;
        req_size -= cum_io_incr;
@@ -317,27 +308,19 @@ size_t req_size;
 
   /* On write, update file size and access time. */
   if (rw_flag == WRITING) {
-       if (cmp64ul(position, vp->v_size) > 0) {
-               if (ex64hi(position) != 0) {
+       if (cmp64ul(new_pos, vp->v_size) > 0) {
+               if (ex64hi(new_pos) != 0) {
                        panic("read_write: file size too big for v_size");
                }
-               vp->v_size = ex64lo(position);
+               vp->v_size = ex64lo(new_pos);
        }
   } else {
-       if (cmp64ul(position, vp->v_size) >= 0) {
-               /* Reset pipe pointers */
+       if (cmp64ul(new_pos, vp->v_size) >= 0) {
+               /* Pipe emtpy; reset size */
                vp->v_size = 0;
-               vp->v_pipe_rd_pos= 0;
-               vp->v_pipe_wr_pos= 0;
-               position = cvu64(0);
        }
   }
 
-  if (rw_flag == READING)
-       vp->v_pipe_rd_pos= cv64ul(position);
-  else
-       vp->v_pipe_wr_pos= cv64ul(position);
-
   if (r == OK) {
        if (partial_pipe) {
                /* partial write on pipe with */
index 0898229317341a80ed8b196a844d8ef18def8042..8c3a13ce7cd10889b321bec0691934fed9c68102 100644 (file)
@@ -451,7 +451,8 @@ static int select_request_pipe(struct filp *f, int *ops, int block)
   orig_ops = *ops;
 
   if ((*ops & (SEL_RD|SEL_ERR))) {
-       err = pipe_check(f->filp_vno, READING, 0, 1, f->filp_pos, 1);
+       /* Check if we can read 1 byte */
+       err = pipe_check(f->filp_vno, READING, 0, 1, 1 /* Check only */);
 
        if (err != SUSPEND)
                r |= SEL_RD;
@@ -466,7 +467,8 @@ static int select_request_pipe(struct filp *f, int *ops, int block)
   }
 
   if ((*ops & (SEL_WR|SEL_ERR))) {
-       err = pipe_check(f->filp_vno, WRITING, 0, 1, f->filp_pos, 1);
+       /* Check if we can write 1 byte */
+       err = pipe_check(f->filp_vno, WRITING, 0, 1, 1 /* Check only */);
 
        if (err != SUSPEND)
                r |= SEL_WR;
index bcea1f063342e869b8a520154be10b72f0f8a6f2..2f01d73790620c68a1c23f93a43532b03ac4d5e4 100644 (file)
@@ -16,8 +16,6 @@ EXTERN struct vnode {
 #if 0
   int v_ref_check;             /* for consistency checks */
 #endif
-  off_t v_pipe_rd_pos;
-  off_t v_pipe_wr_pos;
   endpoint_t v_bfs_e;          /* endpoint number for the FS proces in case
                                   of a block special file */
   dev_t v_dev;                  /* device number on which the corresponding