vfs: do (nearly) lockless generic_file_llseek

The i_mutex lock use of generic _file_llseek hurts.  Independent processes
accessing the same file synchronize over a single lock, even though
they have no need for synchronization at all.

Under high utilization this can cause llseek to scale very poorly on larger
systems.

This patch does some rethinking of the llseek locking model:

First the 64bit f_pos is not necessarily atomic without locks
on 32bit systems. This can already cause races with read() today.
This was discussed on linux-kernel in the past and deemed acceptable.
The patch does not change that.

Let's look at the different seek variants:

SEEK_SET: Doesn't really need any locking.
If there's a race one writer wins, the other loses.

For 32bit the non atomic update races against read()
stay the same. Without a lock they can also happen
against write() now.  The read() race was deemed
acceptable in past discussions, and I think if it's
ok for read it's ok for write too.

=> Don't need a lock.

SEEK_END: This behaves like SEEK_SET plus it reads
the maximum size too. Reading the maximum size would have the
32bit atomic problem. But luckily we already have a way to read
the maximum size without locking (i_size_read), so we
can just use that instead.

Without i_mutex there is no synchronization with write() anymore,
however since the write() update is atomic on 64bit it just behaves
like another racy SEEK_SET.  On non atomic 32bit it's the same
as SEEK_SET.

=> Don't need a lock, but need to use i_size_read()

SEEK_CUR: This has a read-modify-write race window
on the same file. One could argue that any application
doing unsynchronized seeks on the same file is already broken.
But for the sake of not adding a regression here I'm
using the file->f_lock to synchronize this. Using this
lock is much better than the inode mutex because it doesn't
synchronize between processes.

=> So still need a lock, but can use a f_lock.

This patch implements this new scheme in generic_file_llseek.
I dropped generic_file_llseek_unlocked and changed all callers.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>

diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index e4e57d59edb7ca7e6634e1ef251bb142bfaa08b8..1266f6e9cdb22a309d7da8893963fc3765824b65 100644 (file)
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -1821,7 +1821,7 @@ static loff_t btrfs_file_llseek(struct file *file, loff_t offset, int origin)
        switch (origin) {
        case SEEK_END:
        case SEEK_CUR:
-               offset = generic_file_llseek_unlocked(file, offset, origin);
+               offset = generic_file_llseek(file, offset, origin);
                goto out;
        case SEEK_DATA:
        case SEEK_HOLE:

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 54b8f1e7da9465b0f0aad5b245fda0ef7810f84e..db7ce87d37a5919bee6ca27a6d39c20168924f92 100644 (file)
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -723,7 +723,7 @@ static loff_t cifs_llseek(struct file *file, loff_t offset, int origin)
                if (rc < 0)
                        return (loff_t)rc;
        }
-       return generic_file_llseek_unlocked(file, offset, origin);
+       return generic_file_llseek(file, offset, origin);
 }
 
 static int cifs_setlease(struct file *file, long arg, struct file_lock **lease)

diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
index edeb9e80290382b39491d67c40ed3802535aac2b..fe6bc0207818ab80c9aa3d8f490e06d45ea006ad 100644 (file)
--- a/fs/gfs2/file.c
+++ b/fs/gfs2/file.c
@@ -63,11 +63,11 @@ static loff_t gfs2_llseek(struct file *file, loff_t offset, int origin)
                error = gfs2_glock_nq_init(ip->i_gl, LM_ST_SHARED, LM_FLAG_ANY,
                                           &i_gh);
                if (!error) {
-                       error = generic_file_llseek_unlocked(file, offset, origin);
+                       error = generic_file_llseek(file, offset, origin);
                        gfs2_glock_dq_uninit(&i_gh);
                }
        } else
-               error = generic_file_llseek_unlocked(file, offset, origin);
+               error = generic_file_llseek(file, offset, origin);
 
        return error;
 }

diff --git a/fs/nfs/file.c b/fs/nfs/file.c
index 28b8c3f3cda3af002e99044fee907f119ac78afb..12623abcf3d4c8e5dc24999265aa3878ebf3a1ba 100644 (file)
--- a/fs/nfs/file.c
+++ b/fs/nfs/file.c
@@ -198,11 +198,12 @@ static loff_t nfs_file_llseek(struct file *filp, loff_t offset, int origin)
                if (retval < 0)
                        return (loff_t)retval;
 
+               /* AK: should drop this lock. Unlikely to be needed. */
                spin_lock(&inode->i_lock);
-               loff = generic_file_llseek_unlocked(filp, offset, origin);
+               loff = generic_file_llseek(filp, offset, origin);
                spin_unlock(&inode->i_lock);
        } else
-               loff = generic_file_llseek_unlocked(filp, offset, origin);
+               loff = generic_file_llseek(filp, offset, origin);
        return loff;
 }
 

diff --git a/fs/read_write.c b/fs/read_write.c
index 179f1c33ea578481cdc3388d54717089704a51a3..672b187def6296a1de19bba537047a1e1010f9ae 100644 (file)
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -35,23 +35,45 @@ static inline int unsigned_offsets(struct file *file)
        return file->f_mode & FMODE_UNSIGNED_OFFSET;
 }
 
