* [PATCH -next] nbd: Fix hungtask when nbd_config_put
@ 2021-10-20 9:48 Ye Bin
2021-10-20 14:04 ` Josef Bacik
0 siblings, 1 reply; 3+ messages in thread
From: Ye Bin @ 2021-10-20 9:48 UTC (permalink / raw)
To: josef, axboe, linux-block, nbd; +Cc: linux-kernel, Ye Bin
I got follow issue:
[ 247.381177] INFO: task kworker/u10:0:47 blocked for more than 120 seconds.
[ 247.382644] Not tainted 4.19.90-dirty #140
[ 247.383502] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[ 247.385027] Call Trace:
[ 247.388384] schedule+0xb8/0x3c0
[ 247.388966] schedule_timeout+0x2b4/0x380
[ 247.392815] wait_for_completion+0x367/0x510
[ 247.397713] flush_workqueue+0x32b/0x1340
[ 247.402700] drain_workqueue+0xda/0x3c0
[ 247.403442] destroy_workqueue+0x7b/0x690
[ 247.405014] nbd_config_put.cold+0x2f9/0x5b6
[ 247.405823] recv_work+0x1fd/0x2b0
[ 247.406485] process_one_work+0x70b/0x1610
[ 247.407262] worker_thread+0x5a9/0x1060
[ 247.408699] kthread+0x35e/0x430
[ 247.410918] ret_from_fork+0x1f/0x30
We can reprodeuce issue as follows:
1. Inject memory fault in nbd_start_device
@@ -1244,10 +1248,18 @@ static int nbd_start_device(struct nbd_device *nbd)
nbd_dev_dbg_init(nbd);
for (i = 0; i < num_connections; i++) {
struct recv_thread_args *args;
-
- args = kzalloc(sizeof(*args), GFP_KERNEL);
+
+ if (i == 1) {
+ args = NULL;
+ printk("%s: inject malloc error\n", __func__);
+ }
+ else
+ args = kzalloc(sizeof(*args), GFP_KERNEL);
2. Inject delay in recv_work
@@ -757,6 +760,8 @@ static void recv_work(struct work_struct *work)
blk_mq_complete_request(blk_mq_rq_from_pdu(cmd));
}
+ printk("%s: comm=%s pid=%d\n", __func__, current->comm, current->pid);
+ mdelay(5 * 1000);
nbd_config_put(nbd);
atomic_dec(&config->recv_threads);
wake_up(&config->recv_wq);
3. Create nbd server
nbd-server 8000 /tmp/disk
4. Create nbd client
nbd-client localhost 8000 /dev/nbd1
Then will trigger above issue.
Reason is when add delay in recv_work, lead to relase the last reference
of 'nbd->config_refs'. nbd_config_put will call flush_workqueue to make
all work finish. Obviously, it will lead to deadloop.
To solve this issue, we must ensure 'recv_work' all exit before release
the last 'nbd->config_refs' reference count.
Signed-off-by: Ye Bin <yebin10@huawei.com>
---
drivers/block/nbd.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 0ee104fbb628..ba74887e24a8 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -2074,6 +2074,9 @@ static int nbd_genl_connect(struct sk_buff *skb, struct genl_info *info)
set_bit(NBD_RT_HAS_CONFIG_REF, &config->runtime_flags);
refcount_inc(&nbd->config_refs);
nbd_connect_reply(info, nbd->index);
+ } else if (nbd->recv_workq){
+ sock_shutdown(nbd);
+ flush_workqueue(nbd->recv_workq);
}
nbd_config_put(nbd);
if (put_dev)
--
2.31.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH -next] nbd: Fix hungtask when nbd_config_put
2021-10-20 9:48 [PATCH -next] nbd: Fix hungtask when nbd_config_put Ye Bin
@ 2021-10-20 14:04 ` Josef Bacik
2021-10-20 14:23 ` yebin
0 siblings, 1 reply; 3+ messages in thread
From: Josef Bacik @ 2021-10-20 14:04 UTC (permalink / raw)
To: Ye Bin; +Cc: axboe, linux-block, nbd, linux-kernel
On Wed, Oct 20, 2021 at 05:48:30PM +0800, Ye Bin wrote:
> I got follow issue:
> [ 247.381177] INFO: task kworker/u10:0:47 blocked for more than 120 seconds.
> [ 247.382644] Not tainted 4.19.90-dirty #140
> [ 247.383502] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> [ 247.385027] Call Trace:
> [ 247.388384] schedule+0xb8/0x3c0
> [ 247.388966] schedule_timeout+0x2b4/0x380
> [ 247.392815] wait_for_completion+0x367/0x510
> [ 247.397713] flush_workqueue+0x32b/0x1340
> [ 247.402700] drain_workqueue+0xda/0x3c0
> [ 247.403442] destroy_workqueue+0x7b/0x690
> [ 247.405014] nbd_config_put.cold+0x2f9/0x5b6
> [ 247.405823] recv_work+0x1fd/0x2b0
> [ 247.406485] process_one_work+0x70b/0x1610
> [ 247.407262] worker_thread+0x5a9/0x1060
> [ 247.408699] kthread+0x35e/0x430
> [ 247.410918] ret_from_fork+0x1f/0x30
>
> We can reprodeuce issue as follows:
> 1. Inject memory fault in nbd_start_device
> @@ -1244,10 +1248,18 @@ static int nbd_start_device(struct nbd_device *nbd)
> nbd_dev_dbg_init(nbd);
> for (i = 0; i < num_connections; i++) {
> struct recv_thread_args *args;
> -
> - args = kzalloc(sizeof(*args), GFP_KERNEL);
> +
> + if (i == 1) {
> + args = NULL;
> + printk("%s: inject malloc error\n", __func__);
> + }
> + else
> + args = kzalloc(sizeof(*args), GFP_KERNEL);
> 2. Inject delay in recv_work
> @@ -757,6 +760,8 @@ static void recv_work(struct work_struct *work)
>
> blk_mq_complete_request(blk_mq_rq_from_pdu(cmd));
> }
> + printk("%s: comm=%s pid=%d\n", __func__, current->comm, current->pid);
> + mdelay(5 * 1000);
> nbd_config_put(nbd);
> atomic_dec(&config->recv_threads);
> wake_up(&config->recv_wq);
> 3. Create nbd server
> nbd-server 8000 /tmp/disk
> 4. Create nbd client
> nbd-client localhost 8000 /dev/nbd1
> Then will trigger above issue.
>
> Reason is when add delay in recv_work, lead to relase the last reference
> of 'nbd->config_refs'. nbd_config_put will call flush_workqueue to make
> all work finish. Obviously, it will lead to deadloop.
> To solve this issue, we must ensure 'recv_work' all exit before release
> the last 'nbd->config_refs' reference count.
>
> Signed-off-by: Ye Bin <yebin10@huawei.com>
This isn't the way to fix this. The crux of the problem is the fact that we
destroy the recv_workq at config time anyway, since it is tied to the nbd
device.
Fix this instead by getting rid of that, and have the nbd device teardown the
recv_workq when we destory the nbd device. Alloc it on device allocation and
stop alloc'ing it with the config. That way we don't have this weird thing
where we need recv threads to drop their config before the last thing.
And while you're at it would you fix recv_work() so it doesn't modify config
after it drops it's reference? Thanks,
Josef
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH -next] nbd: Fix hungtask when nbd_config_put
2021-10-20 14:04 ` Josef Bacik
@ 2021-10-20 14:23 ` yebin
0 siblings, 0 replies; 3+ messages in thread
From: yebin @ 2021-10-20 14:23 UTC (permalink / raw)
To: Josef Bacik; +Cc: axboe, linux-block, nbd, linux-kernel
On 2021/10/20 22:04, Josef Bacik wrote:
> On Wed, Oct 20, 2021 at 05:48:30PM +0800, Ye Bin wrote:
>> I got follow issue:
>> [ 247.381177] INFO: task kworker/u10:0:47 blocked for more than 120 seconds.
>> [ 247.382644] Not tainted 4.19.90-dirty #140
>> [ 247.383502] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
>> [ 247.385027] Call Trace:
>> [ 247.388384] schedule+0xb8/0x3c0
>> [ 247.388966] schedule_timeout+0x2b4/0x380
>> [ 247.392815] wait_for_completion+0x367/0x510
>> [ 247.397713] flush_workqueue+0x32b/0x1340
>> [ 247.402700] drain_workqueue+0xda/0x3c0
>> [ 247.403442] destroy_workqueue+0x7b/0x690
>> [ 247.405014] nbd_config_put.cold+0x2f9/0x5b6
>> [ 247.405823] recv_work+0x1fd/0x2b0
>> [ 247.406485] process_one_work+0x70b/0x1610
>> [ 247.407262] worker_thread+0x5a9/0x1060
>> [ 247.408699] kthread+0x35e/0x430
>> [ 247.410918] ret_from_fork+0x1f/0x30
>>
>> We can reprodeuce issue as follows:
>> 1. Inject memory fault in nbd_start_device
>> @@ -1244,10 +1248,18 @@ static int nbd_start_device(struct nbd_device *nbd)
>> nbd_dev_dbg_init(nbd);
>> for (i = 0; i < num_connections; i++) {
>> struct recv_thread_args *args;
>> -
>> - args = kzalloc(sizeof(*args), GFP_KERNEL);
>> +
>> + if (i == 1) {
>> + args = NULL;
>> + printk("%s: inject malloc error\n", __func__);
>> + }
>> + else
>> + args = kzalloc(sizeof(*args), GFP_KERNEL);
>> 2. Inject delay in recv_work
>> @@ -757,6 +760,8 @@ static void recv_work(struct work_struct *work)
>>
>> blk_mq_complete_request(blk_mq_rq_from_pdu(cmd));
>> }
>> + printk("%s: comm=%s pid=%d\n", __func__, current->comm, current->pid);
>> + mdelay(5 * 1000);
>> nbd_config_put(nbd);
>> atomic_dec(&config->recv_threads);
>> wake_up(&config->recv_wq);
>> 3. Create nbd server
>> nbd-server 8000 /tmp/disk
>> 4. Create nbd client
>> nbd-client localhost 8000 /dev/nbd1
>> Then will trigger above issue.
>>
>> Reason is when add delay in recv_work, lead to relase the last reference
>> of 'nbd->config_refs'. nbd_config_put will call flush_workqueue to make
>> all work finish. Obviously, it will lead to deadloop.
>> To solve this issue, we must ensure 'recv_work' all exit before release
>> the last 'nbd->config_refs' reference count.
>>
>> Signed-off-by: Ye Bin <yebin10@huawei.com>
> This isn't the way to fix this. The crux of the problem is the fact that we
> destroy the recv_workq at config time anyway, since it is tied to the nbd
> device.
>
> Fix this instead by getting rid of that, and have the nbd device teardown the
> recv_workq when we destory the nbd device. Alloc it on device allocation and
> stop alloc'ing it with the config. That way we don't have this weird thing
> where we need recv threads to drop their config before the last thing.
>
> And while you're at it would you fix recv_work() so it doesn't modify config
> after it drops it's reference? Thanks,
>
> Josef
> .
Thank you for your reply, I'll make another patch according to your
suggestion.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-10-20 14:23 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-20 9:48 [PATCH -next] nbd: Fix hungtask when nbd_config_put Ye Bin
2021-10-20 14:04 ` Josef Bacik
2021-10-20 14:23 ` yebin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).