scsi: target: tcmu: Adjust parameter in call to tcmu_blocks_release()

In commit f7c89771d07d ("scsi: target: tcmu: Replace radix_tree with
XArray") the meaning of last parameter of tcmu_blocks_relea

scsi: target: tcmu: Adjust parameter in call to tcmu_blocks_release()

In commit f7c89771d07d ("scsi: target: tcmu: Replace radix_tree with
XArray") the meaning of last parameter of tcmu_blocks_release() was
changed. So in the callers we should subtract 1 from the previous
parameter.

Unfortunately that change got lost at one of the two places where
tcmu_blocks_release() is called. That does not lead to any problems, but we
should adjust it anyway.

Link: https://lore.kernel.org/r/20210310184458.10741-1-bostroesser@gmail.com
Signed-off-by: Bodo Stroesser <bostroesser@gmail.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

show more ...

scsi: target: tcmu: Use GFP_NOIO while handling cmds or holding cmdr_lock

Especially when using tcmu with tcm_loop, memory allocations with
GFP_KERNEL for a LUN can cause write back to the same LUN.

scsi: target: tcmu: Use GFP_NOIO while handling cmds or holding cmdr_lock

Especially when using tcmu with tcm_loop, memory allocations with
GFP_KERNEL for a LUN can cause write back to the same LUN.

So we have to use GFP_NOIO when allocation is done while handling commands
or while holding cmdr_lock.

Link: https://lore.kernel.org/r/20210305190009.32242-1-bostroesser@gmail.com
Reviewed-by: Mike Christie <michael.christie@oracle.com>
Signed-off-by: Bodo Stroesser <bostroesser@gmail.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

show more ...

scsi: target: tcmu: Replace radix_tree with XArray

An attempt from Matthew Wilcox to replace radix-tree usage by XArray in
tcmu more than 1 year ago unfortunately got lost.

I rebased that work on l

scsi: target: tcmu: Replace radix_tree with XArray

An attempt from Matthew Wilcox to replace radix-tree usage by XArray in
tcmu more than 1 year ago unfortunately got lost.

I rebased that work on latest tcmu and tested it.

Link: https://lore.kernel.org/r/20210224185335.13844-3-bostroesser@gmail.com
Reviewed-by: Mike Christie <michael.christie@oracle.com>
Signed-off-by: Bodo Stroesser <bostroesser@gmail.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

show more ...

scsi: target: tcmu: Replace IDR by XArray

An attempt from Matthew Wilcox to replace IDR usage by XArray in tcmu more
than 1 year ago unfortunately got lost.

I rebased that work on latest tcmu and t

scsi: target: tcmu: Replace IDR by XArray

An attempt from Matthew Wilcox to replace IDR usage by XArray in tcmu more
than 1 year ago unfortunately got lost.

I rebased that work on latest tcmu and tested it.

Link: https://lore.kernel.org/r/20210224185335.13844-2-bostroesser@gmail.com
Reviewed-by: Mike Christie <michael.christie@oracle.com>
Signed-off-by: Bodo Stroesser <bostroesser@gmail.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

show more ...

scsi: target: tcmu: Add backend plug/unplug callouts

This patch adds plug/unplug callouts for tcmu, so we can avoid the number
of times we switch to userspace. Using this driver with tcm_loop is a
c

scsi: target: tcmu: Add backend plug/unplug callouts

This patch adds plug/unplug callouts for tcmu, so we can avoid the number
of times we switch to userspace. Using this driver with tcm_loop is a
common config, and dependng on the nr_hw_queues (nr_hw_queues=1 performs
much better) and fio jobs (lower num jobs around 4) this patch can increase
IOPS by only around 5-10% because we hit other issues like the big per tcmu
device mutex.

Link: https://lore.kernel.org/r/20210227170006.5077-24-michael.christie@oracle.com
Reviewed-by: Bodo Stroesser <bostroesser@gmail.com>
Signed-off-by: Mike Christie <michael.christie@oracle.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

show more ...

scsi: target: tcmu: Fix memory leak caused by wrong uio usage

When user deletes a tcmu device via configFS, tcmu calls
uio_unregister_device(). During that call uio resets its pointer to struct
uio_

scsi: target: tcmu: Fix memory leak caused by wrong uio usage

When user deletes a tcmu device via configFS, tcmu calls
uio_unregister_device(). During that call uio resets its pointer to struct
uio_info provided by tcmu. That means, after uio_unregister_device() uio
will no longer execute any of the callbacks tcmu had set in uio_info.

Especially, if userspace daemon still holds the corresponding uio device
open or mmap'ed while tcmu calls uio_unregister_device(), uio will not call
tcmu_release() when userspace finally closes and munmaps the uio device.

Since tcmu does refcounting for the tcmu device in tcmu_open() and
tcmu_release(), in the decribed case refcount does not drop to 0 and tcmu
does not free tcmu device's resources. In extreme cases this can cause
memory leaking of up to 1 GB for a single tcmu device.

After uio_unregister_device(), uio will reject every open, read, write,
mmap from userspace with -EOI. But userspace daemon can still access the
mmap'ed command ring and data area. Therefore tcmu should wait until
userspace munmaps the uio device before it frees the resources, as we don't
want to cause SIGSEGV or SIGBUS to user space.

That said, current refcounting during tcmu_open and tcmu_release does not
work correctly, and refcounting better should be done in the open and close
callouts of the vm_operations_struct, which tcmu assigns to each mmap of
the uio device (because it wants its own page fault handler).

This patch fixes the memory leak by removing refcounting from tcmu_open and
tcmu_close, and instead adding new tcmu_vma_open() and tcmu_vma_close()
handlers that only do refcounting.

Link: https://lore.kernel.org/r/20210218175039.7829-3-bostroesser@gmail.com
Reviewed-by: Mike Christie <michael.christie@oracle.com>
Signed-off-by: Bodo Stroesser <bostroesser@gmail.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

show more ...

scsi: target: tcmu: Move some functions without code change

This patch just moves one block of code containing some functions inside
target_core_user.c to avoid adding prototypes in next patch.

Lin

scsi: target: tcmu: Move some functions without code change

This patch just moves one block of code containing some functions inside
target_core_user.c to avoid adding prototypes in next patch.

Link: https://lore.kernel.org/r/20210218175039.7829-2-bostroesser@gmail.com
Reviewed-by: Mike Christie <michael.christie@oracle.com>
Signed-off-by: Bodo Stroesser <bostroesser@gmail.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

show more ...

scsi: target: tcmu: Fix use-after-free of se_cmd->priv

Commit a35129024e88 ("scsi: target: tcmu: Use priv pointer in se_cmd")
modified tcmu_free_cmd() to set NULL to priv pointer in se_cmd. However,

scsi: target: tcmu: Fix use-after-free of se_cmd->priv

Commit a35129024e88 ("scsi: target: tcmu: Use priv pointer in se_cmd")
modified tcmu_free_cmd() to set NULL to priv pointer in se_cmd. However,
se_cmd can be already freed by work queue triggered in
target_complete_cmd(). This caused BUG KASAN use-after-free [1].

To fix the bug, do not touch priv pointer in tcmu_free_cmd(). Instead, set
NULL to priv pointer before target_complete_cmd() calls. Also, to avoid
unnecessary priv pointer change in tcmu_queue_cmd(), modify priv pointer in
the function only when tcmu_free_cmd() is not called.

[1]
BUG: KASAN: use-after-free in tcmu_handle_completions+0x1172/0x1770 [target_core_user]
Write of size 8 at addr ffff88814cf79a40 by task cmdproc-uio0/14842

CPU: 2 PID: 14842 Comm: cmdproc-uio0 Not tainted 5.11.0-rc2 #1
Hardware name: Supermicro Super Server/X10SRL-F, BIOS 3.2 11/22/2019
Call Trace:
dump_stack+0x9a/0xcc
? tcmu_handle_completions+0x1172/0x1770 [target_core_user]
print_address_description.constprop.0+0x18/0x130
? tcmu_handle_completions+0x1172/0x1770 [target_core_user]
? tcmu_handle_completions+0x1172/0x1770 [target_core_user]
kasan_report.cold+0x7f/0x10e
? tcmu_handle_completions+0x1172/0x1770 [target_core_user]
tcmu_handle_completions+0x1172/0x1770 [target_core_user]
? queue_tmr_ring+0x5d0/0x5d0 [target_core_user]
tcmu_irqcontrol+0x28/0x60 [target_core_user]
uio_write+0x155/0x230
? uio_vma_fault+0x460/0x460
? security_file_permission+0x4f/0x440
vfs_write+0x1ce/0x860
ksys_write+0xe9/0x1b0
? __ia32_sys_read+0xb0/0xb0
? syscall_enter_from_user_mode+0x27/0x70
? trace_hardirqs_on+0x1c/0x110
do_syscall_64+0x33/0x40
entry_SYSCALL_64_after_hwframe+0x44/0xa9
RIP: 0033:0x7fcf8b61905f
Code: 89 54 24 18 48 89 74 24 10 89 7c 24 08 e8 b9 fc ff ff 48 8b 54 24 18 48 8b 74 24 10 41 89 c0 8b 7c 24 08 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 31 44 89 c7 48 89 44 24 08 e8 0c fd ff ff 48
RSP: 002b:00007fcf7b3e6c30 EFLAGS: 00000293 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007fcf8b61905f
RDX: 0000000000000004 RSI: 00007fcf7b3e6c78 RDI: 000000000000000c
RBP: 00007fcf7b3e6c80 R08: 0000000000000000 R09: 00007fcf7b3e6aa8
R10: 000000000b01c000 R11: 0000000000000293 R12: 00007ffe0c32a52e
R13: 00007ffe0c32a52f R14: 0000000000000000 R15: 00007fcf7b3e7640

Allocated by task 383:
kasan_save_stack+0x1b/0x40
____kasan_kmalloc.constprop.0+0x84/0xa0
kmem_cache_alloc+0x142/0x330
tcm_loop_queuecommand+0x2a/0x4e0 [tcm_loop]
scsi_queue_rq+0x12ec/0x2d20
blk_mq_dispatch_rq_list+0x30a/0x1db0
__blk_mq_do_dispatch_sched+0x326/0x830
__blk_mq_sched_dispatch_requests+0x2c8/0x3f0
blk_mq_sched_dispatch_requests+0xca/0x120
__blk_mq_run_hw_queue+0x93/0xe0
process_one_work+0x7b6/0x1290
worker_thread+0x590/0xf80
kthread+0x362/0x430
ret_from_fork+0x22/0x30

Freed by task 11655:
kasan_save_stack+0x1b/0x40
kasan_set_track+0x1c/0x30
kasan_set_free_info+0x20/0x30
____kasan_slab_free+0xec/0x120
slab_free_freelist_hook+0x53/0x160
kmem_cache_free+0xf4/0x5c0
target_release_cmd_kref+0x3ea/0x9e0 [target_core_mod]
transport_generic_free_cmd+0x28b/0x2f0 [target_core_mod]
target_complete_ok_work+0x250/0xac0 [target_core_mod]
process_one_work+0x7b6/0x1290
worker_thread+0x590/0xf80
kthread+0x362/0x430
ret_from_fork+0x22/0x30

Last potentially related work creation:
kasan_save_stack+0x1b/0x40
kasan_record_aux_stack+0xa3/0xb0
insert_work+0x48/0x2e0
__queue_work+0x4e8/0xdf0
queue_work_on+0x78/0x80
tcmu_handle_completions+0xad0/0x1770 [target_core_user]
tcmu_irqcontrol+0x28/0x60 [target_core_user]
uio_write+0x155/0x230
vfs_write+0x1ce/0x860
ksys_write+0xe9/0x1b0
do_syscall_64+0x33/0x40
entry_SYSCALL_64_after_hwframe+0x44/0xa9

Second to last potentially related work creation:
kasan_save_stack+0x1b/0x40
kasan_record_aux_stack+0xa3/0xb0
insert_work+0x48/0x2e0
__queue_work+0x4e8/0xdf0
queue_work_on+0x78/0x80
tcm_loop_queuecommand+0x1c3/0x4e0 [tcm_loop]
scsi_queue_rq+0x12ec/0x2d20
blk_mq_dispatch_rq_list+0x30a/0x1db0
__blk_mq_do_dispatch_sched+0x326/0x830
__blk_mq_sched_dispatch_requests+0x2c8/0x3f0
blk_mq_sched_dispatch_requests+0xca/0x120
__blk_mq_run_hw_queue+0x93/0xe0
process_one_work+0x7b6/0x1290
worker_thread+0x590/0xf80
kthread+0x362/0x430
ret_from_fork+0x22/0x30

The buggy address belongs to the object at ffff88814cf79800 which belongs
to the cache tcm_loop_cmd_cache of size 896.

Link: https://lore.kernel.org/r/20210113024508.1264992-1-shinichiro.kawasaki@wdc.com
Fixes: a35129024e88 ("scsi: target: tcmu: Use priv pointer in se_cmd")
Cc: stable@vger.kernel.org # v5.9+
Acked-by: Bodo Stroesser <bostroesser@gmail.com>
Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

show more ...

scsi: target: tcmu: scatter_/gather_data_area() rework

scatter_data_area() and gather_data_area() are not easy to understand since
data is copied in nested loops over sg_list and tcmu dbi list. Sinc

scsi: target: tcmu: scatter_/gather_data_area() rework

scatter_data_area() and gather_data_area() are not easy to understand since
data is copied in nested loops over sg_list and tcmu dbi list. Since sg
list can contain only partly filled pages, the loop has to be prepared to
handle sg pages not matching dbi pages one by one.

Existing implementation uses kmap_atomic()/kunmap_atomic() due to
performance reasons. But instead of using these calls strictly nested for
sg and dpi pages, the code holds the mappings in an overlapping way, which
indeed is a bug that would trigger on archs using highmem.

The scatterlist lib contains the sg_miter_start/_next/_stop functions which
can be used to simplify such complicated loops.

The new code now processes the dbi list in the outer loop, while sg list is
handled by the inner one. That way the code can take advantage of the
sg_miter_* family calls.

Calling sg_miter_stop() after the end of the inner loop enforces strict
nesting of atomic kmaps.

Since the nested loops in scatter_/gather_data_area were very similar, I
replaced them by the new helper function tcmu_copy_data().

Link: https://lore.kernel.org/r/20201019115118.11949-1-bostroesser@gmail.com
Acked-by: Mike Christie <michael.christie@oracle.com>
Signed-off-by: Bodo Stroesser <bostroesser@gmail.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

show more ...

scsi: target: tcmu: Return from tcmu_handle_completions() if cmd_id not found

[ Upstream commit 9814b55cde0588b6d9bc496cee43f87316cbc6f1 ]

If tcmu_handle_completions() finds an invalid cmd_id while

scsi: target: tcmu: Return from tcmu_handle_completions() if cmd_id not found

[ Upstream commit 9814b55cde0588b6d9bc496cee43f87316cbc6f1 ]

If tcmu_handle_completions() finds an invalid cmd_id while looping over cmd
responses from userspace it sets TCMU_DEV_BIT_BROKEN and breaks the
loop. This means that it does further handling for the tcmu device.

Skip that handling by replacing 'break' with 'return'.

Additionally change tcmu_handle_completions() from unsigned int to bool,
since the value used in return already is bool.

Link: https://lore.kernel.org/r/20210423150123.24468-1-bostroesser@gmail.com
Signed-off-by: Bodo Stroesser <bostroesser@gmail.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>

show more ...

scsi: target: tcmu: Fix use-after-free of se_cmd->priv

commit 780e1384687d6ecdee9ca789a1027610484ac8a2 upstream.

Commit a35129024e88 ("scsi: target: tcmu: Use priv pointer in se_cmd")
modified tcmu

scsi: target: tcmu: Fix use-after-free of se_cmd->priv

commit 780e1384687d6ecdee9ca789a1027610484ac8a2 upstream.

Commit a35129024e88 ("scsi: target: tcmu: Use priv pointer in se_cmd")
modified tcmu_free_cmd() to set NULL to priv pointer in se_cmd. However,
se_cmd can be already freed by work queue triggered in
target_complete_cmd(). This caused BUG KASAN use-after-free [1].

To fix the bug, do not touch priv pointer in tcmu_free_cmd(). Instead, set
NULL to priv pointer before target_complete_cmd() calls. Also, to avoid
unnecessary priv pointer change in tcmu_queue_cmd(), modify priv pointer in
the function only when tcmu_free_cmd() is not called.

[1]
BUG: KASAN: use-after-free in tcmu_handle_completions+0x1172/0x1770 [target_core_user]
Write of size 8 at addr ffff88814cf79a40 by task cmdproc-uio0/14842

CPU: 2 PID: 14842 Comm: cmdproc-uio0 Not tainted 5.11.0-rc2 #1
Hardware name: Supermicro Super Server/X10SRL-F, BIOS 3.2 11/22/2019
Call Trace:
dump_stack+0x9a/0xcc
? tcmu_handle_completions+0x1172/0x1770 [target_core_user]
print_address_description.constprop.0+0x18/0x130
? tcmu_handle_completions+0x1172/0x1770 [target_core_user]
? tcmu_handle_completions+0x1172/0x1770 [target_core_user]
kasan_report.cold+0x7f/0x10e
? tcmu_handle_completions+0x1172/0x1770 [target_core_user]
tcmu_handle_completions+0x1172/0x1770 [target_core_user]
? queue_tmr_ring+0x5d0/0x5d0 [target_core_user]
tcmu_irqcontrol+0x28/0x60 [target_core_user]
uio_write+0x155/0x230
? uio_vma_fault+0x460/0x460
? security_file_permission+0x4f/0x440
vfs_write+0x1ce/0x860
ksys_write+0xe9/0x1b0
? __ia32_sys_read+0xb0/0xb0
? syscall_enter_from_user_mode+0x27/0x70
? trace_hardirqs_on+0x1c/0x110
do_syscall_64+0x33/0x40
entry_SYSCALL_64_after_hwframe+0x44/0xa9
RIP: 0033:0x7fcf8b61905f
Code: 89 54 24 18 48 89 74 24 10 89 7c 24 08 e8 b9 fc ff ff 48 8b 54 24 18 48 8b 74 24 10 41 89 c0 8b 7c 24 08 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 31 44 89 c7 48 89 44 24 08 e8 0c fd ff ff 48
RSP: 002b:00007fcf7b3e6c30 EFLAGS: 00000293 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007fcf8b61905f
RDX: 0000000000000004 RSI: 00007fcf7b3e6c78 RDI: 000000000000000c
RBP: 00007fcf7b3e6c80 R08: 0000000000000000 R09: 00007fcf7b3e6aa8
R10: 000000000b01c000 R11: 0000000000000293 R12: 00007ffe0c32a52e
R13: 00007ffe0c32a52f R14: 0000000000000000 R15: 00007fcf7b3e7640

Allocated by task 383:
kasan_save_stack+0x1b/0x40
____kasan_kmalloc.constprop.0+0x84/0xa0
kmem_cache_alloc+0x142/0x330
tcm_loop_queuecommand+0x2a/0x4e0 [tcm_loop]
scsi_queue_rq+0x12ec/0x2d20
blk_mq_dispatch_rq_list+0x30a/0x1db0
__blk_mq_do_dispatch_sched+0x326/0x830
__blk_mq_sched_dispatch_requests+0x2c8/0x3f0
blk_mq_sched_dispatch_requests+0xca/0x120
__blk_mq_run_hw_queue+0x93/0xe0
process_one_work+0x7b6/0x1290
worker_thread+0x590/0xf80
kthread+0x362/0x430
ret_from_fork+0x22/0x30

Freed by task 11655:
kasan_save_stack+0x1b/0x40
kasan_set_track+0x1c/0x30
kasan_set_free_info+0x20/0x30
____kasan_slab_free+0xec/0x120
slab_free_freelist_hook+0x53/0x160
kmem_cache_free+0xf4/0x5c0
target_release_cmd_kref+0x3ea/0x9e0 [target_core_mod]
transport_generic_free_cmd+0x28b/0x2f0 [target_core_mod]
target_complete_ok_work+0x250/0xac0 [target_core_mod]
process_one_work+0x7b6/0x1290
worker_thread+0x590/0xf80
kthread+0x362/0x430
ret_from_fork+0x22/0x30

Last potentially related work creation:
kasan_save_stack+0x1b/0x40
kasan_record_aux_stack+0xa3/0xb0
insert_work+0x48/0x2e0
__queue_work+0x4e8/0xdf0
queue_work_on+0x78/0x80
tcmu_handle_completions+0xad0/0x1770 [target_core_user]
tcmu_irqcontrol+0x28/0x60 [target_core_user]
uio_write+0x155/0x230
vfs_write+0x1ce/0x860
ksys_write+0xe9/0x1b0
do_syscall_64+0x33/0x40
entry_SYSCALL_64_after_hwframe+0x44/0xa9

Second to last potentially related work creation:
kasan_save_stack+0x1b/0x40
kasan_record_aux_stack+0xa3/0xb0
insert_work+0x48/0x2e0
__queue_work+0x4e8/0xdf0
queue_work_on+0x78/0x80
tcm_loop_queuecommand+0x1c3/0x4e0 [tcm_loop]
scsi_queue_rq+0x12ec/0x2d20
blk_mq_dispatch_rq_list+0x30a/0x1db0
__blk_mq_do_dispatch_sched+0x326/0x830
__blk_mq_sched_dispatch_requests+0x2c8/0x3f0
blk_mq_sched_dispatch_requests+0xca/0x120
__blk_mq_run_hw_queue+0x93/0xe0
process_one_work+0x7b6/0x1290
worker_thread+0x590/0xf80
kthread+0x362/0x430
ret_from_fork+0x22/0x30

The buggy address belongs to the object at ffff88814cf79800 which belongs
to the cache tcm_loop_cmd_cache of size 896.

Link: https://lore.kernel.org/r/20210113024508.1264992-1-shinichiro.kawasaki@wdc.com
Fixes: a35129024e88 ("scsi: target: tcmu: Use priv pointer in se_cmd")
Cc: stable@vger.kernel.org # v5.9+
Acked-by: Bodo Stroesser <bostroesser@gmail.com>
Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

show more ...

scsi: target: tcmu: Replace zero-length array with flexible-array member

There is a regular need in the kernel to provide a way to declare having a
dynamically sized set of trailing elements in a st

scsi: target: tcmu: Replace zero-length array with flexible-array member

There is a regular need in the kernel to provide a way to declare having a
dynamically sized set of trailing elements in a structure. Kernel code should
always use “flexible array members”[1] for these cases. The older style of
one-element or zero-length arrays should no longer be used[2].

[1] https://en.wikipedia.org/wiki/Flexible_array_member
[2] https://www.kernel.org/doc/html/v5.9-rc1/process/deprecated.html#zero-length-and-one-element-arrays

Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>

show more ...

genetlink: move to smaller ops wherever possible

Bulk of the genetlink users can use smaller ops, move them.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Reviewed-by: Johannes Berg <johannes@sip

genetlink: move to smaller ops wherever possible

Bulk of the genetlink users can use smaller ops, move them.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Reviewed-by: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: David S. Miller <davem@davemloft.net>

show more ...

scsi: target: tcmu: Fix warning: 'page' may be used uninitialized

Corrects drivers/target/target_core_user.c:688:6: warning: 'page' may be
used uninitialized.

Link: https://lore.kernel.org/r/202009

scsi: target: tcmu: Fix warning: 'page' may be used uninitialized

Corrects drivers/target/target_core_user.c:688:6: warning: 'page' may be
used uninitialized.

Link: https://lore.kernel.org/r/20200924001920.43594-1-john.p.donnelly@oracle.com
Fixes: 3c58f737231e ("scsi: target: tcmu: Optimize use of flush_dcache_page")
Cc: Mike Christie <michael.christie@oracle.com>
Acked-by: Mike Christie <michael.christie@oracle.com>
Signed-off-by: John Donnelly <john.p.donnelly@oracle.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

show more ...

scsi: target: tcmu: Optimize scatter_data_area()

scatter_data_area() has two purposes:

1) Create the iovs for the data area buffer of a SCSI cmd.

2) If there is data in DMA_TO_DEVICE direction,

