Re: Update of file offset on write() etc. is non-atomic with I/O

[Linus Torvalds <torvalds@linux-foundation.org>]

[-- Attachment #1: Type: text/plain, Size: 680 bytes --]

On Mon, Mar 3, 2014 at 2:01 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> The thing is, the callers in there do *not* keep struct file * at all -
> they keep struct socket * and use sock->file to get struct file * back
> when they need it.

Not a problem. Just make sockfd_lookup_light() use the broken
inefficient calling convention.

So then the networking code can continue to use the old bad interface,
without it impacting the normal users.

Something like the attached untested patch. This gets rid of
"fget_light()", and instead makes "fdget()" the native interface (same
for the "raw" version).

Totally untested patch (on top of my previous one)

               Linus

[-- Attachment #2: patch.diff --]
[-- Type: text/plain, Size: 3673 bytes --]

 fs/file.c            | 30 +++++++++++++++---------------
 include/linux/file.h | 24 ++----------------------
 net/socket.c         | 17 ++++++++++++-----
 3 files changed, 29 insertions(+), 42 deletions(-)

diff --git a/fs/file.c b/fs/file.c
index db25c2bdfe46..cd0c1c329baf 100644
--- a/fs/file.c
+++ b/fs/file.c
@@ -683,33 +683,33 @@ EXPORT_SYMBOL(fget_raw);
  * The fput_needed flag returned by fget_light should be passed to the
  * corresponding fput_light.
  */
-struct file *__fget_light(unsigned int fd, fmode_t mask, int *fput_needed)
+static struct fd __fdget(unsigned int fd, fmode_t mask)
 {
 	struct files_struct *files = current->files;
-	struct file *file;
+	struct fd f = { NULL, };
 
-	*fput_needed = 0;
 	if (atomic_read(&files->count) == 1) {
-		file = __fcheck_files(files, fd);
-		if (file && (file->f_mode & mask))
-			file = NULL;
+		f.file = __fcheck_files(files, fd);
+		if (f.file && (f.file->f_mode & mask))
+			f.file = NULL;
 	} else {
-		file = __fget(fd, mask);
-		if (file)
-			*fput_needed = 1;
+		f.file = __fget(fd, mask);
+		if (f.file)
+			f.need_put = 1;
 	}
 
-	return file;
+	return f;
 }
-struct file *fget_light(unsigned int fd, int *fput_needed)
+
+struct fd fdget(unsigned int fd)
 {
-	return __fget_light(fd, FMODE_PATH, fput_needed);
+	return __fdget(fd, FMODE_PATH);
 }
-EXPORT_SYMBOL(fget_light);
+EXPORT_SYMBOL(fdget);
 
-struct file *fget_raw_light(unsigned int fd, int *fput_needed)
+struct fd fdget_raw(unsigned int fd)
 {
-	return __fget_light(fd, 0, fput_needed);
+	return __fdget(fd, 0);
 }
 
 void set_close_on_exec(unsigned int fd, int flag)
diff --git a/include/linux/file.h b/include/linux/file.h
index 636634a09c92..253402e6f3da 100644
--- a/include/linux/file.h
+++ b/include/linux/file.h
@@ -20,12 +20,6 @@ struct path;
 extern struct file *alloc_file(struct path *, fmode_t mode,
 	const struct file_operations *fop);
 
-static inline void fput_light(struct file *file, int fput_needed)
-{
-	if (fput_needed)
-		fput(file);
-}
-
 struct fd {
 	struct file *file;
 	unsigned need_put:1, need_pos_unlock:1;
@@ -38,24 +32,10 @@ static inline void fdput(struct fd fd)
 }
 
 extern struct file *fget(unsigned int fd);
-extern struct file *fget_light(unsigned int fd, int *fput_needed);
-
-static inline struct fd fdget(unsigned int fd)
-{
-	int b;
-	struct file *f = fget_light(fd, &b);
-	return (struct fd){f,b,0};
-}
-
 extern struct file *fget_raw(unsigned int fd);
-extern struct file *fget_raw_light(unsigned int fd, int *fput_needed);
 
-static inline struct fd fdget_raw(unsigned int fd)
-{
-	int b;
-	struct file *f = fget_raw_light(fd, &b);
-	return (struct fd){f,b,0};
-}
+extern struct fd fdget(unsigned int fd);
+extern struct fd fdget_raw(unsigned int fd);
 
 extern int f_dupfd(unsigned int from, struct file *file, unsigned flags);
 extern int replace_fd(unsigned fd, struct file *file, unsigned flags);
diff --git a/net/socket.c b/net/socket.c
index 879933aaed4c..4c9dc05f3b4b 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -448,18 +448,25 @@ struct socket *sockfd_lookup(int fd, int *err)
 }
 EXPORT_SYMBOL(sockfd_lookup);
 
+static void fput_light(struct file *file, int fput_needed)
+{
+	struct fd f = { file, fput_needed, 0};
+	fdput(f);
+}
+
 static struct socket *sockfd_lookup_light(int fd, int *err, int *fput_needed)
 {
-	struct file *file;
+	struct fd f;
 	struct socket *sock;
 
 	*err = -EBADF;
-	file = fget_light(fd, fput_needed);
-	if (file) {
-		sock = sock_from_file(file, err);
+	f = fdget(fd);
+	*fput_needed = f.need_put;
+	if (f.file) {
+		sock = sock_from_file(f.file, err);
 		if (sock)
 			return sock;
-		fput_light(file, *fput_needed);
+		fdput(f);
 	}
 	return NULL;
 }