From 5e85b59f1888c9ba98d03b181bc2fc1399b777e7 Mon Sep 17 00:00:00 2001 From: Alan Cox Date: Sun, 28 Aug 2016 23:43:51 +0100 Subject: [PATCH] pipe: first cut at fixing a pile of pipe breakages There is still lots more to deal with - introduce a helper to check if a file is open for read/write - use this to correct the EOF checking Original bugs noted by David Flamand --- Kernel/filesys.c | 15 +++++++++++++++ Kernel/include/kernel.h | 5 +++++ Kernel/inode.c | 37 ++++++++++++++++++++++--------------- 3 files changed, 42 insertions(+), 15 deletions(-) diff --git a/Kernel/filesys.c b/Kernel/filesys.c index 57ac39c2..2dfdde67 100644 --- a/Kernel/filesys.c +++ b/Kernel/filesys.c @@ -687,6 +687,21 @@ int8_t oft_alloc(void) return -1; } +/* Check if an inode is open for reading or for writing. Needed for pipe + * EOF and will one day be needed for shared code segments (8086 etc) + */ +int8_t oft_inuse(inoptr ino, uint8_t rw) +{ + struct oft *ofptr = of_tab; + while(ofptr <= &of_tab[OFTSIZE - 1]) { + if (ofptr->o_refs && ofptr->o_inode == ino && + O_ACCMODE(ofptr->o_access) != rw) + return 1; + ofptr++; + } + return 0; +} + /* * To minimise storage we don't track exclusive locks explicitly. We know * that if we are dropping an exclusive lock then we must be the owner, diff --git a/Kernel/include/kernel.h b/Kernel/include/kernel.h index c9a000e2..01e5e000 100644 --- a/Kernel/include/kernel.h +++ b/Kernel/include/kernel.h @@ -776,6 +776,11 @@ extern void i_free(uint16_t devno, uint16_t ino); extern blkno_t blk_alloc(uint16_t devno); extern void blk_free(uint16_t devno, blkno_t blk); extern int8_t oft_alloc(void); +extern int8_t oft_inuse(inoptr ino, uint8_t rw); +/* Yes these are intentionally backwards - there are 3 modes so the loop looks + for "not this mode" */ +#define INUSE_R O_WRONLY +#define INUSE_W O_RDONLY extern void deflock(struct oft *ofptr); extern void oft_deref(int8_t of); /* returns index of slot, or -1 on failure */ diff --git a/Kernel/inode.c b/Kernel/inode.c index 1c590db9..dc0f2ac1 100644 --- a/Kernel/inode.c +++ b/Kernel/inode.c @@ -10,6 +10,23 @@ #define read_direct(flag) (flag & O_DIRECT) #endif +/* This assumes it's called once before we do I/O. That's wrong and we + need to integrate this into the I/O loop, but when we do it changes + how we handle the psleep_flags bit */ +static uint8_t pipewait(inoptr ino, uint8_t flag) +{ + int8_t n; + while(ino->c_node.i_size == 0) { + n = oft_inuse(ino, INUSE_W); + if (n == 0 || psleep_flags(ino, flag)) { + udata.u_count = 0; + return 0; + } + } + udata.u_count = min(udata.u_count, ino->c_node.i_size); + return 1; +} + /* Writei (and readi) need more i/o error handling */ void readi(inoptr ino, uint8_t flag) { @@ -33,7 +50,6 @@ void readi(inoptr ino, uint8_t flag) udata.u_count = min(udata.u_count, ino->c_node.i_size - udata.u_offset); } - toread = udata.u_count; goto loop; case MODE_R(F_SOCK): @@ -43,25 +59,16 @@ void readi(inoptr ino, uint8_t flag) #endif case MODE_R(F_PIPE): ispipe = true; - while (ino->c_node.i_size == 0 && !(flag & O_NDELAY)) { - if (ino->c_refs == 1) /* No writers */ - break; - /* Sleep if empty pipe */ - if (psleep_flags(ino, flag)) - break; - } - toread = udata.u_count = min(udata.u_count, ino->c_node.i_size); - if (toread == 0) { - udata.u_error = EWOULDBLOCK; - break; - } + /* This bit really needs to be inside the loop for pipe cases */ + if (!pipewait(ino, flag)) + break; goto loop; case MODE_R(F_BDEV): - toread = udata.u_count; dev = *(ino->c_node.i_addr); loop: + toread = udata.u_count; while (toread) { amount = min(toread, BLKSIZE - BLKOFF(udata.u_offset)); pblk = bmap(ino, udata.u_offset >> BLKSHIFT, 1); @@ -157,7 +164,7 @@ void writei(inoptr ino, uint8_t flag) in one go - needs merging into the loop */ while ((towrite = udata.u_count) > (16 * BLKSIZE) - ino->c_node.i_size) { - if (ino->c_refs == 1) { /* No readers */ + if (!oft_inuse(ino, INUSE_R)) { /* No readers */ udata.u_count = (usize_t)-1; udata.u_error = EPIPE; ssig(udata.u_ptab, SIGPIPE); -- 2.34.1