2005-09-10 18:05:44

by Blaisorblade

[permalink] [raw]
Subject: [patch 7/7] uml: retry host close() on EINTR

When calling close() on the host, we must retry the operation when we get
EINTR.

Signed-off-by: Paolo 'Blaisorblade' Giarrusso <[email protected]>
---

arch/um/os-Linux/file.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/arch/um/os-Linux/file.c b/arch/um/os-Linux/file.c
--- a/arch/um/os-Linux/file.c
+++ b/arch/um/os-Linux/file.c
@@ -17,6 +17,7 @@
#include <sys/uio.h>
#include "os.h"
#include "user.h"
+#include "user_util.h"
#include "kern_util.h"

static void copy_stat(struct uml_stat *dst, struct stat64 *src)
@@ -300,7 +301,7 @@ int os_connect_socket(char *name)

void os_close_file(int fd)
{
- close(fd);
+ CATCH_EINTR(close(fd));
}

int os_seek_file(int fd, __u64 offset)

--


2005-09-10 19:00:14

by Linus Torvalds

[permalink] [raw]
Subject: Re: [patch 7/7] uml: retry host close() on EINTR



On Sat, 10 Sep 2005, Paolo 'Blaisorblade' Giarrusso wrote:
>
> When calling close() on the host, we must retry the operation when we get
> EINTR.

Actually, no.

If close() return EINTR, the file descriptor _will_ have been closed. The
error return just tells you that soem error happened on the file: for
example, in the case of EINTR, the close() may not have flushed all the
pending data synchronously.

Re-doing the close() is the wrong thing to do, since in a threaded
environment, something else might have opened another file, gotten the
same file descriptor, and you now close _another_ file.

(Normally, re-doing the close will just return EBADF, of course).

I'm going to drop this patch, but in case you've ever seen a case where
EINTR actually means that the fd didn't get closed, please holler, and we
need to fix it.

Linus

2005-09-11 07:27:09

by Paolo Ornati

[permalink] [raw]
Subject: Re: [patch 7/7] uml: retry host close() on EINTR

On Sat, 10 Sep 2005 12:00:01 -0700 (PDT)
Linus Torvalds <[email protected]> wrote:

> Re-doing the close() is the wrong thing to do, since in a threaded
> environment, something else might have opened another file, gotten
> the same file descriptor, and you now close _another_ file.

So glibc doc is wrong here:

http://www.gnu.org/software/libc/manual/html_node/Opening-and-Closing-Files.html#index-close-1197

-----------------------------------------------------------------------
The normal return value from close is 0; a value of -1 is returned in
case of failure. The following errno error conditions are defined for
this function:

...

EINTR
The close call was interrupted by a signal. See Interrupted
Primitives. Here is an example of how to handle EINTR properly:

TEMP_FAILURE_RETRY (close (desc));
-----------------------------------------------------------------------


And: /usr/include/unistd.h

# define TEMP_FAILURE_RETRY(expression) \
(__extension__ \
({ long int __result; \
do __result = (long int) (expression); \
while (__result == -1L && errno == EINTR); \
__result; }))
#endif


SUSV3:
-------------------------------------------------------------
If close() is interrupted by a signal that is to be caught, it shall
return -1 with errno set to [EINTR] and the state of fildes is
unspecified
-------------------------------------------------------------

Unspecified! ;-)

--
Paolo Ornati
Linux 2.6.13.1 on x86_64

2005-09-11 11:49:28

by Linus Torvalds

[permalink] [raw]
Subject: Re: [patch 7/7] uml: retry host close() on EINTR



On Sun, 11 Sep 2005, Paolo Ornati wrote:
>
> So glibc doc is wrong here:

Yes.

> SUSV3:
> -------------------------------------------------------------
> If close() is interrupted by a signal that is to be caught, it shall
> return -1 with errno set to [EINTR] and the state of fildes is
> unspecified
> -------------------------------------------------------------
>
> Unspecified! ;-)

I don't know of any system where re-trying the close() is the right thing
to do, but I guess they exist. I think the Linux behaviour of "hey, it's
closed, live with it" is pretty universal - almost nobody ever tests the
return value of close().

Even the "careful" users that want to hear about IO errors have to really
do an fsync(), so any IO errors should show up there. Of course, checking
the return value of "close()" in addition to the fsync() is always a good
idea anyway, and I suspect they do.

In Linux, another thread may return with the same fd in open() even _long_
before the close() that released it has even finished. The kernel releases
the fd itself early, and then the rest (anything that could return EINTR -
things like TCP linger stuff etc) is all done with just the "struct file".

So retrying is really really wrong. And not just on Linux. I think this is
true on _most_ if not all unix implementations out there.

Linus