scsi: target: tcmu: Optimize scatter_data_area()

scatter_data_area() has two purposes:

1) Create the iovs for the data area buffer of a SCSI cmd.

2) If there is data in DMA_TO_DEVICE direction, copy
the data from sg_list to data area buffer.

Both are done in a common loop.

In case of DMA_FROM_DEVICE data transfer, scatter_data_area() is called
with parameter copy_data = false. But this flag is just used to skip
memcpy() for data, while radix_tree_lookup still is called for every dbi of
the area area buffer, and kmap and kunmap are called for every page from
sg_list and data_area as well as flush_dcache_page() for the data area
pages. Since the only thing to do with copy_data = false would be to set
up the iovs, this is a noticeable overhead. Rework the iov creation in the
main loop of scatter_data_area() providing the new function
new_block_to_iov(). Based on this, create the short new function
tcmu_setup_iovs() that only writes the iovs with no overhead. This new
function is now called instead of scatter_data_area() for bidi buffers and
for data buffers in those cases where memcpy() would have been skipped.

Link: https://lore.kernel.org/r/20200910155041.17654-4-bstroesser@ts.fujitsu.com
Acked-by: Mike Christie <michael.christie@oracle.com>
Signed-off-by: Bodo Stroesser <bstroesser@ts.fujitsu.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

