Date: Thu, 2 Aug 2012 16:51:03 +0300 From: Konstantin Belousov <kostikbel@gmail.com> To: davidxu@freebsd.org Cc: arch@freebsd.org Subject: Re: short read/write and error code Message-ID: <20120802135103.GX2676@deviant.kiev.zoral.com.ua> In-Reply-To: <501A69EB.9000701@gmail.com> 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>
next in thread | previous in thread | raw e-mail | index | archive | help
--H8/LwUd9h7ybXz/g
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Thu, Aug 02, 2012 at 07:52:11PM +0800, 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)
> > }
> > =20
> > /*
> >- * 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 =3D=3D 0) &&
> >- (uio->uio_resid =3D=3D 0) &&
> >- (error =3D=3D EPIPE)) {
> >+ if (uio->uio_resid !=3D orig_resid && error =3D=3D EPIPE)
> > error =3D 0;
> >- }
> > =20
> > if (error =3D=3D 0)
> > vfs_timestamp(&wpipe->pipe_mtime);
>=20
> I dislike the patch, if I thought it is right, I would have already=20
> 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
I cannot believe that=20
> -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...
And I do think that this behaviour is right.
This only different from the CURRENT by the point where the race between
writer and exiting reader becomes observable. CURRENT reports that reader
side was closed earlier, while my patch pretends that it exited slightly
later.
>=20
> my patch is more compatible with -current. if application does not
> setup a signal handler for SIGPIPE, when short write happens, it is kille=
d,
> it is same as -current. if the application set up a signal handler for=20
> the signal,
> it always should check the return value from write(), this is how=20
> traditional
> code works.
Now assume that application set the SIGPIPE handler, and did a short
write. How can it interpret the delivered signal, if the following
syscall return indicates that the write was successful ?
Let me repeat myself: there are two objections against your patch.
First is the signal generation when no EPIPE is returned to application
(observable as I described in the previous paragraph).
Second is that having the change in generic i/o layer requires impossible
behaviour from the filesystems (ability to roll back failed uio transfer).
>=20
> 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.
>=20
This is right behaviour. If the application wrote all data it wanted to wri=
te,
and went doing something else, then there is no reason to kill it.
--H8/LwUd9h7ybXz/g
Content-Type: application/pgp-signature
Content-Disposition: inline
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (FreeBSD)
iEYEARECAAYFAlAahccACgkQC3+MBN1Mb4hLPgCgzKLF37K8LPJyRhm8ALtrP+vi
GEwAnRHhZPL5T5NXzUfWqnkBPYMPUcFV
=WNw2
-----END PGP SIGNATURE-----
--H8/LwUd9h7ybXz/g--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120802135103.GX2676>