On 10/1/19 8:27 PM, Douglas Gilbert wrote:
-                       kt = ns_to_ktime((u64)delta_jiff * (NSEC_PER_SEC / HZ));
-               } else
-                       kt = ndelay;
+                       u64 ns = jiffies_to_nsecs(delta_jiff);
+
+                       if (sdebug_random && ns < U32_MAX) {
+                               ns = prandom_u32_max((u32)ns);
+                       } else if (sdebug_random) {
+                               ns >>= 12;        /* scale to 4 usec precision 
*/
+                               if (ns < U32_MAX)    /* over 4 hours max */
+                                       ns = prandom_u32_max((u32)ns);
+                               ns <<= 12;

An explanation of how the scaling factor has been chosen is missing.

Is it user friendly to disable randomization silently if delta_jiff exceeds a certain value? How about restricting delta_jiff to e.g. ten times the SCSI timeout?

+               } else {        /* ndelay has a 4.2 second max */
+                       kt = sdebug_random ? prandom_u32_max((u32)ndelay) :
+                                            (u32)ndelay;
+               }

Are these (u32) casts necessary?

+static ssize_t random_show(struct device_driver *ddp, char *buf)
+{
+       return scnprintf(buf, PAGE_SIZE, "%d\n", (int)sdebug_random);
+}

Is the (int) cast necessary?

+static ssize_t random_store(struct device_driver *ddp, const char *buf,
+                           size_t count)
+{
+       int n;
+
+       if (count > 0 && kstrtoint(buf, 10, &n) == 0 && n >= 0) {
+               sdebug_random = (n > 0);
+               return count;
+       }
+       return -EINVAL;
+}
+static DRIVER_ATTR_RW(random);

Is the "count > 0" check useful? I don't think that sysfs ever passes count == 0 to store callback functions.

Thanks,

Bart.

Reply via email to