show more ...

scsi: target: tcmu: Optimize queue_cmd_ring()

queue_cmd_ring() needs to check whether there is enough space in cmd ring
and data area for the cmd to queue.

Currently the sequence is:

1) Calculate

scsi: target: tcmu: Optimize queue_cmd_ring()

queue_cmd_ring() needs to check whether there is enough space in cmd ring
and data area for the cmd to queue.

Currently the sequence is:

1) Calculate size the cmd will occupy on the ring based on estimation of
needed iovs.

2) Check whether there is enough space on the ring based on size from 1)

3) Allocate buffers in data area.

4) Calculate number of iovs the command really needs while copying
incoming data (if any) to data area.

5) Re-calculate real size of cmd on ring based on real number of iovs.

6) Set up possible padding and cmd on the ring.

Step 1) must not underestimate the cmd size so use max possible number of
iovs for the given I/O data size. The resulting overestimation can be
really high so this sequence is not ideal. The earliest the real number of
iovs can be calculated is after data buffer allocation. Therefore rework
the code to implement the following sequence:

A) Allocate buffers on data area and calculate number of necessary iovs
during this.

B) Calculate real size of cmd on ring based on number of iovs.

C) Check whether there is enough space on the ring.

D) Set up possible padding and cmd on the ring.

The new sequence enforces the split of new function tcmu_alloc_data_space()
from is_ring_space_avail(). Using this function, change queue_cmd_ring()
according to the new sequence.

