From owner-freebsd-hackers@FreeBSD.ORG Sun Sep 7 16:40:44 2003 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 8642416A4BF for ; Sun, 7 Sep 2003 16:40:44 -0700 (PDT) Received: from bast.unixathome.org (bast.unixathome.org [66.11.174.150]) by mx1.FreeBSD.org (Postfix) with ESMTP id 32A1243FFB for ; Sun, 7 Sep 2003 16:40:43 -0700 (PDT) (envelope-from dan@langille.org) Received: from wocker (wocker.unixathome.org [192.168.0.99]) by bast.unixathome.org (Postfix) with ESMTP id F12743D28; Sun, 7 Sep 2003 19:40:42 -0400 (EDT) From: "Dan Langille" To: Daniel Eischen Date: Sun, 07 Sep 2003 19:40:35 -0400 MIME-Version: 1.0 Message-ID: <3F5B89B3.11367.112C1E2D@localhost> Priority: normal References: <20030907110314.U78699@xeon.unixathome.org> In-reply-to: X-mailer: Pegasus Mail for Windows (v4.02a) Content-type: text/plain; charset=US-ASCII Content-transfer-encoding: 7BIT Content-description: Mail message body cc: freebsd-hackers@FreeBSD.org cc: Dan Nelson cc: Kern Sibbald cc: Nate Lawson Subject: Re: comments on proposed uthread_write.c changes X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 07 Sep 2003 23:40:44 -0000 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/