mbox series

[v3,0/8] scsi: target: tcmu: Add TMR notification for tcmu

Message ID 20200726153510.13077-1-bstroesser@ts.fujitsu.com (mailing list archive)
Headers show
Series scsi: target: tcmu: Add TMR notification for tcmu | expand

Message

Bodo Stroesser July 26, 2020, 3:35 p.m. UTC
This patch series is made on top of Martin's for-next branch.

ChangeLog:

v2: in patch "scsi: target: tcmu: Implement tmr_notify callback"
    changed new comment's style from "// ..." to "/* ... */"
    and correctly use "/** " for function doc.

V3:
 - Patch 1 "scsi: target: Modify core_tmr_abort_task()":
   fixed wrong spin_lock handling. Nested calls to
   spin_lock_irqsave and spin_lock_irqrestore used the same
   flags field. Inner pair replaced by spin_lock / spin_unlock
    
 - Patches 5,7,8:
   "scsi: target: tcmu: Factor out new helper ring_insert_padding"
   "scsi: target: tcmu: Implement tmr_notify callback"
   "scsi: target: tcmu: Make TMR notification optional"
   New definitions of struct tcmu_dev *dev renamed to *udev.

 - Patch 8 "scsi: target: tcmu: Make TMR notification optional"
   Spacing fixed at function definition.

---

TCM/LIO core handles TMRs without involving backends.
But TMR completion message is sent to initiator only after
commands aborted by the TMR are completed by the backend.
Especially in case of tcmu backend, if userspace executes long
running commands and therefore initiator sends ABORT_TASK on
timeout, the ABORT itself can time out if core still waits for
userspace/tcmu to complete the command.

It would be very helpful for userspace to get a notification
about received TMR and which commands were aborted by that TMR.
Then userspace can decide whether to cancel command processing,
and it can send command completion earlier than it would without
TMR notification.
It is also helpful for userspace traces and device emulation to
get notification about TMR events.

So this patch series in the first two patches implements in
core the usage of a new optional backend callback for TMR
notifications. The core calls it before core waits for
completion of aborted commands (params: se_dev, TMR type,
and list of se_cmds aborted by this TMR).
Of course other backends than tcmu can use this part of the
series also to implement their own TMR notification if
necessary.

The further six patches implement the TMR notify callback for
tcmu. The new configFS attribute tmr_notification allows to
switch on TMR messages on the cmd ring. The default of the
attribute is the old behavior without TMR info on the ring, but
with following changes:
 - if tcmu receives an already aborted command, it immediately
   rejects it. So it will never appear in userspace.
 - if tcmu finds, that according to TMR notification a cmd on
   the qfull_queue was aborted, tcmu removes it from qfull_queue
   and completes it immediately. So userspace will not 'see'
   those commands.

When attribute tmr_notification is set to 1, tcmu additionally
prepares a list of cmd_ids from those commands, that are aborted
by the TMR and are active in cmd ring (not timed out).
This list together with the TMR type is either immediately
written to cmd ring (new TMR entry type) or queued in a separate
tmr queue if ring space is too small.
TMRs in the tmr queue do not time out. If ring space becomes
available, tcmu moves TMRs from tmr queue to ring with higher
priority than cmds from qfull queue.

This mechanism makes sure that userspace receives TMR
notifications as early as possible. Userspace can use the
list of cmd_ids attached to the TMR notification to identify
aborted commands from its list of received and not yet completed
commands. In case userspace has meanwhile completed some of the
cmd_ids on the list, it can just ignore these cmd_ids.
A possible new command having the same cmd_id as one of the
aborted commands will always appear on the ring after the TMR
notification.

Bodo Stroesser (8):
  scsi: target: Modify core_tmr_abort_task()
  scsi: target: Add tmr_notify backend function
  scsi: target: tcmu: Use priv pointer in se_cmd
  scsi: target: tcmu: Do not queue aborted commands
  scsi: target: tcmu: Factor out new helper ring_insert_padding
  scsi: target: tcmu: Fix and simplify timeout handling
  scsi: target: tcmu: Implement tmr_notify callback
  scsi: target: tcmu: Make TMR notification optional

 drivers/target/target_core_tmr.c       |  36 +++-
 drivers/target/target_core_transport.c |   1 +
 drivers/target/target_core_user.c      | 373 +++++++++++++++++++++++++++------
 include/target/target_core_backend.h   |   2 +
 include/target/target_core_base.h      |   1 +
 include/uapi/linux/target_core_user.h  |  25 +++
 6 files changed, 366 insertions(+), 72 deletions(-)

Comments

