Re: [PATCH 4/4] tcmu: make cmd timeout configurable

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 2017-03-20 at 00:05 -0500, Mike Christie wrote:
> On 03/18/2017 06:26 PM, Nicholas A. Bellinger wrote:
> > +static ssize_t tcmu_cmd_time_out_store(struct config_item *item, const char *page,
> > +				       size_t count)
> > +{
> 
> 
> ...
> 
> 
> > +
> > +	if (!val) {
> > +		pr_err("Illegal value for cmd_time_out\n");
> > +		return -EINVAL;
> > +	}
> > +
> 
> Thanks for the patch! I tested with this chunk removed so you can
> disable the timer (0 == disabled), and it all works as expected.
> 

Thanks.  Here's the last patch to drop that part, and avoid the possible
divide-by-zero in the configfs attribute show handler.

diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c
index c6874c3..1d12081 100644
--- a/drivers/target/target_core_user.c
+++ b/drivers/target/target_core_user.c
@@ -1174,7 +1174,8 @@ static ssize_t tcmu_cmd_time_out_show(struct config_item *item, char *page)
 	struct tcmu_dev *udev = container_of(da->da_dev,
 					struct tcmu_dev, se_dev);
 
-	return snprintf(page, PAGE_SIZE, "%lu\n", udev->cmd_time_out / MSEC_PER_SEC);
+	return snprintf(page, PAGE_SIZE, "%lu\n", (!udev->cmd_time_out) ? 0:
+			udev->cmd_time_out / MSEC_PER_SEC);
 }
 
 static ssize_t tcmu_cmd_time_out_store(struct config_item *item, const char *page,
@@ -1196,11 +1197,6 @@ static ssize_t tcmu_cmd_time_out_store(struct config_item *item, const char *pag
 	if (ret < 0)
 		return ret;
 
-	if (!val) {
-		pr_err("Illegal value for cmd_time_out\n");
-		return -EINVAL;
-	}
-
 	udev->cmd_time_out = val * MSEC_PER_SEC;
 	return count;
 }

Btw, one other thing..

The default TCMU_TIME_OUT = 30 seconds value is still used by
tcmu_queue_cmd_ring() when !is_ring_space_avail(), even when the new
cmd_time_out attribute has been set to zero.

AFAICT it should be using a different value than cmd_time_out, and 30
seconds default as-is seems way too high for non Linux initiators..

Perhaps this is a good canidate to become it's own TCMU backend
attribute separate from cmd_time_out..?

--
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



[Index of Archives]     [Linux SCSI]     [Kernel Newbies]     [Linux SCSI Target Infrastructure]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Device Mapper]

  Powered by Linux