Change routines called by tcmu_alloc_data_space() to allow calculating and
returning the iov count. Remove counting of iovs in scatter_data_area().

Link: https://lore.kernel.org/r/20200910155041.17654-3-bstroesser@ts.fujitsu.com
Acked-by: Mike Christie <michael.christie@oracle.com>
Signed-off-by: Bodo Stroesser <bstroesser@ts.fujitsu.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

show more ...

scsi: target: tcmu: Join tcmu_cmd_get_data_length() and tcmu_cmd_get_block_cnt()

Simplify code by joining tcmu_cmd_get_data_length() and
tcmu_cmd_get_block_cnt() into tcmu_cmd_set_block_cnts(). The

scsi: target: tcmu: Join tcmu_cmd_get_data_length() and tcmu_cmd_get_block_cnt()

Simplify code by joining tcmu_cmd_get_data_length() and
tcmu_cmd_get_block_cnt() into tcmu_cmd_set_block_cnts(). The new function
sets tcmu_cmd->dbi_cnt and also the new field tcmu_cmd->dbi_bidi_cnt which
is needed for further enhancements in following patches. Simplify some
code by using tcmu_cmd->dbi(_bidi)_cnt instead of calculation from length.

Please note: The calculation of the number of dbis needed for bidi was
wrong. It was based on the length of the first bidi sg only. I changed it
to correctly sum up entire length of all bidi sgs.

