* Re: iscsi_trx going into D state
From: Robert LeBlanc @ 2016-10-19 16:41 UTC (permalink / raw)
To: Nicholas A. Bellinger
Cc: Zhu Lingshan, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-scsi-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1476858359.8490.97.camel-XoQW25Eq2zviZyQQd+hFbcojREIfoBdhmpATvIKMPHk@public.gmane.org>
Nicholas,
I didn't have high hopes for the patch because we were not seeing
TMR_ABORT_TASK (or 'abort') in dmesg or /var/log/messages, but it
seemed to help regardless. Our clients finally OOMed from the hung
sessions, so we are having to reboot them and we will do some more
testing. We haven't put the updated kernel on our clients yet. Our
clients have iSCSI root disks so I'm not sure if we can get a vmcore
on those, but we will do what we can to get you a vmcore from the
target if it happens again.
As far as our configuration: It is a superMicro box with 6 SAMSUNG
MZ7LM3T8HCJM-00005 SSDs. Two are for root and four are in mdadm
RAID-10 for exporting via iSCSI/iSER. We have ZFS on top of the
RAID-10 for checksum and snapshots only and we export ZVols to the
clients (one or more per VM on the client). We do not persist the
export info (targetcli saveconfig), but regenerate it from scripts.
The client receives two or more of these exports and puts them in a
RAID-1 device. The exports are served by iSER one one port and also by
normal iSCSI on a different port for compatibility, but not normally
used. If you need more info about the config, please let me know. It
was kind of a vague request so I'm not sure what exactly is important
to you.
Thanks for helping us with this,
Robert LeBlanc
When we have problems, we usually see this in the logs:
Oct 17 08:57:50 prv-0-12-sanstack kernel: iSCSI Login timeout on
Network Portal 0.0.0.0:3260
Oct 17 08:57:50 prv-0-12-sanstack kernel: Unexpected ret: -104 send data 48
Oct 17 08:57:50 prv-0-12-sanstack kernel: tx_data returned -32, expecting 48.
Oct 17 08:57:50 prv-0-12-sanstack kernel: iSCSI Login negotiation failed.
I found some backtraces in the logs, not sure if this is helpful, this
is before your patch (your patch booted at Oct 18 10:36:59):
Oct 17 15:43:12 prv-0-12-sanstack kernel: INFO: rcu_sched
self-detected stall on CPU
Oct 17 15:43:12 prv-0-12-sanstack kernel: #0115-...: (41725 ticks this
GP) idle=b59/140000000000001/0 softirq=535/535 fqs=30992
Oct 17 15:43:12 prv-0-12-sanstack kernel: #011 (t=42006 jiffies g=1550
c=1549 q=0)
Oct 17 15:43:12 prv-0-12-sanstack kernel: Task dump for CPU 5:
Oct 17 15:43:12 prv-0-12-sanstack kernel: kworker/u68:2 R running
task 0 17967 2 0x00000008
Oct 17 15:43:12 prv-0-12-sanstack kernel: Workqueue: isert_comp_wq
isert_cq_work [ib_isert]
Oct 17 15:43:12 prv-0-12-sanstack kernel: ffff883f4c0dca80
00000000af8ca7a4 ffff883f7fb43da8 ffffffff810ac83f
Oct 17 15:43:12 prv-0-12-sanstack kernel: 0000000000000005
ffffffff81adb680 ffff883f7fb43dc0 ffffffff810af179
Oct 17 15:43:12 prv-0-12-sanstack kernel: 0000000000000006
ffff883f7fb43df0 ffffffff810e1c10 ffff883f7fb57b80
Oct 17 15:43:12 prv-0-12-sanstack kernel: Call Trace:
Oct 17 15:43:12 prv-0-12-sanstack kernel: <IRQ> [<ffffffff810ac83f>]
sched_show_task+0xaf/0x110
Oct 17 15:43:12 prv-0-12-sanstack kernel: [<ffffffff810af179>]
dump_cpu_task+0x39/0x40
Oct 17 15:43:12 prv-0-12-sanstack kernel: [<ffffffff810e1c10>]
rcu_dump_cpu_stacks+0x80/0xb0
Oct 17 15:43:12 prv-0-12-sanstack kernel: [<ffffffff810e6040>]
rcu_check_callbacks+0x540/0x820
Oct 17 15:43:12 prv-0-12-sanstack kernel: [<ffffffff810afd51>] ?
account_system_time+0x81/0x110
Oct 17 15:43:12 prv-0-12-sanstack kernel: [<ffffffff810fa9a0>] ?
tick_sched_do_timer+0x50/0x50
Oct 17 15:43:12 prv-0-12-sanstack kernel: [<ffffffff810eb4d9>]
update_process_times+0x39/0x60
Oct 17 15:43:12 prv-0-12-sanstack kernel: [<ffffffff810fa755>]
tick_sched_handle.isra.17+0x25/0x60
Oct 17 15:43:12 prv-0-12-sanstack kernel: [<ffffffff810fa9dd>]
tick_sched_timer+0x3d/0x70
Oct 17 15:43:12 prv-0-12-sanstack kernel: [<ffffffff810ec0c2>]
__hrtimer_run_queues+0x102/0x290
Oct 17 15:43:12 prv-0-12-sanstack kernel: [<ffffffff810ec5a8>]
hrtimer_interrupt+0xa8/0x1a0
Oct 17 15:43:12 prv-0-12-sanstack kernel: [<ffffffff81052c65>]
local_apic_timer_interrupt+0x35/0x60
Oct 17 15:43:12 prv-0-12-sanstack kernel: [<ffffffff8172343d>]
smp_apic_timer_interrupt+0x3d/0x50
Oct 17 15:43:12 prv-0-12-sanstack kernel: [<ffffffff817216f7>]
apic_timer_interrupt+0x87/0x90
Oct 17 15:43:12 prv-0-12-sanstack kernel: <EOI> [<ffffffff810d70fe>]
? console_unlock+0x41e/0x4e0
Oct 17 15:43:12 prv-0-12-sanstack kernel: [<ffffffff810d74bc>]
vprintk_emit+0x2fc/0x500
Oct 17 15:43:12 prv-0-12-sanstack kernel: [<ffffffff810d783f>]
vprintk_default+0x1f/0x30
Oct 17 15:43:12 prv-0-12-sanstack kernel: [<ffffffff81174c2a>] printk+0x5d/0x74
Oct 17 15:43:12 prv-0-12-sanstack kernel: [<ffffffff814bc351>]
transport_lookup_cmd_lun+0x1d1/0x200
Oct 17 15:43:12 prv-0-12-sanstack kernel: [<ffffffff814edcf0>]
iscsit_setup_scsi_cmd+0x230/0x540
Oct 17 15:43:12 prv-0-12-sanstack kernel: [<ffffffffa0890bf3>]
isert_rx_do_work+0x3f3/0x7f0 [ib_isert]
Oct 17 15:43:12 prv-0-12-sanstack kernel: [<ffffffffa0891174>]
isert_cq_work+0x184/0x770 [ib_isert]
Oct 17 15:43:12 prv-0-12-sanstack kernel: [<ffffffff8109734f>]
process_one_work+0x14f/0x400
Oct 17 15:43:12 prv-0-12-sanstack kernel: [<ffffffff81097bc4>]
worker_thread+0x114/0x470
Oct 17 15:43:12 prv-0-12-sanstack kernel: [<ffffffff8171c55a>] ?
__schedule+0x34a/0x7f0
Oct 17 15:43:12 prv-0-12-sanstack kernel: [<ffffffff81097ab0>] ?
rescuer_thread+0x310/0x310
Oct 17 15:43:12 prv-0-12-sanstack kernel: [<ffffffff8109d708>] kthread+0xd8/0xf0
Oct 17 15:43:12 prv-0-12-sanstack kernel: [<ffffffff8109d630>] ?
kthread_park+0x60/0x60
Oct 17 15:43:12 prv-0-12-sanstack kernel: [<ffffffff81720c8f>]
ret_from_fork+0x3f/0x70
Oct 17 15:43:12 prv-0-12-sanstack kernel: [<ffffffff8109d630>] ?
kthread_park+0x60/0x60
Oct 17 16:34:03 prv-0-12-sanstack kernel: INFO: rcu_sched
self-detected stall on CPU
Oct 17 16:34:03 prv-0-12-sanstack kernel: #01128-...: (5999 ticks this
GP) idle=2f9/140000000000001/0 softirq=457/457 fqs=4830
Oct 17 16:34:03 prv-0-12-sanstack kernel: #011 (t=6000 jiffies g=3546
c=3545 q=0)
Oct 17 16:34:03 prv-0-12-sanstack kernel: Task dump for CPU 28:
Oct 17 16:34:03 prv-0-12-sanstack kernel: iscsi_np R running
task 0 16597 2 0x0000000c
Oct 17 16:34:03 prv-0-12-sanstack kernel: ffff887f40350000
00000000b98a67bb ffff887f7f503da8 ffffffff810ac8ff
Oct 17 16:34:03 prv-0-12-sanstack kernel: 000000000000001c
ffffffff81adb680 ffff887f7f503dc0 ffffffff810af239
Oct 17 16:34:03 prv-0-12-sanstack kernel: 000000000000001d
ffff887f7f503df0 ffffffff810e1cd0 ffff887f7f517b80
Oct 17 16:34:03 prv-0-12-sanstack kernel: Call Trace:
Oct 17 16:34:03 prv-0-12-sanstack kernel: <IRQ> [<ffffffff810ac8ff>]
sched_show_task+0xaf/0x110
Oct 17 16:34:03 prv-0-12-sanstack kernel: [<ffffffff810af239>]
dump_cpu_task+0x39/0x40
Oct 17 16:34:03 prv-0-12-sanstack kernel: [<ffffffff810e1cd0>]
rcu_dump_cpu_stacks+0x80/0xb0
Oct 17 16:34:03 prv-0-12-sanstack kernel: [<ffffffff810e6100>]
rcu_check_callbacks+0x540/0x820
Oct 17 16:34:03 prv-0-12-sanstack kernel: [<ffffffff810afe11>] ?
account_system_time+0x81/0x110
Oct 17 16:34:03 prv-0-12-sanstack kernel: [<ffffffff810faa60>] ?
tick_sched_do_timer+0x50/0x50
Oct 17 16:34:03 prv-0-12-sanstack kernel: [<ffffffff810eb599>]
update_process_times+0x39/0x60
Oct 17 16:34:03 prv-0-12-sanstack kernel: [<ffffffff810fa815>]
tick_sched_handle.isra.17+0x25/0x60
Oct 17 16:34:03 prv-0-12-sanstack kernel: [<ffffffff810faa9d>]
tick_sched_timer+0x3d/0x70
Oct 17 16:34:03 prv-0-12-sanstack kernel: [<ffffffff810ec182>]
__hrtimer_run_queues+0x102/0x290
Oct 17 16:34:03 prv-0-12-sanstack kernel: [<ffffffff810ec668>]
hrtimer_interrupt+0xa8/0x1a0
Oct 17 16:34:03 prv-0-12-sanstack kernel: [<ffffffff81052c65>]
local_apic_timer_interrupt+0x35/0x60
Oct 17 16:34:03 prv-0-12-sanstack kernel: [<ffffffff81723cbd>]
smp_apic_timer_interrupt+0x3d/0x50
Oct 17 16:34:03 prv-0-12-sanstack kernel: [<ffffffff81721f77>]
apic_timer_interrupt+0x87/0x90
Oct 17 16:34:03 prv-0-12-sanstack kernel: <EOI> [<ffffffff810d71be>]
? console_unlock+0x41e/0x4e0
Oct 17 16:34:03 prv-0-12-sanstack kernel: [<ffffffff810d757c>]
vprintk_emit+0x2fc/0x500
Oct 17 16:34:03 prv-0-12-sanstack kernel: [<ffffffff810d78ff>]
vprintk_default+0x1f/0x30
Oct 17 16:34:03 prv-0-12-sanstack kernel: [<ffffffff81174dde>] printk+0x5d/0x74
Oct 17 16:34:03 prv-0-12-sanstack kernel: [<ffffffff814e71ad>]
iscsi_target_locate_portal+0x62d/0x6f0
Oct 17 16:34:03 prv-0-12-sanstack kernel: [<ffffffff814e5100>]
iscsi_target_login_thread+0x6f0/0xfc0
Oct 17 16:34:03 prv-0-12-sanstack kernel: [<ffffffff814e4a10>] ?
iscsi_target_login_sess_out+0x250/0x250
Oct 17 16:34:03 prv-0-12-sanstack kernel: [<ffffffff8109d7c8>] kthread+0xd8/0xf0
Oct 17 16:34:03 prv-0-12-sanstack kernel: [<ffffffff8109d6f0>] ?
kthread_park+0x60/0x60
Oct 17 16:34:03 prv-0-12-sanstack kernel: [<ffffffff8172150f>]
ret_from_fork+0x3f/0x70
Oct 17 16:34:03 prv-0-12-sanstack kernel: [<ffffffff8109d6f0>] ?
kthread_park+0x60/0x60
I don't think this one is related, but it happened a couple of times:
Oct 17 11:46:52 prv-0-12-sanstack kernel: INFO: rcu_sched
self-detected stall on CPU
Oct 17 11:46:52 prv-0-12-sanstack kernel: #01119-...: (5999 ticks this
GP) idle=727/140000000000001/0 softirq=1346/1346 fqs=4990
Oct 17 11:46:52 prv-0-12-sanstack kernel: #011 (t=6000 jiffies g=4295
c=4294 q=0)
Oct 17 11:46:52 prv-0-12-sanstack kernel: Task dump for CPU 19:
Oct 17 11:46:52 prv-0-12-sanstack kernel: kworker/19:1 R running
task 0 301 2 0x00000008
Oct 17 11:46:52 prv-0-12-sanstack kernel: Workqueue:
events_power_efficient fb_flashcursor
Oct 17 11:46:52 prv-0-12-sanstack kernel: ffff883f6009ca80
00000000010a7cdd ffff883f7fcc3da8 ffffffff810ac8ff
Oct 17 11:46:52 prv-0-12-sanstack kernel: 0000000000000013
ffffffff81adb680 ffff883f7fcc3dc0 ffffffff810af239
Oct 17 11:46:52 prv-0-12-sanstack kernel: 0000000000000014
ffff883f7fcc3df0 ffffffff810e1cd0 ffff883f7fcd7b80
Oct 17 11:46:52 prv-0-12-sanstack kernel: Call Trace:
Oct 17 11:46:52 prv-0-12-sanstack kernel: <IRQ> [<ffffffff810ac8ff>]
sched_show_task+0xaf/0x110
Oct 17 11:46:52 prv-0-12-sanstack kernel: [<ffffffff810af239>]
dump_cpu_task+0x39/0x40
Oct 17 11:46:52 prv-0-12-sanstack kernel: [<ffffffff810e1cd0>]
rcu_dump_cpu_stacks+0x80/0xb0
Oct 17 11:46:52 prv-0-12-sanstack kernel: [<ffffffff810e6100>]
rcu_check_callbacks+0x540/0x820
Oct 17 11:46:52 prv-0-12-sanstack kernel: [<ffffffff810afe11>] ?
account_system_time+0x81/0x110
Oct 17 11:46:52 prv-0-12-sanstack kernel: [<ffffffff810faa60>] ?
tick_sched_do_timer+0x50/0x50
Oct 17 11:46:52 prv-0-12-sanstack kernel: [<ffffffff810eb599>]
update_process_times+0x39/0x60
Oct 17 11:46:52 prv-0-12-sanstack kernel: [<ffffffff810fa815>]
tick_sched_handle.isra.17+0x25/0x60
Oct 17 11:46:52 prv-0-12-sanstack kernel: [<ffffffff810faa9d>]
tick_sched_timer+0x3d/0x70
Oct 17 11:46:52 prv-0-12-sanstack kernel: [<ffffffff810ec182>]
__hrtimer_run_queues+0x102/0x290
Oct 17 11:46:52 prv-0-12-sanstack kernel: [<ffffffff810ec668>]
hrtimer_interrupt+0xa8/0x1a0
Oct 17 11:46:52 prv-0-12-sanstack kernel: [<ffffffff81052c65>]
local_apic_timer_interrupt+0x35/0x60
Oct 17 11:46:52 prv-0-12-sanstack kernel: [<ffffffff81723cbd>]
smp_apic_timer_interrupt+0x3d/0x50
Oct 17 11:46:52 prv-0-12-sanstack kernel: [<ffffffff81721f77>]
apic_timer_interrupt+0x87/0x90
Oct 17 11:46:52 prv-0-12-sanstack kernel: <EOI> [<ffffffff810d71be>]
? console_unlock+0x41e/0x4e0
Oct 17 11:46:52 prv-0-12-sanstack kernel: [<ffffffff813866ad>]
fb_flashcursor+0x5d/0x140
Oct 17 11:46:52 prv-0-12-sanstack kernel: [<ffffffff8138bc00>] ?
bit_clear+0x110/0x110
Oct 17 11:46:52 prv-0-12-sanstack kernel: [<ffffffff8109740f>]
process_one_work+0x14f/0x400
Oct 17 11:46:52 prv-0-12-sanstack kernel: [<ffffffff81097c84>]
worker_thread+0x114/0x470
Oct 17 11:46:52 prv-0-12-sanstack kernel: [<ffffffff8171cdda>] ?
__schedule+0x34a/0x7f0
Oct 17 11:46:52 prv-0-12-sanstack kernel: [<ffffffff81097b70>] ?
rescuer_thread+0x310/0x310
Oct 17 11:46:52 prv-0-12-sanstack kernel: [<ffffffff8109d7c8>] kthread+0xd8/0xf0
Oct 17 11:46:52 prv-0-12-sanstack kernel: [<ffffffff8109d6f0>] ?
kthread_park+0x60/0x60
Oct 17 11:46:52 prv-0-12-sanstack kernel: [<ffffffff8172150f>]
ret_from_fork+0x3f/0x70
Oct 17 11:46:52 prv-0-12-sanstack kernel: [<ffffffff8109d6f0>] ?
kthread_park+0x60/0x60
----------------
Robert LeBlanc
PGP Fingerprint 79A2 9CA4 6CC4 45DD A904 C70E E654 3BB2 FA62 B9F1
On Wed, Oct 19, 2016 at 12:25 AM, Nicholas A. Bellinger
<nab-IzHhD5pYlfBP7FQvKIMDCQ@public.gmane.org> wrote:
> On Tue, 2016-10-18 at 16:13 -0600, Robert LeBlanc wrote:
>> Nicholas,
>>
>> We patched this in and for the first time in many reboots, we didn't
>> have iSCSI going straight into D state. We have had to work on a
>> couple of other things, so we don't know if this is just a coincidence
>> or not. We will reboot back into the old kernel and back a few times
>> and do some more testing, but so far it has given us a little bit of
>> hope that we may be narrowing down on the root cause. We will report
>> back once we have some more info.
>>
>> Thank you,
>> Robert LeBlanc
>> ----------------
>> Robert LeBlanc
>> PGP Fingerprint 79A2 9CA4 6CC4 45DD A904 C70E E654 3BB2 FA62 B9F1
>>
>
> Hello Robert,
>
> Thanks for the update. Btw, if the original /var/log/messages
> reproduction logs for iser-target are still handy, I'm happy to have
> a look to confirm. Feel free to send them along here, or off-list if
> necessary.
>
> For further reference, you can also enable Linux kernel crash dump
> (LKCD) at build time using CONFIG_CRASH_DUMP=y, so it's possible to
> manually generate a vmcore dumpfile of the running system via 'echo c
>> /proc/sysrq-trigger', once the bug occurs.
>
> http://cateee.net/lkddb/web-lkddb/CRASH_DUMP.html
>
> Note in order to fully debug within this in a LKCD environment, it
> requires the vmcore dump from /var/crash/, unstripped vmlinux,
> target_core_mod, iscsi_target_mod and ib_isert modules matching the
> specific particular x86_64 build setup of the running system.
>
> Also, can you share a bit more about the details of your particular
> iser-target + backend setup..?
>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: New providers in rdma-core
From: Christoph Hellwig @ 2016-10-19 16:39 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Amrani, Ram, Lijun Ou, aditr-pghWNbHTmq7QT0dZR+AlfA, Knut Omang,
Doug Ledford, Jason Gunthorpe, linux-rdma
In-Reply-To: <CALq1K=+nEP0jFOCxj30HA0+bg0L1bF5+xiBcAC2bRwUz+M6+eQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Wed, Oct 19, 2016 at 07:18:04PM +0300, Leon Romanovsky wrote:
> Hi All,
>
> I want to remind you that latest kernel is exposing different vendor
> specific structures (include/uapi/rdma/*-abi.,h) and new coming user
> space drivers should use these headers directly and not copy/paste the
> code from them.
>
> In the future, we will convert already accepted drivers too.
Requiring installed super updtodate kernel headers isn't going to
help development. It would be good to keep a copy of the kernel
headers in the rdma-core repo, similar to what most kernel tools
tied to bleeding edge ABIs do.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: Introduction of libqedr to the Consolidated Userspace RDMA Library Repo
From: Leon Romanovsky @ 2016-10-19 16:22 UTC (permalink / raw)
To: Amrani, Ram
Cc: Jason Gunthorpe,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Elior, Ariel,
Kalderon, Michal, Borundia, Rajesh,
dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
In-Reply-To: <SN1PR07MB220719CE0C668A321520311DF8D20-mikhvbZlbf8TSoR2DauN2+FPX92sqiQdvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 634 bytes --]
On Wed, Oct 19, 2016 at 04:08:12PM +0000, Amrani, Ram wrote:
> Hi Jason,
> I've created a user account on github, forked rdma_plumbing and added libqedr in multiple patches,
> after I've verified that it builds warning free and in-line with previous cleanups.
> I've sent a pull request to linux-rdma rather than rdma-plumbing. Is that OK?
Please post them to the ML for review.
Thanks
>
> Thanks,
> Ram
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* New providers in rdma-core
From: Leon Romanovsky @ 2016-10-19 16:18 UTC (permalink / raw)
To: Amrani, Ram, Lijun Ou, aditr-pghWNbHTmq7QT0dZR+AlfA, Knut Omang
Cc: Doug Ledford, Jason Gunthorpe, linux-rdma
Hi All,
I want to remind you that latest kernel is exposing different vendor
specific structures (include/uapi/rdma/*-abi.,h) and new coming user
space drivers should use these headers directly and not copy/paste the
code from them.
In the future, we will convert already accepted drivers too.
Thanks
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 04/11] blk-mq: Introduce blk_mq_quiesce_queue()
From: Bart Van Assche @ 2016-10-19 16:13 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jens Axboe, James Bottomley, Martin K. Petersen, Mike Snitzer,
Doug Ledford, Keith Busch, Ming Lin, Laurence Oberman,
linux-block@vger.kernel.org, linux-scsi@vger.kernel.org,
linux-rdma@vger.kernel.org, linux-nvme@lists.infradead.org
In-Reply-To: <20161019132321.GD6323@lst.de>
On 10/19/2016 06:23 AM, Christoph Hellwig wrote:
>> +/**
>> + * blk_mq_quiesce_queue() - wait until all ongoing queue_rq calls have finished
>> + *
>> + * Note: this function does not prevent that the struct request end_io()
>> + * callback function is invoked. Additionally, it is not prevented that
>> + * new queue_rq() calls occur unless the queue has been stopped first.
>> + */
>> +void blk_mq_quiesce_queue(struct request_queue *q)
>
> If this is intended to be a kerneldoc comment you need to document the 'q'
> parameter. If not you should drop the magic "/**" marker.
Good catch. I will document the 'q' parameter.
>> +static void __blk_mq_run_hw_queue(struct blk_mq_hw_ctx *hctx)
>> +{
>> + int srcu_idx;
>> +
>> + WARN_ON(!cpumask_test_cpu(raw_smp_processor_id(), hctx->cpumask) &&
>> + cpu_online(hctx->next_cpu));
>> +
>> + if (!(hctx->flags & BLK_MQ_F_BLOCKING)) {
>> + rcu_read_lock();
>> + blk_mq_process_rq_list(hctx);
>> + rcu_read_unlock();
>> + } else {
>> + srcu_idx = srcu_read_lock(&hctx->queue_rq_srcu);
>> + blk_mq_process_rq_list(hctx);
>> + srcu_read_unlock(&hctx->queue_rq_srcu, srcu_idx);
>> + }
>> +}
>
> Can you document these synchronization changes in detail in the changelog?
Sure, I will do that.
>> +static void blk_mq_try_issue_directly(struct blk_mq_hw_ctx *hctx,
>> + struct request *rq, blk_qc_t *cookie)
>> +{
>> + if (blk_mq_hctx_stopped(hctx) ||
>> + blk_mq_direct_issue_request(rq, cookie) != 0)
>> + blk_mq_insert_request(rq, false, true, true);
>> +}
>
> Any reason not to merge this function with blk_mq_direct_issue_request?
That sounds like a good idea to me. I will make the proposed change.
Bart.
^ permalink raw reply
* RE: Introduction of libqedr to the Consolidated Userspace RDMA Library Repo
From: Amrani, Ram @ 2016-10-19 16:08 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Elior, Ariel,
Kalderon, Michal, Borundia, Rajesh,
dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
In-Reply-To: <20161014164229.GB6258-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
Hi Jason,
I've created a user account on github, forked rdma_plumbing and added libqedr in multiple patches,
after I've verified that it builds warning free and in-line with previous cleanups.
I've sent a pull request to linux-rdma rather than rdma-plumbing. Is that OK?
Thanks,
Ram
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH net-next 6/6] net: use core MTU range checking in misc drivers
From: Sabrina Dubroca @ 2016-10-19 16:05 UTC (permalink / raw)
To: Jarod Wilson
Cc: linux-kernel, netdev, Stefan Richter, Faisal Latif, linux-rdma,
Cliff Whickman, Robin Holt, Jes Sorensen, Marek Lindner,
Simon Wunderlich, Antonio Quartulli
In-Reply-To: <20161019023333.15760-7-jarod@redhat.com>
2016-10-18, 22:33:33 -0400, Jarod Wilson wrote:
[...]
> diff --git a/drivers/firewire/net.c b/drivers/firewire/net.c
> index 309311b..b5f125c 100644
> --- a/drivers/firewire/net.c
> +++ b/drivers/firewire/net.c
> @@ -1349,15 +1349,6 @@ static netdev_tx_t fwnet_tx(struct sk_buff *skb, struct net_device *net)
> return NETDEV_TX_OK;
> }
>
> -static int fwnet_change_mtu(struct net_device *net, int new_mtu)
> -{
> - if (new_mtu < 68)
> - return -EINVAL;
> -
> - net->mtu = new_mtu;
> - return 0;
> -}
> -
This doesn't do any upper bound checking.
> static const struct ethtool_ops fwnet_ethtool_ops = {
> .get_link = ethtool_op_get_link,
> };
> @@ -1366,7 +1357,6 @@ static const struct net_device_ops fwnet_netdev_ops = {
> .ndo_open = fwnet_open,
> .ndo_stop = fwnet_stop,
> .ndo_start_xmit = fwnet_tx,
> - .ndo_change_mtu = fwnet_change_mtu,
> };
>
> static void fwnet_init_dev(struct net_device *net)
> @@ -1481,6 +1471,8 @@ static int fwnet_probe(struct fw_unit *unit,
> max_mtu = (1 << (card->max_receive + 1))
> - sizeof(struct rfc2734_header) - IEEE1394_GASP_HDR_SIZE;
> net->mtu = min(1500U, max_mtu);
> + net->min_mtu = ETH_MIN_MTU;
> + net->max_mtu = net->mtu;
But that will now prevent increasing the MTU above the initial value?
--
Sabrina
^ permalink raw reply
* Re: [PATCH v3 02/11] blk-mq: Introduce blk_mq_hctx_stopped()
From: Bart Van Assche @ 2016-10-19 15:58 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jens Axboe, James Bottomley, Martin K. Petersen, Mike Snitzer,
Doug Ledford, Keith Busch, Ming Lin, Laurence Oberman,
linux-block@vger.kernel.org, linux-scsi@vger.kernel.org,
linux-rdma@vger.kernel.org, linux-nvme@lists.infradead.org
In-Reply-To: <20161019131900.GB6323@lst.de>
On 10/19/2016 06:19 AM, Christoph Hellwig wrote:
> On Tue, Oct 18, 2016 at 02:49:09PM -0700, Bart Van Assche wrote:
>> Multiple functions test the BLK_MQ_S_STOPPED bit so introduce
>> a helper function that performs this test.
>
> Looks sensible. Any reason to have it in the public blk-mq.h instead
> of the private one, though? I see that dm is using it with this patch,
> but that usage should go away once your full series is merged, right?
Hello Christoph,
Moving the blk_mq_hctx_stopped() declaration from the public to the
private blk-mq.h header file should be possible. I will look into this.
Bart.
^ permalink raw reply
* Re: [PATCH 6/8] IB/hns: Replace counting semaphore event_sem with wait condition
From: Arnd Bergmann @ 2016-10-19 15:15 UTC (permalink / raw)
To: Binoy Jayan
Cc: Doug Ledford, Sean Hefty, Hal Rosenstock, linux-rdma,
Linux kernel mailing list
In-Reply-To: <CAHv-k__KG4wZ+dCEALRjw1U_eS2eQA63Qy=mzFjXD5SqFz85ew@mail.gmail.com>
On Tuesday, October 18, 2016 10:46:45 AM CEST Binoy Jayan wrote:
> Thank you for replying for the questions. I''ll look for alternatives
> for patches 6,7 and 8 and resend the series.
Ok, thanks!
I also looked at patch 8 some more and noticed that those four
functions all do the exact same sequence:
- initialize a mlx5_ib_umr_context on the stack
- assign "umrwr.wr.wr_cqe = &umr_context.cqe"
- take the semaphore
- call ib_post_send with a single ib_send_wr
- wait for the mlx5_ib_umr_done() function to get called
- if we get back a failure, print a warning and return -EFAULT.
- release the semaphore
Moving all of these into a shared helper function would be
a good cleanup, and it leaves only a single function using
the semaphore, which can then be rewritten to use something
else.
The existing completion in there can be simplified to a
wait_event, since we are waiting for the return value to
be filled.
Arnd
^ permalink raw reply
* Re: [PATCH net-next 6/6] net: use core MTU range checking in misc drivers
From: Robin Holt @ 2016-10-19 14:37 UTC (permalink / raw)
To: Jarod Wilson
Cc: LKML, netdev, Stefan Richter, Faisal Latif, linux-rdma,
Cliff Whickman, Jes Sorensen, Marek Lindner, Simon Wunderlich,
Antonio Quartulli
In-Reply-To: <20161019023333.15760-7-jarod@redhat.com>
On Tue, Oct 18, 2016 at 9:33 PM, Jarod Wilson <jarod@redhat.com> wrote:
> CC: netdev@vger.kernel.org
> CC: Stefan Richter <stefanr@s5r6.in-berlin.de>
> CC: Faisal Latif <faisal.latif@intel.com>
> CC: linux-rdma@vger.kernel.org
> CC: Cliff Whickman <cpw@sgi.com>
Acked-by: Robin Holt <robinmholt@gmail.com>
> CC: Jes Sorensen <jes@trained-monkey.org>
> CC: Marek Lindner <mareklindner@neomailbox.ch>
> CC: Simon Wunderlich <sw@simonwunderlich.de>
> CC: Antonio Quartulli <a@unstable.cc>
> Signed-off-by: Jarod Wilson <jarod@redhat.com>
^ permalink raw reply
* Re: [PATCHv12 0/3] rdmacg: IB/core: rdma controller support
From: Tejun Heo @ 2016-10-19 14:33 UTC (permalink / raw)
To: Parav Pandit
Cc: Leon Romanovsky, cgroups-u79uwXL29TY76Z2rM5mHXA, linux-rdma,
Li Zefan, Johannes Weiner, Doug Ledford, Christoph Hellwig,
Liran Liss, Hefty, Sean, Jason Gunthorpe, Haggai Eran,
james.l.morris-QHcLZuEGTsvQT0dZR+AlfA, Or Gerlitz, Matan Barak
In-Reply-To: <CAG53R5UEvkPBM0yFrR=fvEzyCrku2q=rLZyDVrSs9q+3hgbSmQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hello,
On Wed, Oct 19, 2016 at 03:04:52PM +0530, Parav Pandit wrote:
> > Heh, we can go for per-mil and use %0 as the suffix if absolutely
> > necessary but is this a real issue?
>
> We need to provide current version to exceed the 100 containers limit first. :-)
> Please let me know if such user interface already exist as reference point.
> I prefer to avoid introducing such configuration.
We don't have such usage yet. rdmacg would be the first one doing
something like this.
> > I think it's a lot more consistent to implement all absoulte limits
> > through max.
>
> I agree. I will implement reporting actual max values instead of
> reporting "max" string so that percentage configuration can be done by
> the user space tools.
> (post merging of v12).
> Waiting for direction from Doug.
That doesn't sound like a good idea. What happens when the amount of
available resources changes either through underlying resource changes
or config changes in one of the ancestors? What "max" means is "no
limit is imposed" which is different from "limit it to 100% of what's
currently available".
Thanks.
--
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 11/11] nvme: Fix a race condition
From: Christoph Hellwig @ 2016-10-19 13:41 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, Christoph Hellwig, James Bottomley,
Martin K. Petersen, Mike Snitzer, Doug Ledford, Keith Busch,
Ming Lin, Laurence Oberman, linux-block@vger.kernel.org,
linux-scsi@vger.kernel.org, linux-rdma@vger.kernel.org,
linux-nvme@lists.infradead.org
In-Reply-To: <2a29b7ec-0113-3450-9a36-b925b47b1fb0@sandisk.com>
Hi Bart,
this looks great!
Reviewed-by: Christoph Hellwig <hch@lst.de>
Some minor nitpicks below:
> void nvme_requeue_req(struct request *req)
> {
> + blk_mq_requeue_request(req, true);
> }
> EXPORT_SYMBOL_GPL(nvme_requeue_req);
Please just remove the nvme_requeue_req wrapper.
>
> @@ -2074,11 +2068,14 @@ EXPORT_SYMBOL_GPL(nvme_kill_queues);
> void nvme_stop_queues(struct nvme_ctrl *ctrl)
> {
> struct nvme_ns *ns;
> + struct request_queue *q;
>
> mutex_lock(&ctrl->namespaces_mutex);
> list_for_each_entry(ns, &ctrl->namespaces, list) {
> + q = ns->queue;
> + blk_mq_cancel_requeue_work(q);
> + blk_mq_stop_hw_queues(q);
> + blk_mq_quiesce_queue(q);
> }
I'd keep the q declaration in the minimal scope, e.g.
list_for_each_entry(ns, &ctrl->namespaces, list) {
struct request_queue *q = ns->queue;
blk_mq_cancel_requeue_work(q);
blk_mq_stop_hw_queues(q);
blk_mq_quiesce_queue(q);
}
^ permalink raw reply
* Re: [PATCH v3 10/11] nvme: Use BLK_MQ_S_STOPPED instead of QUEUE_FLAG_STOPPED in blk-mq code
From: Christoph Hellwig @ 2016-10-19 13:39 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, Christoph Hellwig, James Bottomley,
Martin K. Petersen, Mike Snitzer, Doug Ledford, Keith Busch,
Ming Lin, Laurence Oberman, linux-block@vger.kernel.org,
linux-scsi@vger.kernel.org, linux-rdma@vger.kernel.org,
linux-nvme@lists.infradead.org
In-Reply-To: <39c07429-2508-4123-8fe1-ecf407f8571d@sandisk.com>
Looks fine,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [PATCH v3 09/11] SRP transport, scsi-mq: Wait for .queue_rq() if necessary
From: Christoph Hellwig @ 2016-10-19 13:39 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, Christoph Hellwig, James Bottomley,
Martin K. Petersen, Mike Snitzer, Doug Ledford, Keith Busch,
Ming Lin, Laurence Oberman, linux-block@vger.kernel.org,
linux-scsi@vger.kernel.org, linux-rdma@vger.kernel.org,
linux-nvme@lists.infradead.org
In-Reply-To: <23d3615f-6905-0b60-a531-4a4d2cd9cf3f@sandisk.com>
Looks fine,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [PATCH v3 08/11] SRP transport: Move queuecommand() wait code to SCSI core
From: Christoph Hellwig @ 2016-10-19 13:38 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, Christoph Hellwig, James Bottomley,
Martin K. Petersen, Mike Snitzer, Doug Ledford, Keith Busch,
Ming Lin, Laurence Oberman,
linux-block-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <09a8efcb-d0e0-d5bd-288c-686b553e6326-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
Looks fine,
Reviewed-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 07/11] dm: Fix a race condition related to stopping and starting queues
From: Christoph Hellwig @ 2016-10-19 13:30 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, Christoph Hellwig, James Bottomley,
Martin K. Petersen, Mike Snitzer, Doug Ledford, Keith Busch,
Ming Lin, Laurence Oberman, linux-block@vger.kernel.org,
linux-scsi@vger.kernel.org, linux-rdma@vger.kernel.org,
linux-nvme@lists.infradead.org
In-Reply-To: <1e89a7d7-5cae-40b1-ce31-09d630e6f33d@sandisk.com>
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
FYI, I wonder how many of the blk_mq_stop_hw_queues do not need
the quiesce call. In the long run it might be better to have
blk_mq_stop_hw_queues to stop an quiesce, and have a
__blk_mq_stop_hw_queues variant to just stop.
^ permalink raw reply
* Re: [PATCH v3 06/11] dm: Use BLK_MQ_S_STOPPED instead of QUEUE_FLAG_STOPPED in blk-mq code
From: Christoph Hellwig @ 2016-10-19 13:28 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, Christoph Hellwig, James Bottomley,
Martin K. Petersen, Mike Snitzer, Doug Ledford, Keith Busch,
Ming Lin, Laurence Oberman, linux-block@vger.kernel.org,
linux-scsi@vger.kernel.org, linux-rdma@vger.kernel.org,
linux-nvme@lists.infradead.org
In-Reply-To: <e5facfd0-82d5-10cb-712b-13d7950b78b8@sandisk.com>
This looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
On Tue, Oct 18, 2016 at 02:51:33PM -0700, Bart Van Assche wrote:
> static void dm_mq_start_queue(struct request_queue *q)
> {
> - unsigned long flags;
> -
> - spin_lock_irqsave(q->queue_lock, flags);
> - queue_flag_clear(QUEUE_FLAG_STOPPED, q);
> - spin_unlock_irqrestore(q->queue_lock, flags);
> -
> blk_mq_start_stopped_hw_queues(q, true);
> blk_mq_kick_requeue_list(q);
FYI, I'm tempted to say we should always call blk_mq_kick_requeue_list
from blk_mq_start_stopped_hw_queues, but that's a separate issue.
^ permalink raw reply
* Re: [PATCH v3 05/11] blk-mq: Add a kick_requeue_list argument to blk_mq_requeue_request()
From: Christoph Hellwig @ 2016-10-19 13:23 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, Christoph Hellwig, James Bottomley,
Martin K. Petersen, Mike Snitzer, Doug Ledford, Keith Busch,
Ming Lin, Laurence Oberman, linux-block@vger.kernel.org,
linux-scsi@vger.kernel.org, linux-rdma@vger.kernel.org,
linux-nvme@lists.infradead.org
In-Reply-To: <7b52319b-03f7-e432-909a-2f46232770c6@sandisk.com>
On Tue, Oct 18, 2016 at 02:51:02PM -0700, Bart Van Assche wrote:
> Most blk_mq_requeue_request() and blk_mq_add_to_requeue_list() calls
> are followed by kicking the requeue list. Hence add an argument to
> these two functions that allows to kick the requeue list. This was
> proposed by Christoph Hellwig.
>
> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Thanks Bart, this looks fine to me:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [PATCH v3 04/11] blk-mq: Introduce blk_mq_quiesce_queue()
From: Christoph Hellwig @ 2016-10-19 13:23 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, Christoph Hellwig, James Bottomley,
Martin K. Petersen, Mike Snitzer, Doug Ledford, Keith Busch,
Ming Lin, Laurence Oberman, linux-block@vger.kernel.org,
linux-scsi@vger.kernel.org, linux-rdma@vger.kernel.org,
linux-nvme@lists.infradead.org
In-Reply-To: <e42a952c-f245-eb39-d0a1-0336035573f9@sandisk.com>
> +/**
> + * blk_mq_quiesce_queue() - wait until all ongoing queue_rq calls have finished
> + *
> + * Note: this function does not prevent that the struct request end_io()
> + * callback function is invoked. Additionally, it is not prevented that
> + * new queue_rq() calls occur unless the queue has been stopped first.
> + */
> +void blk_mq_quiesce_queue(struct request_queue *q)
If this is intended to be a kerneldoc comment you need to document the 'q'
parameter. If not you should drop the magic "/**" marker.
> +static void __blk_mq_run_hw_queue(struct blk_mq_hw_ctx *hctx)
> +{
> + int srcu_idx;
> +
> + WARN_ON(!cpumask_test_cpu(raw_smp_processor_id(), hctx->cpumask) &&
> + cpu_online(hctx->next_cpu));
> +
> + if (!(hctx->flags & BLK_MQ_F_BLOCKING)) {
> + rcu_read_lock();
> + blk_mq_process_rq_list(hctx);
> + rcu_read_unlock();
> + } else {
> + srcu_idx = srcu_read_lock(&hctx->queue_rq_srcu);
> + blk_mq_process_rq_list(hctx);
> + srcu_read_unlock(&hctx->queue_rq_srcu, srcu_idx);
> + }
> +}
Can you document these synchronization changes in detail in the changelog?
> +static void blk_mq_try_issue_directly(struct blk_mq_hw_ctx *hctx,
> + struct request *rq, blk_qc_t *cookie)
> +{
> + if (blk_mq_hctx_stopped(hctx) ||
> + blk_mq_direct_issue_request(rq, cookie) != 0)
> + blk_mq_insert_request(rq, false, true, true);
> +}
Any reason not to merge this function with blk_mq_direct_issue_request?
Otherwise this change looks fine to me.
^ permalink raw reply
* Re: [PATCH v3 03/11] blk-mq: Introduce blk_mq_queue_stopped()
From: Christoph Hellwig @ 2016-10-19 13:19 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, Christoph Hellwig, James Bottomley,
Martin K. Petersen, Mike Snitzer, Doug Ledford, Keith Busch,
Ming Lin, Laurence Oberman,
linux-block-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <b4e8cd03-0654-3e62-a559-ecc996676807-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
Looks good,
Reviewed-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 02/11] blk-mq: Introduce blk_mq_hctx_stopped()
From: Christoph Hellwig @ 2016-10-19 13:19 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, Christoph Hellwig, James Bottomley,
Martin K. Petersen, Mike Snitzer, Doug Ledford, Keith Busch,
Ming Lin, Laurence Oberman, linux-block@vger.kernel.org,
linux-scsi@vger.kernel.org, linux-rdma@vger.kernel.org,
linux-nvme@lists.infradead.org
In-Reply-To: <b5b0ffcc-767d-1b90-9c5a-52c57690dea8@sandisk.com>
On Tue, Oct 18, 2016 at 02:49:09PM -0700, Bart Van Assche wrote:
> Multiple functions test the BLK_MQ_S_STOPPED bit so introduce
> a helper function that performs this test.
Looks sensible. Any reason to have it in the public blk-mq.h instead
of the private one, though? I see that dm is using it with this patch,
but that usage should go away once your full series is merged, right?
^ permalink raw reply
* Re: [PATCH v3 01/11] blk-mq: Do not invoke .queue_rq() for a stopped queue
From: Christoph Hellwig @ 2016-10-19 13:17 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, Christoph Hellwig, James Bottomley,
Martin K. Petersen, Mike Snitzer, Doug Ledford, Keith Busch,
Ming Lin, Laurence Oberman, linux-block@vger.kernel.org,
linux-scsi@vger.kernel.org, linux-rdma@vger.kernel.org,
linux-nvme@lists.infradead.org
In-Reply-To: <595d4b59-3892-ef24-ef91-b7cab6611af7@sandisk.com>
Good catch,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* [PATCH 2/2] IB/rdmavt: Handle the kthread worker using the new API
From: Petr Mladek @ 2016-10-19 12:07 UTC (permalink / raw)
To: Doug Ledford, Sean Hefty
Cc: Dennis Dalessandro, Hal Rosenstock, linux-rdma, Tejun Heo,
linux-kernel, Petr Mladek
In-Reply-To: <1476878840-14548-1-git-send-email-pmladek@suse.com>
Use the new API to create and destroy the cq kthread worker.
The API hides some implementation details.
In particular, kthread_create_worker() allocates and initializes
struct kthread_worker. It runs the kthread the right way and stores
task_struct into the worker structure. In addition, the *on_cpu()
variant binds the kthread to the given cpu and the related memory
node.
kthread_destroy_worker() flushes all pending works, stops
the kthread and frees the structure.
This patch does not change the existing behavior. Note that we must
use the on_cpu() variant because the function starts the kthread
and it must bind it to the right CPU before waking. The numa node
is associated for given CPU as well.
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
drivers/infiniband/sw/rdmavt/cq.c | 34 +++++++++++-----------------------
1 file changed, 11 insertions(+), 23 deletions(-)
diff --git a/drivers/infiniband/sw/rdmavt/cq.c b/drivers/infiniband/sw/rdmavt/cq.c
index 223ec4589fc7..4d0b6992e847 100644
--- a/drivers/infiniband/sw/rdmavt/cq.c
+++ b/drivers/infiniband/sw/rdmavt/cq.c
@@ -503,33 +503,23 @@ int rvt_poll_cq(struct ib_cq *ibcq, int num_entries, struct ib_wc *entry)
*/
int rvt_driver_cq_init(struct rvt_dev_info *rdi)
{
- int ret = 0;
int cpu;
- struct task_struct *task;
+ struct kthread_worker *worker;
if (rdi->worker)
return 0;
+
spin_lock_init(&rdi->n_cqs_lock);
- rdi->worker = kzalloc(sizeof(*rdi->worker), GFP_KERNEL);
- if (!rdi->worker)
- return -ENOMEM;
- kthread_init_worker(rdi->worker);
- task = kthread_create_on_node(
- kthread_worker_fn,
- rdi->worker,
- rdi->dparms.node,
- "%s", rdi->dparms.cq_name);
- if (IS_ERR(task)) {
- kfree(rdi->worker);
- rdi->worker = NULL;
- return PTR_ERR(task);
- }
- set_user_nice(task, MIN_NICE);
cpu = cpumask_first(cpumask_of_node(rdi->dparms.node));
- kthread_bind(task, cpu);
- wake_up_process(task);
- return ret;
+ worker = kthread_create_worker_on_cpu(cpu, 0,
+ "%s", rdi->dparms.cq_name);
+ if (IS_ERR(worker))
+ return PTR_ERR(worker);
+
+ set_user_nice(worker->task, MIN_NICE);
+ rdi->worker = worker;
+ return 0;
}
/**
@@ -549,7 +539,5 @@ void rvt_cq_exit(struct rvt_dev_info *rdi)
rdi->worker = NULL;
spin_unlock_irq(&rdi->n_cqs_lock);
- kthread_flush_worker(worker);
- kthread_stop(worker->task);
- kfree(worker);
+ kthread_destroy_worker(worker);
}
--
1.8.5.6
^ permalink raw reply related
* [PATCH 1/2] IB/rdmavt: Avoid queuing work into a destroyed cq kthread worker
From: Petr Mladek @ 2016-10-19 12:07 UTC (permalink / raw)
To: Doug Ledford, Sean Hefty
Cc: Dennis Dalessandro, Hal Rosenstock, linux-rdma, Tejun Heo,
linux-kernel, Petr Mladek
In-Reply-To: <1476878840-14548-1-git-send-email-pmladek@suse.com>
The memory barrier is not enough to protect queuing works into
a destroyed cq kthread. Just imagine the following situation:
CPU1 CPU2
rvt_cq_enter()
worker = cq->rdi->worker;
rvt_cq_exit()
rdi->worker = NULL;
smp_wmb();
kthread_flush_worker(worker);
kthread_stop(worker->task);
kfree(worker);
// nothing queued yet =>
// nothing flushed and
// happily stopped and freed
if (likely(worker)) {
// true => read before CPU2 acted
cq->notify = RVT_CQ_NONE;
cq->triggered++;
kthread_queue_work(worker, &cq->comptask);
BANG: worker has been flushed/stopped/freed in the meantime.
This patch solves this by protecting the critical sections by
rdi->n_cqs_lock. It seems that this lock is not much contended
and looks reasonable for this purpose.
One catch is that rvt_cq_enter() might be called from IRQ context.
Therefore we must always take the lock with IRQs disabled to avoid
a possible deadlock.
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
drivers/infiniband/sw/rdmavt/cq.c | 30 ++++++++++++++++--------------
1 file changed, 16 insertions(+), 14 deletions(-)
diff --git a/drivers/infiniband/sw/rdmavt/cq.c b/drivers/infiniband/sw/rdmavt/cq.c
index 6d9904a4a0ab..223ec4589fc7 100644
--- a/drivers/infiniband/sw/rdmavt/cq.c
+++ b/drivers/infiniband/sw/rdmavt/cq.c
@@ -119,18 +119,17 @@ void rvt_cq_enter(struct rvt_cq *cq, struct ib_wc *entry, bool solicited)
if (cq->notify == IB_CQ_NEXT_COMP ||
(cq->notify == IB_CQ_SOLICITED &&
(solicited || entry->status != IB_WC_SUCCESS))) {
- struct kthread_worker *worker;
/*
* This will cause send_complete() to be called in
* another thread.
*/
- smp_read_barrier_depends(); /* see rvt_cq_exit */
- worker = cq->rdi->worker;
- if (likely(worker)) {
+ spin_lock(&cq->rdi->n_cqs_lock);
+ if (likely(cq->rdi->worker)) {
cq->notify = RVT_CQ_NONE;
cq->triggered++;
- kthread_queue_work(worker, &cq->comptask);
+ kthread_queue_work(cq->rdi->worker, &cq->comptask);
}
+ spin_unlock(&cq->rdi->n_cqs_lock);
}
spin_unlock_irqrestore(&cq->lock, flags);
@@ -240,15 +239,15 @@ struct ib_cq *rvt_create_cq(struct ib_device *ibdev,
}
}
- spin_lock(&rdi->n_cqs_lock);
+ spin_lock_irq(&rdi->n_cqs_lock);
if (rdi->n_cqs_allocated == rdi->dparms.props.max_cq) {
- spin_unlock(&rdi->n_cqs_lock);
+ spin_unlock_irq(&rdi->n_cqs_lock);
ret = ERR_PTR(-ENOMEM);
goto bail_ip;
}
rdi->n_cqs_allocated++;
- spin_unlock(&rdi->n_cqs_lock);
+ spin_unlock_irq(&rdi->n_cqs_lock);
if (cq->ip) {
spin_lock_irq(&rdi->pending_lock);
@@ -296,9 +295,9 @@ int rvt_destroy_cq(struct ib_cq *ibcq)
struct rvt_dev_info *rdi = cq->rdi;
kthread_flush_work(&cq->comptask);
- spin_lock(&rdi->n_cqs_lock);
+ spin_lock_irq(&rdi->n_cqs_lock);
rdi->n_cqs_allocated--;
- spin_unlock(&rdi->n_cqs_lock);
+ spin_unlock_irq(&rdi->n_cqs_lock);
if (cq->ip)
kref_put(&cq->ip->ref, rvt_release_mmap_info);
else
@@ -541,12 +540,15 @@ void rvt_cq_exit(struct rvt_dev_info *rdi)
{
struct kthread_worker *worker;
- worker = rdi->worker;
- if (!worker)
+ /* block future queuing from send_complete() */
+ spin_lock_irq(&rdi->n_cqs_lock);
+ if (!rdi->worker) {
+ spin_unlock_irq(&rdi->n_cqs_lock);
return;
- /* blocks future queuing from send_complete() */
+ }
rdi->worker = NULL;
- smp_wmb(); /* See rdi_cq_enter */
+ spin_unlock_irq(&rdi->n_cqs_lock);
+
kthread_flush_worker(worker);
kthread_stop(worker->task);
kfree(worker);
--
1.8.5.6
^ permalink raw reply related
* [PATCH 0/2] IB/rdmavt: cq ktrhead worker fix and API update
From: Petr Mladek @ 2016-10-19 12:07 UTC (permalink / raw)
To: Doug Ledford, Sean Hefty
Cc: Dennis Dalessandro, Hal Rosenstock, linux-rdma, Tejun Heo,
linux-kernel, Petr Mladek
The kthread worker API has been improved in 4.9-rc1. The 2nd
patch uses the new functions and simplifies the kthread worker
creation and destroying.
I have found a possible race when working on the API conversion.
A proposed fix is in the 1st patch.
Both changes are compile tested only. I did not find an easy way
how to test them at runtime.
Petr Mladek (2):
IB/rdmavt: Avoid queuing work into a destroyed cq kthread worker
IB/rdmavt: Handle the kthread worker using the new API
drivers/infiniband/sw/rdmavt/cq.c | 64 +++++++++++++++++----------------------
1 file changed, 27 insertions(+), 37 deletions(-)
--
1.8.5.6
^ 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