Re: [PATCH] scsi: tcmu: setup one timeout timer for each cmd
On 2018/9/19 0:42, Mike Christie wrote:
On 09/18/2018 05:32 AM, xiu...@redhat.com wrote:
From: Xiubo Li
Currently there has one cmd timeout timer for each udev, and whenever
there has any new coming cmd it will update the timer. And for some
corner case the timer is always working only for the ringbuffer's
newest cmd. That's to say the timer won't be fired even there has one
cmd stuck for a very long time.
Signed-off-by: Xiubo Li
---
drivers/target/target_core_user.c | 69 +++
1 file changed, 34 insertions(+), 35 deletions(-)
diff --git a/drivers/target/target_core_user.c
b/drivers/target/target_core_user.c
index 9cd404acdb82..ed3d6f055037 100644
--- a/drivers/target/target_core_user.c
+++ b/drivers/target/target_core_user.c
@@ -157,7 +157,6 @@ struct tcmu_dev {
struct idr commands;
- struct timer_list cmd_timer;
unsigned int cmd_time_out;
struct timer_list qfull_timer;
@@ -190,6 +189,7 @@ struct tcmu_cmd {
uint32_t *dbi;
unsigned long deadline;
+ struct timer_list cmd_timer;
I think you can fix this for qfull at the same time
Sure, will add this.
and we will not have
to do a mix of per cmd timer along with per device handling.
In tcmu_check_expired_cmd/check_timedout_devices add a next_deadline
variable and when we loop over all the cmds with the idr_for_each have
tcmu_check_expired_cmd return the cmd->deadline. check_timedout_devices
will then track the next timeout value to use and do a mod_timer to set it.
Yeah, make sense and this looks good to me.
In tcmu_setup_cmd_timer you would only mod_timer if the timer is not
already pending already, so we do not keep resetting it like we do today.
Thanks Mike, I will update it.
BRs
Re: [PATCH] scsi: tcmu: setup one timeout timer for each cmd
On 09/18/2018 05:32 AM, xiu...@redhat.com wrote:
> From: Xiubo Li
>
> Currently there has one cmd timeout timer for each udev, and whenever
> there has any new coming cmd it will update the timer. And for some
> corner case the timer is always working only for the ringbuffer's
> newest cmd. That's to say the timer won't be fired even there has one
> cmd stuck for a very long time.
>
> Signed-off-by: Xiubo Li
> ---
> drivers/target/target_core_user.c | 69 +++
> 1 file changed, 34 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/target/target_core_user.c
> b/drivers/target/target_core_user.c
> index 9cd404acdb82..ed3d6f055037 100644
> --- a/drivers/target/target_core_user.c
> +++ b/drivers/target/target_core_user.c
> @@ -157,7 +157,6 @@ struct tcmu_dev {
>
> struct idr commands;
>
> - struct timer_list cmd_timer;
> unsigned int cmd_time_out;
>
> struct timer_list qfull_timer;
> @@ -190,6 +189,7 @@ struct tcmu_cmd {
> uint32_t *dbi;
>
> unsigned long deadline;
> + struct timer_list cmd_timer;
>
I think you can fix this for qfull at the same time and we will not have
to do a mix of per cmd timer along with per device handling.
In tcmu_check_expired_cmd/check_timedout_devices add a next_deadline
variable and when we loop over all the cmds with the idr_for_each have
tcmu_check_expired_cmd return the cmd->deadline. check_timedout_devices
will then track the next timeout value to use and do a mod_timer to set it.
In tcmu_setup_cmd_timer you would only mod_timer if the timer is not
already pending already, so we do not keep resetting it like we do today.
