linux.git/drivers/target/target_core_user.c, branch v5.9

scsi: target: tcmu: Make TMR notification optional

2020-07-29T02:25:31+00:00

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 
Signed-off-by: Bodo Stroesser 
Signed-off-by: Martin K. Petersen

scsi: target: tcmu: Implement tmr_notify callback

2020-07-29T02:25:30+00:00

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 
Signed-off-by: Bodo Stroesser 
Signed-off-by: Martin K. Petersen

scsi: target: tcmu: Fix and simplify timeout handling

2020-07-29T02:25:29+00:00

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 
Signed-off-by: Bodo Stroesser 
Signed-off-by: Martin K. Petersen

scsi: target: tcmu: Factor out new helper ring_insert_padding

2020-07-29T02:25:28+00:00

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 
Signed-off-by: Bodo Stroesser 
Signed-off-by: Martin K. Petersen

scsi: target: tcmu: Do not queue aborted commands

2020-07-29T02:25:28+00:00

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 
Signed-off-by: Bodo Stroesser 
Signed-off-by: Martin K. Petersen

scsi: target: tcmu: Use priv pointer in se_cmd

2020-07-29T02:25:27+00:00

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 
Signed-off-by: Bodo Stroesser 
Signed-off-by: Martin K. Petersen

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

2020-06-30T01:44:57+00:00

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 
Acked-by: Mike Christie 
Signed-off-by: Bodo Stroesser 
Signed-off-by: Martin K. Petersen

scsi: target: tcmu: Fix crash in tcmu_flush_dcache_range on ARM

2020-06-20T03:22:48+00:00

This patch fixes the following crash (see
https://bugzilla.kernel.org/show_bug.cgi?id=208045)

 Process iscsi_trx (pid: 7496, stack limit = 0x0000000010dd111a)
 CPU: 0 PID: 7496 Comm: iscsi_trx Not tainted 4.19.118-0419118-generic
        #202004230533
 Hardware name: Greatwall QingTian DF720/F601, BIOS 601FBE20 Sep 26 2019
 pstate: 80400005 (Nzcv daif +PAN -UAO)
 pc : flush_dcache_page+0x18/0x40
 lr : is_ring_space_avail+0x68/0x2f8 [target_core_user]
 sp : ffff000015123a80
 x29: ffff000015123a80 x28: 0000000000000000
 x27: 0000000000001000 x26: ffff000023ea5000
 x25: ffffcfa25bbe08b8 x24: 0000000000000078
 x23: ffff7e0000000000 x22: ffff000023ea5001
 x21: ffffcfa24b79c000 x20: 0000000000000fff
 x19: ffff7e00008fa940 x18: 0000000000000000
 x17: 0000000000000000 x16: ffff2d047e709138
 x15: 0000000000000000 x14: 0000000000000000
 x13: 0000000000000000 x12: ffff2d047fbd0a40
 x11: 0000000000000000 x10: 0000000000000030
 x9 : 0000000000000000 x8 : ffffc9a254820a00
 x7 : 00000000000013b0 x6 : 000000000000003f
 x5 : 0000000000000040 x4 : ffffcfa25bbe08e8
 x3 : 0000000000001000 x2 : 0000000000000078
 x1 : ffffcfa25bbe08b8 x0 : ffff2d040bc88a18
 Call trace:
  flush_dcache_page+0x18/0x40
  is_ring_space_avail+0x68/0x2f8 [target_core_user]
  queue_cmd_ring+0x1f8/0x680 [target_core_user]
  tcmu_queue_cmd+0xe4/0x158 [target_core_user]
  __target_execute_cmd+0x30/0xf0 [target_core_mod]
  target_execute_cmd+0x294/0x390 [target_core_mod]
  transport_generic_new_cmd+0x1e8/0x358 [target_core_mod]
  transport_handle_cdb_direct+0x50/0xb0 [target_core_mod]
  iscsit_execute_cmd+0x2b4/0x350 [iscsi_target_mod]
  iscsit_sequence_cmd+0xd8/0x1d8 [iscsi_target_mod]
  iscsit_process_scsi_cmd+0xac/0xf8 [iscsi_target_mod]
  iscsit_get_rx_pdu+0x404/0xd00 [iscsi_target_mod]
  iscsi_target_rx_thread+0xb8/0x130 [iscsi_target_mod]
  kthread+0x130/0x138
  ret_from_fork+0x10/0x18
 Code: f9000bf3 aa0003f3 aa1e03e0 d503201f (f9400260)
 ---[ end trace 1e451c73f4266776 ]---

The solution is based on patch:

  "scsi: target: tcmu: Optimize use of flush_dcache_page"

which restricts the use of tcmu_flush_dcache_range() to addresses from
vmalloc'ed areas only.

This patch now replaces the virt_to_page() call in
tcmu_flush_dcache_range() - which is wrong for vmalloced addrs - by
vmalloc_to_page().

The patch was tested on ARM with kernel 4.19.118 and 5.7.2

Link: https://lore.kernel.org/r/20200618131632.32748-3-bstroesser@ts.fujitsu.com
Tested-by: JiangYu 
Tested-by: Daniel Meyerholt 
Acked-by: Mike Christie 
Signed-off-by: Bodo Stroesser 
Signed-off-by: Martin K. Petersen

scsi: target: tcmu: Optimize use of flush_dcache_page

2020-06-20T03:22:47+00:00

(scatter|gather)_data_area() need to flush dcache after writing data to or
before reading data from a page in uio data area.  The two routines are
able to handle data transfer to/from such a page in fragments and flush the
cache after each fragment was copied by calling the wrapper
tcmu_flush_dcache_range().

That means:

1) flush_dcache_page() can be called multiple times for the same page.

2) Calling flush_dcache_page() indirectly using the wrapper does not make
   sense, because each call of the wrapper is for one single page only and
   the calling routine already has the correct page pointer.

Change (scatter|gather)_data_area() such that, instead of calling
tcmu_flush_dcache_range() before/after each memcpy, it now calls
flush_dcache_page() before unmapping a page (when writing is complete for
that page) or after mapping a page (when starting to read the page).

After this change only calls to tcmu_flush_dcache_range() for addresses in
vmalloc'ed command ring are left over.

The patch was tested on ARM with kernel 4.19.118 and 5.7.2

Link: https://lore.kernel.org/r/20200618131632.32748-2-bstroesser@ts.fujitsu.com
Tested-by: JiangYu 
Tested-by: Daniel Meyerholt 
Acked-by: Mike Christie 
Signed-off-by: Bodo Stroesser 
Signed-off-by: Martin K. Petersen

scsi: target: tcmu: Remove unnecessary bit TCMU_CMD_BIT_INFLIGHT

2020-06-20T03:00:32+00:00

Since commit 61fb24822166 ("scsi: target: tcmu: Userspace must not complete
queued commands") tcmu_cmd bit TCMU_CMD_BIT_INFLIGHT is set but never
checked.  So we can remove it safely.

[mkp: fixed Mike's email address]

Link: https://lore.kernel.org/r/20200619173806.5016-1-bstroesser@ts.fujitsu.com
Signed-off-by: Bodo Stroesser 
Acked-by: Mike Christie 
Signed-off-by: Martin K. Petersen