scsi: sg: 4.4.89

[crinteanu...@gmail.com]

hi, just noticed that one of the following commits introduced in 4.4.89 borks some apps (like nerolinux and norton ghost 12)

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=3e0097499839e0fe3af380410eababe5a47c4cf9

https://lkml.org/lkml/2017/9/24/457

https://patchwork.kernel.org/patch/9968779/

https://lkml.org/lkml/2017/9/24/356

nerolinux fail to detect any drives on my pc with

Call Trace:
[<c02de73f>] ? dump_stack+0x44/0x57
[<c013b639>] ? warn_slowpath_common+0x85/0x9a
[<c03e1116>] ? sg_rq_end_io+0x42/0x277
[<c013b6f1>] ? warn_slowpath_null+0xd/0x10
[<c03e1116>] ? sg_rq_end_io+0x42/0x277
[<c02bd08c>] ? blk_account_io_done+0xc/0xea
[<c02bd1cd>] ? blk_finish_request+0x63/0x84
[<c03c9f87>] ? scsi_end_request+0x11a/0x155
[<c03cae32>] ? scsi_io_completion+0x1c7/0x4aa
[<c02c1c2c>] ? blk_done_softirq+0x56/0x70
[<c013df28>] ? __do_softirq+0xa6/0x18b
[<c013de82>] ? tasklet_action+0x90/0x90
[<c0103980>] ? do_softirq_own_stack+0x1a/0x1f
<IRQ> [<c013e144>] ? irq_exit+0x3c/0x7b
[<c0103463>] ? do_IRQ+0x7a/0x8b
[<c05343b3>] ? common_interrupt+0x33/0x38
[<c016007b>] ? unregister_console+0x87/0xad
[<c0476295>] ? cpuidle_enter_state+0xdb/0x17d
[<c0155e6b>] ? cpu_startup_entry+0x17f/0x1e2
[<c0129344>] ? start_secondary+0x13b/0x153
---[ end trace 36fc4958c27e64df ]---

and norton ghost 12

