Re: [PATCH] Introduce a method to catch mmap_region (was: Recent kernel "mount" slow)

Linus Torvalds
2012-11-28T20:33:07Z

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

On Wed, Nov 28, 2012 at 12:13 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> I dunno. Maybe there is some fundamental reason why the above is
> broken, but it seems to be a much simpler approach. Sure, you need to
> guarantee that the people who get the write-lock cannot possibly cause
> IO while holding it, but since the only reason to get the write lock
> would be to change the block size, that should be pretty simple, no?

Here is a *COMPLETELY* untested patch. Caveat emptor. It will probably
do unspeakable things to your family and pets.

Linus

[-- Attachment #2: patch.diff --]
[-- Type: application/octet-stream, Size: 7590 bytes --]

fs/block_dev.c | 138 +++++++++++++++++++++------------------------------------
1 file changed, 50 insertions(+), 88 deletions(-)

diff --git a/fs/block_dev.c b/fs/block_dev.c
index 1a1e5e3b1eaf..98709446f5f4 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -116,8 +116,6 @@ EXPORT_SYMBOL(invalidate_bdev);

int set_blocksize(struct block_device *bdev, int size)
{
- struct address_space *mapping;
-
/* Size must be a power of two, and between 512 and PAGE_SIZE */
if (size > PAGE_SIZE || size < 512 || !is_power_of_2(size))
return -EINVAL;
@@ -126,29 +124,18 @@ int set_blocksize(struct block_device *bdev, int size)
if (size < bdev_logical_block_size(bdev))
return -EINVAL;

- /* Prevent starting I/O or mapping the device */
- percpu_down_write(&bdev->bd_block_size_semaphore);
-
- /* Check that the block device is not memory mapped */
- mapping = bdev->bd_inode->i_mapping;
- mutex_lock(&mapping->i_mmap_mutex);
- if (mapping_mapped(mapping)) {
- mutex_unlock(&mapping->i_mmap_mutex);
- percpu_up_write(&bdev->bd_block_size_semaphore);
- return -EBUSY;
- }
- mutex_unlock(&mapping->i_mmap_mutex);
-
/* Don't change the size if it is same as current */
if (bdev->bd_block_size != size) {
- sync_blockdev(bdev);
+ percpu_down_write(&bdev->bd_block_size_semaphore);
bdev->bd_block_size = size;
bdev->bd_inode->i_blkbits = blksize_bits(size);
+ percpu_up_write(&bdev->bd_block_size_semaphore);
+
+ /* Sync and kill any old size buffers */
+ sync_blockdev(bdev);
kill_bdev(bdev);
}

- percpu_up_write(&bdev->bd_block_size_semaphore);
-
return 0;
}

@@ -233,9 +220,14 @@ blkdev_direct_IO(int rw, struct kiocb *iocb, const struct iovec *iov,
{
struct file *file = iocb->ki_filp;
struct inode *inode = file->f_mapping->host;
+ struct block_device *bdev = I_BDEV(inode);
+ ssize_t ret;

- return __blockdev_direct_IO(rw, iocb, inode, I_BDEV(inode), iov, offset,
+ percpu_down_read(&bdev->bd_block_size_semaphore);
+ ret = __blockdev_direct_IO(rw, iocb, inode, I_BDEV(inode), iov, offset,
nr_segs, blkdev_get_blocks, NULL, NULL, 0);
+ percpu_up_read(&bdev->bd_block_size_semaphore);
+ return ret;
}

int __sync_blockdev(struct block_device *bdev, int wait)
@@ -358,20 +350,48 @@ EXPORT_SYMBOL(thaw_bdev);

static int blkdev_writepage(struct page *page, struct writeback_control *wbc)
{
- return block_write_full_page(page, blkdev_get_block, wbc);
+ int ret;
+ struct inode *inode = page_mapping(page)->host;
+ struct block_device *bdev = I_BDEV(inode);
+
+ percpu_down_read(&bdev->bd_block_size_semaphore);
+ ret = block_write_full_page(page, blkdev_get_block, wbc);
+ percpu_up_read(&bdev->bd_block_size_semaphore);
+
+ return ret;
}

static int blkdev_readpage(struct file * file, struct page * page)
{
- return block_read_full_page(page, blkdev_get_block);
+ int ret;
+ struct inode *inode = page_mapping(page)->host;
+ struct block_device *bdev = I_BDEV(inode);
+
+ percpu_down_read(&bdev->bd_block_size_semaphore);
+ ret = block_read_full_page(page, blkdev_get_block);
+ percpu_up_read(&bdev->bd_block_size_semaphore);
+
+ return ret;
}

static int blkdev_write_begin(struct file *file, struct address_space *mapping,
loff_t pos, unsigned len, unsigned flags,
struct page **pagep, void **fsdata)
{
- return block_write_begin(mapping, pos, len, flags, pagep,
+ int ret;
+ struct inode *inode = mapping->host;
+ struct block_device *bdev = I_BDEV(inode);
+
+ percpu_down_read(&bdev->bd_block_size_semaphore);
+ ret = block_write_begin(mapping, pos, len, flags, pagep,
blkdev_get_block);
+ /*
+ * If we had an error, we need to release the size
+ * semaphore, because write_end() will never be called.
+ */
+ if (ret)
+ percpu_up_read(&bdev->bd_block_size_semaphore);
+ return ret;
}

static int blkdev_write_end(struct file *file, struct address_space *mapping,
@@ -379,8 +399,11 @@ static int blkdev_write_end(struct file *file, struct address_space *mapping,
struct page *page, void *fsdata)
{
int ret;
- ret = block_write_end(file, mapping, pos, len, copied, page, fsdata);
+ struct inode *inode = mapping->host;
+ struct block_device *bdev = I_BDEV(inode);

+ ret = block_write_end(file, mapping, pos, len, copied, page, fsdata);
+ percpu_up_read(&bdev->bd_block_size_semaphore);
unlock_page(page);
page_cache_release(page);

@@ -1596,16 +1619,7 @@ static long block_ioctl(struct file *file, unsigned cmd, unsigned long arg)
ssize_t blkdev_aio_read(struct kiocb *iocb, const struct iovec *iov,
unsigned long nr_segs, loff_t pos)
{
- ssize_t ret;
- struct block_device *bdev = I_BDEV(iocb->ki_filp->f_mapping->host);
-
- percpu_down_read(&bdev->bd_block_size_semaphore);
-
- ret = generic_file_aio_read(iocb, iov, nr_segs, pos);
-
- percpu_up_read(&bdev->bd_block_size_semaphore);
-
- return ret;
+ return generic_file_aio_read(iocb, iov, nr_segs, pos);
}
EXPORT_SYMBOL_GPL(blkdev_aio_read);

@@ -1620,7 +1634,6 @@ ssize_t blkdev_aio_write(struct kiocb *iocb, const struct iovec *iov,
unsigned long nr_segs, loff_t pos)
{
struct file *file = iocb->ki_filp;
- struct block_device *bdev = I_BDEV(file->f_mapping->host);
struct blk_plug plug;
ssize_t ret;

@@ -1628,8 +1641,6 @@ ssize_t blkdev_aio_write(struct kiocb *iocb, const struct iovec *iov,

blk_start_plug(&plug);

- percpu_down_read(&bdev->bd_block_size_semaphore);
-
ret = __generic_file_aio_write(iocb, iov, nr_segs, &iocb->ki_pos);
if (ret > 0 || ret == -EIOCBQUEUED) {
ssize_t err;
@@ -1639,61 +1650,12 @@ ssize_t blkdev_aio_write(struct kiocb *iocb, const struct iovec *iov,
ret = err;
}

- percpu_up_read(&bdev->bd_block_size_semaphore);
-
blk_finish_plug(&plug);

return ret;
}
EXPORT_SYMBOL_GPL(blkdev_aio_write);

-static int blkdev_mmap(struct file *file, struct vm_area_struct *vma)
-{
- int ret;
- struct block_device *bdev = I_BDEV(file->f_mapping->host);
-
- percpu_down_read(&bdev->bd_block_size_semaphore);
-
- ret = generic_file_mmap(file, vma);
-
- percpu_up_read(&bdev->bd_block_size_semaphore);
-
- return ret;
-}
-
-static ssize_t blkdev_splice_read(struct file *file, loff_t *ppos,
- struct pipe_inode_info *pipe, size_t len,
- unsigned int flags)
-{
- ssize_t ret;
- struct block_device *bdev = I_BDEV(file->f_mapping->host);
-
- percpu_down_read(&bdev->bd_block_size_semaphore);
-
- ret = generic_file_splice_read(file, ppos, pipe, len, flags);
-
- percpu_up_read(&bdev->bd_block_size_semaphore);
-
- return ret;
-}
-
-static ssize_t blkdev_splice_write(struct pipe_inode_info *pipe,
- struct file *file, loff_t *ppos, size_t len,
- unsigned int flags)
-{
- ssize_t ret;
- struct block_device *bdev = I_BDEV(file->f_mapping->host);
-
- percpu_down_read(&bdev->bd_block_size_semaphore);
-
- ret = generic_file_splice_write(pipe, file, ppos, len, flags);
-
- percpu_up_read(&bdev->bd_block_size_semaphore);
-
- return ret;
-}
-
-
/*
* Try to release a page associated with block device when the system
* is under memory pressure.
@@ -1726,14 +1688,14 @@ const struct file_operations def_blk_fops = {
.write = do_sync_write,
.aio_read = blkdev_aio_read,
.aio_write = blkdev_aio_write,
- .mmap = blkdev_mmap,
+ .mmap = generic_file_mmap,
.fsync = blkdev_fsync,
.unlocked_ioctl = block_ioctl,
#ifdef CONFIG_COMPAT
.compat_ioctl = compat_blkdev_ioctl,
#endif
- .splice_read = blkdev_splice_read,
- .splice_write = blkdev_splice_write,
+ .splice_read = generic_file_splice_read,
+ .splice_write = generic_file_splice_write,
};

int ioctl_by_bdev(struct block_device *bdev, unsigned cmd, unsigned long arg)