* [PATCH] nbd: use bdget_disk to get bdev
From: Josef Bacik @ 2017-02-13 17:31 UTC (permalink / raw)
To: axboe, nbd-general, linux-block, kernel-team
In preparation for the upcoming netlink interface we need to not rely on
already having the bdev for the NBD device we are doing operations on.
Instead of passing the bdev from the bdev ioctl around, use bdget_disk()
wherever we need the bdev and pass that down to the helpers as
necessary.
Signed-off-by: Josef Bacik <jbacik@fb.com>
---
drivers/block/nbd.c | 88 ++++++++++++++++++++++++++++++-----------------------
1 file changed, 50 insertions(+), 38 deletions(-)
diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 25891a1..0623f8f 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -168,13 +168,18 @@ static void nbd_size_update(struct nbd_device *nbd, struct block_device *bdev)
kobject_uevent(&nbd_to_dev(nbd)->kobj, KOBJ_CHANGE);
}
-static void nbd_size_set(struct nbd_device *nbd, struct block_device *bdev,
- loff_t blocksize, loff_t nr_blocks)
+static int nbd_size_set(struct nbd_device *nbd, loff_t blocksize,
+ loff_t nr_blocks)
{
+ struct block_device *bdev = bdget_disk(nbd->disk, 0);
+ if (!bdev)
+ return -EINVAL;
nbd->blksize = blocksize;
nbd->bytesize = blocksize * nr_blocks;
if (nbd_is_connected(nbd))
nbd_size_update(nbd, bdev);
+ bdput(bdev);
+ return 0;
}
static void nbd_end_request(struct nbd_cmd *cmd)
@@ -704,8 +709,7 @@ static int nbd_wait_for_socks(struct nbd_device *nbd)
return ret;
}
-static int nbd_add_socket(struct nbd_device *nbd, struct block_device *bdev,
- unsigned long arg)
+static int nbd_add_socket(struct nbd_device *nbd, unsigned long arg)
{
struct socket *sock;
struct nbd_sock **socks;
@@ -749,8 +753,6 @@ static int nbd_add_socket(struct nbd_device *nbd, struct block_device *bdev,
nsock->sock = sock;
socks[nbd->num_connections++] = nsock;
- if (max_part)
- bdev->bd_invalidated = 1;
err = 0;
out:
mutex_unlock(&nbd->socks_lock);
@@ -771,18 +773,19 @@ static void nbd_reset(struct nbd_device *nbd)
static void nbd_bdev_reset(struct block_device *bdev)
{
- set_device_ro(bdev, false);
- bdev->bd_inode->i_size = 0;
+ bd_set_size(bdev, 0);
if (max_part > 0) {
blkdev_reread_part(bdev);
bdev->bd_invalidated = 1;
}
}
-static void nbd_parse_flags(struct nbd_device *nbd, struct block_device *bdev)
+static void nbd_parse_flags(struct nbd_device *nbd)
{
if (nbd->flags & NBD_FLAG_READ_ONLY)
- set_device_ro(bdev, true);
+ set_disk_ro(nbd->disk, true);
+ else
+ set_disk_ro(nbd->disk, false);
if (nbd->flags & NBD_FLAG_SEND_TRIM)
queue_flag_set_unlocked(QUEUE_FLAG_DISCARD, nbd->disk->queue);
if (nbd->flags & NBD_FLAG_SEND_FLUSH)
@@ -807,16 +810,11 @@ static void send_disconnects(struct nbd_device *nbd)
}
}
-static int nbd_disconnect(struct nbd_device *nbd, struct block_device *bdev)
+static int nbd_disconnect(struct nbd_device *nbd)
{
dev_info(disk_to_dev(nbd->disk), "NBD_DISCONNECT\n");
if (!nbd_socks_get_unless_zero(nbd))
return -EINVAL;
-
- mutex_unlock(&nbd->config_lock);
- fsync_bdev(bdev);
- mutex_lock(&nbd->config_lock);
-
if (!test_and_set_bit(NBD_DISCONNECT_REQUESTED,
&nbd->runtime_flags))
send_disconnects(nbd);
@@ -824,28 +822,40 @@ static int nbd_disconnect(struct nbd_device *nbd, struct block_device *bdev)
return 0;
}
-static int nbd_clear_sock(struct nbd_device *nbd, struct block_device *bdev)
+static int nbd_clear_sock(struct nbd_device *nbd)
{
+ struct block_device *bdev = bdget_disk(nbd->disk, 0);
+
sock_shutdown(nbd);
nbd_clear_que(nbd);
- kill_bdev(bdev);
- nbd_bdev_reset(bdev);
+ if (bdev) {
+ kill_bdev(bdev);
+ nbd_bdev_reset(bdev);
+ bdput(bdev);
+ }
nbd->task_setup = NULL;
if (test_and_clear_bit(NBD_HAS_SOCKS_REF, &nbd->runtime_flags))
nbd_socks_put(nbd);
return 0;
}
-static int nbd_start_device(struct nbd_device *nbd, struct block_device *bdev)
+static int nbd_start_device(struct nbd_device *nbd)
{
struct recv_thread_args *args;
+ struct block_device *bdev = bdget_disk(nbd->disk, 0);
int num_connections = nbd->num_connections;
int error = 0, i;
- if (nbd->task_recv)
+ if (!bdev)
+ return -EINVAL;
+ if (nbd->task_recv) {
+ bdput(bdev);
return -EBUSY;
- if (!nbd->socks)
+ }
+ if (!nbd->socks) {
+ bdput(bdev);
return -EINVAL;
+ }
if (num_connections > 1 &&
!(nbd->flags & NBD_FLAG_CAN_MULTI_CONN)) {
dev_err(disk_to_dev(nbd->disk), "server does not support multiple connections per device.\n");
@@ -853,6 +863,9 @@ static int nbd_start_device(struct nbd_device *nbd, struct block_device *bdev)
goto out_err;
}
+ if (max_part)
+ bdev->bd_invalidated = 1;
+
blk_mq_update_nr_hw_queues(&nbd->tag_set, nbd->num_connections);
args = kcalloc(num_connections, sizeof(*args), GFP_KERNEL);
if (!args) {
@@ -862,7 +875,7 @@ static int nbd_start_device(struct nbd_device *nbd, struct block_device *bdev)
nbd->task_recv = current;
mutex_unlock(&nbd->config_lock);
- nbd_parse_flags(nbd, bdev);
+ nbd_parse_flags(nbd);
error = device_create_file(disk_to_dev(nbd->disk), &pid_attr);
if (error) {
@@ -893,7 +906,7 @@ static int nbd_start_device(struct nbd_device *nbd, struct block_device *bdev)
mutex_lock(&nbd->config_lock);
nbd->task_recv = NULL;
out_err:
- nbd_clear_sock(nbd, bdev);
+ nbd_clear_sock(nbd);
/* user requested, ignore socket errors */
if (test_bit(NBD_DISCONNECT_REQUESTED, &nbd->runtime_flags))
@@ -902,31 +915,30 @@ static int nbd_start_device(struct nbd_device *nbd, struct block_device *bdev)
error = -ETIMEDOUT;
nbd_reset(nbd);
+ bdput(bdev);
return error;
}
/* Must be called with config_lock held */
-static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
- unsigned int cmd, unsigned long arg)
+static int __nbd_ioctl(struct nbd_device *nbd, unsigned int cmd,
+ unsigned long arg)
{
switch (cmd) {
case NBD_DISCONNECT:
- return nbd_disconnect(nbd, bdev);
+ return nbd_disconnect(nbd);
case NBD_CLEAR_SOCK:
- return nbd_clear_sock(nbd, bdev);
+ return nbd_clear_sock(nbd);
case NBD_SET_SOCK:
- return nbd_add_socket(nbd, bdev, arg);
+ return nbd_add_socket(nbd, arg);
case NBD_SET_BLKSIZE:
- nbd_size_set(nbd, bdev, arg,
- div_s64(nbd->bytesize, arg));
+ return nbd_size_set(nbd, arg,
+ div_s64(nbd->bytesize, arg));
return 0;
case NBD_SET_SIZE:
- nbd_size_set(nbd, bdev, nbd->blksize,
- div_s64(arg, nbd->blksize));
- return 0;
+ return nbd_size_set(nbd, nbd->blksize,
+ div_s64(arg, nbd->blksize));
case NBD_SET_SIZE_BLOCKS:
- nbd_size_set(nbd, bdev, nbd->blksize, arg);
- return 0;
+ return nbd_size_set(nbd, nbd->blksize, arg);
case NBD_SET_TIMEOUT:
nbd->tag_set.timeout = arg * HZ;
return 0;
@@ -935,7 +947,7 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
nbd->flags = arg;
return 0;
case NBD_DO_IT:
- return nbd_start_device(nbd, bdev);
+ return nbd_start_device(nbd);
case NBD_CLEAR_QUE:
/*
* This is for compatibility only. The queue is always cleared
@@ -964,7 +976,7 @@ static int nbd_ioctl(struct block_device *bdev, fmode_t mode,
BUG_ON(nbd->magic != NBD_MAGIC);
mutex_lock(&nbd->config_lock);
- error = __nbd_ioctl(bdev, nbd, cmd, arg);
+ error = __nbd_ioctl(nbd, cmd, arg);
mutex_unlock(&nbd->config_lock);
return error;
--
2.7.4
^ permalink raw reply related
* RE: [PATCH V5 3/4] Move stack parameters for sed_ioctl to prevent oversized stack with CONFIG_KASAN
From: David Laight @ 2017-02-13 17:07 UTC (permalink / raw)
To: 'Arnd Bergmann', Scott Bauer
Cc: keith.busch@intel.com, hch@infradead.org,
linux-kernel@vger.kernel.org, linux-nvme@lists.infradead.org,
axboe@fb.com, linux-block@vger.kernel.org,
jonathan.derrick@intel.com
In-Reply-To: <CAK8P3a3-4UOkGjLWQpt9Rh=+QJNnJxJSHyha3R_-rGrqQ_LmFw@mail.gmail.com>
From: Arnd Bergmann
> Sent: 13 February 2017 17:02
...
> >> > + ioctl_ptr = memdup_user(arg, _IOC_SIZE(cmd));
> >> > + if (IS_ERR_OR_NULL(ioctl_ptr)) {
> >> > + ret = PTR_ERR(ioctl_ptr);
> >> > + goto out;
> >> ...
> >> > + out:
> >> > + kfree(ioctl_ptr);
> >> > + return ret;
...
> >> That error path doesn't look quite right to me.
...
> > good god, this is a mess this morning. Thanks for the catch, I'll review these
> > more aggressively before sending out.
>
> memdup_user() never returns NULL, and generally speaking any use of
> IS_ERR_OR_NULL() indicates that there is something wrong with the
> interface, so aside from passing the right pointer (or NULL) into kfree()
> I think using IS_ERR() is the correct solution.
You missed the problem entirely.
Code needs to be:
if (IS_ERR(ioctl_ptr))
return PTR_ERR(ioctl_ptr);
David
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply
* Re: [PATCH V5 3/4] Move stack parameters for sed_ioctl to prevent oversized stack with CONFIG_KASAN
From: Arnd Bergmann @ 2017-02-13 17:01 UTC (permalink / raw)
To: Scott Bauer
Cc: David Laight, linux-nvme@lists.infradead.org, axboe@fb.com,
keith.busch@intel.com, jonathan.derrick@intel.com,
hch@infradead.org, linux-kernel@vger.kernel.org,
linux-block@vger.kernel.org
In-Reply-To: <20170213162941.GB18913@sbauer-Z170X-UD5>
On Mon, Feb 13, 2017 at 5:29 PM, Scott Bauer <scott.bauer@intel.com> wrote:
> On Mon, Feb 13, 2017 at 04:30:36PM +0000, David Laight wrote:
>> From: Scott Bauer Sent: 13 February 2017 16:11
>> > When CONFIG_KASAN is enabled, compilation fails:
>> >
>> > block/sed-opal.c: In function 'sed_ioctl':
>> > block/sed-opal.c:2447:1: error: the frame size of 2256 bytes is larger than 2048 bytes [-Werror=frame-
>> > larger-than=]
>> >
>> > Moved all the ioctl structures off the stack and dynamically activate
>> > using _IOC_SIZE()
>>
>> Think I'd not that this simplifies the code considerably.
>> AFAICT CONFIG_KASAN is a just brainf*ck.
>> It at least needs annotation that copy_from_user() has properties
>> similar to memset().
>> So if the size matches that of the type then no guard space (etc)
>> is required.
I think it still would, as the pointer to the local variable gets passed through
dev->func_data[].
>> ...
>> > + ioctl_ptr = memdup_user(arg, _IOC_SIZE(cmd));
>> > + if (IS_ERR_OR_NULL(ioctl_ptr)) {
>> > + ret = PTR_ERR(ioctl_ptr);
>> > + goto out;
>> ...
>> > + out:
>> > + kfree(ioctl_ptr);
>> > + return ret;
>> > }
>
>
>>
>> That error path doesn't look quite right to me.
>>
>> David
>>
>
> good god, this is a mess this morning. Thanks for the catch, I'll review these
> more aggressively before sending out.
memdup_user() never returns NULL, and generally speaking any use of
IS_ERR_OR_NULL() indicates that there is something wrong with the
interface, so aside from passing the right pointer (or NULL) into kfree()
I think using IS_ERR() is the correct solution.
Arnd
^ permalink raw reply
* Re: [PATCH V5 3/4] Move stack parameters for sed_ioctl to prevent oversized stack with CONFIG_KASAN
From: Scott Bauer @ 2017-02-13 16:29 UTC (permalink / raw)
To: David Laight
Cc: linux-nvme@lists.infradead.org, arnd@arndb.de, axboe@fb.com,
keith.busch@intel.com, jonathan.derrick@intel.com,
hch@infradead.org, linux-kernel@vger.kernel.org,
linux-block@vger.kernel.org
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DB028519B@AcuExch.aculab.com>
On Mon, Feb 13, 2017 at 04:30:36PM +0000, David Laight wrote:
> From: Scott Bauer Sent: 13 February 2017 16:11
> > When CONFIG_KASAN is enabled, compilation fails:
> >
> > block/sed-opal.c: In function 'sed_ioctl':
> > block/sed-opal.c:2447:1: error: the frame size of 2256 bytes is larger than 2048 bytes [-Werror=frame-
> > larger-than=]
> >
> > Moved all the ioctl structures off the stack and dynamically activate
> > using _IOC_SIZE()
>
> Think I'd not that this simplifies the code considerably.
> AFAICT CONFIG_KASAN is a just brainf*ck.
> It at least needs annotation that copy_from_user() has properties
> similar to memset().
> So if the size matches that of the type then no guard space (etc)
> is required.
>
I don't really follow what you're saying. Do you want me to just include that
the patch cleans up the ioctl path a bit. I need to include the KASAN part since
there was build breakage and the series does fix it even if it simplifies the path
as well. As for the memset part, we never copy back to userland so there's no chance
of data leakage which is what it seems you're hinting at.
> ...
> > + ioctl_ptr = memdup_user(arg, _IOC_SIZE(cmd));
> > + if (IS_ERR_OR_NULL(ioctl_ptr)) {
> > + ret = PTR_ERR(ioctl_ptr);
> > + goto out;
> ...
> > + out:
> > + kfree(ioctl_ptr);
> > + return ret;
> > }
>
> That error path doesn't look quite right to me.
>
> David
>
good god, this is a mess this morning. Thanks for the catch, I'll review these
more aggressively before sending out.
^ permalink raw reply
* Re: [PATCH] block/loop: fix race between I/O and set_status
From: Jens Axboe @ 2017-02-13 16:37 UTC (permalink / raw)
To: Ming Lei, linux-kernel
Cc: linux-block, Christoph Hellwig, Dmitry Vyukov, stable
In-Reply-To: <1486784445-25300-1-git-send-email-tom.leiming@gmail.com>
On 02/10/2017 08:40 PM, Ming Lei wrote:
> Inside set_status, transfer need to setup again, so
> we have to drain IO before the transition, otherwise
> oops may be triggered like the following:
>
> divide error: 0000 [#1] SMP KASAN
> CPU: 0 PID: 2935 Comm: loop7 Not tainted 4.10.0-rc7+ #213
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs
> 01/01/2011
> task: ffff88006ba1e840 task.stack: ffff880067338000
> RIP: 0010:transfer_xor+0x1d1/0x440 drivers/block/loop.c:110
> RSP: 0018:ffff88006733f108 EFLAGS: 00010246
> RAX: 0000000000000000 RBX: ffff8800688d7000 RCX: 0000000000000059
> RDX: 0000000000000000 RSI: 1ffff1000d743f43 RDI: ffff880068891c08
> RBP: ffff88006733f160 R08: ffff8800688d7001 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000000 R12: ffff8800688d7000
> R13: ffff880067b7d000 R14: dffffc0000000000 R15: 0000000000000000
> FS: 0000000000000000(0000) GS:ffff88006d000000(0000)
> knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00000000006c17e0 CR3: 0000000066e3b000 CR4: 00000000001406f0
> Call Trace:
> lo_do_transfer drivers/block/loop.c:251 [inline]
> lo_read_transfer drivers/block/loop.c:392 [inline]
> do_req_filebacked drivers/block/loop.c:541 [inline]
> loop_handle_cmd drivers/block/loop.c:1677 [inline]
> loop_queue_work+0xda0/0x49b0 drivers/block/loop.c:1689
> kthread_worker_fn+0x4c3/0xa30 kernel/kthread.c:630
> kthread+0x326/0x3f0 kernel/kthread.c:227
> ret_from_fork+0x31/0x40 arch/x86/entry/entry_64.S:430
> Code: 03 83 e2 07 41 29 df 42 0f b6 04 30 4d 8d 44 24 01 38 d0 7f 08
> 84 c0 0f 85 62 02 00 00 44 89 f8 41 0f b6 48 ff 25 ff 01 00 00 99 <f7>
> 7d c8 48 63 d2 48 03 55 d0 48 89 d0 48 89 d7 48 c1 e8 03 83
> RIP: transfer_xor+0x1d1/0x440 drivers/block/loop.c:110 RSP:
> ffff88006733f108
> ---[ end trace 0166f7bd3b0c0933 ]---
Thanks Ming, added for 4.11.
--
Jens Axboe
^ permalink raw reply
* RE: [PATCH V5 3/4] Move stack parameters for sed_ioctl to prevent oversized stack with CONFIG_KASAN
From: David Laight @ 2017-02-13 16:30 UTC (permalink / raw)
To: 'Scott Bauer', linux-nvme@lists.infradead.org
Cc: arnd@arndb.de, axboe@fb.com, keith.busch@intel.com,
jonathan.derrick@intel.com, hch@infradead.org,
linux-kernel@vger.kernel.org, linux-block@vger.kernel.org
In-Reply-To: <1487002272-17940-4-git-send-email-scott.bauer@intel.com>
From: Scott Bauer Sent: 13 February 2017 16:11
> When CONFIG_KASAN is enabled, compilation fails:
>
> block/sed-opal.c: In function 'sed_ioctl':
> block/sed-opal.c:2447:1: error: the frame size of 2256 bytes is larger than 2048 bytes [-Werror=frame-
> larger-than=]
>
> Moved all the ioctl structures off the stack and dynamically activate
> using _IOC_SIZE()
Think I'd not that this simplifies the code considerably.
AFAICT CONFIG_KASAN is a just brainf*ck.
It at least needs annotation that copy_from_user() has properties
similar to memset().
So if the size matches that of the type then no guard space (etc)
is required.
...
> + ioctl_ptr = memdup_user(arg, _IOC_SIZE(cmd));
> + if (IS_ERR_OR_NULL(ioctl_ptr)) {
> + ret = PTR_ERR(ioctl_ptr);
> + goto out;
...
> + out:
> + kfree(ioctl_ptr);
> + return ret;
> }
That error path doesn't look quite right to me.
David
^ permalink raw reply
* Re: [PATCHv6 08/37] filemap: handle huge pages in do_generic_file_read()
From: Matthew Wilcox @ 2017-02-13 16:28 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: Theodore Ts'o, Andreas Dilger, Jan Kara, Andrew Morton,
Alexander Viro, Hugh Dickins, Andrea Arcangeli, Dave Hansen,
Vlastimil Babka, Ross Zwisler, linux-ext4, linux-fsdevel,
linux-kernel, linux-mm, linux-block
In-Reply-To: <20170126115819.58875-9-kirill.shutemov@linux.intel.com>
On Thu, Jan 26, 2017 at 02:57:50PM +0300, Kirill A. Shutemov wrote:
> Most of work happans on head page. Only when we need to do copy data to
> userspace we find relevant subpage.
>
> We are still limited by PAGE_SIZE per iteration. Lifting this limitation
> would require some more work.
Now that I debugged that bit of my brain, here's a more sensible suggestion.
> @@ -1886,6 +1886,7 @@ static ssize_t do_generic_file_read(struct file *filp, loff_t *ppos,
> if (unlikely(page == NULL))
> goto no_cached_page;
> }
> + page = compound_head(page);
> if (PageReadahead(page)) {
> page_cache_async_readahead(mapping,
> ra, filp, page,
We're going backwards and forwards a lot between subpages and page heads.
I'd like to see us do this:
static inline struct page *pagecache_get_page(struct address_space *mapping,
pgoff_t offset, int fgp_flags, gfp_t cache_gfp_mask)
{
struct page *page = pagecache_get_head(mapping, offset, fgp_flags,
cache_gfp_mask);
return page ? find_subpage(page, offset) : NULL;
}
static inline struct page *find_get_head(struct address_space *mapping,
pgoff_t offset)
{
return pagecache_get_head(mapping, offset, 0, 0);
}
and then we can switch do_generic_file_read() to call find_get_head(),
eliminating the conversion back and forth between subpages and head pages.
^ permalink raw reply
* Re: [PATCH V5 1/4] block: sed-opal: change ioctl to take user pointer instead of unsinged long
From: Scott Bauer @ 2017-02-13 16:15 UTC (permalink / raw)
To: linux-nvme
Cc: David.Laight, arnd, axboe, keith.busch, jonathan.derrick, hch,
linux-kernel, linux-block
In-Reply-To: <1487002272-17940-2-git-send-email-scott.bauer@intel.com>
esOn Mon, Feb 13, 2017 at 09:11:09AM -0700, Scott Bauer wrote:
> Signed-off-by: Scott Bauer <scott.bauer@intel.com>
> ---
> block/sed-opal.c | 6 ++++--
> drivers/nvme/host/core.c | 3 ++-
> include/linux/sed-opal.h | 4 ++--
> 3 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/block/sed-opal.c b/block/sed-opal.c
> index bf1406e..2448d4a 100644
> --- a/block/sed-opal.c
> +++ b/block/sed-opal.c
> @@ -2344,9 +2344,11 @@ bool opal_unlock_from_suspend(struct opal_dev *dev)
> }
> EXPORT_SYMBOL(opal_unlock_from_suspend);
>
> -int sed_ioctl(struct opal_dev *dev, unsigned int cmd, unsigned long ptr)
> +int sed_ioctl(struct opal_dev *dev, unsigned int cmd, void __user *arg)
> {
> - void __user *arg = (void __user *)ptr;
> + void *ioctl_ptr;
> + int ret = -ENOTTY;
> + unsigned int cmd_size = _IOC_SIZE(cmd);
ugh, I apparently messed up my rebase these should be in patch 2 or maybe I should
sqash p1 and p2 together.
^ permalink raw reply
* [PATCH V5 3/4] Move stack parameters for sed_ioctl to prevent oversized stack with CONFIG_KASAN
From: Scott Bauer @ 2017-02-13 16:11 UTC (permalink / raw)
To: linux-nvme
Cc: David.Laight, arnd, axboe, keith.busch, jonathan.derrick, hch,
linux-kernel, linux-block, Scott Bauer
In-Reply-To: <1487002272-17940-1-git-send-email-scott.bauer@intel.com>
When CONFIG_KASAN is enabled, compilation fails:
block/sed-opal.c: In function 'sed_ioctl':
block/sed-opal.c:2447:1: error: the frame size of 2256 bytes is larger than 2048 bytes [-Werror=frame-larger-than=]
Moved all the ioctl structures off the stack and dynamically activate
using _IOC_SIZE()
Fixes: 455a7b238cd6 ("block: Add Sed-opal library")
Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Scott Bauer <scott.bauer@intel.com>
---
block/sed-opal.c | 130 +++++++++++++++++++------------------------------------
1 file changed, 45 insertions(+), 85 deletions(-)
diff --git a/block/sed-opal.c b/block/sed-opal.c
index 2448d4a..5733248 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -2348,7 +2348,6 @@ int sed_ioctl(struct opal_dev *dev, unsigned int cmd, void __user *arg)
{
void *ioctl_ptr;
int ret = -ENOTTY;
- unsigned int cmd_size = _IOC_SIZE(cmd);
if (!capable(CAP_SYS_ADMIN))
return -EACCES;
@@ -2357,94 +2356,55 @@ int sed_ioctl(struct opal_dev *dev, unsigned int cmd, void __user *arg)
return -ENOTSUPP;
}
- switch (cmd) {
- case IOC_OPAL_SAVE: {
- struct opal_lock_unlock lk_unlk;
-
- if (copy_from_user(&lk_unlk, arg, sizeof(lk_unlk)))
- return -EFAULT;
- return opal_save(dev, &lk_unlk);
- }
- case IOC_OPAL_LOCK_UNLOCK: {
- struct opal_lock_unlock lk_unlk;
-
- if (copy_from_user(&lk_unlk, arg, sizeof(lk_unlk)))
- return -EFAULT;
- return opal_lock_unlock(dev, &lk_unlk);
- }
- case IOC_OPAL_TAKE_OWNERSHIP: {
- struct opal_key opal_key;
-
- if (copy_from_user(&opal_key, arg, sizeof(opal_key)))
- return -EFAULT;
- return opal_take_ownership(dev, &opal_key);
- }
- case IOC_OPAL_ACTIVATE_LSP: {
- struct opal_lr_act opal_lr_act;
-
- if (copy_from_user(&opal_lr_act, arg, sizeof(opal_lr_act)))
- return -EFAULT;
- return opal_activate_lsp(dev, &opal_lr_act);
- }
- case IOC_OPAL_SET_PW: {
- struct opal_new_pw opal_pw;
-
- if (copy_from_user(&opal_pw, arg, sizeof(opal_pw)))
- return -EFAULT;
- return opal_set_new_pw(dev, &opal_pw);
- }
- case IOC_OPAL_ACTIVATE_USR: {
- struct opal_session_info session;
-
- if (copy_from_user(&session, arg, sizeof(session)))
- return -EFAULT;
- return opal_activate_user(dev, &session);
- }
- case IOC_OPAL_REVERT_TPR: {
- struct opal_key opal_key;
-
- if (copy_from_user(&opal_key, arg, sizeof(opal_key)))
- return -EFAULT;
- return opal_reverttper(dev, &opal_key);
- }
- case IOC_OPAL_LR_SETUP: {
- struct opal_user_lr_setup lrs;
-
- if (copy_from_user(&lrs, arg, sizeof(lrs)))
- return -EFAULT;
- return opal_setup_locking_range(dev, &lrs);
- }
- case IOC_OPAL_ADD_USR_TO_LR: {
- struct opal_lock_unlock lk_unlk;
-
- if (copy_from_user(&lk_unlk, arg, sizeof(lk_unlk)))
- return -EFAULT;
- return opal_add_user_to_lr(dev, &lk_unlk);
- }
- case IOC_OPAL_ENABLE_DISABLE_MBR: {
- struct opal_mbr_data mbr;
-
- if (copy_from_user(&mbr, arg, sizeof(mbr)))
- return -EFAULT;
- return opal_enable_disable_shadow_mbr(dev, &mbr);
- }
- case IOC_OPAL_ERASE_LR: {
- struct opal_session_info session;
-
- if (copy_from_user(&session, arg, sizeof(session)))
- return -EFAULT;
- return opal_erase_locking_range(dev, &session);
+ ioctl_ptr = memdup_user(arg, _IOC_SIZE(cmd));
+ if (IS_ERR_OR_NULL(ioctl_ptr)) {
+ ret = PTR_ERR(ioctl_ptr);
+ goto out;
}
- case IOC_OPAL_SECURE_ERASE_LR: {
- struct opal_session_info session;
- if (copy_from_user(&session, arg, sizeof(session)))
- return -EFAULT;
- return opal_secure_erase_locking_range(dev, &session);
- }
+ switch (cmd) {
+ case IOC_OPAL_SAVE:
+ ret = opal_save(dev, ioctl_ptr);
+ break;
+ case IOC_OPAL_LOCK_UNLOCK:
+ ret = opal_lock_unlock(dev, ioctl_ptr);
+ break;
+ case IOC_OPAL_TAKE_OWNERSHIP:
+ ret = opal_take_ownership(dev, ioctl_ptr);
+ break;
+ case IOC_OPAL_ACTIVATE_LSP:
+ ret = opal_activate_lsp(dev, ioctl_ptr);
+ break;
+ case IOC_OPAL_SET_PW:
+ ret = opal_set_new_pw(dev, ioctl_ptr);
+ break;
+ case IOC_OPAL_ACTIVATE_USR:
+ ret = opal_activate_user(dev, ioctl_ptr);
+ break;
+ case IOC_OPAL_REVERT_TPR:
+ ret = opal_reverttper(dev, ioctl_ptr);
+ break;
+ case IOC_OPAL_LR_SETUP:
+ ret = opal_setup_locking_range(dev, ioctl_ptr);
+ break;
+ case IOC_OPAL_ADD_USR_TO_LR:
+ ret = opal_add_user_to_lr(dev, ioctl_ptr);
+ break;
+ case IOC_OPAL_ENABLE_DISABLE_MBR:
+ ret = opal_enable_disable_shadow_mbr(dev, ioctl_ptr);
+ break;
+ case IOC_OPAL_ERASE_LR:
+ ret = opal_erase_locking_range(dev, ioctl_ptr);
+ break;
+ case IOC_OPAL_SECURE_ERASE_LR:
+ ret = opal_secure_erase_locking_range(dev, ioctl_ptr);
+ break;
default:
pr_warn("No such Opal Ioctl %u\n", cmd);
}
- return -ENOTTY;
+
+ out:
+ kfree(ioctl_ptr);
+ return ret;
}
EXPORT_SYMBOL_GPL(sed_ioctl);
--
2.7.4
^ permalink raw reply related
* [PATCH V5 4/4] Maintainers: Modify SED list from nvme to block
From: Scott Bauer @ 2017-02-13 16:11 UTC (permalink / raw)
To: linux-nvme
Cc: David.Laight, arnd, axboe, keith.busch, jonathan.derrick, hch,
linux-kernel, linux-block, Scott Bauer
In-Reply-To: <1487002272-17940-1-git-send-email-scott.bauer@intel.com>
Signed-off-by: Scott Bauer <scott.bauer@intel.com>
---
MAINTAINERS | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index e325373..b983b25 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11094,7 +11094,7 @@ SECURE ENCRYPTING DEVICE (SED) OPAL DRIVER
M: Scott Bauer <scott.bauer@intel.com>
M: Jonathan Derrick <jonathan.derrick@intel.com>
M: Rafael Antognolli <rafael.antognolli@intel.com>
-L: linux-nvme@lists.infradead.org
+L: linux-block@vger.kernel.org
S: Supported
F: block/sed*
F: block/opal_proto.h
--
2.7.4
^ permalink raw reply related
* [PATCH V5 2/4] uapi: sed-opal fix IOW for activate lsp to use correct struct
From: Scott Bauer @ 2017-02-13 16:11 UTC (permalink / raw)
To: linux-nvme
Cc: David.Laight, arnd, axboe, keith.busch, jonathan.derrick, hch,
linux-kernel, linux-block, Scott Bauer
In-Reply-To: <1487002272-17940-1-git-send-email-scott.bauer@intel.com>
the IOW for the IOC_OPAL_ACTIVATE_LSP took the wrong strcure which
would give us the wrong size when using _IOC_SIZE, switch it to the
right structure.
Fixes: 058f8a2 ("Include: Uapi: Add user ABI for Sed/Opal")
Signed-off-by: Scott Bauer <scott.bauer@intel.com>
---
include/uapi/linux/sed-opal.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/uapi/linux/sed-opal.h b/include/uapi/linux/sed-opal.h
index fc06e3a..c72e073 100644
--- a/include/uapi/linux/sed-opal.h
+++ b/include/uapi/linux/sed-opal.h
@@ -106,7 +106,7 @@ struct opal_mbr_data {
#define IOC_OPAL_SAVE _IOW('p', 220, struct opal_lock_unlock)
#define IOC_OPAL_LOCK_UNLOCK _IOW('p', 221, struct opal_lock_unlock)
#define IOC_OPAL_TAKE_OWNERSHIP _IOW('p', 222, struct opal_key)
-#define IOC_OPAL_ACTIVATE_LSP _IOW('p', 223, struct opal_key)
+#define IOC_OPAL_ACTIVATE_LSP _IOW('p', 223, struct opal_lr_act)
#define IOC_OPAL_SET_PW _IOW('p', 224, struct opal_new_pw)
#define IOC_OPAL_ACTIVATE_USR _IOW('p', 225, struct opal_session_info)
#define IOC_OPAL_REVERT_TPR _IOW('p', 226, struct opal_key)
--
2.7.4
^ permalink raw reply related
* SED Opal Fixups
From: Scott Bauer @ 2017-02-13 16:11 UTC (permalink / raw)
To: linux-nvme
Cc: David.Laight, arnd, axboe, keith.busch, jonathan.derrick, hch,
linux-kernel, linux-block
So we have a few patches here, they're pretty small. First patch changes
the sed-opal ioctl function parameters to take a void __user* instead of
an unsigned long, this required a small cast in the nvme driver.
Patch 2 is a UAPI fixup for the IOW to make an ioctl
the right size. Patch 3 fixes a compiliation error when building using
KSAN due to the stack frame being too large. And lastly we move the
Maintainers list from nvme to linux-block.
^ permalink raw reply
* [PATCH V5 1/4] block: sed-opal: change ioctl to take user pointer instead of unsinged long
From: Scott Bauer @ 2017-02-13 16:11 UTC (permalink / raw)
To: linux-nvme
Cc: keith.busch, arnd, hch, linux-kernel, axboe, David.Laight,
linux-block, Scott Bauer, jonathan.derrick
In-Reply-To: <1487002272-17940-1-git-send-email-scott.bauer@intel.com>
Signed-off-by: Scott Bauer <scott.bauer@intel.com>
---
block/sed-opal.c | 6 ++++--
drivers/nvme/host/core.c | 3 ++-
include/linux/sed-opal.h | 4 ++--
3 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/block/sed-opal.c b/block/sed-opal.c
index bf1406e..2448d4a 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -2344,9 +2344,11 @@ bool opal_unlock_from_suspend(struct opal_dev *dev)
}
EXPORT_SYMBOL(opal_unlock_from_suspend);
-int sed_ioctl(struct opal_dev *dev, unsigned int cmd, unsigned long ptr)
+int sed_ioctl(struct opal_dev *dev, unsigned int cmd, void __user *arg)
{
- void __user *arg = (void __user *)ptr;
+ void *ioctl_ptr;
+ int ret = -ENOTTY;
+ unsigned int cmd_size = _IOC_SIZE(cmd);
if (!capable(CAP_SYS_ADMIN))
return -EACCES;
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index d9f4903..04c48e7 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -811,7 +811,8 @@ static int nvme_ioctl(struct block_device *bdev, fmode_t mode,
return nvme_nvm_ioctl(ns, cmd, arg);
#endif
if (is_sed_ioctl(cmd))
- return sed_ioctl(&ns->ctrl->opal_dev, cmd, arg);
+ return sed_ioctl(&ns->ctrl->opal_dev, cmd,
+ (void __user *) arg);
return -ENOTTY;
}
}
diff --git a/include/linux/sed-opal.h b/include/linux/sed-opal.h
index af1a85e..205d520 100644
--- a/include/linux/sed-opal.h
+++ b/include/linux/sed-opal.h
@@ -132,7 +132,7 @@ struct opal_dev {
#ifdef CONFIG_BLK_SED_OPAL
bool opal_unlock_from_suspend(struct opal_dev *dev);
void init_opal_dev(struct opal_dev *opal_dev, sec_send_recv *send_recv);
-int sed_ioctl(struct opal_dev *dev, unsigned int cmd, unsigned long ptr);
+int sed_ioctl(struct opal_dev *dev, unsigned int cmd, void __user *ioctl_ptr);
static inline bool is_sed_ioctl(unsigned int cmd)
{
@@ -160,7 +160,7 @@ static inline bool is_sed_ioctl(unsigned int cmd)
}
static inline int sed_ioctl(struct opal_dev *dev, unsigned int cmd,
- unsigned long ptr)
+ void __user *ioctl_ptr)
{
return 0;
}
--
2.7.4
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related
* Re: [PATCHv6 08/37] filemap: handle huge pages in do_generic_file_read()
From: Matthew Wilcox @ 2017-02-13 16:09 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: Kirill A. Shutemov, Theodore Ts'o, Andreas Dilger, Jan Kara,
Andrew Morton, Alexander Viro, Hugh Dickins, Andrea Arcangeli,
Dave Hansen, Vlastimil Babka, Ross Zwisler, linux-ext4,
linux-fsdevel, linux-kernel, linux-mm, linux-block
In-Reply-To: <20170213160117.GA6416@bombadil.infradead.org>
On Mon, Feb 13, 2017 at 08:01:17AM -0800, Matthew Wilcox wrote:
> On Mon, Feb 13, 2017 at 06:33:42PM +0300, Kirill A. Shutemov wrote:
> > No. pagecache_get_page() returns subpage. See description of the first
> > patch.
Oh, I re-read patch 1 and it made sense now. I missed the bit where
pagecache_get_page() called page_subpage().
^ permalink raw reply
* Re: [PATCH] blk-mq: Call bio_io_error if request returned is NULL
From: Jens Axboe @ 2017-02-13 16:07 UTC (permalink / raw)
To: Goldwyn Rodrigues, linux-block; +Cc: Goldwyn Rodrigues
In-Reply-To: <20170213160454.30869-1-rgoldwyn@suse.de>
On 02/13/2017 09:04 AM, Goldwyn Rodrigues wrote:
> From: Goldwyn Rodrigues <rgoldwyn@suse.com>
>
> If blk_mq_map_request returns NULL, bio_endio() function is not
> called. Call bio_io_error() in case request returned is NULL.
That's currently not a possible condition, since the main
request mapper functions block on rq allocation. So we can
never return NULL there.
--
Jens Axboe
^ permalink raw reply
* [PATCH] blk-mq: Call bio_io_error if request returned is NULL
From: Goldwyn Rodrigues @ 2017-02-13 16:04 UTC (permalink / raw)
To: linux-block; +Cc: Goldwyn Rodrigues
From: Goldwyn Rodrigues <rgoldwyn@suse.com>
If blk_mq_map_request returns NULL, bio_endio() function is not
called. Call bio_io_error() in case request returned is NULL.
Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
---
block/blk-mq.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 81caceb..7cd8912 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1286,8 +1286,10 @@ static blk_qc_t blk_mq_make_request(struct request_queue *q, struct bio *bio)
return BLK_QC_T_NONE;
rq = blk_mq_map_request(q, bio, &data);
- if (unlikely(!rq))
+ if (unlikely(!rq)) {
+ bio_io_error(bio);
return BLK_QC_T_NONE;
+ }
cookie = blk_tag_to_qc_t(rq->tag, data.hctx->queue_num);
@@ -1381,8 +1383,10 @@ static blk_qc_t blk_sq_make_request(struct request_queue *q, struct bio *bio)
request_count = blk_plug_queued_count(q);
rq = blk_mq_map_request(q, bio, &data);
- if (unlikely(!rq))
+ if (unlikely(!rq)) {
+ bio_io_error(bio);
return BLK_QC_T_NONE;
+ }
cookie = blk_tag_to_qc_t(rq->tag, data.hctx->queue_num);
--
2.10.2
^ permalink raw reply related
* Re: [PATCHv6 08/37] filemap: handle huge pages in do_generic_file_read()
From: Matthew Wilcox @ 2017-02-13 16:01 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: Kirill A. Shutemov, Theodore Ts'o, Andreas Dilger, Jan Kara,
Andrew Morton, Alexander Viro, Hugh Dickins, Andrea Arcangeli,
Dave Hansen, Vlastimil Babka, Ross Zwisler, linux-ext4,
linux-fsdevel, linux-kernel, linux-mm, linux-block
In-Reply-To: <20170213153342.GE20394@node.shutemov.name>
On Mon, Feb 13, 2017 at 06:33:42PM +0300, Kirill A. Shutemov wrote:
> No. pagecache_get_page() returns subpage. See description of the first
> patch.
Your description says:
> We also change interface for page-cache lookup function:
>
> - functions that lookup for pages[1] would return subpages of THP
> relevant for requested indexes;
>
> - functions that lookup for entries[2] would return one entry per-THP
> and index will point to index of head page (basically, round down to
> HPAGE_PMD_NR);
>
> This would provide balanced exposure of multi-order entires to the rest
> of the kernel.
>
> [1] find_get_pages(), pagecache_get_page(), pagevec_lookup(), etc.
> [2] find_get_entry(), find_get_entries(), pagevec_lookup_entries(), etc.
I'm saying:
> > We got this page from find_get_page(), which gets it from
> > pagecache_get_page(), which gets it from find_get_entry() ... which
> > (unless I'm lost in your patch series) returns the head page.
Am I guilty of debugging documentation rather than code?
^ permalink raw reply
* [PATCH V2] nbd: set the logical and physical blocksize properly
From: Josef Bacik @ 2017-02-13 15:39 UTC (permalink / raw)
To: axboe, linux-block, kernel-team, nbd-general
In-Reply-To: <1486750004-2988-1-git-send-email-jbacik@fb.com>
We noticed when trying to do O_DIRECT to an export on the server side
that we were getting requests smaller than the 4k sectorsize of the
device. This is because the client isn't setting the logical and
physical blocksizes properly for the underlying device. Fix this up by
setting the queue blocksizes and then calling bd_set_size.
Signed-off-by: Josef Bacik <jbacik@fb.com>
---
V1->V2:
-remove an debug printk that I forgot to delete.
drivers/block/nbd.c | 38 +++++++++++++++-----------------------
1 file changed, 15 insertions(+), 23 deletions(-)
diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index afb1353..25891a1 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -152,7 +152,7 @@ static void nbd_socks_put(struct nbd_device *nbd)
static int nbd_size_clear(struct nbd_device *nbd, struct block_device *bdev)
{
- bdev->bd_inode->i_size = 0;
+ bd_set_size(bdev, 0);
set_capacity(nbd->disk, 0);
kobject_uevent(&nbd_to_dev(nbd)->kobj, KOBJ_CHANGE);
@@ -161,29 +161,20 @@ static int nbd_size_clear(struct nbd_device *nbd, struct block_device *bdev)
static void nbd_size_update(struct nbd_device *nbd, struct block_device *bdev)
{
- if (!nbd_is_connected(nbd))
- return;
-
- bdev->bd_inode->i_size = nbd->bytesize;
+ blk_queue_logical_block_size(nbd->disk->queue, nbd->blksize);
+ blk_queue_physical_block_size(nbd->disk->queue, nbd->blksize);
+ bd_set_size(bdev, nbd->bytesize);
set_capacity(nbd->disk, nbd->bytesize >> 9);
kobject_uevent(&nbd_to_dev(nbd)->kobj, KOBJ_CHANGE);
}
-static int nbd_size_set(struct nbd_device *nbd, struct block_device *bdev,
+static void nbd_size_set(struct nbd_device *nbd, struct block_device *bdev,
loff_t blocksize, loff_t nr_blocks)
{
- int ret;
-
- ret = set_blocksize(bdev, blocksize);
- if (ret)
- return ret;
-
nbd->blksize = blocksize;
nbd->bytesize = blocksize * nr_blocks;
-
- nbd_size_update(nbd, bdev);
-
- return 0;
+ if (nbd_is_connected(nbd))
+ nbd_size_update(nbd, bdev);
}
static void nbd_end_request(struct nbd_cmd *cmd)
@@ -926,15 +917,16 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
case NBD_SET_SOCK:
return nbd_add_socket(nbd, bdev, arg);
case NBD_SET_BLKSIZE:
- return nbd_size_set(nbd, bdev, arg,
- div_s64(nbd->bytesize, arg));
+ nbd_size_set(nbd, bdev, arg,
+ div_s64(nbd->bytesize, arg));
+ return 0;
case NBD_SET_SIZE:
- return nbd_size_set(nbd, bdev, nbd->blksize,
- div_s64(arg, nbd->blksize));
-
+ nbd_size_set(nbd, bdev, nbd->blksize,
+ div_s64(arg, nbd->blksize));
+ return 0;
case NBD_SET_SIZE_BLOCKS:
- return nbd_size_set(nbd, bdev, nbd->blksize, arg);
-
+ nbd_size_set(nbd, bdev, nbd->blksize, arg);
+ return 0;
case NBD_SET_TIMEOUT:
nbd->tag_set.timeout = arg * HZ;
return 0;
--
2.7.4
^ permalink raw reply related
* Re: [PATCHv6 08/37] filemap: handle huge pages in do_generic_file_read()
From: Kirill A. Shutemov @ 2017-02-13 15:33 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Kirill A. Shutemov, Theodore Ts'o, Andreas Dilger, Jan Kara,
Andrew Morton, Alexander Viro, Hugh Dickins, Andrea Arcangeli,
Dave Hansen, Vlastimil Babka, Ross Zwisler, linux-ext4,
linux-fsdevel, linux-kernel, linux-mm, linux-block
In-Reply-To: <20170209215505.GW2267@bombadil.infradead.org>
On Thu, Feb 09, 2017 at 01:55:05PM -0800, Matthew Wilcox wrote:
> On Thu, Jan 26, 2017 at 02:57:50PM +0300, Kirill A. Shutemov wrote:
> > +++ b/mm/filemap.c
> > @@ -1886,6 +1886,7 @@ static ssize_t do_generic_file_read(struct file *filp, loff_t *ppos,
> > if (unlikely(page == NULL))
> > goto no_cached_page;
> > }
> > + page = compound_head(page);
>
> We got this page from find_get_page(), which gets it from
> pagecache_get_page(), which gets it from find_get_entry() ... which
> (unless I'm lost in your patch series) returns the head page. So this
> line is redundant, right?
No. pagecache_get_page() returns subpage. See description of the first
patch.
> But then down in filemap_fault, we have:
>
> VM_BUG_ON_PAGE(page->index != offset, page);
>
> ... again, maybe I'm lost somewhere in your patch series, but I don't see
> anywhere you remove that line (or modify it).
This should be fine as find_get_page() returns subpage.
> So are you not testing
> with VM debugging enabled, or are you not doing a test which includes
> mapping a file with huge pages, reading from it (to get the page in cache),
> then faulting on an address that is not in the first 4kB of that 2MB?
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
--
Kirill A. Shutemov
^ permalink raw reply
* Re: [PATCHv6 07/37] filemap: allocate huge page in page_cache_read(), if allowed
From: Kirill A. Shutemov @ 2017-02-13 15:17 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Kirill A. Shutemov, Theodore Ts'o, Andreas Dilger, Jan Kara,
Andrew Morton, Alexander Viro, Hugh Dickins, Andrea Arcangeli,
Dave Hansen, Vlastimil Babka, Ross Zwisler, linux-ext4,
linux-fsdevel, linux-kernel, linux-mm, linux-block
In-Reply-To: <20170209211835.GV2267@bombadil.infradead.org>
On Thu, Feb 09, 2017 at 01:18:35PM -0800, Matthew Wilcox wrote:
> On Thu, Jan 26, 2017 at 02:57:49PM +0300, Kirill A. Shutemov wrote:
> > Later we can add logic to accumulate information from shadow entires to
> > return to caller (average eviction time?).
>
> I would say minimum rather than average. That will become the refault
> time of the entire page, so minimum would probably have us making better
> decisions?
Yes, makes sense.
> > + /* Wipe shadow entires */
> > + radix_tree_for_each_slot(slot, &mapping->page_tree, &iter,
> > + page->index) {
> > + if (iter.index >= page->index + hpage_nr_pages(page))
> > + break;
> >
> > p = radix_tree_deref_slot_protected(slot, &mapping->tree_lock);
> > - if (!radix_tree_exceptional_entry(p))
> > + if (!p)
> > + continue;
>
> Just FYI, this can't happen. You're holding the tree lock so nobody
> else gets to remove things from the tree. radix_tree_for_each_slot()
> only gives you the full slots; it skips the empty ones for you. I'm
> OK if you want to leave it in out of an abundance of caution.
I'll drop it.
> > + __radix_tree_replace(&mapping->page_tree, iter.node, slot, NULL,
> > + workingset_update_node, mapping);
>
> I may add an update_node argument to radix_tree_join at some point,
> so you can use it here. Or maybe we don't need to do that, and what
> you have here works just fine.
>
> > mapping->nrexceptional--;
>
> ... because adjusting the exceptional count is going to be a pain.
Yeah..
> > + error = __radix_tree_insert(&mapping->page_tree,
> > + page->index, compound_order(page), page);
> > + /* This shouldn't happen */
> > + if (WARN_ON_ONCE(error))
> > + return error;
>
> A lesser man would have just ignored the return value from
> __radix_tree_insert. I salute you.
>
> > @@ -2078,18 +2155,34 @@ static int page_cache_read(struct file *file, pgoff_t offset, gfp_t gfp_mask)
> > {
> > struct address_space *mapping = file->f_mapping;
> > struct page *page;
> > + pgoff_t hoffset;
> > int ret;
> >
> > do {
> > - page = __page_cache_alloc(gfp_mask|__GFP_COLD);
> > + page = page_cache_alloc_huge(mapping, offset, gfp_mask);
> > +no_huge:
> > + if (!page)
> > + page = __page_cache_alloc(gfp_mask|__GFP_COLD);
> > if (!page)
> > return -ENOMEM;
> >
> > - ret = add_to_page_cache_lru(page, mapping, offset, gfp_mask & GFP_KERNEL);
> > - if (ret == 0)
> > + if (PageTransHuge(page))
> > + hoffset = round_down(offset, HPAGE_PMD_NR);
> > + else
> > + hoffset = offset;
> > +
> > + ret = add_to_page_cache_lru(page, mapping, hoffset,
> > + gfp_mask & GFP_KERNEL);
> > +
> > + if (ret == -EEXIST && PageTransHuge(page)) {
> > + put_page(page);
> > + page = NULL;
> > + goto no_huge;
> > + } else if (ret == 0) {
> > ret = mapping->a_ops->readpage(file, page);
> > - else if (ret == -EEXIST)
> > + } else if (ret == -EEXIST) {
> > ret = 0; /* losing race to add is OK */
> > + }
> >
> > put_page(page);
>
> If the filesystem returns AOP_TRUNCATED_PAGE, you'll go round this loop
> again trying the huge page again, even if the huge page didn't work
> the first time. I would tend to think that if the huge page failed the
> first time, we shouldn't try it again, so I propose this:
AOP_TRUNCATED_PAGE is positive, so I don't see how you avoid try_huge on
second iteration. Hm?
>
> struct address_space *mapping = file->f_mapping;
> struct page *page;
> pgoff_t index;
> int ret;
> bool try_huge = true;
>
> do {
> if (try_huge) {
> page = page_cache_alloc_huge(gfp_mask|__GFP_COLD);
> if (page)
> index = round_down(offset, HPAGE_PMD_NR);
> else
> try_huge = false;
> }
>
> if (!try_huge) {
> page = __page_cache_alloc(gfp_mask|__GFP_COLD);
> index = offset;
> }
>
> if (!page)
> return -ENOMEM;
>
> ret = add_to_page_cache_lru(page, mapping, index,
> gfp_mask & GFP_KERNEL);
> if (ret < 0) {
> if (try_huge) {
> try_huge = false;
> ret = AOP_TRUNCATED_PAGE;
> } else if (ret == -EEXIST)
> ret = 0; /* losing race to add is OK */
> } else {
> ret = mapping->a_ops->readpage(file, page);
> }
>
> put_page(page);
> } while (ret == AOP_TRUNCATED_PAGE);
>
> But ... maybe it's OK to retry the huge page. I mean, not many
> filesystems return AOP_TRUNCATED_PAGE, and they only do so rarely.
What about this:
struct address_space *mapping = file->f_mapping;
struct page *page;
pgoff_t hoffset;
int ret;
bool try_huge = true;
do {
if (try_huge) {
page = page_cache_alloc_huge(mapping, offset, gfp_mask);
hoffset = round_down(offset, HPAGE_PMD_NR);
/* Try to allocate huge page once */
try_huge = false;
}
if (!page) {
page = __page_cache_alloc(gfp_mask|__GFP_COLD);
hoffset = offset;
}
if (!page)
return -ENOMEM;
ret = add_to_page_cache_lru(page, mapping, hoffset,
gfp_mask & GFP_KERNEL);
if (ret == -EEXIST && PageTransHuge(page)) {
/* Retry with small page */
put_page(page);
page = NULL;
continue;
} else if (ret == 0) {
ret = mapping->a_ops->readpage(file, page);
} else if (ret == -EEXIST) {
ret = 0; /* losing race to add is OK */
}
put_page(page);
} while (ret == AOP_TRUNCATED_PAGE);
return ret;
> Anyway, I'm fine with the patch going in as-is. I just wanted to type out
> my review notes.
>
> Reviewed-by: Matthew Wilcox <mawilcox@microsoft.com>
Thanks!
--
Kirill A. Shutemov
^ permalink raw reply
* Re: [BUG] Potential deadlock in the block layer
From: Jens Axboe @ 2017-02-13 15:02 UTC (permalink / raw)
To: Thomas Gleixner, LKML
Cc: linux-block, Gabriel C, Steven Rostedt, Peter Zijlstra,
Linus Torvalds
In-Reply-To: <alpine.DEB.2.20.1702131505580.3619@nanos>
On 02/13/2017 07:14 AM, Thomas Gleixner wrote:
> Gabriel reported the lockdep splat below while investigating something
> different.
>
> Explanation for the splat is in the function comment above
> del_timer_sync().
>
> I can reproduce it as well and it's clearly broken.
Agree, the disable logic is broken. The below isn't super pretty, but it
should do the trick for 4.10.
diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
index c73a6fcaeb9d..838f07e2b64a 100644
--- a/block/cfq-iosched.c
+++ b/block/cfq-iosched.c
@@ -3758,7 +3758,7 @@ static void cfq_init_cfqq(struct cfq_data *cfqd, struct cfq_queue *cfqq,
}
#ifdef CONFIG_CFQ_GROUP_IOSCHED
-static void check_blkcg_changed(struct cfq_io_cq *cic, struct bio *bio)
+static bool check_blkcg_changed(struct cfq_io_cq *cic, struct bio *bio)
{
struct cfq_data *cfqd = cic_to_cfqd(cic);
struct cfq_queue *cfqq;
@@ -3775,15 +3775,7 @@ static void check_blkcg_changed(struct cfq_io_cq *cic, struct bio *bio)
* spuriously on a newly created cic but there's no harm.
*/
if (unlikely(!cfqd) || likely(cic->blkcg_serial_nr == serial_nr))
- return;
-
- /*
- * If we have a non-root cgroup, we can depend on that to
- * do proper throttling of writes. Turn off wbt for that
- * case, if it was enabled by default.
- */
- if (nonroot_cg)
- wbt_disable_default(cfqd->queue);
+ return nonroot_cg;
/*
* Drop reference to queues. New queues will be assigned in new
@@ -3804,9 +3796,13 @@ static void check_blkcg_changed(struct cfq_io_cq *cic, struct bio *bio)
}
cic->blkcg_serial_nr = serial_nr;
+ return nonroot_cg;
}
#else
-static inline void check_blkcg_changed(struct cfq_io_cq *cic, struct bio *bio) { }
+static inline bool check_blkcg_changed(struct cfq_io_cq *cic, struct bio *bio)
+{
+ return false;
+}
#endif /* CONFIG_CFQ_GROUP_IOSCHED */
static struct cfq_queue **
@@ -4448,11 +4444,12 @@ cfq_set_request(struct request_queue *q, struct request *rq, struct bio *bio,
const int rw = rq_data_dir(rq);
const bool is_sync = rq_is_sync(rq);
struct cfq_queue *cfqq;
+ bool disable_wbt;
spin_lock_irq(q->queue_lock);
check_ioprio_changed(cic, bio);
- check_blkcg_changed(cic, bio);
+ disable_wbt = check_blkcg_changed(cic, bio);
new_queue:
cfqq = cic_to_cfqq(cic, is_sync);
if (!cfqq || cfqq == &cfqd->oom_cfqq) {
@@ -4488,6 +4485,10 @@ cfq_set_request(struct request_queue *q, struct request *rq, struct bio *bio,
rq->elv.priv[0] = cfqq;
rq->elv.priv[1] = cfqq->cfqg;
spin_unlock_irq(q->queue_lock);
+
+ if (disable_wbt)
+ wbt_disable_default(q);
+
return 0;
}
--
Jens Axboe
^ permalink raw reply related
* Re: [PATCHv6 05/37] thp: try to free page's buffers before attempt split
From: Kirill A. Shutemov @ 2017-02-13 14:32 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Kirill A. Shutemov, Theodore Ts'o, Andreas Dilger, Jan Kara,
Andrew Morton, Alexander Viro, Hugh Dickins, Andrea Arcangeli,
Dave Hansen, Vlastimil Babka, Ross Zwisler, linux-ext4,
linux-fsdevel, linux-kernel, linux-mm, linux-block
In-Reply-To: <20170209201416.GS2267@bombadil.infradead.org>
On Thu, Feb 09, 2017 at 12:14:16PM -0800, Matthew Wilcox wrote:
> On Thu, Jan 26, 2017 at 02:57:47PM +0300, Kirill A. Shutemov wrote:
> > @@ -2146,6 +2146,23 @@ int split_huge_page_to_list(struct page *page, struct list_head *list)
> > goto out;
> > }
> >
> > + /* Try to free buffers before attempt split */
> > + if (!PageSwapBacked(head) && PagePrivate(page)) {
> > + /*
> > + * We cannot trigger writeback from here due possible
> > + * recursion if triggered from vmscan, only wait.
> > + *
> > + * Caller can trigger writeback it on its own, if safe.
> > + */
>
> It took me a few reads to get this. May I suggest:
>
> /*
> * Cannot split a page with buffers. If the caller has
> * already started writeback, we can wait for it to finish,
> * but we cannot start writeback if we were called from vmscan
> */
Yeah, that's better.
> > + if (!PageSwapBacked(head) && PagePrivate(page)) {
>
> Also, it looks weird to test PageSwapBacked of *head* and PagePrivate
> of *page*. I think it's correct, but it still looks weird.
I'll change this.
> Reviewed-by: Matthew Wilcox <mawilcox@microsoft.com>
Thanks!
--
Kirill A. Shutemov
^ permalink raw reply
* Re: [PATCH 0/10] block: Fix block device shutdown related races
From: Thiago Jung Bauermann @ 2017-02-13 14:27 UTC (permalink / raw)
To: Jan Kara
Cc: Jens Axboe, linux-block, Christoph Hellwig, Tejun Heo,
Dan Williams, NeilBrown, lekshmicpillai
In-Reply-To: <20170209124433.2626-1-jack@suse.cz>
Hi,
Am Donnerstag, 9. Februar 2017, 13:44:23 BRST schrieb Jan Kara:
> People, please have a look at patches. The are mostly simple however the
> interactions are rather complex so I may have missed something. Also I'm
> happy for any additional testing these patches can get - I've stressed them
> with Omar's script, tested memcg writeback, tested static (not udev managed)
> device inodes.
Lekshmi has been stress-testing these patches for 4 days now without problems.
If this is the version that goes in, you can add:
Tested-by: Lekshmi Pillai <lekshmicpillai@in.ibm.com>
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply
* [BUG] Potential deadlock in the block layer
From: Thomas Gleixner @ 2017-02-13 14:14 UTC (permalink / raw)
To: LKML
Cc: linux-block, Jens Axboe, Gabriel C, Steven Rostedt,
Peter Zijlstra, Linus Torvalds
Gabriel reported the lockdep splat below while investigating something
different.
Explanation for the splat is in the function comment above
del_timer_sync().
I can reproduce it as well and it's clearly broken.
Thanks,
tglx
---
[ 81.518032] ======================================================
[ 81.520151] [ INFO: HARDIRQ-safe -> HARDIRQ-unsafe lock order detected ]
[ 81.522276] 4.10.0-rc8-debug-dirty #1 Tainted: G I
[ 81.524445] ------------------------------------------------------
[ 81.526560] (systemd)/1725 [HC0[0]:SC0[0]:HE0:SE1] is trying to acquire:
[ 81.528672] (((&rwb->window_timer))){+.-...}, at: [<ffffffff810e38e0>] del_timer_sync+0x0/0xd0
[ 81.530973]
and this task is already holding:
[ 81.535399] (&(&q->__queue_lock)->rlock){-.-...}, at: [<ffffffff81378fc0>] cfq_set_request+0x80/0x2c0
[ 81.537637] which would create a new lock dependency:
[ 81.539870] (&(&q->__queue_lock)->rlock){-.-...} -> (((&rwb->window_timer))){+.-...}
[ 81.542145]
but this new dependency connects a HARDIRQ-irq-safe lock:
[ 81.546717] (&(&q->__queue_lock)->rlock){-.-...}
[ 81.546720]
... which became HARDIRQ-irq-safe at:
[ 81.553643] __lock_acquire+0x24f/0x19e0
[ 81.555970] lock_acquire+0xa5/0xd0
[ 81.558302] _raw_spin_lock_irqsave+0x54/0x90
[ 81.560667] ata_qc_schedule_eh+0x56/0x80 [libata]
[ 81.563035] ata_qc_complete+0x99/0x1c0 [libata]
[ 81.565410] ata_do_link_abort+0x93/0xd0 [libata]
[ 81.567786] ata_port_abort+0xb/0x10 [libata]
[ 81.570150] ahci_handle_port_interrupt+0x41e/0x570 [libahci]
[ 81.572533] ahci_handle_port_intr+0x74/0xb0 [libahci]
[ 81.574918] ahci_single_level_irq_intr+0x3b/0x60 [libahci]
[ 81.577311] __handle_irq_event_percpu+0x35/0x100
[ 81.579696] handle_irq_event_percpu+0x1e/0x50
[ 81.582065] handle_irq_event+0x34/0x60
[ 81.584427] handle_edge_irq+0x112/0x140
[ 81.586776] handle_irq+0x18/0x20
[ 81.589109] do_IRQ+0x87/0x110
[ 81.591403] ret_from_intr+0x0/0x20
[ 81.593666] cpuidle_enter_state+0x102/0x230
[ 81.595939] cpuidle_enter+0x12/0x20
[ 81.598202] call_cpuidle+0x38/0x40
[ 81.600443] do_idle+0x16e/0x1e0
[ 81.602670] cpu_startup_entry+0x5d/0x60
[ 81.604890] rest_init+0x12c/0x140
[ 81.607080] start_kernel+0x45f/0x46c
[ 81.609252] x86_64_start_reservations+0x2a/0x2c
[ 81.611399] x86_64_start_kernel+0xeb/0xf8
[ 81.613505] verify_cpu+0x0/0xfc
[ 81.615574]
to a HARDIRQ-irq-unsafe lock:
[ 81.619611] (((&rwb->window_timer))){+.-...}
[ 81.619614]
... which became HARDIRQ-irq-unsafe at:
[ 81.625562] ...
[ 81.625565] __lock_acquire+0x2ba/0x19e0
[ 81.629504] lock_acquire+0xa5/0xd0
[ 81.631467] call_timer_fn+0x74/0x110
[ 81.633397] expire_timers+0xaa/0xd0
[ 81.635287] run_timer_softirq+0x72/0x140
[ 81.637145] __do_softirq+0x143/0x290
[ 81.638974] irq_exit+0x6a/0xd0
[ 81.640818] smp_trace_apic_timer_interrupt+0x79/0x90
[ 81.642698] smp_apic_timer_interrupt+0x9/0x10
[ 81.644572] apic_timer_interrupt+0x93/0xa0
[ 81.646445] cpuidle_enter_state+0x102/0x230
[ 81.648332] cpuidle_enter+0x12/0x20
[ 81.650211] call_cpuidle+0x38/0x40
[ 81.652096] do_idle+0x16e/0x1e0
[ 81.653984] cpu_startup_entry+0x5d/0x60
[ 81.655855] start_secondary+0x150/0x180
[ 81.657696] verify_cpu+0x0/0xfc
[ 81.659497]
other info that might help us debug this:
[ 81.664802] Possible interrupt unsafe locking scenario:
[ 81.668296] CPU0 CPU1
[ 81.670010] ---- ----
[ 81.671685] lock(((&rwb->window_timer)));
[ 81.673358] local_irq_disable();
[ 81.675018] lock(&(&q->__queue_lock)->rlock);
[ 81.676659] lock(((&rwb->window_timer)));
[ 81.678275] <Interrupt>
[ 81.679845] lock(&(&q->__queue_lock)->rlock);
[ 81.681419]
*** DEADLOCK ***
[ 81.685989] 1 lock held by (systemd)/1725:
[ 81.687518] #0: (&(&q->__queue_lock)->rlock){-.-...}, at: [<ffffffff81378fc0>] cfq_set_request+0x80/0x2c0
[ 81.689125]
the dependencies between HARDIRQ-irq-safe lock and the holding lock:
[ 81.692327] -> (&(&q->__queue_lock)->rlock){-.-...} ops: 70140 {
[ 81.693971] IN-HARDIRQ-W at:
[ 81.695606] __lock_acquire+0x24f/0x19e0
[ 81.697262] lock_acquire+0xa5/0xd0
[ 81.698906] _raw_spin_lock_irqsave+0x54/0x90
[ 81.700568] ata_qc_schedule_eh+0x56/0x80 [libata]
[ 81.702229] ata_qc_complete+0x99/0x1c0 [libata]
[ 81.703884] ata_do_link_abort+0x93/0xd0 [libata]
[ 81.705540] ata_port_abort+0xb/0x10 [libata]
[ 81.707199] ahci_handle_port_interrupt+0x41e/0x570 [libahci]
[ 81.708881] ahci_handle_port_intr+0x74/0xb0 [libahci]
[ 81.710563] ahci_single_level_irq_intr+0x3b/0x60 [libahci]
[ 81.712256] __handle_irq_event_percpu+0x35/0x100
[ 81.713952] handle_irq_event_percpu+0x1e/0x50
[ 81.715651] handle_irq_event+0x34/0x60
[ 81.717350] handle_edge_irq+0x112/0x140
[ 81.719050] handle_irq+0x18/0x20
[ 81.720744] do_IRQ+0x87/0x110
[ 81.722432] ret_from_intr+0x0/0x20
[ 81.724123] cpuidle_enter_state+0x102/0x230
[ 81.725823] cpuidle_enter+0x12/0x20
[ 81.727509] call_cpuidle+0x38/0x40
[ 81.729196] do_idle+0x16e/0x1e0
[ 81.730879] cpu_startup_entry+0x5d/0x60
[ 81.732565] rest_init+0x12c/0x140
[ 81.734246] start_kernel+0x45f/0x46c
[ 81.735927] x86_64_start_reservations+0x2a/0x2c
[ 81.737617] x86_64_start_kernel+0xeb/0xf8
[ 81.739297] verify_cpu+0x0/0xfc
[ 81.740964] IN-SOFTIRQ-W at:
[ 81.742628] __lock_acquire+0x268/0x19e0
[ 81.744311] lock_acquire+0xa5/0xd0
[ 81.745994] _raw_spin_lock_irqsave+0x54/0x90
[ 81.747699] scsi_end_request+0x144/0x1a0 [scsi_mod]
[ 81.749417] scsi_io_completion+0x268/0x610 [scsi_mod]
[ 81.751133] scsi_finish_command+0x103/0x110 [scsi_mod]
[ 81.752856] scsi_softirq_done+0x11f/0x130 [scsi_mod]
[ 81.754575] blk_done_softirq+0x9b/0xb0
[ 81.756288] __do_softirq+0x143/0x290
[ 81.757992] irq_exit+0x6a/0xd0
[ 81.759688] smp_trace_apic_timer_interrupt+0x79/0x90
[ 81.761407] smp_apic_timer_interrupt+0x9/0x10
[ 81.763131] apic_timer_interrupt+0x93/0xa0
[ 81.764862] _raw_spin_unlock_irqrestore+0x47/0x70
[ 81.766606] release_pages+0x51/0x390
[ 81.768347] free_pages_and_swap_cache+0x2a/0xb0
[ 81.770100] tlb_flush_mmu_free+0x34/0x50
[ 81.771851] tlb_finish_mmu+0x17/0x50
[ 81.773592] unmap_region+0xe8/0x100
[ 81.775322] do_munmap+0x264/0x3e0
[ 81.777040] SyS_munmap+0x49/0x70
[ 81.778752] entry_SYSCALL_64_fastpath+0x1f/0xc2
[ 81.780483] INITIAL USE at:
[ 81.782221] __lock_acquire+0x1c9/0x19e0
[ 81.783978] lock_acquire+0xa5/0xd0
[ 81.785721] _raw_spin_lock_irq+0x48/0x80
[ 81.787470] blkcg_init_queue+0xad/0x1b0
[ 81.789217] blk_alloc_queue_node+0x27d/0x2d0
[ 81.790959] blk_mq_init_queue+0x1b/0x60
[ 81.792695] loop_add+0xf4/0x280
[ 81.794427] loop_init+0x101/0x142
[ 81.796166] do_one_initcall+0x6f/0x190
[ 81.797914] kernel_init_freeable+0x321/0x3b7
[ 81.799659] kernel_init+0x9/0xf0
[ 81.801360] ret_from_fork+0x31/0x40
[ 81.803029] }
[ 81.804660] ... key at: [<ffffffff82f1e728>] __key.42689+0x0/0x8
[ 81.806315] ... acquired at:
[ 81.807937] check_irq_usage+0x4d/0xa0
[ 81.809555] __lock_acquire+0x1110/0x19e0
[ 81.811176] lock_acquire+0xa5/0xd0
[ 81.812783] del_timer_sync+0x49/0xd0
[ 81.814385] wbt_disable_default+0x1e/0x60
[ 81.815991] check_blkcg_changed+0x22d/0x380
[ 81.817594] cfq_set_request+0xe5/0x2c0
[ 81.819194] elv_set_request+0x1a/0x20
[ 81.820775] get_request+0x88d/0x900
[ 81.822355] blk_queue_bio+0x1e9/0x320
[ 81.823925] generic_make_request+0xbe/0x170
[ 81.825492] submit_bio+0x121/0x150
[ 81.827050] btrfs_map_bio+0x282/0x2a0
[ 81.828613] btree_submit_bio_hook+0x71/0xe0
[ 81.830181] submit_one_bio+0x65/0x90
[ 81.831750] read_extent_buffer_pages+0x1fa/0x2f0
[ 81.833336] btree_read_extent_buffer_pages+0x59/0xf0
[ 81.834932] read_tree_block+0x2d/0x50
[ 81.836528] read_block_for_search.isra.10+0x296/0x2e0
[ 81.838140] btrfs_search_slot+0x6cc/0x920
[ 81.839754] btrfs_lookup_csum+0x3e/0x120
[ 81.841370] __btrfs_lookup_bio_sums+0x258/0x4d0
[ 81.842993] btrfs_lookup_bio_sums+0x11/0x20
[ 81.844621] btrfs_submit_compressed_read+0x3d0/0x4c0
[ 81.846259] btrfs_submit_bio_hook+0xbb/0x170
[ 81.847889] submit_one_bio+0x65/0x90
[ 81.849516] extent_readpages+0x1dd/0x1f0
[ 81.851145] btrfs_readpages+0x1a/0x20
[ 81.852769] __do_page_cache_readahead+0x249/0x320
[ 81.854396] ondemand_readahead+0x47e/0x4b0
[ 81.856025] page_cache_sync_readahead+0x3c/0x40
[ 81.857656] generic_file_read_iter+0x20e/0x770
[ 81.859279] __vfs_read+0xe5/0x120
[ 81.860897] vfs_read+0xc7/0x170
[ 81.862503] SyS_read+0x44/0xa0
[ 81.864102] entry_SYSCALL_64_fastpath+0x1f/0xc2
[ 81.867299]
the dependencies between the lock to be acquired
[ 81.867300] and HARDIRQ-irq-unsafe lock:
[ 81.872089] -> (((&rwb->window_timer))){+.-...} ops: 463 {
[ 81.873716] HARDIRQ-ON-W at:
[ 81.875336] __lock_acquire+0x2ba/0x19e0
[ 81.876969] lock_acquire+0xa5/0xd0
[ 81.878598] call_timer_fn+0x74/0x110
[ 81.880225] expire_timers+0xaa/0xd0
[ 81.881853] run_timer_softirq+0x72/0x140
[ 81.883486] __do_softirq+0x143/0x290
[ 81.885127] irq_exit+0x6a/0xd0
[ 81.886765] smp_trace_apic_timer_interrupt+0x79/0x90
[ 81.888419] smp_apic_timer_interrupt+0x9/0x10
[ 81.890066] apic_timer_interrupt+0x93/0xa0
[ 81.891706] cpuidle_enter_state+0x102/0x230
[ 81.893349] cpuidle_enter+0x12/0x20
[ 81.894982] call_cpuidle+0x38/0x40
[ 81.896610] do_idle+0x16e/0x1e0
[ 81.898235] cpu_startup_entry+0x5d/0x60
[ 81.899866] start_secondary+0x150/0x180
[ 81.901497] verify_cpu+0x0/0xfc
[ 81.903126] IN-SOFTIRQ-W at:
[ 81.904753] __lock_acquire+0x268/0x19e0
[ 81.906402] lock_acquire+0xa5/0xd0
[ 81.908052] call_timer_fn+0x74/0x110
[ 81.909702] expire_timers+0xaa/0xd0
[ 81.911347] run_timer_softirq+0x72/0x140
[ 81.912980] __do_softirq+0x143/0x290
[ 81.914605] irq_exit+0x6a/0xd0
[ 81.916227] smp_trace_apic_timer_interrupt+0x79/0x90
[ 81.917867] smp_apic_timer_interrupt+0x9/0x10
[ 81.919510] apic_timer_interrupt+0x93/0xa0
[ 81.921161] cpuidle_enter_state+0x102/0x230
[ 81.922807] cpuidle_enter+0x12/0x20
[ 81.924444] call_cpuidle+0x38/0x40
[ 81.926082] do_idle+0x16e/0x1e0
[ 81.927718] cpu_startup_entry+0x5d/0x60
[ 81.929358] start_secondary+0x150/0x180
[ 81.930996] verify_cpu+0x0/0xfc
[ 81.932628] INITIAL USE at:
[ 81.934256] __lock_acquire+0x1c9/0x19e0
[ 81.935907] lock_acquire+0xa5/0xd0
[ 81.937550] call_timer_fn+0x74/0x110
[ 81.939192] expire_timers+0xaa/0xd0
[ 81.940826] run_timer_softirq+0x72/0x140
[ 81.942456] __do_softirq+0x143/0x290
[ 81.944075] irq_exit+0x6a/0xd0
[ 81.945685] smp_trace_apic_timer_interrupt+0x79/0x90
[ 81.947304] smp_apic_timer_interrupt+0x9/0x10
[ 81.948926] apic_timer_interrupt+0x93/0xa0
[ 81.950547] cpuidle_enter_state+0x102/0x230
[ 81.952162] cpuidle_enter+0x12/0x20
[ 81.953766] call_cpuidle+0x38/0x40
[ 81.955369] do_idle+0x16e/0x1e0
[ 81.956971] cpu_startup_entry+0x5d/0x60
[ 81.958580] start_secondary+0x150/0x180
[ 81.960191] verify_cpu+0x0/0xfc
[ 81.961787] }
[ 81.963360] ... key at: [<ffffffff82f1f7e0>] __key.37597+0x0/0x8
[ 81.964971] ... acquired at:
[ 81.966566] check_irq_usage+0x4d/0xa0
[ 81.968162] __lock_acquire+0x1110/0x19e0
[ 81.969767] lock_acquire+0xa5/0xd0
[ 81.971374] del_timer_sync+0x49/0xd0
[ 81.972973] wbt_disable_default+0x1e/0x60
[ 81.974574] check_blkcg_changed+0x22d/0x380
[ 81.976192] cfq_set_request+0xe5/0x2c0
[ 81.977800] elv_set_request+0x1a/0x20
[ 81.979384] get_request+0x88d/0x900
[ 81.980954] blk_queue_bio+0x1e9/0x320
[ 81.982506] generic_make_request+0xbe/0x170
[ 81.984057] submit_bio+0x121/0x150
[ 81.985606] btrfs_map_bio+0x282/0x2a0
[ 81.987157] btree_submit_bio_hook+0x71/0xe0
[ 81.988713] submit_one_bio+0x65/0x90
[ 81.990269] read_extent_buffer_pages+0x1fa/0x2f0
[ 81.991829] btree_read_extent_buffer_pages+0x59/0xf0
[ 81.993394] read_tree_block+0x2d/0x50
[ 81.994950] read_block_for_search.isra.10+0x296/0x2e0
[ 81.996513] btrfs_search_slot+0x6cc/0x920
[ 81.998073] btrfs_lookup_csum+0x3e/0x120
[ 81.999624] __btrfs_lookup_bio_sums+0x258/0x4d0
[ 82.001190] btrfs_lookup_bio_sums+0x11/0x20
[ 82.002767] btrfs_submit_compressed_read+0x3d0/0x4c0
[ 82.004352] btrfs_submit_bio_hook+0xbb/0x170
[ 82.005933] submit_one_bio+0x65/0x90
[ 82.007513] extent_readpages+0x1dd/0x1f0
[ 82.009092] btrfs_readpages+0x1a/0x20
[ 82.010667] __do_page_cache_readahead+0x249/0x320
[ 82.012241] ondemand_readahead+0x47e/0x4b0
[ 82.013816] page_cache_sync_readahead+0x3c/0x40
[ 82.015398] generic_file_read_iter+0x20e/0x770
[ 82.016989] __vfs_read+0xe5/0x120
[ 82.018577] vfs_read+0xc7/0x170
[ 82.020155] SyS_read+0x44/0xa0
[ 82.021731] entry_SYSCALL_64_fastpath+0x1f/0xc2
[ 82.024866]
stack backtrace:
[ 82.027929] CPU: 13 PID: 1725 Comm: (systemd) Tainted: G I 4.10.0-rc8-debug-dirty #1
[ 82.029530] Hardware name: FUJITSU PRIMERGY TX200 S5 /D2709, BIOS 6.00 Rev. 1.14.2709 02/04/2013
[ 82.031213] Call Trace:
[ 82.032870] dump_stack+0x86/0xc1
[ 82.034527] check_usage+0x5fc/0x630
[ 82.036191] check_irq_usage+0x4d/0xa0
[ 82.037852] __lock_acquire+0x1110/0x19e0
[ 82.039510] ? __lock_acquire+0x18f0/0x19e0
[ 82.041160] ? __lock_acquire+0x18f0/0x19e0
[ 82.042792] lock_acquire+0xa5/0xd0
[ 82.044425] ? collect_expired_timers+0xb0/0xb0
[ 82.046069] del_timer_sync+0x49/0xd0
[ 82.047711] ? collect_expired_timers+0xb0/0xb0
[ 82.049366] wbt_disable_default+0x1e/0x60
[ 82.051024] check_blkcg_changed+0x22d/0x380
[ 82.052678] ? cfq_pd_offline+0x110/0x110
[ 82.054327] ? debug_smp_processor_id+0x17/0x20
[ 82.055977] ? get_lock_stats+0x19/0x50
[ 82.057623] ? cfq_set_request+0x80/0x2c0
[ 82.059276] cfq_set_request+0xe5/0x2c0
[ 82.060926] ? _raw_spin_unlock_irq+0x27/0x50
[ 82.062656] ? __this_cpu_preempt_check+0x13/0x20
[ 82.064314] ? trace_hardirqs_on_caller+0x1db/0x210
[ 82.065976] ? trace_hardirqs_on+0xd/0x10
[ 82.067630] ? _raw_spin_unlock_irq+0x38/0x50
[ 82.069291] elv_set_request+0x1a/0x20
[ 82.070937] get_request+0x88d/0x900
[ 82.072568] ? get_request+0x47/0x900
[ 82.074202] ? prepare_to_wait_event+0x180/0x180
[ 82.075842] blk_queue_bio+0x1e9/0x320
[ 82.077476] generic_make_request+0xbe/0x170
[ 82.079117] submit_bio+0x121/0x150
[ 82.080754] ? __percpu_counter_add+0x8b/0xb0
[ 82.082390] btrfs_map_bio+0x282/0x2a0
[ 82.084027] ? free_root_pointers+0x60/0x60
[ 82.085673] btree_submit_bio_hook+0x71/0xe0
[ 82.087320] submit_one_bio+0x65/0x90
[ 82.088969] read_extent_buffer_pages+0x1fa/0x2f0
[ 82.090628] ? free_root_pointers+0x60/0x60
[ 82.092284] btree_read_extent_buffer_pages+0x59/0xf0
[ 82.093952] read_tree_block+0x2d/0x50
[ 82.095612] read_block_for_search.isra.10+0x296/0x2e0
[ 82.097282] btrfs_search_slot+0x6cc/0x920
[ 82.098946] ? __this_cpu_preempt_check+0x13/0x20
[ 82.100617] btrfs_lookup_csum+0x3e/0x120
[ 82.102280] __btrfs_lookup_bio_sums+0x258/0x4d0
[ 82.103951] ? kmem_cache_alloc+0x11a/0x130
[ 82.105613] btrfs_lookup_bio_sums+0x11/0x20
[ 82.107280] btrfs_submit_compressed_read+0x3d0/0x4c0
[ 82.108953] btrfs_submit_bio_hook+0xbb/0x170
[ 82.110610] submit_one_bio+0x65/0x90
[ 82.112260] extent_readpages+0x1dd/0x1f0
[ 82.113910] ? btrfs_direct_IO+0x320/0x320
[ 82.115559] ? alloc_pages_current+0x14a/0x1e0
[ 82.117219] ? __page_cache_alloc+0xcc/0x150
[ 82.118873] btrfs_readpages+0x1a/0x20
[ 82.120524] __do_page_cache_readahead+0x249/0x320
[ 82.122181] ? __do_page_cache_readahead+0xb4/0x320
[ 82.123836] ondemand_readahead+0x47e/0x4b0
[ 82.125491] page_cache_sync_readahead+0x3c/0x40
[ 82.127148] generic_file_read_iter+0x20e/0x770
[ 82.128817] ? __do_page_fault+0x325/0x410
[ 82.130490] __vfs_read+0xe5/0x120
[ 82.132152] vfs_read+0xc7/0x170
[ 82.133809] SyS_read+0x44/0xa0
[ 82.135479] entry_SYSCALL_64_fastpath+0x1f/0xc2
[ 82.137166] RIP: 0033:0x7f4eace2ff27
[ 82.138825] RSP: 002b:00007ffdd7f77318 EFLAGS: 00000202 ORIG_RAX: 0000000000000000
[ 82.140477] RAX: ffffffffffffffda RBX: ffffffff813b0823 RCX: 00007f4eace2ff27
[ 82.142106] RDX: 0000000000000340 RSI: 00007ffdd7f77408 RDI: 0000000000000008
[ 82.143751] RBP: ffffc9000ccb7f88 R08: 0000000090000002 R09: 00007ffdd7f773ef
[ 82.145408] R10: 00007f4eace310d0 R11: 0000000000000202 R12: 00007ffdd7f773ef
[ 82.147072] R13: 0000000000000340 R14: 0000000000000008 R15: 00007ffdd7f77400
[ 82.148745] ? __this_cpu_preempt_check+0x13/0x20
^ permalink raw reply
* Re: [PATCHv6 03/37] page-flags: relax page flag policy for few flags
From: Kirill A. Shutemov @ 2017-02-13 13:59 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Kirill A. Shutemov, Theodore Ts'o, Andreas Dilger, Jan Kara,
Andrew Morton, Alexander Viro, Hugh Dickins, Andrea Arcangeli,
Dave Hansen, Vlastimil Babka, Ross Zwisler, linux-ext4,
linux-fsdevel, linux-kernel, linux-mm, linux-block
In-Reply-To: <20170209040113.GR2267@bombadil.infradead.org>
On Wed, Feb 08, 2017 at 08:01:13PM -0800, Matthew Wilcox wrote:
> On Thu, Jan 26, 2017 at 02:57:45PM +0300, Kirill A. Shutemov wrote:
> > These flags are in use for filesystems with backing storage: PG_error,
> > PG_writeback and PG_readahead.
>
> Oh ;-) Then I amend my comment on patch 1 to be "patch 3 needs to go
> ahead of patch 1" ;-)
It doesn't really matter as long as both before patch 37 :P
--
Kirill A. Shutemov
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox