Hi Mike, Andy
Thanks very much.
Here add one 'void *data' parameter just piont to the entry hdr in
ring buffer:
struct tcmu_cmd {
...
+ void *data; ===> pionting to entry hdr
...
}
When some cmds were expired, target_core_user will set the opcode in
the entry hdr to TCMU_OP_EXP(or reuse TCMU_OP_PAD here?), and then
the tcmu-runner will just skip all the TCMU_OP_EXP cmds, so is the
target_core_user.
I think this may cause race conditions. Once we "give" part of the cmd
ring to userspace, it's racy to change it because we don't know if
userspace has already accessed it, or not.
We could put the new OP_EXP opcode on the cmd ring as a new entry. This
would ensure the old cmd's processing was terminated and not finish
out-of-order with a new cmd the initiator sent to retry the aborted one,
avoiding corruption. Implementing this could get a little complicated
When tcmu-runner sees the new OP_EXP opcode, what does it do with it? I
was not sure how it prevents retries and new commands from executing
before the expired command. Will tcmu-runner not start new commands
before the OP_EXP is handled?
If the expired the cmds in ring buffer just call
target_complete_cmd(cmd->se_cmd, SAM_STAT_CHECK_CONDITION),
will the initiator retry it, for now I'm not very sure what the
initiator will
do without HA ?
Should the TCMU or tcmu-runner's duty to retry the cmd again ?
If not the TCMU or tcmu-runner will just drop the expired cmds:
For now, when the tcmu_cmds are expired in ring buffer and set
OP_EXP in kernel space, possible the tcmu-runner has died or is
processing other new commands.
For the case Andy pointed out yesterday is very possible. But in my
opinion, though the racy is exist, it can still run well. Because the
cmd_tail only could be update by tcmu-runner in userspace. The
most bad case is the runner will still handle the expired one(OP_EXP)
raced, but it could skip most other expired cmds.
IMHO, there are two thing to do at the same time :
1, when the tcmu-runner is running again and before processing the
cmds in ringbuffer, just disable timeout handler in kernel.
2, the runner should just skip the expired cmds, which have already expired
before the tcmu-runner come back, in ringbuffer.
If you do a HA target, then the initiator could retry IO on other paths,
so I think we would want to abort the command in tcmu-runner and the
handler,
Agreed.
before having LIO return status for the command or a TMF for
it. So if OP_EXP means for tcmu-runner and the handlers to kill commands
then it would work for me.
Yes, for now the OP_EXP means kill the commands for TCMU and tcmu-runner.
What is the point of the target_core_user timer though? The initiators
already do the same thing, so it seems like one more thing to configure.
For example, for tape commands we will want to increase it, so why
configure it twice. Is it just a hack around target_core_tmr.c not being
able to call into the backend modules to perform TMFs?
If there is a good use for the timer, should it be in userspace? It
seems like it would be easier for tcmu-runner to drive it. We could have
a tcmu-runner timer in userspace, call into the handlers and when they
kill a command we can fail it with a task aborted status or something
like that.
Sounds good.
Is it just supposed to detect when tcmu-runner has died?
Maybe, we could have one timer handler in kernel space just will
work when there has no consumers in userspace. Or the timer
handler will be disabled in kernel space and the consumer's timer
handler will work....?
Thanks,
BRs
Xiubo Li
because we'd need to handle if the cmd ring was full, and prioritize
queued OP_EXP opcodes over OP_CMD when ring space became available.
Mike made me aware of this as an issue recently -- do you have any
further thoughts for how best to solve this?
Regards -- Andy
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