Link: https://lore.kernel.org/r/20200910155041.17654-2-bstroesser@ts.fujitsu.com
Acked-by: Mike Christie <michael.christie@oracle.com>
Signed-off-by: Bodo Stroesser <bstroesser@ts.fujitsu.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

show more ...

scsi: target: tcmu: Add missing newline when printing parameters

The tcmu 'global_max_data_area_mb' parameter in sysfs is missing a
newline. Add it.

Link: https://lore.kernel.org/r/1599132573-33818

scsi: target: tcmu: Add missing newline when printing parameters

The tcmu 'global_max_data_area_mb' parameter in sysfs is missing a
newline. Add it.

Link: https://lore.kernel.org/r/1599132573-33818-1-git-send-email-wangxiongfeng2@huawei.com
Signed-off-by: Xiongfeng Wang <wangxiongfeng2@huawei.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

show more ...

scsi: target: tcmu: Make TMR notification optional

Add "tmr_notification" configfs attribute to tcmu devices. If the default
value 0 is used, tcmu only removes aborted commands from qfull_queue. I

scsi: target: tcmu: Make TMR notification optional

Add "tmr_notification" configfs attribute to tcmu devices. If the default
value 0 is used, tcmu only removes aborted commands from qfull_queue. If
user changes tmr_notification to 1, additionally TMR notifications will be
written to the cmd ring.

