Date: Tue, 9 Sep 2003 19:46:12 -0400 (EDT) From: Daniel Eischen <eischen@vigrid.com> To: Dan Langille <dan@langille.org> Cc: Nate Lawson <nate@root.org> Subject: Re: comments on proposed uthread_write.c changes Message-ID: <Pine.GSO.4.10.10309091929500.13114-100000@pcnet5.pcnet.com> In-Reply-To: <3F5B89B3.11367.112C1E2D@localhost>
next in thread | previous in thread | raw e-mail | index | archive | help
[ Redirected to -standards ] Synopsis: libc_r wraps write() and changes a blocking file descriptor to non-blocking behind the scenes. When __sys_write() returns less than the number of bytes requested, libc_r's write() continues looping trying to finish the requested write. Problem: A return of 0 for a write() to a tape device means end-of-file. Libc_r's write treats a return of 0 from __sys_write() as a partial write and continues looping. The end-of-file is passed and a few more blocks may be written before the physical end-of-tape is reached. [ In this context, end-of-file means a the shiny end-of-tape marker on the tape (at least that's what it looks like on 7 & 9 track tapes -- am I dating myself?). It doesn't mean an EOF marker. ] I've been reading the POSIX spec in regards to write(): http://www.opengroup.org/onlinepubs/007904975/toc.htm particularly the Rationale at the end. I think it is OK for libc_r to special case a return of 0 from __sys_write() and pass it back to the caller. Any comments? On Sun, 7 Sep 2003, Dan Langille wrote: > On 7 Sep 2003 at 12:32, Daniel Eischen wrote: > > > On Sun, 7 Sep 2003, Dan Langille wrote: > > > > > A problem with pthreads and EOT has been identified. See PR 56274. It > > > was suggested the solution was probably just a matter of changing one of > > > the >0 tests to >=0 in uthread_write.c > > > > > > Any comments on that? > > > > I don't know that a return of 0 isn't valid for other devices. > > If this is the case, a return of 0 for blocking writes may break > > other applications. > > > > The patch isn't quite correct (at least looking at -current srcs). > > Lines 98-99 are: > > > > if (blocking && ((n < 0 && (errno == EWOULDBLOCK || > > errno == EAGAIN)) || (n >= 0 && num < nbytes))) { > > > > This will get entered first if n == 0, and I don't think your > > proposed patch would have any effect. I think you would have > > to change the "n >= 0" above to be "n > 0" in conjunction with > > your patch. > > Ahh thank you. That explains why the test results with the original > patch did not differ from -STABLE or 5.1-RELEASE. After adding your > suggestions, we have had success. > > The points to note: > > 1. The status that stopped the writing was 0 > 2. It wrote 17,256 blocks, and read 17,256 blocks. > > Point 1 is key to determining EOT. Point 2 is what you always want > to have... > > [dan@undef:~/tape-test] $ sudo ./tapetest /dev/nsa0 > *rewind > Rewound /dev/nsa0 > *rawfill > Begin writing blocks of 64512 bytes. > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > +++++ > weof_dev > Wrote EOF to /dev/nsa0 > Write failed. Last block written=17256. stat=0 ERR=Unknown error: 0 > *rewind > Rewound /dev/nsa0 > *scan > Starting scan at file 0 > 17256 blocks of 64512 bytes in file 0 > End of File mark. > End of File mark. > End of tape > Total files=1, blocks=17256, bytes = 1113219072 > * > > > This could be solved at the application level by selecting on > > the tape device and performing non-blocking writes to it. Since > > the application knows that a return of 0 is end-of-tape, it > > must also know the difference between talking to a tape device > > and talking to a regular file, socket, etc. > > True. But *if* the code is wrong, it should be fixed. > > FWIW, the patch follows. As always, opinions and suggestions are > welcome. > > --- uthread_write.c.org Sun Sep 7 10:58:31 2003 > +++ uthread_write.c Sun Sep 7 15:41:34 2003 > @@ -93,7 +93,7 @@ > * write: > */ > if (blocking && ((n < 0 && (errno == EWOULDBLOCK || > - errno == EAGAIN)) || (n >= 0 && num < nbytes))) { > + errno == EAGAIN)) || (n > 0 && num < nbytes))) { > curthread->data.fd.fd = fd; > _thread_kern_set_timeout(NULL); > > @@ -131,7 +131,7 @@ > * If there was an error, return partial success > * (if any bytes were written) or else the error: > */ > - } else if (n < 0) { > + } else if (n <= 0) { > if (num > 0) > ret = num; > else > > -- > Dan Langille : http://www.langille.org/ > -- Dan Eischen
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.GSO.4.10.10309091929500.13114-100000>