+static loff_t lseek_execute(struct file *file, struct inode *inode,
+               loff_t offset, loff_t maxsize)
+{
+       if (offset < 0 && !unsigned_offsets(file))
+               return -EINVAL;
+       if (offset > maxsize)
+               return -EINVAL;
+
+       if (offset != file->f_pos) {
+               file->f_pos = offset;
+               file->f_version = 0;
+       }
+       return offset;
+}
+
 /**
- * generic_file_llseek_unlocked - lockless generic llseek implementation
+ * generic_file_llseek - generic llseek implementation for regular files
  * @file:      file structure to seek on
  * @offset:    file offset to seek to
  * @origin:    type of seek
  *
- * Updates the file offset to the value specified by @offset and @origin.
- * Locking must be provided by the caller.
+ * This is a generic implemenation of ->llseek usable for all normal local
+ * filesystems.  It just updates the file offset to the value specified by
+ * @offset and @origin under i_mutex.
+ *
+ * Synchronization:
+ * SEEK_SET is unsynchronized (but atomic on 64bit platforms)
+ * SEEK_CUR is synchronized against other SEEK_CURs, but not read/writes.
+ * read/writes behave like SEEK_SET against seeks.
+ * SEEK_END
  */
 loff_t
-generic_file_llseek_unlocked(struct file *file, loff_t offset, int origin)
+generic_file_llseek(struct file *file, loff_t offset, int origin)
 {
        struct inode *inode = file->f_mapping->host;
 
        switch (origin) {
        case SEEK_END:
-               offset += inode->i_size;
+               offset += i_size_read(inode);
                break;
        case SEEK_CUR:
                /*
@@ -62,14 +84,22 @@ generic_file_llseek_unlocked(struct file *file, loff_t offset, int origin)
                 */
                if (offset == 0)
                        return file->f_pos;
-               offset += file->f_pos;
-               break;
+               /*
+                * f_lock protects against read/modify/write race with other
+                * SEEK_CURs. Note that parallel writes and reads behave
+                * like SEEK_SET.
+                */
+               spin_lock(&file->f_lock);
+               offset = lseek_execute(file, inode, file->f_pos + offset,
+                                      inode->i_sb->s_maxbytes);
+               spin_unlock(&file->f_lock);
+               return offset;
        case SEEK_DATA:
                /*
                 * In the generic case the entire file is data, so as long as
                 * offset isn't at the end of the file then the offset is data.
                 */
-               if (offset >= inode->i_size)
+               if (offset >= i_size_read(inode))
                        return -ENXIO;
                break;
        case SEEK_HOLE:
@@ -77,46 +107,13 @@ generic_file_llseek_unlocked(struct file *file, loff_t offset, int origin)
                 * There is a virtual hole at the end of the file, so as long as
                 * offset isn't i_size or larger, return i_size.
                 */
-               if (offset >= inode->i_size)
+               if (offset >= i_size_read(inode))
                        return -ENXIO;
-               offset = inode->i_size;
+               offset = i_size_read(inode);
                break;
        }
 
-       if (offset < 0 && !unsigned_offsets(file))
-               return -EINVAL;
-       if (offset > inode->i_sb->s_maxbytes)
-               return -EINVAL;
-
-       /* Special lock needed here? */
-       if (offset != file->f_pos) {
-               file->f_pos = offset;
-               file->f_version = 0;
-       }
-
-       return offset;
-}
-EXPORT_SYMBOL(generic_file_llseek_unlocked);
-
-/**
- * generic_file_llseek - generic llseek implementation for regular files
- * @file:      file structure to seek on
- * @offset:    file offset to seek to
- * @origin:    type of seek
- *
- * This is a generic implemenation of ->llseek useable for all normal local
- * filesystems.  It just updates the file offset to the value specified by
- * @offset and @origin under i_mutex.
- */
-loff_t generic_file_llseek(struct file *file, loff_t offset, int origin)
-{
-       loff_t rval;
-
-       mutex_lock(&file->f_dentry->d_inode->i_mutex);
-       rval = generic_file_llseek_unlocked(file, offset, origin);
-       mutex_unlock(&file->f_dentry->d_inode->i_mutex);
-
-       return rval;
+       return lseek_execute(file, inode, offset, inode->i_sb->s_maxbytes);
 }
 EXPORT_SYMBOL(generic_file_llseek);
 

diff --git a/include/linux/fs.h b/include/linux/fs.h
index c1884e974ff4bfd65e6d56df46c5736a3944fdc4..db85196f63088d878645db104c3e1fc172a746eb 100644 (file)
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -964,7 +964,12 @@ struct file {
 #define f_dentry       f_path.dentry
 #define f_vfsmnt       f_path.mnt
        const struct file_operations    *f_op;
-       spinlock_t              f_lock;  /* f_ep_links, f_flags, no IRQ */
+
+       /*
+        * Protects f_ep_links, f_flags, f_pos vs i_size in lseek SEEK_CUR.
+        * Must not be taken from IRQ context.
+        */
+       spinlock_t              f_lock;
 #ifdef CONFIG_SMP
        int                     f_sb_list_cpu;
 #endif
@@ -2398,8 +2403,6 @@ file_ra_state_init(struct file_ra_state *ra, struct address_space *mapping);
 extern loff_t noop_llseek(struct file *file, loff_t offset, int origin);
 extern loff_t no_llseek(struct file *file, loff_t offset, int origin);
 extern loff_t generic_file_llseek(struct file *file, loff_t offset, int origin);
-extern loff_t generic_file_llseek_unlocked(struct file *file, loff_t offset,
-                       int origin);
 extern int generic_file_open(struct inode * inode, struct file * filp);
 extern int nonseekable_open(struct inode * inode, struct file * filp);