Link: https://lore.kernel.org/r/20200726153510.13077-9-bstroesser@ts.fujitsu.com
Reviewed-by: Mike Christie <michael.christie@oracle.com>
Signed-off-by: Bodo Stroesser <bstroesser@ts.fujitsu.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

show more ...

scsi: target: tcmu: Implement tmr_notify callback

This patch implements the tmr_notify callback for tcmu. When the callback
is called, tcmu checks the list of aborted commands it received as
parame

scsi: target: tcmu: Implement tmr_notify callback

This patch implements the tmr_notify callback for tcmu. When the callback
is called, tcmu checks the list of aborted commands it received as
parameter:

- aborted commands in the qfull_queue are removed from the queue and
target_complete_command is called

- from the cmd_ids of aborted commands currently uncompleted in cmd ring
it creates a list of aborted cmd_ids.

Finally a TMR notification is written to cmd ring containing TMR type and
cmd_id list. If there is no space in ring, the TMR notification is queued
on a TMR specific queue.

The TMR specific queue 'tmr_queue' can be seen as a extension of the cmd
ring. At the end of each iexecution of tcmu_complete_commands() we check
whether tmr_queue contains TMRs and try to move them onto the ring. If
tmr_queue is not empty after that, we don't call run_qfull_queue() because
commands must not overtake TMRs.

