From 39be79c16f2b8eb07dd0d4e965cddfe39cc0534a Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Thu, 27 Oct 2011 23:53:08 +0200 Subject: vfs: iov_iter: have iov_iter_advance decrement nr_segs appropriately Currently, when you call iov_iter_advance, then the pointer to the iovec array can be incremented, but it does not decrement the nr_segs value in the iov_iter struct. The result is a iov_iter struct with a nr_segs value that goes beyond the end of the array. While I'm not aware of anything that's specifically broken by this, it seems odd and a bit dangerous not to decrement that value. If someone were to trust the nr_segs value to be correct, then they could end up walking off the end of the array. Changing this might also provide some micro-optimization when dealing with the last iovec in an array. Many of the other routines that deal with iov_iter have optimized codepaths when nr_segs == 1. Cc: Nick Piggin Signed-off-by: Jeff Layton Signed-off-by: Christoph Hellwig --- mm/filemap.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/mm/filemap.c b/mm/filemap.c index 7771871..5cf820a 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -2115,6 +2115,7 @@ void iov_iter_advance(struct iov_iter *i, size_t bytes) } else { const struct iovec *iov = i->iov; size_t base = i->iov_offset; + unsigned long nr_segs = i->nr_segs; /* * The !iov->iov_len check ensures we skip over unlikely @@ -2130,11 +2131,13 @@ void iov_iter_advance(struct iov_iter *i, size_t bytes) base += copy; if (iov->iov_len == base) { iov++; + nr_segs--; base = 0; } } i->iov = iov; i->iov_offset = base; + i->nr_segs = nr_segs; } } EXPORT_SYMBOL(iov_iter_advance); -- cgit v1.1 From 814e1d25a59662f9552e6dc1305d1df3616fc87e Mon Sep 17 00:00:00 2001 From: Wang Sheng-Hui Date: Thu, 1 Sep 2011 08:22:57 +0800 Subject: cleanup: vfs: small comment fix for block_invalidatepage The patch is aganist 3.1-rc3. Signed-off-by: Wang Sheng-Hui Signed-off-by: Christoph Hellwig --- fs/buffer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/buffer.c b/fs/buffer.c index 1a80b04..936d603 100644 --- a/fs/buffer.c +++ b/fs/buffer.c @@ -1470,13 +1470,13 @@ static void discard_buffer(struct buffer_head * bh) } /** - * block_invalidatepage - invalidate part of all of a buffer-backed page + * block_invalidatepage - invalidate part or all of a buffer-backed page * * @page: the page which is affected * @offset: the index of the truncation point * * block_invalidatepage() is called when all or part of the page has become - * invalidatedby a truncate operation. + * invalidated by a truncate operation. * * block_invalidatepage() does not have to release all buffers, but it must * ensure that no dirty buffer is left outside @offset and that no I/O -- cgit v1.1 From a877ee03ac010ded434b77f7831f43cbb1fcc60f Mon Sep 17 00:00:00 2001 From: Bryan Schumaker Date: Fri, 7 Oct 2011 13:41:15 -0400 Subject: vfs: add "device" tag to /proc/self/mountstats nfsiostat was failing to find mounted filesystems on kernels after 2.6.38 because of changes to show_vfsstat() by commit c7f404b40a3665d9f4e9a927cc5c1ee0479ed8f9. This patch adds back the "device" tag before the nfs server entry so scripts can parse the mountstats file correctly. Signed-off-by: Bryan Schumaker CC: stable@kernel.org [>=2.6.39] Signed-off-by: Christoph Hellwig --- fs/namespace.c | 1 + 1 file changed, 1 insertion(+) diff --git a/fs/namespace.c b/fs/namespace.c index b4febb2..e5e1c7d 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -1109,6 +1109,7 @@ static int show_vfsstat(struct seq_file *m, void *v) /* device */ if (mnt->mnt_sb->s_op->show_devname) { + seq_puts(m, "device "); err = mnt->mnt_sb->s_op->show_devname(m, mnt); } else { if (mnt->mnt_devname) { -- cgit v1.1 From 1448c721e4fa2faf742029a0403b4b787fccb7fa Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Mon, 17 Oct 2011 13:40:02 -0700 Subject: compat: sync compat_stats with statfs. This was found by inspection while tracking a similar bug in compat_statfs64, that has been fixed in mainline since decemeber. - This fixes a bug where not all of the f_spare fields were cleared on mips and s390. - Add the f_flags field to struct compat_statfs - Copy f_flags to userspace in case someone cares. - Use __clear_user to copy the f_spare field to userspace to ensure that all of the elements of f_spare are cleared. On some architectures f_spare is has 5 ints and on some architectures f_spare only has 4 ints. Which makes the previous technique of clearing each int individually broken. I don't expect anyone actually uses the old statfs system call anymore but if they do let them benefit from having the compat and the native version working the same. Signed-off-by: Eric W. Biederman Signed-off-by: Christoph Hellwig --- arch/mips/include/asm/compat.h | 3 ++- arch/parisc/include/asm/compat.h | 3 ++- arch/powerpc/include/asm/compat.h | 3 ++- arch/s390/include/asm/compat.h | 3 ++- arch/sparc/include/asm/compat.h | 3 ++- arch/x86/include/asm/compat.h | 3 ++- fs/compat.c | 7 ++----- 7 files changed, 14 insertions(+), 11 deletions(-) diff --git a/arch/mips/include/asm/compat.h b/arch/mips/include/asm/compat.h index dbc5106..b77df03 100644 --- a/arch/mips/include/asm/compat.h +++ b/arch/mips/include/asm/compat.h @@ -111,7 +111,8 @@ struct compat_statfs { int f_bavail; compat_fsid_t f_fsid; int f_namelen; - int f_spare[6]; + int f_flags; + int f_spare[5]; }; #define COMPAT_RLIM_INFINITY 0x7fffffffUL diff --git a/arch/parisc/include/asm/compat.h b/arch/parisc/include/asm/compat.h index efa0b60c..760f331 100644 --- a/arch/parisc/include/asm/compat.h +++ b/arch/parisc/include/asm/compat.h @@ -105,7 +105,8 @@ struct compat_statfs { __kernel_fsid_t f_fsid; s32 f_namelen; s32 f_frsize; - s32 f_spare[5]; + s32 f_flags; + s32 f_spare[4]; }; struct compat_sigcontext { diff --git a/arch/powerpc/include/asm/compat.h b/arch/powerpc/include/asm/compat.h index 91010e8..88e602f 100644 --- a/arch/powerpc/include/asm/compat.h +++ b/arch/powerpc/include/asm/compat.h @@ -100,7 +100,8 @@ struct compat_statfs { compat_fsid_t f_fsid; int f_namelen; /* SunOS ignores this field. */ int f_frsize; - int f_spare[5]; + int f_flags; + int f_spare[4]; }; #define COMPAT_RLIM_OLD_INFINITY 0x7fffffff diff --git a/arch/s390/include/asm/compat.h b/arch/s390/include/asm/compat.h index da359ca..cdb9b78 100644 --- a/arch/s390/include/asm/compat.h +++ b/arch/s390/include/asm/compat.h @@ -131,7 +131,8 @@ struct compat_statfs { compat_fsid_t f_fsid; s32 f_namelen; s32 f_frsize; - s32 f_spare[6]; + s32 f_flags; + s32 f_spare[5]; }; #define COMPAT_RLIM_OLD_INFINITY 0x7fffffff diff --git a/arch/sparc/include/asm/compat.h b/arch/sparc/include/asm/compat.h index 6f57325..b8be20d 100644 --- a/arch/sparc/include/asm/compat.h +++ b/arch/sparc/include/asm/compat.h @@ -134,7 +134,8 @@ struct compat_statfs { compat_fsid_t f_fsid; int f_namelen; /* SunOS ignores this field. */ int f_frsize; - int f_spare[5]; + int f_flags; + int f_spare[4]; }; #define COMPAT_RLIM_INFINITY 0x7fffffff diff --git a/arch/x86/include/asm/compat.h b/arch/x86/include/asm/compat.h index 1d9cd27..30d737e 100644 --- a/arch/x86/include/asm/compat.h +++ b/arch/x86/include/asm/compat.h @@ -108,7 +108,8 @@ struct compat_statfs { compat_fsid_t f_fsid; int f_namelen; /* SunOS ignores this field. */ int f_frsize; - int f_spare[5]; + int f_flags; + int f_spare[4]; }; #define COMPAT_RLIM_OLD_INFINITY 0x7fffffff diff --git a/fs/compat.c b/fs/compat.c index 58b1da4..0f1ab46 100644 --- a/fs/compat.c +++ b/fs/compat.c @@ -247,11 +247,8 @@ static int put_compat_statfs(struct compat_statfs __user *ubuf, struct kstatfs * __put_user(kbuf->f_fsid.val[0], &ubuf->f_fsid.val[0]) || __put_user(kbuf->f_fsid.val[1], &ubuf->f_fsid.val[1]) || __put_user(kbuf->f_frsize, &ubuf->f_frsize) || - __put_user(0, &ubuf->f_spare[0]) || - __put_user(0, &ubuf->f_spare[1]) || - __put_user(0, &ubuf->f_spare[2]) || - __put_user(0, &ubuf->f_spare[3]) || - __put_user(0, &ubuf->f_spare[4])) + __put_user(kbuf->f_flags, &ubuf->f_flags) || + __clear_user(ubuf->f_spare, sizeof(ubuf->f_spare))) return -EFAULT; return 0; } -- cgit v1.1 From 8fd90c8d1dacb5ff0f372217c97f57a9e61559cd Mon Sep 17 00:00:00 2001 From: Andreas Gruenbacher Date: Sun, 23 Oct 2011 23:13:30 +0530 Subject: vfs: indicate that the permission functions take all the MAY_* flags Acked-by: J. Bruce Fields Acked-by: David Howells Signed-off-by: Andreas Gruenbacher Signed-off-by: Aneesh Kumar K.V Signed-off-by: Christoph Hellwig --- fs/namei.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/namei.c b/fs/namei.c index 0b3138d..2a4574f 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -257,7 +257,7 @@ other_perms: /** * generic_permission - check for access rights on a Posix-like filesystem * @inode: inode to check access rights for - * @mask: right to check for (%MAY_READ, %MAY_WRITE, %MAY_EXEC) + * @mask: right to check for (%MAY_READ, %MAY_WRITE, %MAY_EXEC, ...) * * Used to check for read/write/execute permissions on a file. * We use "fsuid" for this, letting us set arbitrary permissions @@ -331,7 +331,7 @@ static inline int do_inode_permission(struct inode *inode, int mask) /** * inode_permission - check for access rights to a given inode * @inode: inode to check permission on - * @mask: right to check for (%MAY_READ, %MAY_WRITE, %MAY_EXEC) + * @mask: right to check for (%MAY_READ, %MAY_WRITE, %MAY_EXEC, ...) * * Used to check for read/write/execute permissions on an inode. * We use "fsuid" for this, letting us set arbitrary permissions -- cgit v1.1 From 8522ca5818652c4da6808c66a307abce75462212 Mon Sep 17 00:00:00 2001 From: "Aneesh Kumar K.V" Date: Sun, 23 Oct 2011 23:13:31 +0530 Subject: vfs: add hex format for MAY_* flag values We are going to add more flags and having them in hex format make it simpler Acked-by: J. Bruce Fields Acked-by: David Howells Signed-off-by: Aneesh Kumar K.V Signed-off-by: Christoph Hellwig --- include/linux/fs.h | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/include/linux/fs.h b/include/linux/fs.h index 277f497..c1884e9 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -58,14 +58,15 @@ struct inodes_stat_t { #define NR_FILE 8192 /* this can well be larger on a larger system */ -#define MAY_EXEC 1 -#define MAY_WRITE 2 -#define MAY_READ 4 -#define MAY_APPEND 8 -#define MAY_ACCESS 16 -#define MAY_OPEN 32 -#define MAY_CHDIR 64 -#define MAY_NOT_BLOCK 128 /* called from RCU mode, don't block */ +#define MAY_EXEC 0x00000001 +#define MAY_WRITE 0x00000002 +#define MAY_READ 0x00000004 +#define MAY_APPEND 0x00000008 +#define MAY_ACCESS 0x00000010 +#define MAY_OPEN 0x00000020 +#define MAY_CHDIR 0x00000040 +/* called from RCU mode, don't block */ +#define MAY_NOT_BLOCK 0x00000080 /* * flags in file.f_mode. Note that FMODE_READ and FMODE_WRITE must correspond -- cgit v1.1 From d124b60a838141bb9cac1b6567e9ca4539d1fff0 Mon Sep 17 00:00:00 2001 From: Andreas Gruenbacher Date: Sun, 23 Oct 2011 23:13:32 +0530 Subject: vfs: pass all mask flags check_acl and posix_acl_permission Acked-by: J. Bruce Fields Acked-by: David Howells Signed-off-by: Andreas Gruenbacher Signed-off-by: Aneesh Kumar K.V Signed-off-by: Christoph Hellwig --- fs/namei.c | 2 -- fs/posix_acl.c | 2 ++ 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/namei.c b/fs/namei.c index 2a4574f..276cd30 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -227,8 +227,6 @@ static int acl_permission_check(struct inode *inode, int mask) { unsigned int mode = inode->i_mode; - mask &= MAY_READ | MAY_WRITE | MAY_EXEC | MAY_NOT_BLOCK; - if (current_user_ns() != inode_userns(inode)) goto other_perms; diff --git a/fs/posix_acl.c b/fs/posix_acl.c index 10027b4..cea4623 100644 --- a/fs/posix_acl.c +++ b/fs/posix_acl.c @@ -218,6 +218,8 @@ posix_acl_permission(struct inode *inode, const struct posix_acl *acl, int want) const struct posix_acl_entry *pa, *pe, *mask_obj; int found = 0; + want &= MAY_READ | MAY_WRITE | MAY_EXEC | MAY_NOT_BLOCK; + FOREACH_ACL_ENTRY(pa, acl, pe) { switch(pa->e_tag) { case ACL_USER_OBJ: -- cgit v1.1 From 948409c74d217f6cf054b8c927765a1c3fe16b53 Mon Sep 17 00:00:00 2001 From: Andreas Gruenbacher Date: Sun, 23 Oct 2011 23:13:33 +0530 Subject: vfs: add a comment to inode_permission() Acked-by: J. Bruce Fields Acked-by: David Howells Signed-off-by: Andreas Gruenbacher Signed-off-by: Aneesh Kumar K.V Signed-off-by: Christoph Hellwig --- fs/namei.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/fs/namei.c b/fs/namei.c index 276cd30..9061157 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -221,7 +221,7 @@ static int check_acl(struct inode *inode, int mask) } /* - * This does basic POSIX ACL permission checking + * This does the basic permission checking */ static int acl_permission_check(struct inode *inode, int mask) { @@ -271,7 +271,7 @@ int generic_permission(struct inode *inode, int mask) int ret; /* - * Do the basic POSIX ACL permission checks. + * Do the basic permission checks. */ ret = acl_permission_check(inode, mask); if (ret != -EACCES) @@ -335,6 +335,8 @@ static inline int do_inode_permission(struct inode *inode, int mask) * We use "fsuid" for this, letting us set arbitrary permissions * for filesystem access without changing the "normal" uids which * are used for other things. + * + * When checking for MAY_APPEND, MAY_WRITE must also be set in @mask. */ int inode_permission(struct inode *inode, int mask) { -- cgit v1.1 From 62a3ddef6181d7d932c565d97552d2f7b9ab4d28 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Fri, 28 Oct 2011 10:03:41 +0200 Subject: vfs: fix spinning prevention in prune_icache_sb We need to move the inode to the end of the list to actually make the spinning prevention explained in the comment above it work. With a plain list_move it will simply stay in place as we're always reclaiming from the head of the list. Signed-off-by: Christoph Hellwig --- fs/inode.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/inode.c b/fs/inode.c index ec79246..ecbb68d 100644 --- a/fs/inode.c +++ b/fs/inode.c @@ -634,7 +634,7 @@ void prune_icache_sb(struct super_block *sb, int nr_to_scan) * inode to the back of the list so we don't spin on it. */ if (!spin_trylock(&inode->i_lock)) { - list_move(&inode->i_lru, &sb->s_inode_lru); + list_move_tail(&inode->i_lru, &sb->s_inode_lru); continue; } -- cgit v1.1 From eb28be2b4c0a0608e54f0a8fc237372c674eb7d0 Mon Sep 17 00:00:00 2001 From: Andi Kleen Date: Mon, 1 Aug 2011 21:38:03 -0700 Subject: direct-io: separate fields only used in the submission path from struct dio This large, but largely mechanic, patch moves all fields in struct dio that are only used in the submission path into a separate on stack data structure. This has the advantage that the memory is very likely cache hot, which is not guaranteed for memory fresh out of kmalloc. This also gives gcc more optimization potential because it can easier determine that there are no external aliases for these variables. The sdio initialization is a initialization now instead of memset. This allows gcc to break sdio into individual fields and optimize away unnecessary zeroing (after all the functions are inlined) Signed-off-by: Andi Kleen Acked-by: Jeff Moyer Signed-off-by: Christoph Hellwig --- fs/direct-io.c | 389 +++++++++++++++++++++++++++++---------------------------- 1 file changed, 201 insertions(+), 188 deletions(-) diff --git a/fs/direct-io.c b/fs/direct-io.c index 44a360c..02ccf76 100644 --- a/fs/direct-io.c +++ b/fs/direct-io.c @@ -55,13 +55,10 @@ * blocksize. */ -struct dio { - /* BIO submission state */ +/* dio_state only used in the submission path */ + +struct dio_submit { struct bio *bio; /* bio under assembly */ - struct inode *inode; - int rw; - loff_t i_size; /* i_size when submitted */ - int flags; /* doesn't change */ unsigned blkbits; /* doesn't change */ unsigned blkfactor; /* When we're using an alignment which is finer than the filesystem's soft @@ -81,13 +78,12 @@ struct dio { int boundary; /* prev block is at a boundary */ int reap_counter; /* rate limit reaping */ get_block_t *get_block; /* block mapping function */ - dio_iodone_t *end_io; /* IO completion function */ dio_submit_t *submit_io; /* IO submition function */ + loff_t logical_offset_in_bio; /* current first logical block in bio */ sector_t final_block_in_bio; /* current final block in bio + 1 */ sector_t next_block_for_io; /* next block to be put under IO, in dio_blocks units */ - struct buffer_head map_bh; /* last get_block() result */ /* * Deferred addition of a page to the dio. These variables are @@ -100,18 +96,6 @@ struct dio { sector_t cur_page_block; /* Where it starts */ loff_t cur_page_fs_offset; /* Offset in file */ - /* BIO completion state */ - spinlock_t bio_lock; /* protects BIO fields below */ - unsigned long refcount; /* direct_io_worker() and bios */ - struct bio *bio_list; /* singly linked via bi_private */ - struct task_struct *waiter; /* waiting task (NULL if none) */ - - /* AIO related stuff */ - struct kiocb *iocb; /* kiocb */ - int is_async; /* is IO async ? */ - int io_error; /* IO error in completion path */ - ssize_t result; /* IO result */ - /* * Page fetching state. These variables belong to dio_refill_pages(). */ @@ -125,6 +109,30 @@ struct dio { */ unsigned head; /* next page to process */ unsigned tail; /* last valid page + 1 */ +}; + +/* dio_state communicated between submission path and end_io */ +struct dio { + int flags; /* doesn't change */ + struct inode *inode; + int rw; + loff_t i_size; /* i_size when submitted */ + dio_iodone_t *end_io; /* IO completion function */ + struct buffer_head map_bh; /* last get_block() result */ + + + /* BIO completion state */ + spinlock_t bio_lock; /* protects BIO fields below */ + unsigned long refcount; /* direct_io_worker() and bios */ + struct bio *bio_list; /* singly linked via bi_private */ + struct task_struct *waiter; /* waiting task (NULL if none) */ + + /* AIO related stuff */ + struct kiocb *iocb; /* kiocb */ + int is_async; /* is IO async ? */ + int io_error; /* IO error in completion path */ + ssize_t result; /* IO result */ + int page_errors; /* errno from get_user_pages() */ /* @@ -182,27 +190,27 @@ EXPORT_SYMBOL_GPL(inode_dio_done); /* * How many pages are in the queue? */ -static inline unsigned dio_pages_present(struct dio *dio) +static inline unsigned dio_pages_present(struct dio_submit *sdio) { - return dio->tail - dio->head; + return sdio->tail - sdio->head; } /* * Go grab and pin some userspace pages. Typically we'll get 64 at a time. */ -static int dio_refill_pages(struct dio *dio) +static int dio_refill_pages(struct dio *dio, struct dio_submit *sdio) { int ret; int nr_pages; - nr_pages = min(dio->total_pages - dio->curr_page, DIO_PAGES); + nr_pages = min(sdio->total_pages - sdio->curr_page, DIO_PAGES); ret = get_user_pages_fast( - dio->curr_user_address, /* Where from? */ + sdio->curr_user_address, /* Where from? */ nr_pages, /* How many pages? */ dio->rw == READ, /* Write to memory? */ &dio->pages[0]); /* Put results here */ - if (ret < 0 && dio->blocks_available && (dio->rw & WRITE)) { + if (ret < 0 && sdio->blocks_available && (dio->rw & WRITE)) { struct page *page = ZERO_PAGE(0); /* * A memory fault, but the filesystem has some outstanding @@ -213,17 +221,17 @@ static int dio_refill_pages(struct dio *dio) dio->page_errors = ret; page_cache_get(page); dio->pages[0] = page; - dio->head = 0; - dio->tail = 1; + sdio->head = 0; + sdio->tail = 1; ret = 0; goto out; } if (ret >= 0) { - dio->curr_user_address += ret * PAGE_SIZE; - dio->curr_page += ret; - dio->head = 0; - dio->tail = ret; + sdio->curr_user_address += ret * PAGE_SIZE; + sdio->curr_page += ret; + sdio->head = 0; + sdio->tail = ret; ret = 0; } out: @@ -236,17 +244,17 @@ out: * decent number of pages, less frequently. To provide nicer use of the * L1 cache. */ -static struct page *dio_get_page(struct dio *dio) +static struct page *dio_get_page(struct dio *dio, struct dio_submit *sdio) { - if (dio_pages_present(dio) == 0) { + if (dio_pages_present(sdio) == 0) { int ret; - ret = dio_refill_pages(dio); + ret = dio_refill_pages(dio, sdio); if (ret) return ERR_PTR(ret); - BUG_ON(dio_pages_present(dio) == 0); + BUG_ON(dio_pages_present(sdio) == 0); } - return dio->pages[dio->head++]; + return dio->pages[sdio->head++]; } /** @@ -368,8 +376,9 @@ void dio_end_io(struct bio *bio, int error) EXPORT_SYMBOL_GPL(dio_end_io); static void -dio_bio_alloc(struct dio *dio, struct block_device *bdev, - sector_t first_sector, int nr_vecs) +dio_bio_alloc(struct dio *dio, struct dio_submit *sdio, + struct block_device *bdev, + sector_t first_sector, int nr_vecs) { struct bio *bio; @@ -386,8 +395,8 @@ dio_bio_alloc(struct dio *dio, struct block_device *bdev, else bio->bi_end_io = dio_bio_end_io; - dio->bio = bio; - dio->logical_offset_in_bio = dio->cur_page_fs_offset; + sdio->bio = bio; + sdio->logical_offset_in_bio = sdio->cur_page_fs_offset; } /* @@ -397,9 +406,9 @@ dio_bio_alloc(struct dio *dio, struct block_device *bdev, * * bios hold a dio reference between submit_bio and ->end_io. */ -static void dio_bio_submit(struct dio *dio) +static void dio_bio_submit(struct dio *dio, struct dio_submit *sdio) { - struct bio *bio = dio->bio; + struct bio *bio = sdio->bio; unsigned long flags; bio->bi_private = dio; @@ -411,24 +420,24 @@ static void dio_bio_submit(struct dio *dio) if (dio->is_async && dio->rw == READ) bio_set_pages_dirty(bio); - if (dio->submit_io) - dio->submit_io(dio->rw, bio, dio->inode, - dio->logical_offset_in_bio); + if (sdio->submit_io) + sdio->submit_io(dio->rw, bio, dio->inode, + sdio->logical_offset_in_bio); else submit_bio(dio->rw, bio); - dio->bio = NULL; - dio->boundary = 0; - dio->logical_offset_in_bio = 0; + sdio->bio = NULL; + sdio->boundary = 0; + sdio->logical_offset_in_bio = 0; } /* * Release any resources in case of a failure */ -static void dio_cleanup(struct dio *dio) +static void dio_cleanup(struct dio *dio, struct dio_submit *sdio) { - while (dio_pages_present(dio)) - page_cache_release(dio_get_page(dio)); + while (dio_pages_present(sdio)) + page_cache_release(dio_get_page(dio, sdio)); } /* @@ -518,11 +527,11 @@ static void dio_await_completion(struct dio *dio) * * This also helps to limit the peak amount of pinned userspace memory. */ -static int dio_bio_reap(struct dio *dio) +static int dio_bio_reap(struct dio *dio, struct dio_submit *sdio) { int ret = 0; - if (dio->reap_counter++ >= 64) { + if (sdio->reap_counter++ >= 64) { while (dio->bio_list) { unsigned long flags; struct bio *bio; @@ -536,14 +545,14 @@ static int dio_bio_reap(struct dio *dio) if (ret == 0) ret = ret2; } - dio->reap_counter = 0; + sdio->reap_counter = 0; } return ret; } /* * Call into the fs to map some more disk blocks. We record the current number - * of available blocks at dio->blocks_available. These are in units of the + * of available blocks at sdio->blocks_available. These are in units of the * fs blocksize, (1 << inode->i_blkbits). * * The fs is allowed to map lots of blocks at once. If it wants to do that, @@ -564,7 +573,7 @@ static int dio_bio_reap(struct dio *dio) * buffer_mapped(). However the direct-io code will only process holes one * block at a time - it will repeatedly call get_block() as it walks the hole. */ -static int get_more_blocks(struct dio *dio) +static int get_more_blocks(struct dio *dio, struct dio_submit *sdio) { int ret; struct buffer_head *map_bh = &dio->map_bh; @@ -580,11 +589,11 @@ static int get_more_blocks(struct dio *dio) */ ret = dio->page_errors; if (ret == 0) { - BUG_ON(dio->block_in_file >= dio->final_block_in_request); - fs_startblk = dio->block_in_file >> dio->blkfactor; - dio_count = dio->final_block_in_request - dio->block_in_file; - fs_count = dio_count >> dio->blkfactor; - blkmask = (1 << dio->blkfactor) - 1; + BUG_ON(sdio->block_in_file >= sdio->final_block_in_request); + fs_startblk = sdio->block_in_file >> sdio->blkfactor; + dio_count = sdio->final_block_in_request - sdio->block_in_file; + fs_count = dio_count >> sdio->blkfactor; + blkmask = (1 << sdio->blkfactor) - 1; if (dio_count & blkmask) fs_count++; @@ -604,12 +613,12 @@ static int get_more_blocks(struct dio *dio) */ create = dio->rw & WRITE; if (dio->flags & DIO_SKIP_HOLES) { - if (dio->block_in_file < (i_size_read(dio->inode) >> - dio->blkbits)) + if (sdio->block_in_file < (i_size_read(dio->inode) >> + sdio->blkbits)) create = 0; } - ret = (*dio->get_block)(dio->inode, fs_startblk, + ret = (*sdio->get_block)(dio->inode, fs_startblk, map_bh, create); } return ret; @@ -618,20 +627,21 @@ static int get_more_blocks(struct dio *dio) /* * There is no bio. Make one now. */ -static int dio_new_bio(struct dio *dio, sector_t start_sector) +static int dio_new_bio(struct dio *dio, struct dio_submit *sdio, + sector_t start_sector) { sector_t sector; int ret, nr_pages; - ret = dio_bio_reap(dio); + ret = dio_bio_reap(dio, sdio); if (ret) goto out; - sector = start_sector << (dio->blkbits - 9); - nr_pages = min(dio->pages_in_io, bio_get_nr_vecs(dio->map_bh.b_bdev)); + sector = start_sector << (sdio->blkbits - 9); + nr_pages = min(sdio->pages_in_io, bio_get_nr_vecs(dio->map_bh.b_bdev)); nr_pages = min(nr_pages, BIO_MAX_PAGES); BUG_ON(nr_pages <= 0); - dio_bio_alloc(dio, dio->map_bh.b_bdev, sector, nr_pages); - dio->boundary = 0; + dio_bio_alloc(dio, sdio, dio->map_bh.b_bdev, sector, nr_pages); + sdio->boundary = 0; out: return ret; } @@ -643,21 +653,21 @@ out: * * Return zero on success. Non-zero means the caller needs to start a new BIO. */ -static int dio_bio_add_page(struct dio *dio) +static int dio_bio_add_page(struct dio_submit *sdio) { int ret; - ret = bio_add_page(dio->bio, dio->cur_page, - dio->cur_page_len, dio->cur_page_offset); - if (ret == dio->cur_page_len) { + ret = bio_add_page(sdio->bio, sdio->cur_page, + sdio->cur_page_len, sdio->cur_page_offset); + if (ret == sdio->cur_page_len) { /* * Decrement count only, if we are done with this page */ - if ((dio->cur_page_len + dio->cur_page_offset) == PAGE_SIZE) - dio->pages_in_io--; - page_cache_get(dio->cur_page); - dio->final_block_in_bio = dio->cur_page_block + - (dio->cur_page_len >> dio->blkbits); + if ((sdio->cur_page_len + sdio->cur_page_offset) == PAGE_SIZE) + sdio->pages_in_io--; + page_cache_get(sdio->cur_page); + sdio->final_block_in_bio = sdio->cur_page_block + + (sdio->cur_page_len >> sdio->blkbits); ret = 0; } else { ret = 1; @@ -675,14 +685,14 @@ static int dio_bio_add_page(struct dio *dio) * The caller of this function is responsible for removing cur_page from the * dio, and for dropping the refcount which came from that presence. */ -static int dio_send_cur_page(struct dio *dio) +static int dio_send_cur_page(struct dio *dio, struct dio_submit *sdio) { int ret = 0; - if (dio->bio) { - loff_t cur_offset = dio->cur_page_fs_offset; - loff_t bio_next_offset = dio->logical_offset_in_bio + - dio->bio->bi_size; + if (sdio->bio) { + loff_t cur_offset = sdio->cur_page_fs_offset; + loff_t bio_next_offset = sdio->logical_offset_in_bio + + sdio->bio->bi_size; /* * See whether this new request is contiguous with the old. @@ -698,28 +708,28 @@ static int dio_send_cur_page(struct dio *dio) * be the next logical offset in the bio, submit the bio we * have. */ - if (dio->final_block_in_bio != dio->cur_page_block || + if (sdio->final_block_in_bio != sdio->cur_page_block || cur_offset != bio_next_offset) - dio_bio_submit(dio); + dio_bio_submit(dio, sdio); /* * Submit now if the underlying fs is about to perform a * metadata read */ - else if (dio->boundary) - dio_bio_submit(dio); + else if (sdio->boundary) + dio_bio_submit(dio, sdio); } - if (dio->bio == NULL) { - ret = dio_new_bio(dio, dio->cur_page_block); + if (sdio->bio == NULL) { + ret = dio_new_bio(dio, sdio, sdio->cur_page_block); if (ret) goto out; } - if (dio_bio_add_page(dio) != 0) { - dio_bio_submit(dio); - ret = dio_new_bio(dio, dio->cur_page_block); + if (dio_bio_add_page(sdio) != 0) { + dio_bio_submit(dio, sdio); + ret = dio_new_bio(dio, sdio, sdio->cur_page_block); if (ret == 0) { - ret = dio_bio_add_page(dio); + ret = dio_bio_add_page(sdio); BUG_ON(ret != 0); } } @@ -745,7 +755,7 @@ out: * page to the dio instead. */ static int -submit_page_section(struct dio *dio, struct page *page, +submit_page_section(struct dio *dio, struct dio_submit *sdio, struct page *page, unsigned offset, unsigned len, sector_t blocknr) { int ret = 0; @@ -760,20 +770,20 @@ submit_page_section(struct dio *dio, struct page *page, /* * Can we just grow the current page's presence in the dio? */ - if ( (dio->cur_page == page) && - (dio->cur_page_offset + dio->cur_page_len == offset) && - (dio->cur_page_block + - (dio->cur_page_len >> dio->blkbits) == blocknr)) { - dio->cur_page_len += len; + if (sdio->cur_page == page && + sdio->cur_page_offset + sdio->cur_page_len == offset && + sdio->cur_page_block + + (sdio->cur_page_len >> sdio->blkbits) == blocknr) { + sdio->cur_page_len += len; /* - * If dio->boundary then we want to schedule the IO now to + * If sdio->boundary then we want to schedule the IO now to * avoid metadata seeks. */ - if (dio->boundary) { - ret = dio_send_cur_page(dio); - page_cache_release(dio->cur_page); - dio->cur_page = NULL; + if (sdio->boundary) { + ret = dio_send_cur_page(dio, sdio); + page_cache_release(sdio->cur_page); + sdio->cur_page = NULL; } goto out; } @@ -781,20 +791,20 @@ submit_page_section(struct dio *dio, struct page *page, /* * If there's a deferred page already there then send it. */ - if (dio->cur_page) { - ret = dio_send_cur_page(dio); - page_cache_release(dio->cur_page); - dio->cur_page = NULL; + if (sdio->cur_page) { + ret = dio_send_cur_page(dio, sdio); + page_cache_release(sdio->cur_page); + sdio->cur_page = NULL; if (ret) goto out; } page_cache_get(page); /* It is in dio */ - dio->cur_page = page; - dio->cur_page_offset = offset; - dio->cur_page_len = len; - dio->cur_page_block = blocknr; - dio->cur_page_fs_offset = dio->block_in_file << dio->blkbits; + sdio->cur_page = page; + sdio->cur_page_offset = offset; + sdio->cur_page_len = len; + sdio->cur_page_block = blocknr; + sdio->cur_page_fs_offset = sdio->block_in_file << sdio->blkbits; out: return ret; } @@ -826,19 +836,19 @@ static void clean_blockdev_aliases(struct dio *dio) * `end' is zero if we're doing the start of the IO, 1 at the end of the * IO. */ -static void dio_zero_block(struct dio *dio, int end) +static void dio_zero_block(struct dio *dio, struct dio_submit *sdio, int end) { unsigned dio_blocks_per_fs_block; unsigned this_chunk_blocks; /* In dio_blocks */ unsigned this_chunk_bytes; struct page *page; - dio->start_zero_done = 1; - if (!dio->blkfactor || !buffer_new(&dio->map_bh)) + sdio->start_zero_done = 1; + if (!sdio->blkfactor || !buffer_new(&dio->map_bh)) return; - dio_blocks_per_fs_block = 1 << dio->blkfactor; - this_chunk_blocks = dio->block_in_file & (dio_blocks_per_fs_block - 1); + dio_blocks_per_fs_block = 1 << sdio->blkfactor; + this_chunk_blocks = sdio->block_in_file & (dio_blocks_per_fs_block - 1); if (!this_chunk_blocks) return; @@ -850,14 +860,14 @@ static void dio_zero_block(struct dio *dio, int end) if (end) this_chunk_blocks = dio_blocks_per_fs_block - this_chunk_blocks; - this_chunk_bytes = this_chunk_blocks << dio->blkbits; + this_chunk_bytes = this_chunk_blocks << sdio->blkbits; page = ZERO_PAGE(0); - if (submit_page_section(dio, page, 0, this_chunk_bytes, - dio->next_block_for_io)) + if (submit_page_section(dio, sdio, page, 0, this_chunk_bytes, + sdio->next_block_for_io)) return; - dio->next_block_for_io += this_chunk_blocks; + sdio->next_block_for_io += this_chunk_blocks; } /* @@ -876,9 +886,9 @@ static void dio_zero_block(struct dio *dio, int end) * it should set b_size to PAGE_SIZE or more inside get_block(). This gives * fine alignment but still allows this function to work in PAGE_SIZE units. */ -static int do_direct_IO(struct dio *dio) +static int do_direct_IO(struct dio *dio, struct dio_submit *sdio) { - const unsigned blkbits = dio->blkbits; + const unsigned blkbits = sdio->blkbits; const unsigned blocks_per_page = PAGE_SIZE >> blkbits; struct page *page; unsigned block_in_page; @@ -886,10 +896,10 @@ static int do_direct_IO(struct dio *dio) int ret = 0; /* The I/O can start at any block offset within the first page */ - block_in_page = dio->first_block_in_page; + block_in_page = sdio->first_block_in_page; - while (dio->block_in_file < dio->final_block_in_request) { - page = dio_get_page(dio); + while (sdio->block_in_file < sdio->final_block_in_request) { + page = dio_get_page(dio, sdio); if (IS_ERR(page)) { ret = PTR_ERR(page); goto out; @@ -901,14 +911,14 @@ static int do_direct_IO(struct dio *dio) unsigned this_chunk_blocks; /* # of blocks */ unsigned u; - if (dio->blocks_available == 0) { + if (sdio->blocks_available == 0) { /* * Need to go and map some more disk */ unsigned long blkmask; unsigned long dio_remainder; - ret = get_more_blocks(dio); + ret = get_more_blocks(dio, sdio); if (ret) { page_cache_release(page); goto out; @@ -916,18 +926,18 @@ static int do_direct_IO(struct dio *dio) if (!buffer_mapped(map_bh)) goto do_holes; - dio->blocks_available = - map_bh->b_size >> dio->blkbits; - dio->next_block_for_io = - map_bh->b_blocknr << dio->blkfactor; + sdio->blocks_available = + map_bh->b_size >> sdio->blkbits; + sdio->next_block_for_io = + map_bh->b_blocknr << sdio->blkfactor; if (buffer_new(map_bh)) clean_blockdev_aliases(dio); - if (!dio->blkfactor) + if (!sdio->blkfactor) goto do_holes; - blkmask = (1 << dio->blkfactor) - 1; - dio_remainder = (dio->block_in_file & blkmask); + blkmask = (1 << sdio->blkfactor) - 1; + dio_remainder = (sdio->block_in_file & blkmask); /* * If we are at the start of IO and that IO @@ -941,8 +951,8 @@ static int do_direct_IO(struct dio *dio) * on-disk */ if (!buffer_new(map_bh)) - dio->next_block_for_io += dio_remainder; - dio->blocks_available -= dio_remainder; + sdio->next_block_for_io += dio_remainder; + sdio->blocks_available -= dio_remainder; } do_holes: /* Handle holes */ @@ -961,7 +971,7 @@ do_holes: */ i_size_aligned = ALIGN(i_size_read(dio->inode), 1 << blkbits); - if (dio->block_in_file >= + if (sdio->block_in_file >= i_size_aligned >> blkbits) { /* We hit eof */ page_cache_release(page); @@ -969,7 +979,7 @@ do_holes: } zero_user(page, block_in_page << blkbits, 1 << blkbits); - dio->block_in_file++; + sdio->block_in_file++; block_in_page++; goto next_block; } @@ -979,38 +989,40 @@ do_holes: * is finer than the underlying fs, go check to see if * we must zero out the start of this block. */ - if (unlikely(dio->blkfactor && !dio->start_zero_done)) - dio_zero_block(dio, 0); + if (unlikely(sdio->blkfactor && !sdio->start_zero_done)) + dio_zero_block(dio, sdio, 0); /* * Work out, in this_chunk_blocks, how much disk we * can add to this page */ - this_chunk_blocks = dio->blocks_available; + this_chunk_blocks = sdio->blocks_available; u = (PAGE_SIZE - offset_in_page) >> blkbits; if (this_chunk_blocks > u) this_chunk_blocks = u; - u = dio->final_block_in_request - dio->block_in_file; + u = sdio->final_block_in_request - sdio->block_in_file; if (this_chunk_blocks > u) this_chunk_blocks = u; this_chunk_bytes = this_chunk_blocks << blkbits; BUG_ON(this_chunk_bytes == 0); - dio->boundary = buffer_boundary(map_bh); - ret = submit_page_section(dio, page, offset_in_page, - this_chunk_bytes, dio->next_block_for_io); + sdio->boundary = buffer_boundary(map_bh); + ret = submit_page_section(dio, sdio, page, + offset_in_page, + this_chunk_bytes, + sdio->next_block_for_io); if (ret) { page_cache_release(page); goto out; } - dio->next_block_for_io += this_chunk_blocks; + sdio->next_block_for_io += this_chunk_blocks; - dio->block_in_file += this_chunk_blocks; + sdio->block_in_file += this_chunk_blocks; block_in_page += this_chunk_blocks; - dio->blocks_available -= this_chunk_blocks; + sdio->blocks_available -= this_chunk_blocks; next_block: - BUG_ON(dio->block_in_file > dio->final_block_in_request); - if (dio->block_in_file == dio->final_block_in_request) + BUG_ON(sdio->block_in_file > sdio->final_block_in_request); + if (sdio->block_in_file == sdio->final_block_in_request) break; } @@ -1026,7 +1038,7 @@ static ssize_t direct_io_worker(int rw, struct kiocb *iocb, struct inode *inode, const struct iovec *iov, loff_t offset, unsigned long nr_segs, unsigned blkbits, get_block_t get_block, dio_iodone_t end_io, - dio_submit_t submit_io, struct dio *dio) + dio_submit_t submit_io, struct dio *dio, struct dio_submit *sdio) { unsigned long user_addr; unsigned long flags; @@ -1037,15 +1049,15 @@ direct_io_worker(int rw, struct kiocb *iocb, struct inode *inode, dio->inode = inode; dio->rw = rw; - dio->blkbits = blkbits; - dio->blkfactor = inode->i_blkbits - blkbits; - dio->block_in_file = offset >> blkbits; + sdio->blkbits = blkbits; + sdio->blkfactor = inode->i_blkbits - blkbits; + sdio->block_in_file = offset >> blkbits; - dio->get_block = get_block; + sdio->get_block = get_block; dio->end_io = end_io; - dio->submit_io = submit_io; - dio->final_block_in_bio = -1; - dio->next_block_for_io = -1; + sdio->submit_io = submit_io; + sdio->final_block_in_bio = -1; + sdio->next_block_for_io = -1; dio->iocb = iocb; dio->i_size = i_size_read(inode); @@ -1057,45 +1069,45 @@ direct_io_worker(int rw, struct kiocb *iocb, struct inode *inode, * In case of non-aligned buffers, we may need 2 more * pages since we need to zero out first and last block. */ - if (unlikely(dio->blkfactor)) - dio->pages_in_io = 2; + if (unlikely(sdio->blkfactor)) + sdio->pages_in_io = 2; for (seg = 0; seg < nr_segs; seg++) { user_addr = (unsigned long)iov[seg].iov_base; - dio->pages_in_io += + sdio->pages_in_io += ((user_addr+iov[seg].iov_len +PAGE_SIZE-1)/PAGE_SIZE - user_addr/PAGE_SIZE); } for (seg = 0; seg < nr_segs; seg++) { user_addr = (unsigned long)iov[seg].iov_base; - dio->size += bytes = iov[seg].iov_len; + sdio->size += bytes = iov[seg].iov_len; /* Index into the first page of the first block */ - dio->first_block_in_page = (user_addr & ~PAGE_MASK) >> blkbits; - dio->final_block_in_request = dio->block_in_file + + sdio->first_block_in_page = (user_addr & ~PAGE_MASK) >> blkbits; + sdio->final_block_in_request = sdio->block_in_file + (bytes >> blkbits); /* Page fetching state */ - dio->head = 0; - dio->tail = 0; - dio->curr_page = 0; + sdio->head = 0; + sdio->tail = 0; + sdio->curr_page = 0; - dio->total_pages = 0; + sdio->total_pages = 0; if (user_addr & (PAGE_SIZE-1)) { - dio->total_pages++; + sdio->total_pages++; bytes -= PAGE_SIZE - (user_addr & (PAGE_SIZE - 1)); } - dio->total_pages += (bytes + PAGE_SIZE - 1) / PAGE_SIZE; - dio->curr_user_address = user_addr; + sdio->total_pages += (bytes + PAGE_SIZE - 1) / PAGE_SIZE; + sdio->curr_user_address = user_addr; - ret = do_direct_IO(dio); + ret = do_direct_IO(dio, sdio); dio->result += iov[seg].iov_len - - ((dio->final_block_in_request - dio->block_in_file) << + ((sdio->final_block_in_request - sdio->block_in_file) << blkbits); if (ret) { - dio_cleanup(dio); + dio_cleanup(dio, sdio); break; } } /* end iovec loop */ @@ -1111,23 +1123,23 @@ direct_io_worker(int rw, struct kiocb *iocb, struct inode *inode, * There may be some unwritten disk at the end of a part-written * fs-block-sized block. Go zero that now. */ - dio_zero_block(dio, 1); + dio_zero_block(dio, sdio, 1); - if (dio->cur_page) { - ret2 = dio_send_cur_page(dio); + if (sdio->cur_page) { + ret2 = dio_send_cur_page(dio, sdio); if (ret == 0) ret = ret2; - page_cache_release(dio->cur_page); - dio->cur_page = NULL; + page_cache_release(sdio->cur_page); + sdio->cur_page = NULL; } - if (dio->bio) - dio_bio_submit(dio); + if (sdio->bio) + dio_bio_submit(dio, sdio); /* * It is possible that, we return short IO due to end of file. * In that case, we need to release all the pages we got hold on. */ - dio_cleanup(dio); + dio_cleanup(dio, sdio); /* * All block lookups have been performed. For READ requests @@ -1146,7 +1158,7 @@ direct_io_worker(int rw, struct kiocb *iocb, struct inode *inode, */ BUG_ON(ret == -EIOCBQUEUED); if (dio->is_async && ret == 0 && dio->result && - ((rw & READ) || (dio->result == dio->size))) + ((rw & READ) || (dio->result == sdio->size))) ret = -EIOCBQUEUED; if (ret != -EIOCBQUEUED) @@ -1211,6 +1223,7 @@ __blockdev_direct_IO(int rw, struct kiocb *iocb, struct inode *inode, ssize_t retval = -EINVAL; loff_t end = offset; struct dio *dio; + struct dio_submit sdio = { 0, }; if (rw & WRITE) rw = WRITE_ODIRECT; @@ -1290,7 +1303,7 @@ __blockdev_direct_IO(int rw, struct kiocb *iocb, struct inode *inode, retval = direct_io_worker(rw, iocb, inode, iov, offset, nr_segs, blkbits, get_block, end_io, - submit_io, dio); + submit_io, dio, &sdio); out: return retval; -- cgit v1.1 From cde1ecb3247f67c167918ea6326159209996fd54 Mon Sep 17 00:00:00 2001 From: Andi Kleen Date: Mon, 1 Aug 2011 21:38:04 -0700 Subject: direct-io: fix a wrong comment There's nothing on the stack, even before my changes. Signed-off-by: Andi Kleen Acked-by: Jeff Moyer Signed-off-by: Christoph Hellwig --- fs/direct-io.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/direct-io.c b/fs/direct-io.c index 02ccf76..c1e03ae 100644 --- a/fs/direct-io.c +++ b/fs/direct-io.c @@ -39,7 +39,7 @@ /* * How many user pages to map in one call to get_user_pages(). This determines - * the size of a structure on the stack. + * the size of a structure in the slab cache */ #define DIO_PAGES 64 -- cgit v1.1 From 0dc2bc49be545626a2dc6da133202ffe69ac3fcc Mon Sep 17 00:00:00 2001 From: Andi Kleen Date: Mon, 1 Aug 2011 21:38:05 -0700 Subject: direct-io: rearrange fields in dio/dio_submit to avoid holes Fix most problems reported by pahole. There is still a weird 104 byte hole after map_bh. I'm not sure what causes this. Signed-off-by: Andi Kleen Acked-by: Jeff Moyer Signed-off-by: Christoph Hellwig --- fs/direct-io.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/fs/direct-io.c b/fs/direct-io.c index c1e03ae..b01ea0d 100644 --- a/fs/direct-io.c +++ b/fs/direct-io.c @@ -73,10 +73,10 @@ struct dio_submit { sector_t block_in_file; /* Current offset into the underlying file in dio_block units. */ unsigned blocks_available; /* At block_in_file. changes */ + int reap_counter; /* rate limit reaping */ sector_t final_block_in_request;/* doesn't change */ unsigned first_block_in_page; /* doesn't change, Used only once */ int boundary; /* prev block is at a boundary */ - int reap_counter; /* rate limit reaping */ get_block_t *get_block; /* block mapping function */ dio_submit_t *submit_io; /* IO submition function */ @@ -114,27 +114,26 @@ struct dio_submit { /* dio_state communicated between submission path and end_io */ struct dio { int flags; /* doesn't change */ - struct inode *inode; int rw; + struct inode *inode; loff_t i_size; /* i_size when submitted */ dio_iodone_t *end_io; /* IO completion function */ - struct buffer_head map_bh; /* last get_block() result */ /* BIO completion state */ spinlock_t bio_lock; /* protects BIO fields below */ + int page_errors; /* errno from get_user_pages() */ + int is_async; /* is IO async ? */ + int io_error; /* IO error in completion path */ unsigned long refcount; /* direct_io_worker() and bios */ struct bio *bio_list; /* singly linked via bi_private */ struct task_struct *waiter; /* waiting task (NULL if none) */ /* AIO related stuff */ struct kiocb *iocb; /* kiocb */ - int is_async; /* is IO async ? */ - int io_error; /* IO error in completion path */ ssize_t result; /* IO result */ - int page_errors; /* errno from get_user_pages() */ - + struct buffer_head map_bh; /* last get_block() result */ /* * pages[] (and any fields placed after it) are not zeroed out at * allocation time. Don't add new fields after pages[] unless you -- cgit v1.1 From 6e8267f532a17165ab551ac5fdafcba5333dcca5 Mon Sep 17 00:00:00 2001 From: Andi Kleen Date: Mon, 1 Aug 2011 21:38:06 -0700 Subject: direct-io: use a slab cache for struct dio A direct slab call is slightly faster than kmalloc and can be better cached per CPU. It also avoids rounding to the next kmalloc slab. In addition this enforces cache line alignment for struct dio to avoid any false sharing. Signed-off-by: Andi Kleen Acked-by: Jeff Moyer Signed-off-by: Christoph Hellwig --- fs/direct-io.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/fs/direct-io.c b/fs/direct-io.c index b01ea0d..6bb0440 100644 --- a/fs/direct-io.c +++ b/fs/direct-io.c @@ -140,7 +140,9 @@ struct dio { * wish that they not be zeroed. */ struct page *pages[DIO_PAGES]; /* page buffer */ -}; +} ____cacheline_aligned_in_smp; + +static struct kmem_cache *dio_cache __read_mostly; static void __inode_dio_wait(struct inode *inode) { @@ -330,7 +332,7 @@ static void dio_bio_end_aio(struct bio *bio, int error) if (remaining == 0) { dio_complete(dio, dio->iocb->ki_pos, 0, true); - kfree(dio); + kmem_cache_free(dio_cache, dio); } } @@ -1180,7 +1182,7 @@ direct_io_worker(int rw, struct kiocb *iocb, struct inode *inode, if (ret2 == 0) { ret = dio_complete(dio, offset, ret, false); - kfree(dio); + kmem_cache_free(dio_cache, dio); } else BUG_ON(ret != -EIOCBQUEUED); @@ -1256,7 +1258,7 @@ __blockdev_direct_IO(int rw, struct kiocb *iocb, struct inode *inode, if (rw == READ && end == offset) return 0; - dio = kmalloc(sizeof(*dio), GFP_KERNEL); + dio = kmem_cache_alloc(dio_cache, GFP_KERNEL); retval = -ENOMEM; if (!dio) goto out; @@ -1280,7 +1282,7 @@ __blockdev_direct_IO(int rw, struct kiocb *iocb, struct inode *inode, end - 1); if (retval) { mutex_unlock(&inode->i_mutex); - kfree(dio); + kmem_cache_free(dio_cache, dio); goto out; } } @@ -1308,3 +1310,10 @@ out: return retval; } EXPORT_SYMBOL(__blockdev_direct_IO); + +static __init int dio_init(void) +{ + dio_cache = KMEM_CACHE(dio, SLAB_PANIC); + return 0; +} +module_init(dio_init) -- cgit v1.1 From 18772641dbe2c89c6122c603f81f6a9574aee556 Mon Sep 17 00:00:00 2001 From: Andi Kleen Date: Mon, 1 Aug 2011 21:38:07 -0700 Subject: direct-io: separate map_bh from dio Only a single b_private field in the map_bh buffer head is needed after the submission path. Move map_bh separately to avoid storing this information in the long term slab. This avoids the weird 104 byte hole in struct dio_submit which also needed to be memseted early. Signed-off-by: Andi Kleen Signed-off-by: Christoph Hellwig --- fs/direct-io.c | 66 ++++++++++++++++++++++++++++++++-------------------------- 1 file changed, 37 insertions(+), 29 deletions(-) diff --git a/fs/direct-io.c b/fs/direct-io.c index 6bb0440..edf3174 100644 --- a/fs/direct-io.c +++ b/fs/direct-io.c @@ -119,6 +119,7 @@ struct dio { loff_t i_size; /* i_size when submitted */ dio_iodone_t *end_io; /* IO completion function */ + void *private; /* copy from map_bh.b_private */ /* BIO completion state */ spinlock_t bio_lock; /* protects BIO fields below */ @@ -133,7 +134,6 @@ struct dio { struct kiocb *iocb; /* kiocb */ ssize_t result; /* IO result */ - struct buffer_head map_bh; /* last get_block() result */ /* * pages[] (and any fields placed after it) are not zeroed out at * allocation time. Don't add new fields after pages[] unless you @@ -301,7 +301,7 @@ static ssize_t dio_complete(struct dio *dio, loff_t offset, ssize_t ret, bool is if (dio->end_io && dio->result) { dio->end_io(dio->iocb, offset, transferred, - dio->map_bh.b_private, ret, is_async); + dio->private, ret, is_async); } else { if (is_async) aio_complete(dio->iocb, ret, 0); @@ -574,10 +574,10 @@ static int dio_bio_reap(struct dio *dio, struct dio_submit *sdio) * buffer_mapped(). However the direct-io code will only process holes one * block at a time - it will repeatedly call get_block() as it walks the hole. */ -static int get_more_blocks(struct dio *dio, struct dio_submit *sdio) +static int get_more_blocks(struct dio *dio, struct dio_submit *sdio, + struct buffer_head *map_bh) { int ret; - struct buffer_head *map_bh = &dio->map_bh; sector_t fs_startblk; /* Into file, in filesystem-sized blocks */ unsigned long fs_count; /* Number of filesystem-sized blocks */ unsigned long dio_count;/* Number of dio_block-sized blocks */ @@ -621,6 +621,9 @@ static int get_more_blocks(struct dio *dio, struct dio_submit *sdio) ret = (*sdio->get_block)(dio->inode, fs_startblk, map_bh, create); + + /* Store for completion */ + dio->private = map_bh->b_private; } return ret; } @@ -629,7 +632,7 @@ static int get_more_blocks(struct dio *dio, struct dio_submit *sdio) * There is no bio. Make one now. */ static int dio_new_bio(struct dio *dio, struct dio_submit *sdio, - sector_t start_sector) + sector_t start_sector, struct buffer_head *map_bh) { sector_t sector; int ret, nr_pages; @@ -638,10 +641,10 @@ static int dio_new_bio(struct dio *dio, struct dio_submit *sdio, if (ret) goto out; sector = start_sector << (sdio->blkbits - 9); - nr_pages = min(sdio->pages_in_io, bio_get_nr_vecs(dio->map_bh.b_bdev)); + nr_pages = min(sdio->pages_in_io, bio_get_nr_vecs(map_bh->b_bdev)); nr_pages = min(nr_pages, BIO_MAX_PAGES); BUG_ON(nr_pages <= 0); - dio_bio_alloc(dio, sdio, dio->map_bh.b_bdev, sector, nr_pages); + dio_bio_alloc(dio, sdio, map_bh->b_bdev, sector, nr_pages); sdio->boundary = 0; out: return ret; @@ -686,7 +689,8 @@ static int dio_bio_add_page(struct dio_submit *sdio) * The caller of this function is responsible for removing cur_page from the * dio, and for dropping the refcount which came from that presence. */ -static int dio_send_cur_page(struct dio *dio, struct dio_submit *sdio) +static int dio_send_cur_page(struct dio *dio, struct dio_submit *sdio, + struct buffer_head *map_bh) { int ret = 0; @@ -721,14 +725,14 @@ static int dio_send_cur_page(struct dio *dio, struct dio_submit *sdio) } if (sdio->bio == NULL) { - ret = dio_new_bio(dio, sdio, sdio->cur_page_block); + ret = dio_new_bio(dio, sdio, sdio->cur_page_block, map_bh); if (ret) goto out; } if (dio_bio_add_page(sdio) != 0) { dio_bio_submit(dio, sdio); - ret = dio_new_bio(dio, sdio, sdio->cur_page_block); + ret = dio_new_bio(dio, sdio, sdio->cur_page_block, map_bh); if (ret == 0) { ret = dio_bio_add_page(sdio); BUG_ON(ret != 0); @@ -757,7 +761,8 @@ out: */ static int submit_page_section(struct dio *dio, struct dio_submit *sdio, struct page *page, - unsigned offset, unsigned len, sector_t blocknr) + unsigned offset, unsigned len, sector_t blocknr, + struct buffer_head *map_bh) { int ret = 0; @@ -782,7 +787,7 @@ submit_page_section(struct dio *dio, struct dio_submit *sdio, struct page *page, * avoid metadata seeks. */ if (sdio->boundary) { - ret = dio_send_cur_page(dio, sdio); + ret = dio_send_cur_page(dio, sdio, map_bh); page_cache_release(sdio->cur_page); sdio->cur_page = NULL; } @@ -793,7 +798,7 @@ submit_page_section(struct dio *dio, struct dio_submit *sdio, struct page *page, * If there's a deferred page already there then send it. */ if (sdio->cur_page) { - ret = dio_send_cur_page(dio, sdio); + ret = dio_send_cur_page(dio, sdio, map_bh); page_cache_release(sdio->cur_page); sdio->cur_page = NULL; if (ret) @@ -815,16 +820,16 @@ out: * file blocks. Only called for S_ISREG files - blockdevs do not set * buffer_new */ -static void clean_blockdev_aliases(struct dio *dio) +static void clean_blockdev_aliases(struct dio *dio, struct buffer_head *map_bh) { unsigned i; unsigned nblocks; - nblocks = dio->map_bh.b_size >> dio->inode->i_blkbits; + nblocks = map_bh->b_size >> dio->inode->i_blkbits; for (i = 0; i < nblocks; i++) { - unmap_underlying_metadata(dio->map_bh.b_bdev, - dio->map_bh.b_blocknr + i); + unmap_underlying_metadata(map_bh->b_bdev, + map_bh->b_blocknr + i); } } @@ -837,7 +842,8 @@ static void clean_blockdev_aliases(struct dio *dio) * `end' is zero if we're doing the start of the IO, 1 at the end of the * IO. */ -static void dio_zero_block(struct dio *dio, struct dio_submit *sdio, int end) +static void dio_zero_block(struct dio *dio, struct dio_submit *sdio, int end, + struct buffer_head *map_bh) { unsigned dio_blocks_per_fs_block; unsigned this_chunk_blocks; /* In dio_blocks */ @@ -845,7 +851,7 @@ static void dio_zero_block(struct dio *dio, struct dio_submit *sdio, int end) struct page *page; sdio->start_zero_done = 1; - if (!sdio->blkfactor || !buffer_new(&dio->map_bh)) + if (!sdio->blkfactor || !buffer_new(map_bh)) return; dio_blocks_per_fs_block = 1 << sdio->blkfactor; @@ -865,7 +871,7 @@ static void dio_zero_block(struct dio *dio, struct dio_submit *sdio, int end) page = ZERO_PAGE(0); if (submit_page_section(dio, sdio, page, 0, this_chunk_bytes, - sdio->next_block_for_io)) + sdio->next_block_for_io, map_bh)) return; sdio->next_block_for_io += this_chunk_blocks; @@ -887,13 +893,13 @@ static void dio_zero_block(struct dio *dio, struct dio_submit *sdio, int end) * it should set b_size to PAGE_SIZE or more inside get_block(). This gives * fine alignment but still allows this function to work in PAGE_SIZE units. */ -static int do_direct_IO(struct dio *dio, struct dio_submit *sdio) +static int do_direct_IO(struct dio *dio, struct dio_submit *sdio, + struct buffer_head *map_bh) { const unsigned blkbits = sdio->blkbits; const unsigned blocks_per_page = PAGE_SIZE >> blkbits; struct page *page; unsigned block_in_page; - struct buffer_head *map_bh = &dio->map_bh; int ret = 0; /* The I/O can start at any block offset within the first page */ @@ -919,7 +925,7 @@ static int do_direct_IO(struct dio *dio, struct dio_submit *sdio) unsigned long blkmask; unsigned long dio_remainder; - ret = get_more_blocks(dio, sdio); + ret = get_more_blocks(dio, sdio, map_bh); if (ret) { page_cache_release(page); goto out; @@ -932,7 +938,7 @@ static int do_direct_IO(struct dio *dio, struct dio_submit *sdio) sdio->next_block_for_io = map_bh->b_blocknr << sdio->blkfactor; if (buffer_new(map_bh)) - clean_blockdev_aliases(dio); + clean_blockdev_aliases(dio, map_bh); if (!sdio->blkfactor) goto do_holes; @@ -991,7 +997,7 @@ do_holes: * we must zero out the start of this block. */ if (unlikely(sdio->blkfactor && !sdio->start_zero_done)) - dio_zero_block(dio, sdio, 0); + dio_zero_block(dio, sdio, 0, map_bh); /* * Work out, in this_chunk_blocks, how much disk we @@ -1011,7 +1017,8 @@ do_holes: ret = submit_page_section(dio, sdio, page, offset_in_page, this_chunk_bytes, - sdio->next_block_for_io); + sdio->next_block_for_io, + map_bh); if (ret) { page_cache_release(page); goto out; @@ -1047,6 +1054,7 @@ direct_io_worker(int rw, struct kiocb *iocb, struct inode *inode, ssize_t ret = 0; ssize_t ret2; size_t bytes; + struct buffer_head map_bh = { 0, }; dio->inode = inode; dio->rw = rw; @@ -1101,7 +1109,7 @@ direct_io_worker(int rw, struct kiocb *iocb, struct inode *inode, sdio->total_pages += (bytes + PAGE_SIZE - 1) / PAGE_SIZE; sdio->curr_user_address = user_addr; - ret = do_direct_IO(dio, sdio); + ret = do_direct_IO(dio, sdio, &map_bh); dio->result += iov[seg].iov_len - ((sdio->final_block_in_request - sdio->block_in_file) << @@ -1124,10 +1132,10 @@ direct_io_worker(int rw, struct kiocb *iocb, struct inode *inode, * There may be some unwritten disk at the end of a part-written * fs-block-sized block. Go zero that now. */ - dio_zero_block(dio, sdio, 1); + dio_zero_block(dio, sdio, 1, &map_bh); if (sdio->cur_page) { - ret2 = dio_send_cur_page(dio, sdio); + ret2 = dio_send_cur_page(dio, sdio, &map_bh); if (ret == 0) ret = ret2; page_cache_release(sdio->cur_page); -- cgit v1.1 From ba253fbf6d3502c54e1ac8792e7ac8290a1f5b8d Mon Sep 17 00:00:00 2001 From: Andi Kleen Date: Mon, 1 Aug 2011 21:38:08 -0700 Subject: direct-io: inline the complete submission path Add inlines to all the submission path functions. While this increases code size it also gives gcc a lot of optimization opportunities in this critical hotpath. In particular -- together with some other changes -- this allows gcc to get rid of the unnecessary clearing of sdio at the beginning and optimize the messy parameter passing. Any non inlining of a function which takes a sdio parameter would break this optimization because they cannot be done if the address of a structure is taken. Note that benefits are only seen with CONFIG_OPTIMIZE_INLINING and CONFIG_CC_OPTIMIZE_FOR_SIZE both set to off. This gives about 2.2% improvement on a large database benchmark with a high IOPS rate. Signed-off-by: Andi Kleen Signed-off-by: Christoph Hellwig --- fs/direct-io.c | 36 +++++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/fs/direct-io.c b/fs/direct-io.c index edf3174..6d42582 100644 --- a/fs/direct-io.c +++ b/fs/direct-io.c @@ -199,7 +199,7 @@ static inline unsigned dio_pages_present(struct dio_submit *sdio) /* * Go grab and pin some userspace pages. Typically we'll get 64 at a time. */ -static int dio_refill_pages(struct dio *dio, struct dio_submit *sdio) +static inline int dio_refill_pages(struct dio *dio, struct dio_submit *sdio) { int ret; int nr_pages; @@ -245,7 +245,8 @@ out: * decent number of pages, less frequently. To provide nicer use of the * L1 cache. */ -static struct page *dio_get_page(struct dio *dio, struct dio_submit *sdio) +static inline struct page *dio_get_page(struct dio *dio, + struct dio_submit *sdio) { if (dio_pages_present(sdio) == 0) { int ret; @@ -376,7 +377,7 @@ void dio_end_io(struct bio *bio, int error) } EXPORT_SYMBOL_GPL(dio_end_io); -static void +static inline void dio_bio_alloc(struct dio *dio, struct dio_submit *sdio, struct block_device *bdev, sector_t first_sector, int nr_vecs) @@ -407,7 +408,7 @@ dio_bio_alloc(struct dio *dio, struct dio_submit *sdio, * * bios hold a dio reference between submit_bio and ->end_io. */ -static void dio_bio_submit(struct dio *dio, struct dio_submit *sdio) +static inline void dio_bio_submit(struct dio *dio, struct dio_submit *sdio) { struct bio *bio = sdio->bio; unsigned long flags; @@ -435,7 +436,7 @@ static void dio_bio_submit(struct dio *dio, struct dio_submit *sdio) /* * Release any resources in case of a failure */ -static void dio_cleanup(struct dio *dio, struct dio_submit *sdio) +static inline void dio_cleanup(struct dio *dio, struct dio_submit *sdio) { while (dio_pages_present(sdio)) page_cache_release(dio_get_page(dio, sdio)); @@ -528,7 +529,7 @@ static void dio_await_completion(struct dio *dio) * * This also helps to limit the peak amount of pinned userspace memory. */ -static int dio_bio_reap(struct dio *dio, struct dio_submit *sdio) +static inline int dio_bio_reap(struct dio *dio, struct dio_submit *sdio) { int ret = 0; @@ -631,8 +632,8 @@ static int get_more_blocks(struct dio *dio, struct dio_submit *sdio, /* * There is no bio. Make one now. */ -static int dio_new_bio(struct dio *dio, struct dio_submit *sdio, - sector_t start_sector, struct buffer_head *map_bh) +static inline int dio_new_bio(struct dio *dio, struct dio_submit *sdio, + sector_t start_sector, struct buffer_head *map_bh) { sector_t sector; int ret, nr_pages; @@ -657,7 +658,7 @@ out: * * Return zero on success. Non-zero means the caller needs to start a new BIO. */ -static int dio_bio_add_page(struct dio_submit *sdio) +static inline int dio_bio_add_page(struct dio_submit *sdio) { int ret; @@ -689,8 +690,8 @@ static int dio_bio_add_page(struct dio_submit *sdio) * The caller of this function is responsible for removing cur_page from the * dio, and for dropping the refcount which came from that presence. */ -static int dio_send_cur_page(struct dio *dio, struct dio_submit *sdio, - struct buffer_head *map_bh) +static inline int dio_send_cur_page(struct dio *dio, struct dio_submit *sdio, + struct buffer_head *map_bh) { int ret = 0; @@ -759,7 +760,7 @@ out: * If that doesn't work out then we put the old page into the bio and add this * page to the dio instead. */ -static int +static inline int submit_page_section(struct dio *dio, struct dio_submit *sdio, struct page *page, unsigned offset, unsigned len, sector_t blocknr, struct buffer_head *map_bh) @@ -842,8 +843,8 @@ static void clean_blockdev_aliases(struct dio *dio, struct buffer_head *map_bh) * `end' is zero if we're doing the start of the IO, 1 at the end of the * IO. */ -static void dio_zero_block(struct dio *dio, struct dio_submit *sdio, int end, - struct buffer_head *map_bh) +static inline void dio_zero_block(struct dio *dio, struct dio_submit *sdio, + int end, struct buffer_head *map_bh) { unsigned dio_blocks_per_fs_block; unsigned this_chunk_blocks; /* In dio_blocks */ @@ -1042,7 +1043,7 @@ out: return ret; } -static ssize_t +static inline ssize_t direct_io_worker(int rw, struct kiocb *iocb, struct inode *inode, const struct iovec *iov, loff_t offset, unsigned long nr_segs, unsigned blkbits, get_block_t get_block, dio_iodone_t end_io, @@ -1216,6 +1217,11 @@ direct_io_worker(int rw, struct kiocb *iocb, struct inode *inode, * expected that filesystem provide exclusion between new direct I/O * and truncates. For DIO_LOCKING filesystems this is done by i_mutex, * but other filesystems need to take care of this on their own. + * + * NOTE: if you pass "sdio" to anything by pointer make sure that function + * is always inlined. Otherwise gcc is unable to split the structure into + * individual fields and will generate much worse code. This is important + * for the whole file. */ ssize_t __blockdev_direct_IO(int rw, struct kiocb *iocb, struct inode *inode, -- cgit v1.1 From 847cc6371ba820763773e993000410d6d8d23515 Mon Sep 17 00:00:00 2001 From: Andi Kleen Date: Mon, 1 Aug 2011 21:38:09 -0700 Subject: direct-io: merge direct_io_walker into __blockdev_direct_IO This doesn't change anything for the compiler, but hch thought it would make the code clearer. I moved the reference counting into its own little inline. Signed-off-by: Andi Kleen Acked-by: Jeff Moyer Signed-off-by: Christoph Hellwig --- fs/direct-io.c | 271 ++++++++++++++++++++++++++++----------------------------- 1 file changed, 132 insertions(+), 139 deletions(-) diff --git a/fs/direct-io.c b/fs/direct-io.c index 6d42582..d740ab6 100644 --- a/fs/direct-io.c +++ b/fs/direct-io.c @@ -1043,136 +1043,10 @@ out: return ret; } -static inline ssize_t -direct_io_worker(int rw, struct kiocb *iocb, struct inode *inode, - const struct iovec *iov, loff_t offset, unsigned long nr_segs, - unsigned blkbits, get_block_t get_block, dio_iodone_t end_io, - dio_submit_t submit_io, struct dio *dio, struct dio_submit *sdio) +static inline int drop_refcount(struct dio *dio) { - unsigned long user_addr; + int ret2; unsigned long flags; - int seg; - ssize_t ret = 0; - ssize_t ret2; - size_t bytes; - struct buffer_head map_bh = { 0, }; - - dio->inode = inode; - dio->rw = rw; - sdio->blkbits = blkbits; - sdio->blkfactor = inode->i_blkbits - blkbits; - sdio->block_in_file = offset >> blkbits; - - sdio->get_block = get_block; - dio->end_io = end_io; - sdio->submit_io = submit_io; - sdio->final_block_in_bio = -1; - sdio->next_block_for_io = -1; - - dio->iocb = iocb; - dio->i_size = i_size_read(inode); - - spin_lock_init(&dio->bio_lock); - dio->refcount = 1; - - /* - * In case of non-aligned buffers, we may need 2 more - * pages since we need to zero out first and last block. - */ - if (unlikely(sdio->blkfactor)) - sdio->pages_in_io = 2; - - for (seg = 0; seg < nr_segs; seg++) { - user_addr = (unsigned long)iov[seg].iov_base; - sdio->pages_in_io += - ((user_addr+iov[seg].iov_len +PAGE_SIZE-1)/PAGE_SIZE - - user_addr/PAGE_SIZE); - } - - for (seg = 0; seg < nr_segs; seg++) { - user_addr = (unsigned long)iov[seg].iov_base; - sdio->size += bytes = iov[seg].iov_len; - - /* Index into the first page of the first block */ - sdio->first_block_in_page = (user_addr & ~PAGE_MASK) >> blkbits; - sdio->final_block_in_request = sdio->block_in_file + - (bytes >> blkbits); - /* Page fetching state */ - sdio->head = 0; - sdio->tail = 0; - sdio->curr_page = 0; - - sdio->total_pages = 0; - if (user_addr & (PAGE_SIZE-1)) { - sdio->total_pages++; - bytes -= PAGE_SIZE - (user_addr & (PAGE_SIZE - 1)); - } - sdio->total_pages += (bytes + PAGE_SIZE - 1) / PAGE_SIZE; - sdio->curr_user_address = user_addr; - - ret = do_direct_IO(dio, sdio, &map_bh); - - dio->result += iov[seg].iov_len - - ((sdio->final_block_in_request - sdio->block_in_file) << - blkbits); - - if (ret) { - dio_cleanup(dio, sdio); - break; - } - } /* end iovec loop */ - - if (ret == -ENOTBLK) { - /* - * The remaining part of the request will be - * be handled by buffered I/O when we return - */ - ret = 0; - } - /* - * There may be some unwritten disk at the end of a part-written - * fs-block-sized block. Go zero that now. - */ - dio_zero_block(dio, sdio, 1, &map_bh); - - if (sdio->cur_page) { - ret2 = dio_send_cur_page(dio, sdio, &map_bh); - if (ret == 0) - ret = ret2; - page_cache_release(sdio->cur_page); - sdio->cur_page = NULL; - } - if (sdio->bio) - dio_bio_submit(dio, sdio); - - /* - * It is possible that, we return short IO due to end of file. - * In that case, we need to release all the pages we got hold on. - */ - dio_cleanup(dio, sdio); - - /* - * All block lookups have been performed. For READ requests - * we can let i_mutex go now that its achieved its purpose - * of protecting us from looking up uninitialized blocks. - */ - if (rw == READ && (dio->flags & DIO_LOCKING)) - mutex_unlock(&dio->inode->i_mutex); - - /* - * The only time we want to leave bios in flight is when a successful - * partial aio read or full aio write have been setup. In that case - * bio completion will call aio_complete. The only time it's safe to - * call aio_complete is when we return -EIOCBQUEUED, so we key on that. - * This had *better* be the only place that raises -EIOCBQUEUED. - */ - BUG_ON(ret == -EIOCBQUEUED); - if (dio->is_async && ret == 0 && dio->result && - ((rw & READ) || (dio->result == sdio->size))) - ret = -EIOCBQUEUED; - - if (ret != -EIOCBQUEUED) - dio_await_completion(dio); /* * Sync will always be dropping the final ref and completing the @@ -1188,14 +1062,7 @@ direct_io_worker(int rw, struct kiocb *iocb, struct inode *inode, spin_lock_irqsave(&dio->bio_lock, flags); ret2 = --dio->refcount; spin_unlock_irqrestore(&dio->bio_lock, flags); - - if (ret2 == 0) { - ret = dio_complete(dio, offset, ret, false); - kmem_cache_free(dio_cache, dio); - } else - BUG_ON(ret != -EIOCBQUEUED); - - return ret; + return ret2; } /* @@ -1239,6 +1106,9 @@ __blockdev_direct_IO(int rw, struct kiocb *iocb, struct inode *inode, loff_t end = offset; struct dio *dio; struct dio_submit sdio = { 0, }; + unsigned long user_addr; + size_t bytes; + struct buffer_head map_bh = { 0, }; if (rw & WRITE) rw = WRITE_ODIRECT; @@ -1316,9 +1186,132 @@ __blockdev_direct_IO(int rw, struct kiocb *iocb, struct inode *inode, dio->is_async = !is_sync_kiocb(iocb) && !((rw & WRITE) && (end > i_size_read(inode))); - retval = direct_io_worker(rw, iocb, inode, iov, offset, - nr_segs, blkbits, get_block, end_io, - submit_io, dio, &sdio); + retval = 0; + + dio->inode = inode; + dio->rw = rw; + sdio.blkbits = blkbits; + sdio.blkfactor = inode->i_blkbits - blkbits; + sdio.block_in_file = offset >> blkbits; + + sdio.get_block = get_block; + dio->end_io = end_io; + sdio.submit_io = submit_io; + sdio.final_block_in_bio = -1; + sdio.next_block_for_io = -1; + + dio->iocb = iocb; + dio->i_size = i_size_read(inode); + + spin_lock_init(&dio->bio_lock); + dio->refcount = 1; + + /* + * In case of non-aligned buffers, we may need 2 more + * pages since we need to zero out first and last block. + */ + if (unlikely(sdio.blkfactor)) + sdio.pages_in_io = 2; + + for (seg = 0; seg < nr_segs; seg++) { + user_addr = (unsigned long)iov[seg].iov_base; + sdio.pages_in_io += + ((user_addr + iov[seg].iov_len + PAGE_SIZE-1) / + PAGE_SIZE - user_addr / PAGE_SIZE); + } + + for (seg = 0; seg < nr_segs; seg++) { + user_addr = (unsigned long)iov[seg].iov_base; + sdio.size += bytes = iov[seg].iov_len; + + /* Index into the first page of the first block */ + sdio.first_block_in_page = (user_addr & ~PAGE_MASK) >> blkbits; + sdio.final_block_in_request = sdio.block_in_file + + (bytes >> blkbits); + /* Page fetching state */ + sdio.head = 0; + sdio.tail = 0; + sdio.curr_page = 0; + + sdio.total_pages = 0; + if (user_addr & (PAGE_SIZE-1)) { + sdio.total_pages++; + bytes -= PAGE_SIZE - (user_addr & (PAGE_SIZE - 1)); + } + sdio.total_pages += (bytes + PAGE_SIZE - 1) / PAGE_SIZE; + sdio.curr_user_address = user_addr; + + retval = do_direct_IO(dio, &sdio, &map_bh); + + dio->result += iov[seg].iov_len - + ((sdio.final_block_in_request - sdio.block_in_file) << + blkbits); + + if (retval) { + dio_cleanup(dio, &sdio); + break; + } + } /* end iovec loop */ + + if (retval == -ENOTBLK) { + /* + * The remaining part of the request will be + * be handled by buffered I/O when we return + */ + retval = 0; + } + /* + * There may be some unwritten disk at the end of a part-written + * fs-block-sized block. Go zero that now. + */ + dio_zero_block(dio, &sdio, 1, &map_bh); + + if (sdio.cur_page) { + ssize_t ret2; + + ret2 = dio_send_cur_page(dio, &sdio, &map_bh); + if (retval == 0) + retval = ret2; + page_cache_release(sdio.cur_page); + sdio.cur_page = NULL; + } + if (sdio.bio) + dio_bio_submit(dio, &sdio); + + /* + * It is possible that, we return short IO due to end of file. + * In that case, we need to release all the pages we got hold on. + */ + dio_cleanup(dio, &sdio); + + /* + * All block lookups have been performed. For READ requests + * we can let i_mutex go now that its achieved its purpose + * of protecting us from looking up uninitialized blocks. + */ + if (rw == READ && (dio->flags & DIO_LOCKING)) + mutex_unlock(&dio->inode->i_mutex); + + /* + * The only time we want to leave bios in flight is when a successful + * partial aio read or full aio write have been setup. In that case + * bio completion will call aio_complete. The only time it's safe to + * call aio_complete is when we return -EIOCBQUEUED, so we key on that. + * This had *better* be the only place that raises -EIOCBQUEUED. + */ + BUG_ON(retval == -EIOCBQUEUED); + if (dio->is_async && retval == 0 && dio->result && + ((rw & READ) || (dio->result == sdio.size))) + retval = -EIOCBQUEUED; + + if (retval != -EIOCBQUEUED) + dio_await_completion(dio); + + if (drop_refcount(dio) == 0) { + retval = dio_complete(dio, offset, retval, false); + kmem_cache_free(dio_cache, dio); + } else + BUG_ON(retval != -EIOCBQUEUED); out: return retval; -- cgit v1.1 From ef3d0fd27e90f67e35da516dafc1482c82939a60 Mon Sep 17 00:00:00 2001 From: Andi Kleen Date: Thu, 15 Sep 2011 16:06:48 -0700 Subject: 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 Signed-off-by: Christoph Hellwig --- fs/btrfs/file.c | 2 +- fs/cifs/cifsfs.c | 2 +- fs/gfs2/file.c | 4 +-- fs/nfs/file.c | 5 ++-- fs/read_write.c | 85 ++++++++++++++++++++++++++---------------------------- include/linux/fs.h | 9 ++++-- 6 files changed, 54 insertions(+), 53 deletions(-) diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c index e4e57d5..1266f6e 100644 --- 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 54b8f1e..db7ce87 100644 --- 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 edeb9e8..fe6bc02 100644 --- 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 28b8c3f..12623ab 100644 --- 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 179f1c3..672b187 100644 --- 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 c1884e9..db85196 100644 --- 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); -- cgit v1.1 From 5760495a872d63a182962680a13c2af29235237c Mon Sep 17 00:00:00 2001 From: Andi Kleen Date: Thu, 15 Sep 2011 16:06:50 -0700 Subject: vfs: add generic_file_llseek_size Add a generic_file_llseek variant to the VFS that allows passing in the maximum file size of the file system, instead of always using maxbytes from the superblock. This can be used to eliminate some cut'n'paste seek code in ext4. Signed-off-by: Andi Kleen Signed-off-by: Christoph Hellwig --- fs/read_write.c | 37 ++++++++++++++++++++++++++++--------- include/linux/fs.h | 2 ++ 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/fs/read_write.c b/fs/read_write.c index 672b187..dfd1257 100644 --- a/fs/read_write.c +++ b/fs/read_write.c @@ -51,23 +51,23 @@ static loff_t lseek_execute(struct file *file, struct inode *inode, } /** - * generic_file_llseek - generic llseek implementation for regular files + * generic_file_llseek_size - generic llseek implementation for regular files * @file: file structure to seek on * @offset: file offset to seek to * @origin: type of seek + * @size: max size of file system * - * 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. + * This is a variant of generic_file_llseek that allows passing in a custom + * file size. * * Synchronization: - * SEEK_SET is unsynchronized (but atomic on 64bit platforms) + * SEEK_SET and SEEK_END are 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(struct file *file, loff_t offset, int origin) +generic_file_llseek_size(struct file *file, loff_t offset, int origin, + loff_t maxsize) { struct inode *inode = file->f_mapping->host; @@ -91,7 +91,7 @@ generic_file_llseek(struct file *file, loff_t offset, int origin) */ spin_lock(&file->f_lock); offset = lseek_execute(file, inode, file->f_pos + offset, - inode->i_sb->s_maxbytes); + maxsize); spin_unlock(&file->f_lock); return offset; case SEEK_DATA: @@ -113,7 +113,26 @@ generic_file_llseek(struct file *file, loff_t offset, int origin) break; } - return lseek_execute(file, inode, offset, inode->i_sb->s_maxbytes); + return lseek_execute(file, inode, offset, maxsize); +} +EXPORT_SYMBOL(generic_file_llseek_size); + +/** + * 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) +{ + struct inode *inode = file->f_mapping->host; + + return generic_file_llseek_size(file, offset, origin, + inode->i_sb->s_maxbytes); } EXPORT_SYMBOL(generic_file_llseek); diff --git a/include/linux/fs.h b/include/linux/fs.h index db85196..d055cc7 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -2403,6 +2403,8 @@ 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_size(struct file *file, loff_t offset, + int origin, loff_t maxsize); extern int generic_file_open(struct inode * inode, struct file * filp); extern int nonseekable_open(struct inode * inode, struct file * filp); -- cgit v1.1 From 4cce0e28b932c11454f75d1c1fae674600c23fbf Mon Sep 17 00:00:00 2001 From: Andi Kleen Date: Thu, 15 Sep 2011 16:06:51 -0700 Subject: ext4: replace cut'n'pasted llseek code with generic_file_llseek_size This gives ext4 the benefits of unlocked llseek. Cc: tytso@mit.edu Signed-off-by: Andi Kleen Signed-off-by: Christoph Hellwig --- fs/ext4/file.c | 47 +---------------------------------------------- 1 file changed, 1 insertion(+), 46 deletions(-) diff --git a/fs/ext4/file.c b/fs/ext4/file.c index e4095e9..b9548f4 100644 --- a/fs/ext4/file.c +++ b/fs/ext4/file.c @@ -224,53 +224,8 @@ loff_t ext4_llseek(struct file *file, loff_t offset, int origin) maxbytes = EXT4_SB(inode->i_sb)->s_bitmap_maxbytes; else maxbytes = inode->i_sb->s_maxbytes; - mutex_lock(&inode->i_mutex); - switch (origin) { - case SEEK_END: - offset += inode->i_size; - break; - case SEEK_CUR: - if (offset == 0) { - mutex_unlock(&inode->i_mutex); - return file->f_pos; - } - offset += file->f_pos; - break; - 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) { - mutex_unlock(&inode->i_mutex); - return -ENXIO; - } - break; - case SEEK_HOLE: - /* - * 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) { - mutex_unlock(&inode->i_mutex); - return -ENXIO; - } - offset = inode->i_size; - break; - } - - if (offset < 0 || offset > maxbytes) { - mutex_unlock(&inode->i_mutex); - return -EINVAL; - } - - if (offset != file->f_pos) { - file->f_pos = offset; - file->f_version = 0; - } - mutex_unlock(&inode->i_mutex); - return offset; + return generic_file_llseek_size(file, offset, origin, maxbytes); } const struct file_operations ext4_file_operations = { -- cgit v1.1 From 79835a710d6ff811659c8de46f89c7577c3b8cc6 Mon Sep 17 00:00:00 2001 From: Andi Kleen Date: Thu, 15 Sep 2011 16:06:52 -0700 Subject: nfs: drop unnecessary locking in llseek This makes NFS follow the standard generic_file_llseek locking scheme. Cc: Trond.Myklebust@netapp.com Signed-off-by: Andi Kleen Signed-off-by: Christoph Hellwig --- fs/nfs/file.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/fs/nfs/file.c b/fs/nfs/file.c index 12623ab..91c01f0 100644 --- a/fs/nfs/file.c +++ b/fs/nfs/file.c @@ -180,8 +180,6 @@ force_reval: static loff_t nfs_file_llseek(struct file *filp, loff_t offset, int origin) { - loff_t loff; - dprintk("NFS: llseek file(%s/%s, %lld, %d)\n", filp->f_path.dentry->d_parent->d_name.name, filp->f_path.dentry->d_name.name, @@ -197,14 +195,9 @@ static loff_t nfs_file_llseek(struct file *filp, loff_t offset, int origin) int retval = nfs_revalidate_file_size(inode, filp); 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(filp, offset, origin); - spin_unlock(&inode->i_lock); - } else - loff = generic_file_llseek(filp, offset, origin); - return loff; + return generic_file_llseek(filp, offset, origin); } /* -- cgit v1.1 From f3c7691e8d30d88899b514675c7c86d19057b5fd Mon Sep 17 00:00:00 2001 From: "J. Bruce Fields" Date: Wed, 21 Sep 2011 10:58:13 -0400 Subject: leases: fix write-open/read-lease race In setlease, we use i_writecount to decide whether we can give out a read lease. In open, we break leases before incrementing i_writecount. There is therefore a window between the break lease and the i_writecount increment when setlease could add a new read lease. This would leave us with a simultaneous write open and read lease, which shouldn't happen. Signed-off-by: J. Bruce Fields Signed-off-by: Christoph Hellwig --- fs/namei.c | 5 +---- fs/open.c | 4 ++++ 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/fs/namei.c b/fs/namei.c index 9061157..7657be4 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -2035,10 +2035,7 @@ static int may_open(struct path *path, int acc_mode, int flag) if (flag & O_NOATIME && !inode_owner_or_capable(inode)) return -EPERM; - /* - * Ensure there are no outstanding leases on the file. - */ - return break_lease(inode, flag); + return 0; } static int handle_truncate(struct file *filp) diff --git a/fs/open.c b/fs/open.c index f711921..22c41b5 100644 --- a/fs/open.c +++ b/fs/open.c @@ -685,6 +685,10 @@ static struct file *__dentry_open(struct dentry *dentry, struct vfsmount *mnt, if (error) goto cleanup_all; + error = break_lease(inode, f->f_flags); + if (error) + goto cleanup_all; + if (!open && f->f_op) open = f->f_op->open; if (open) { -- cgit v1.1