*********************************
Date : Wed Nov 29 22:34:34 2017
Error Number: 36000
Message: A signal or windows exception occurred
Version: 12.0.0.10561(Nov 2 2017, Build=10561
OS Version: Linux 4.4.100-pclos1.pae #1 SMP Tue Nov 21 16:10:25 EET 2017 i686
Command line arguments:
Active Switches :
AutoName
PathName :
FlagImplode : 0
FlagExplode : 0

Operation Details :
Total size.........0
MB copied..........0
MB remaining.......0
Percent complete...0%
Speed..............0 MB/min
Time elapsed.......0:00
Time remaining.....0:00

Processor exception
Generated at HardExceptionHandlerLinux.cpp:230

Program Call Stack
VolumeContainerManagerLvm::addIfPv
VolumeContainerManagerLvm::loadPvs
VolumeContainerManagerLvm::loadVolumeGroups
VolumeContainerManagerLvm::loadVolumeGroups
VolumeContainerManagerLvm::constructor
VolumeContainerManager::loadVolumeManagers
VolumeContainerManager::constructor
sub_main
main

Call Stack
0xeba0f62a
0xb77e8a0c
0x084644f6
0x084645ce
0x086ad8e0
0x086b3aac
0x0870f00e
0x0870f3a3
0x0870f51b
0x0870f7ab
0x0870f8de
0x086ff373
0x086ffdab
0x08700039
0x08284c59
0x08286679
0x082893df
0x08052d25
0x080541b4
End Call Stack

Anyway i reversed the scsi: sg: changes introduced in 4.4.89 and everything works fine (see the patch below) - now i'm running 4.4.100 with the patch
thx!

--- a/sg.c 2017-11-30 10:37:28.000000000 +0200
+++ b/sg.c 2017-09-14 00:10:05.000000000 +0300
@@ -133,7 +133,7 @@
struct sg_fd;

typedef struct sg_request { /* SG_MAX_QUEUE requests outstanding per file */
- struct list_head entry; /* list entry */
+ struct sg_request *nextrp; /* NULL -> tail request (slist) */
struct sg_fd *parentfp; /* NULL -> not in use */
Sg_scatter_hold data; /* hold buffer, perhaps scatter list */
sg_io_hdr_t header; /* scsi command+info, see <scsi/sg.h> */
@@ -157,7 +157,8 @@
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 */
- struct list_head rq_list; /* head of request list */
+ unsigned save_scat_len; /* original length of trunc. scat. element */
+ Sg_request *headrp; /* head of request slist, NULL->empty */
struct fasync_struct *async_qp; /* used by asynchronous notification */
Sg_request req_arr[SG_MAX_QUEUE]; /* used as singly-linked list */
char low_dma; /* as in parent but possibly overridden to 1 */
@@ -839,39 +840,6 @@
return max_sectors << 9;
}

-static void
-sg_fill_request_table(Sg_fd *sfp, sg_req_info_t *rinfo)
-{
- Sg_request *srp;
- int val;
- unsigned int ms;
-
- val = 0;
- list_for_each_entry(srp, &sfp->rq_list, entry) {
- if (val >= SG_MAX_QUEUE)
- break;
- rinfo[val].req_state = srp->done + 1;
- rinfo[val].problem =
- srp->header.masked_status &
- srp->header.host_status &
- srp->header.driver_status;
- if (srp->done)
- rinfo[val].duration =
- srp->header.duration;
- else {
- ms = jiffies_to_msecs(jiffies);
- rinfo[val].duration =
- (ms > srp->header.duration) ?
- (ms - srp->header.duration) : 0;
- }
- rinfo[val].orphan = srp->orphan;
- rinfo[val].sg_io_owned = srp->sg_io_owned;
- rinfo[val].pack_id = srp->header.pack_id;
- rinfo[val].usr_ptr = srp->header.usr_ptr;
- val++;
- }
-}
-
static long
sg_ioctl(struct file *filp, unsigned int cmd_in, unsigned long arg)
{
@@ -983,7 +951,7 @@
if (!access_ok(VERIFY_WRITE, ip, sizeof (int)))
return -EFAULT;
read_lock_irqsave(&sfp->rq_list_lock, iflags);
- list_for_each_entry(srp, &sfp->rq_list, entry) {
+ for (srp = sfp->headrp; srp; srp = srp->nextrp) {
if ((1 == srp->done) && (!srp->sg_io_owned)) {
read_unlock_irqrestore(&sfp->rq_list_lock,
iflags);
@@ -996,8 +964,7 @@
return 0;
case SG_GET_NUM_WAITING:
read_lock_irqsave(&sfp->rq_list_lock, iflags);
- val = 0;
- list_for_each_entry(srp, &sfp->rq_list, entry) {
+ for (val = 0, srp = sfp->headrp; srp; srp = srp->nextrp) {
if ((1 == srp->done) && (!srp->sg_io_owned))
++val;
}
@@ -1065,15 +1032,42 @@
return -EFAULT;
else {
sg_req_info_t *rinfo;
+ unsigned int ms;

- rinfo = kzalloc(SZ_SG_REQ_INFO * SG_MAX_QUEUE,
- GFP_KERNEL);
+ rinfo = kmalloc(SZ_SG_REQ_INFO * SG_MAX_QUEUE,
+ GFP_KERNEL);
if (!rinfo)
return -ENOMEM;
read_lock_irqsave(&sfp->rq_list_lock, iflags);
- sg_fill_request_table(sfp, rinfo);
+ for (srp = sfp->headrp, val = 0; val < SG_MAX_QUEUE;
+ ++val, srp = srp ? srp->nextrp : srp) {
+ memset(&rinfo[val], 0, SZ_SG_REQ_INFO);
+ if (srp) {
+ rinfo[val].req_state = srp->done + 1;
+ rinfo[val].problem =
+ srp->header.masked_status &
+ srp->header.host_status &
+ srp->header.driver_status;
+ if (srp->done)
+ rinfo[val].duration =
+ srp->header.duration;
+ else {
+ ms = jiffies_to_msecs(jiffies);
+ rinfo[val].duration =
+ (ms > srp->header.duration) ?
+ (ms - srp->header.duration) : 0;
+ }
+ rinfo[val].orphan = srp->orphan;
+ rinfo[val].sg_io_owned =
+ srp->sg_io_owned;
+ rinfo[val].pack_id =
+ srp->header.pack_id;
+ rinfo[val].usr_ptr =
+ srp->header.usr_ptr;
+ }
+ }
read_unlock_irqrestore(&sfp->rq_list_lock, iflags);
- result = __copy_to_user(p, rinfo,
+ result = __copy_to_user(p, rinfo,
SZ_SG_REQ_INFO * SG_MAX_QUEUE);
result = result ? -EFAULT : 0;
kfree(rinfo);
@@ -1179,7 +1173,7 @@
return POLLERR;
poll_wait(filp, &sfp->read_wait, wait);
read_lock_irqsave(&sfp->rq_list_lock, iflags);
- list_for_each_entry(srp, &sfp->rq_list, entry) {
+ for (srp = sfp->headrp; srp; srp = srp->nextrp) {
/* if any read waiting, flag it */
if ((0 == res) && (1 == srp->done) && (!srp->sg_io_owned))
res = POLLIN | POLLRDNORM;
@@ -2065,6 +2059,7 @@
req_schp->pages = NULL;
req_schp->page_order = 0;
req_schp->sglist_len = 0;
+ sfp->save_scat_len = 0;
srp->res_used = 0;
/* Called without mutex lock to avoid deadlock */
sfp->res_in_use = 0;
@@ -2077,7 +2072,7 @@
unsigned long iflags;

write_lock_irqsave(&sfp->rq_list_lock, iflags);
- list_for_each_entry(resp, &sfp->rq_list, entry) {
+ for (resp = sfp->headrp; resp; resp = resp->nextrp) {
/* look for requests that are ready + not SG_IO owned */
if ((1 == resp->done) && (!resp->sg_io_owned) &&
((-1 == pack_id) || (resp->header.pack_id == pack_id))) {
@@ -2095,45 +2090,70 @@
{
int k;
unsigned long iflags;
+ Sg_request *resp;
Sg_request *rp = sfp->req_arr;

write_lock_irqsave(&sfp->rq_list_lock, iflags);
- if (!list_empty(&sfp->rq_list)) {
- if (!sfp->cmd_q)
- goto out_unlock;
-
- for (k = 0; k < SG_MAX_QUEUE; ++k, ++rp) {
- if (!rp->parentfp)
- break;
+ resp = sfp->headrp;
+ if (!resp) {
+ memset(rp, 0, sizeof (Sg_request));
+ rp->parentfp = sfp;
+ resp = rp;
+ sfp->headrp = resp;
+ } else {
+ if (0 == sfp->cmd_q)
+ resp = NULL; /* command queuing disallowed */
+ else {
+ for (k = 0; k < SG_MAX_QUEUE; ++k, ++rp) {
+ if (!rp->parentfp)
+ break;
+ }
+ if (k < SG_MAX_QUEUE) {
+ memset(rp, 0, sizeof (Sg_request));
+ rp->parentfp = sfp;
+ while (resp->nextrp)
+ resp = resp->nextrp;
+ resp->nextrp = rp;
+ resp = rp;
+ } else
+ resp = NULL;
}
- if (k >= SG_MAX_QUEUE)
- goto out_unlock;
}
- memset(rp, 0, sizeof (Sg_request));
- rp->parentfp = sfp;
- rp->header.duration = jiffies_to_msecs(jiffies);
- list_add_tail(&rp->entry, &sfp->rq_list);
- write_unlock_irqrestore(&sfp->rq_list_lock, iflags);
- return rp;
-out_unlock:
+ if (resp) {
+ resp->nextrp = NULL;
+ resp->header.duration = jiffies_to_msecs(jiffies);
+ }
write_unlock_irqrestore(&sfp->rq_list_lock, iflags);
- return NULL;
+ return resp;
}

/* Return of 1 for found; 0 for not found */
static int
sg_remove_request(Sg_fd * sfp, Sg_request * srp)
{
+ Sg_request *prev_rp;
+ Sg_request *rp;
unsigned long iflags;
int res = 0;

- if (!sfp || !srp || list_empty(&sfp->rq_list))
+ if ((!sfp) || (!srp) || (!sfp->headrp))
return res;
write_lock_irqsave(&sfp->rq_list_lock, iflags);
- if (!list_empty(&srp->entry)) {
- list_del(&srp->entry);
- srp->parentfp = NULL;
+ prev_rp = sfp->headrp;
+ if (srp == prev_rp) {
+ sfp->headrp = prev_rp->nextrp;
+ prev_rp->parentfp = NULL;
res = 1;
+ } else {
+ while ((rp = prev_rp->nextrp)) {
+ if (srp == rp) {
+ prev_rp->nextrp = rp->nextrp;
+ rp->parentfp = NULL;
+ res = 1;
+ break;
+ }
+ prev_rp = rp;
+ }
}
write_unlock_irqrestore(&sfp->rq_list_lock, iflags);
return res;
@@ -2152,7 +2172,7 @@

init_waitqueue_head(&sfp->read_wait);
rwlock_init(&sfp->rq_list_lock);
- INIT_LIST_HEAD(&sfp->rq_list);
+
kref_init(&sfp->f_ref);
mutex_init(&sfp->f_mutex);
sfp->timeout = SG_DEFAULT_TIMEOUT;
@@ -2193,13 +2213,10 @@
{
struct sg_fd *sfp = container_of(work, struct sg_fd, ew.work);
struct sg_device *sdp = sfp->parentdp;
- Sg_request *srp;

/* Cleanup any responses which were never read(). */
- while (!list_empty(&sfp->rq_list)) {
- srp = list_first_entry(&sfp->rq_list, Sg_request, entry);
- sg_finish_rem_req(srp);
- }
+ while (sfp->headrp)
+ sg_finish_rem_req(sfp->headrp);

if (sfp->reserve.bufflen > 0) {
SCSI_LOG_TIMEOUT(6, sg_printk(KERN_INFO, sdp,
@@ -2602,7 +2619,7 @@
/* must be called while holding sg_index_lock */
static void sg_proc_debug_helper(struct seq_file *s, Sg_device * sdp)
{
- int k, new_interface, blen, usg;
+ int k, m, new_interface, blen, usg;
Sg_request *srp;
Sg_fd *fp;
const sg_io_hdr_t *hp;
@@ -2622,11 +2639,13 @@
seq_printf(s, " cmd_q=%d f_packid=%d k_orphan=%d closed=0\n",
(int) fp->cmd_q, (int) fp->force_packid,
(int) fp->keep_orphan);
- list_for_each_entry(srp, &fp->rq_list, entry) {
+ for (m = 0, srp = fp->headrp;
+ srp != NULL;
+ ++m, srp = srp->nextrp) {
hp = &srp->header;
new_interface = (hp->interface_id == '\0') ? 0 : 1;
if (srp->res_used) {
- if (new_interface &&
+ if (new_interface &&
(SG_FLAG_MMAP_IO & hp->flags))
cp = " mmap>> ";
else
@@ -2657,7 +2676,7 @@
seq_printf(s, "ms sgat=%d op=0x%02x\n", usg,
(int) srp->data.cmd_opcode);
}
- if (list_empty(&fp->rq_list))
+ if (0 == m)
seq_puts(s, " No requests active\n");
read_unlock(&fp->rq_list_lock);
}