This way we guarantee that cmd_ids in TMR notification received by
userspace either match an active, not yet completed command or are no
longer valid due to userspace having complete some cmd_ids meanwhile.

New commands that were assigned to an aborted cmd_id will always appear on
the cmd ring _after_ the TMR.

Link: https://lore.kernel.org/r/20200726153510.13077-8-bstroesser@ts.fujitsu.com
Reviewed-by: Mike Christie <michael.christie@oracle.com>
Signed-off-by: Bodo Stroesser <bstroesser@ts.fujitsu.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

show more ...

scsi: target: tcmu: Fix and simplify timeout handling

During cmd timeout handling in check_timedout_devices(), due to a race, it
can happen that tcmu_set_next_deadline() does not start a timer as
ex

scsi: target: tcmu: Fix and simplify timeout handling

During cmd timeout handling in check_timedout_devices(), due to a race, it
can happen that tcmu_set_next_deadline() does not start a timer as
expected:

1) Either tcmu_check_expired_ring_cmd() checks the inflight_queue or
tcmu_check_expired_queue_cmd() checks the qfull_queue while jiffies has
the value X

2) At the end of the check the queue contains one remaining command with
deadline X (time_after(X, X) is false and thus the command is not
handled as being timed out).

3) After tcmu_check_expired_xxxxx_cmd() a timer interrupt happens and
jiffies is incremented to X+1.

