From owner-freebsd-arch@FreeBSD.ORG Thu Aug 2 11:53:55 2012 Return-Path: Delivered-To: arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 0D276106566C; Thu, 2 Aug 2012 11:53:55 +0000 (UTC) (envelope-from listlog2011@gmail.com) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id E2DB08FC14; Thu, 2 Aug 2012 11:53:54 +0000 (UTC) Received: from [127.0.0.1] (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.5/8.14.5) with ESMTP id q72BrorA040243; Thu, 2 Aug 2012 11:53:51 GMT (envelope-from listlog2011@gmail.com) Message-ID: <501A6A4B.8060209@gmail.com> Date: Thu, 02 Aug 2012 19:53:47 +0800 From: David Xu User-Agent: Mozilla/5.0 (Windows NT 6.1; rv:14.0) Gecko/20120713 Thunderbird/14.0 MIME-Version: 1.0 To: davidxu@freebsd.org References: <5018992C.8000207@freebsd.org> <20120801071934.GJ2676@deviant.kiev.zoral.com.ua> <20120801183240.K1291@besplex.bde.org> <20120801162836.GO2676@deviant.kiev.zoral.com.ua> <20120802040542.G2978@besplex.bde.org> <20120802100240.GV2676@deviant.kiev.zoral.com.ua> <501A69EB.9000701@gmail.com> In-Reply-To: <501A69EB.9000701@gmail.com> Content-Type: multipart/mixed; boundary="------------020705050204070000090402" Cc: Konstantin Belousov , arch@freebsd.org Subject: Re: short read/write and error code X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: davidxu@freebsd.org List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 02 Aug 2012 11:53:55 -0000 This is a multi-part message in MIME format. --------------020705050204070000090402 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit On 2012/8/2 19:52, David Xu wrote: > On 2012/8/2 18:02, Konstantin Belousov wrote: >> diff --git a/sys/kern/sys_pipe.c b/sys/kern/sys_pipe.c >> index 338256c..1a61b93 100644 >> --- a/sys/kern/sys_pipe.c >> +++ b/sys/kern/sys_pipe.c >> @@ -1286,13 +1286,13 @@ pipe_write(fp, uio, active_cred, flags, td) >> } >> /* >> - * Don't return EPIPE if I/O was successful >> + * Don't return EPIPE if any byte was written. >> + * EINTR and other interrupts are handled by generic I/O layer. >> + * Do not pretend that I/O succeeded for obvious user error >> + * like EFAULT. >> */ >> - if ((wpipe->pipe_buffer.cnt == 0) && >> - (uio->uio_resid == 0) && >> - (error == EPIPE)) { >> + if (uio->uio_resid != orig_resid && error == EPIPE) >> error = 0; >> - } >> if (error == 0) >> vfs_timestamp(&wpipe->pipe_mtime); > > I dislike the patch, if I thought it is right, I would have already > submit such > a patch. Now let us see why your patch is wore than my version > (attached): > -current: > short write is done, some bytes were sent > dofilewrite returns -1, errno is EPIPE > dofilewrite sends SIGPIPE > application is killed by SIGPIPE > -my attached version: > short write is done, some bytes were sent > dofilewrite return number of bytes sent, errno is zero > dofilewrite sends SIGPIPE. > application is killed by SIGPIPE > -you version: > short write is done, some bytes were sent. > dofilewrite returns number of bytes sent, errno is zero. > dofilewrite does not send SIGPIPE signal > application is not killed > application does not check return value from write() > application thinks it is successful, application does other things, > application might begin a bank account transaction. > ... > application never comes back... > > my patch is more compatible with -current. if application does not > setup a signal handler for SIGPIPE, when short write happens, it is > killed, > it is same as -current. if the application set up a signal handler for > the signal, > it always should check the return value from write(), this is how > traditional > code works. > > in your patch, short write does not kill application, you can not assume > that the application will request a next write() on the pipe, and you > hope > the second write to kill the application, but there is always exception, > the next write does not happen, application works on other things. > This is too incompatible with -current. > > Oops, I forgot to attach it. --------------020705050204070000090402 Content-Type: text/plain; charset=gb18030; name="sys_generic_short_pipe_write.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="sys_generic_short_pipe_write.diff" Index: sys_generic.c =================================================================== --- sys_generic.c (revision 238927) +++ sys_generic.c (working copy) @@ -539,15 +539,21 @@ if (fp->f_type == DTYPE_VNODE) bwillwrite(); if ((error = fo_write(fp, auio, td->td_ucred, flags, td))) { - if (auio->uio_resid != cnt && (error == ERESTART || - error == EINTR || error == EWOULDBLOCK)) - error = 0; /* Socket layer is responsible for issuing SIGPIPE. */ if (fp->f_type != DTYPE_SOCKET && error == EPIPE) { PROC_LOCK(td->td_proc); tdsignal(td, SIGPIPE); PROC_UNLOCK(td->td_proc); } + if (auio->uio_resid != cnt) { + if (error == ERESTART || error == EINTR || + error == EWOULDBLOCK) + error = 0; + else if (error == EPIPE && + (fp->f_type == DTYPE_PIPE || + fp->f_type == DTYPE_FIFO)) + error = 0; + } } cnt -= auio->uio_resid; #ifdef KTRACE --------------020705050204070000090402--