On 7/13/20 7:03 AM, Bodo Stroesser wrote:
On 2020-07-12 03:15, Mike Christie wrote:
On 7/10/20 5:48 AM, Bodo Stroesser wrote:
...
@@ -844,6 +854,9 @@ static bool is_ring_space_avail(struct tcmu_dev
*udev, struct tcmu_cmd *cmd,
return false;
}
+ if (!data_needed)
+ return true;
+
/* try to check and get the data blocks as needed */
space = spc_bitmap_free(udev->data_bitmap, udev->dbi_thresh);
if ((space * DATA_BLOCK_SIZE) < data_needed) {
@@ -1106,6 +1119,61 @@ static int queue_cmd_ring(struct tcmu_cmd
*tcmu_cmd, sense_reason_t *scsi_err)
return 1;
}
+/*
We do 2 stars for this type of comment:
/**
+ * queue_tmr_ring - queue tmr info to ring or internally
+ *
No extra newline.
Ah, thank you. I'll fix both.
@@ -1141,6 +1209,85 @@ static void tcmu_set_next_deadline(struct
list_head *queue,
del_timer(timer);
}
+static int
+tcmu_tmr_type(enum tcm_tmreq_table tmf)
+{
+ switch (tmf) {
+ case TMR_ABORT_TASK: return TCMU_TMR_ABORT_TASK;
+ case TMR_ABORT_TASK_SET: return TCMU_TMR_ABORT_TASK_SET;
+ case TMR_CLEAR_ACA: return TCMU_TMR_CLEAR_ACA;
+ case TMR_CLEAR_TASK_SET: return TCMU_TMR_CLEAR_TASK_SET;
+ case TMR_LUN_RESET: return TCMU_TMR_LUN_RESET;
+ case TMR_TARGET_WARM_RESET: return TCMU_TMR_TARGET_WARM_RESET;
+ case TMR_TARGET_COLD_RESET: return TCMU_TMR_TARGET_COLD_RESET;
+ case TMR_LUN_RESET_PRO: return TCMU_TMR_LUN_RESET_PRO;
+ default: return TCMU_TMR_UNKNOWN;
+ }
+}
+
+static void
+tcmu_tmr_notify(struct se_device *se_dev, enum tcm_tmreq_table tmf,
+ struct list_head *cmd_list)
+{
+ int i = 0, cmd_cnt = 0;
+ bool unqueued = false;
+ uint16_t *cmd_ids = NULL;
+ struct tcmu_cmd *cmd;
+ struct se_cmd *se_cmd;
+ struct tcmu_tmr *tmr;
+ struct tcmu_dev *dev = TCMU_DEV(se_dev);
+
+ mutex_lock(&dev->cmdr_lock);
+
+ // First we check for aborted commands in qfull_queue
I know the coding style doc does not say to never use // anymore, but
just use the same style we have already in the rest of the code for
single line comments:
/* comment */
Ok, I'll fix.
+struct tcmu_tmr_entry {
+ struct tcmu_cmd_entry_hdr hdr;
+
+#define TCMU_TMR_UNKNOWN 0
+#define TCMU_TMR_ABORT_TASK 1
+#define TCMU_TMR_ABORT_TASK_SET 2
+#define TCMU_TMR_CLEAR_ACA 3
+#define TCMU_TMR_CLEAR_TASK_SET 4
+#define TCMU_TMR_LUN_RESET 5
+#define TCMU_TMR_TARGET_WARM_RESET 6
+#define TCMU_TMR_TARGET_COLD_RESET 7
+/* Pseudo reset due to received PR OUT */
+#define TCMU_TMR_LUN_RESET_PRO 128
+ __u8 tmr_type;
+
+ __u8 __pad1;
+ __u16 __pad2;
+ __u32 cmd_cnt;
+ __u64 __pad3;
+ __u64 __pad4;
+ __u16 cmd_ids[0];
+} __packed;
+
Is this new request and the tmr_notify callback just supposed to clean
up the requested commands or is it supposed to perform the actions of
the task management function defined in the specs?
The callback name makes it feel like it's just a notification, but the
names above make it seem like we are supposed to execute the TMF in
userspace. But if the latter, then how do we notify the kernel if the
TMF was successful or failed?
My plan is to have a notification only. IMHO userspace (and also tcmu
or another backend) must not interfere with core's TMR handling.
The new callback tmr_notify just allows backend to be helpful during
TMR handling by completing in core aborted, but in backend/userspace
still running commands early.
Do you refer to the TCMU_TMR_* definitions? I just defined these names
because TMR_* definitions are in target_core_base.h which is not exposed
to userspace programs. Knowing the type of TMR that aborted a command is
useful at least for userspace tracin
I see where you are going. Makes sense to me now.
BTW: I hope there are enough padding fields in the tcmu_tmr_entry to
allow additional session info later?
Yes.
One question on that. Were you going to use the tcmu_cmd_entry_hdr
flags, or add a flag field to tcmu_tmr_entry?
Or will userspace just know its enabled because we would eventually add
a add/delete session callback to the backend modules. And from the add
callout, we would then notify userspace of the new session and that
other commands like tcmu_tmr_entry have session info in it.