Mike Christie July 26, 2020, 6:38 p.m. UTC | #1
On 7/26/20 10:35 AM, Bodo Stroesser wrote:
> This patch series is made on top of Martin's for-next branch.
> 
> ChangeLog:
> 
> v2: in patch "scsi: target: tcmu: Implement tmr_notify callback"
>     changed new comment's style from "// ..." to "/* ... */"
>     and correctly use "/** " for function doc.
> 
> V3:
>  - Patch 1 "scsi: target: Modify core_tmr_abort_task()":
>    fixed wrong spin_lock handling. Nested calls to
>    spin_lock_irqsave and spin_lock_irqrestore used the same
>    flags field. Inner pair replaced by spin_lock / spin_unlock
>     
>  - Patches 5,7,8:
>    "scsi: target: tcmu: Factor out new helper ring_insert_padding"
>    "scsi: target: tcmu: Implement tmr_notify callback"
>    "scsi: target: tcmu: Make TMR notification optional"
>    New definitions of struct tcmu_dev *dev renamed to *udev.
> 
>  - Patch 8 "scsi: target: tcmu: Make TMR notification optional"
>    Spacing fixed at function definition.
> 
> ---
> 
> TCM/LIO core handles TMRs without involving backends.
> But TMR completion message is sent to initiator only after
> commands aborted by the TMR are completed by the backend.
> Especially in case of tcmu backend, if userspace executes long
> running commands and therefore initiator sends ABORT_TASK on
> timeout, the ABORT itself can time out if core still waits for
> userspace/tcmu to complete the command.
> 
> It would be very helpful for userspace to get a notification
> about received TMR and which commands were aborted by that TMR.
> Then userspace can decide whether to cancel command processing,
> and it can send command completion earlier than it would without
> TMR notification.
> It is also helpful for userspace traces and device emulation to
> get notification about TMR events.
> 
> So this patch series in the first two patches implements in
> core the usage of a new optional backend callback for TMR
> notifications. The core calls it before core waits for
> completion of aborted commands (params: se_dev, TMR type,
> and list of se_cmds aborted by this TMR).
> Of course other backends than tcmu can use this part of the
> series also to implement their own TMR notification if
> necessary.
> 
> The further six patches implement the TMR notify callback for
> tcmu. The new configFS attribute tmr_notification allows to
> switch on TMR messages on the cmd ring. The default of the
> attribute is the old behavior without TMR info on the ring, but
> with following changes:
>  - if tcmu receives an already aborted command, it immediately
>    rejects it. So it will never appear in userspace.
>  - if tcmu finds, that according to TMR notification a cmd on
>    the qfull_queue was aborted, tcmu removes it from qfull_queue
>    and completes it immediately. So userspace will not 'see'
>    those commands.
> 
> When attribute tmr_notification is set to 1, tcmu additionally
> prepares a list of cmd_ids from those commands, that are aborted
> by the TMR and are active in cmd ring (not timed out).
> This list together with the TMR type is either immediately
> written to cmd ring (new TMR entry type) or queued in a separate
> tmr queue if ring space is too small.
> TMRs in the tmr queue do not time out. If ring space becomes
> available, tcmu moves TMRs from tmr queue to ring with higher
> priority than cmds from qfull queue.
> 
> This mechanism makes sure that userspace receives TMR
> notifications as early as possible. Userspace can use the
> list of cmd_ids attached to the TMR notification to identify
> aborted commands from its list of received and not yet completed
> commands. In case userspace has meanwhile completed some of the
> cmd_ids on the list, it can just ignore these cmd_ids.
> A possible new command having the same cmd_id as one of the
> aborted commands will always appear on the ring after the TMR
> notification.
> 

Thanks for all the work on this.

Reviewed-by: Mike Christie <michael.christie@oracle.com>
Martin K. Petersen July 29, 2020, 4:10 a.m. UTC | #2
On Sun, 26 Jul 2020 17:35:02 +0200, Bodo Stroesser wrote:

> This patch series is made on top of Martin's for-next branch.
> 
> ChangeLog:
> 
> v2: in patch "scsi: target: tcmu: Implement tmr_notify callback"
>     changed new comment's style from "// ..." to "/* ... */"
>     and correctly use "/** " for function doc.
> 
> [...]

Applied to 5.9/scsi-queue, thanks!

[1/8] scsi: target: Modify core_tmr_abort_task()
      https://git.kernel.org/mkp/scsi/c/f5e2714ad1a6
[2/8] scsi: target: Add tmr_notify backend function
      https://git.kernel.org/mkp/scsi/c/2e45a1a9c75d
[3/8] scsi: target: tcmu: Use priv pointer in se_cmd
      https://git.kernel.org/mkp/scsi/c/a35129024e88
[4/8] scsi: target: tcmu: Do not queue aborted commands
      https://git.kernel.org/mkp/scsi/c/c96849276211
[5/8] scsi: target: tcmu: Factor out new helper ring_insert_padding
      https://git.kernel.org/mkp/scsi/c/3d3f9d56a570
[6/8] scsi: target: tcmu: Fix and simplify timeout handling
      https://git.kernel.org/mkp/scsi/c/ed212ca87897
[7/8] scsi: target: tcmu: Implement tmr_notify callback
      https://git.kernel.org/mkp/scsi/c/bc2d214af5db
[8/8] scsi: target: tcmu: Make TMR notification optional
      https://git.kernel.org/mkp/scsi/c/59526d7a187f