4) Now tcmu_set_next_deadline() is called, but it skips the command, since
time_after(X+1, X) is true. Therefore tcmu_set_next_deadline() finds no
new deadline and stops the timer, which it shouldn't.

Since commands that time out are removed from inflight_queue or
qfull_queue, we don't need the check with time_after() in
tcmu_set_next_deadline() but can use the deadline from the first cmd in
the queue.

Additionally, replace the remaining time_after() calls in
tcmu_check_expired_xxxxx_cmd() with time_after_eq(), because it is not
useful to set the timeout to deadline but then check for jiffies being
greater than deadline.

Simplify the end of tcmu_handle_completions() and change the check for no
more pending commands from

mb->cmd_tail == mb->cmd_head
to

idr_is_empty(&udev->commands)

because the old check doesn't work correctly if paddings or in the future
TMRs are in the ring.

Finally tcmu_set_next_deadline() was shifted in the source as
preparation for later implementation of tmr_notify callback.

Link: https://lore.kernel.org/r/20200726153510.13077-7-bstroesser@ts.fujitsu.com
Reviewed-by: Mike Christie <michael.christie@oracle.com>
Signed-off-by: Bodo Stroesser <bstroesser@ts.fujitsu.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

show more ...

scsi: target: tcmu: Factor out new helper ring_insert_padding

The new helper ring_insert_padding is split off from and then called by
queue_cmd_ring. It inserts a padding if necessary. The new help

scsi: target: tcmu: Factor out new helper ring_insert_padding

The new helper ring_insert_padding is split off from and then called by
queue_cmd_ring. It inserts a padding if necessary. The new helper will in
a subsequent patch be used during writing of TMR notifications to command
ring.

Link: https://lore.kernel.org/r/20200726153510.13077-6-bstroesser@ts.fujitsu.com
Reviewed-by: Mike Christie <michael.christie@oracle.com>
Signed-off-by: Bodo Stroesser <bstroesser@ts.fujitsu.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

show more ...

scsi: target: tcmu: Do not queue aborted commands

If tcmu receives an already aborted command, tcmu_queue_cmd() should reject
it.

Link: https://lore.kernel.org/r/20200726153510.13077-5-bstroesser@t

scsi: target: tcmu: Do not queue aborted commands

If tcmu receives an already aborted command, tcmu_queue_cmd() should reject
it.

Link: https://lore.kernel.org/r/20200726153510.13077-5-bstroesser@ts.fujitsu.com
Reviewed-by: Mike Christie <michael.christie@oracle.com>
Signed-off-by: Bodo Stroesser <bstroesser@ts.fujitsu.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

show more ...

scsi: target: tcmu: Use priv pointer in se_cmd

We initialize and clean up the se_cmd's priv pointer under cmd_ring_lock to
point to the corresponding tcmu_cmd.

In the patch that implements tmr_noti

scsi: target: tcmu: Use priv pointer in se_cmd

We initialize and clean up the se_cmd's priv pointer under cmd_ring_lock to
point to the corresponding tcmu_cmd.

In the patch that implements tmr_notify callback in tcmu we will use the
priv pointer.

Link: https://lore.kernel.org/r/20200726153510.13077-4-bstroesser@ts.fujitsu.com
Reviewed-by: Mike Christie <michael.christie@oracle.com>
Signed-off-by: Bodo Stroesser <bstroesser@ts.fujitsu.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

show more ...

scsi: target: tcmu: Fix crash on ARM during cmd completion

If tcmu_handle_completions() has to process a padding shorter than
sizeof(struct tcmu_cmd_entry), the current call to
tcmu_flush_dcache_ran

scsi: target: tcmu: Fix crash on ARM during cmd completion

If tcmu_handle_completions() has to process a padding shorter than
sizeof(struct tcmu_cmd_entry), the current call to
tcmu_flush_dcache_range() with sizeof(struct tcmu_cmd_entry) as length
param is wrong and causes crashes on e.g. ARM, because
tcmu_flush_dcache_range() in this case calls
flush_dcache_page(vmalloc_to_page(start)); with start being an invalid
address above the end of the vmalloc'ed area.

The fix is to use the minimum of remaining ring space and sizeof(struct
tcmu_cmd_entry) as the length param.

The patch was tested on kernel 4.19.118.

See https://bugzilla.kernel.org/show_bug.cgi?id=208045#c10

Link: https://lore.kernel.org/r/20200629093756.8947-1-bstroesser@ts.fujitsu.com
Tested-by: JiangYu <lnsyyj@hotmail.com>
Acked-by: Mike Christie <michael.christie@oracle.com>
Signed-off-by: Bodo Stroesser <bstroesser@ts.fujitsu.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

show more ...