Commit 8885e375 authored by Rolf Fokkens's avatar Rolf Fokkens Committed by James Bottomley

[PATCH] sg.c and USER_HZ, kernel 2.5.37

Hi!

Since the introduction of USER_HZ the SG_[GS]ET_TIMEOUT ioctls may have
a serious BUG as userspace uses a different HZ from the HZ in kernelspace.

In x86 HZ=1000 and USER_HZ=100, resulting in confusing timouts as the
kernel measures time 10 times as fast as userspace.

This patch is an attempt to fix this by transforming USER_HZ based timing to
HZ based timing before storing it in timeout. To make sure that SG_GET_TIMEOUT
and SG_SET_TIMEOUT behave consistently a field timeout_user is added which
stores the exact value that's passed by SG_SET_TIMEOUT and it's returned on
SG_GET_TIMEOUT.

Rolf Fokkens
fokkensr@fokkensr.vertis.nl

P.S. this is the second post of this patch
parent dfa944ae
...@@ -82,6 +82,19 @@ static void sg_proc_cleanup(void); ...@@ -82,6 +82,19 @@ static void sg_proc_cleanup(void);
#define SG_MAX_DEVS_MASK ((1U << KDEV_MINOR_BITS) - 1) #define SG_MAX_DEVS_MASK ((1U << KDEV_MINOR_BITS) - 1)
/*
* Suppose you want to calculate the formula muldiv(x,m,d)=int(x * m / d)
* Then when using 32 bit integers x * m may overflow during the calculation.
* Replacing muldiv(x) by muldiv(x)=((x % d) * m) / d + int(x / d) * m
* calculates the same, but prevents the overflow when both m and d
* are "small" numbers (like HZ and USER_HZ).
* Of course an overflow is inavoidable if the result of muldiv doesn't fit
* in 32 bits.
*/
#define MULDIV(X,MUL,DIV) ((((X % DIV) * MUL) / DIV) + ((X / DIV) * MUL))
#define SG_DEFAULT_TIMEOUT MULDIV(SG_DEFAULT_TIMEOUT_USER, HZ, USER_HZ)
int sg_big_buff = SG_DEF_RESERVED_SIZE; int sg_big_buff = SG_DEF_RESERVED_SIZE;
/* N.B. This variable is readable and writeable via /* N.B. This variable is readable and writeable via
/proc/scsi/sg/def_reserved_size . Each time sg_open() is called a buffer /proc/scsi/sg/def_reserved_size . Each time sg_open() is called a buffer
...@@ -150,7 +163,8 @@ typedef struct sg_fd { /* holds the state of a file descriptor */ ...@@ -150,7 +163,8 @@ typedef struct sg_fd { /* holds the state of a file descriptor */
struct sg_device *parentdp; /* owning device */ struct sg_device *parentdp; /* owning device */
wait_queue_head_t read_wait; /* queue read until command done */ wait_queue_head_t read_wait; /* queue read until command done */
rwlock_t rq_list_lock; /* protect access to list in req_arr */ rwlock_t rq_list_lock; /* protect access to list in req_arr */
int timeout; /* defaults to SG_DEFAULT_TIMEOUT */ int timeout; /* defaults to SG_DEFAULT_TIMEOUT */
int timeout_user; /* defaults to SG_DEFAULT_TIMEOUT_USER */
Sg_scatter_hold reserve; /* buffer held for this file descriptor */ Sg_scatter_hold reserve; /* buffer held for this file descriptor */
unsigned save_scat_len; /* original length of trunc. scat. element */ unsigned save_scat_len; /* original length of trunc. scat. element */
Sg_request *headrp; /* head of request slist, NULL->empty */ Sg_request *headrp; /* head of request slist, NULL->empty */
...@@ -790,10 +804,15 @@ sg_ioctl(struct inode *inode, struct file *filp, ...@@ -790,10 +804,15 @@ sg_ioctl(struct inode *inode, struct file *filp,
return result; return result;
if (val < 0) if (val < 0)
return -EIO; return -EIO;
sfp->timeout = val; if (val >= MULDIV (INT_MAX, USER_HZ, HZ))
val = MULDIV (INT_MAX, USER_HZ, HZ);
sfp->timeout_user = val;
sfp->timeout = MULDIV (val, HZ, USER_HZ);
return 0; return 0;
case SG_GET_TIMEOUT: /* N.B. User receives timeout as return value */ case SG_GET_TIMEOUT: /* N.B. User receives timeout as return value */
return sfp->timeout; /* strange ..., for backward compatibility */ /* strange ..., for backward compatibility */
return sfp->timeout_user;
case SG_SET_FORCE_LOW_DMA: case SG_SET_FORCE_LOW_DMA:
result = get_user(val, (int *) arg); result = get_user(val, (int *) arg);
if (result) if (result)
...@@ -2432,6 +2451,7 @@ sg_add_sfp(Sg_device * sdp, int dev) ...@@ -2432,6 +2451,7 @@ sg_add_sfp(Sg_device * sdp, int dev)
sfp->rq_list_lock = RW_LOCK_UNLOCKED; sfp->rq_list_lock = RW_LOCK_UNLOCKED;
sfp->timeout = SG_DEFAULT_TIMEOUT; sfp->timeout = SG_DEFAULT_TIMEOUT;
sfp->timeout_user = SG_DEFAULT_TIMEOUT_USER;
sfp->force_packid = SG_DEF_FORCE_PACK_ID; sfp->force_packid = SG_DEF_FORCE_PACK_ID;
sfp->low_dma = (SG_DEF_FORCE_LOW_DMA == 0) ? sfp->low_dma = (SG_DEF_FORCE_LOW_DMA == 0) ?
sdp->device->host->unchecked_isa_dma : 1; sdp->device->host->unchecked_isa_dma : 1;
......
...@@ -304,7 +304,12 @@ struct sg_header ...@@ -304,7 +304,12 @@ struct sg_header
/* Defaults, commented if they differ from original sg driver */ /* Defaults, commented if they differ from original sg driver */
#define SG_DEFAULT_TIMEOUT (60*HZ) /* HZ == 'jiffies in 1 second' */ #ifdef __KERNEL__
#define SG_DEFAULT_TIMEOUT_USER (60*USER_HZ) /* HZ == 'jiffies in 1 second' */
#else
#define SG_DEFAULT_TIMEOUT (60*HZ) /* HZ == 'jiffies in 1 second' */
#endif
#define SG_DEF_COMMAND_Q 0 /* command queuing is always on when #define SG_DEF_COMMAND_Q 0 /* command queuing is always on when
the new interface is used */ the new interface is used */
#define SG_DEF_UNDERRUN_FLAG 0 #define SG_DEF_UNDERRUN_FLAG 0
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment