[PATCH 1/1] target: target_core_xcopy: Remove version.h header inclusion
version.h header inclusion is not necessary as detected by versioncheck. Signed-off-by: Sachin Kamat --- drivers/target/target_core_xcopy.c |1 - 1 file changed, 1 deletion(-) diff --git a/drivers/target/target_core_xcopy.c b/drivers/target/target_core_xcopy.c index 7e12286..fb4b0c5 100644 --- a/drivers/target/target_core_xcopy.c +++ b/drivers/target/target_core_xcopy.c @@ -21,7 +21,6 @@ * **/ -#include #include #include #include -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[Bug 60644] MPT2SAS drops all HDDs when under high I/O
https://bugzilla.kernel.org/show_bug.cgi?id=60644 --- Comment #27 from livea...@live.com --- Hi again . I set up two disks as two seperate BTRFS volumes (No RAID) , and did some tests . One of the disks failed to complete the process given to it , but it wasn't dropped , and checking the mountpoint shows that it still mounted . The error it gives is "Stale file handle" . The other drive , completed all the processes successfully . It seems that RAID is indeed a problem if not THE problem . Thank you . -- You are receiving this mail because: You are watching the assignee of the bug. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[Bug 60758] module scsi_wait_scan not found kernel panic on boot
https://bugzilla.kernel.org/show_bug.cgi?id=60758 Jeff Zhou changed: What|Removed |Added CC||jz.researc...@yahoo.com --- Comment #3 from Jeff Zhou --- Looks like a dependency failure, would you like to show the two .config files? -- You are receiving this mail because: You are the assignee for the bug. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1 v1.1] arcmsr: Support Areca new SATA Raid Adapter ARC1214/1224/1264/1284 (Resend renew)
On Thu, 2013-08-29 at 12:55 +0800, 黃清隆 wrote: > Update the patch code. This patch is whitespace damaged. Please send the email to yourself and make sure it applies correctly before resending to the lists. If necessary, read Documentation/email-clients.txt cheers, Joe -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[Bug 49461] scsi/bfa/bfad.c:1037: possible off by one in strncpy ?
https://bugzilla.kernel.org/show_bug.cgi?id=49461
Jeff Zhou changed:
What|Removed |Added
CC||jz.researc...@yahoo.com
--- Comment #1 from Jeff Zhou ---
In 3.10.9, it is correct:
scsi/bfa/bfad.c : 1036
strncpy(driver_info.os_device_name, bfad->pci_name,
sizeof(driver_info.os_device_name) - 1);
scsi/bfa/bfad.c : 1014
struct bfa_fcs_driver_info_s driver_info;
scsi/bfa/bfa_fcs.h : 672
struct bfa_fcs_driver_info_s {
u8 version[BFA_VERSION_LEN];/* Driver Version */
u8 host_machine_name[BFA_FCS_OS_STR_LEN];
u8 host_os_name[BFA_FCS_OS_STR_LEN]; /* OS name and version */
u8 host_os_patch[BFA_FCS_OS_STR_LEN]; /* patch or service pack */
u8 os_device_name[BFA_FCS_OS_STR_LEN]; /* Driver Device Name */
};
The copy length here is (BFA_FCS_OS_STR_LEN - 1), which should be.
--
You are receiving this mail because:
You are the assignee for the bug.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 0/4][SCSI] sg: fix race condition in sg_open
There is a race when open sg with O_EXCL flag. Also a race may happen between sg_open and sg_remove. Changes from v6: * [1/4] remove double if. * [3/4] fix via IS_ERR Changes from v5: Patches based on v3.11-rc7 and passed sg_tst_excl test. * [1/4] * remove unused variables - res,sdp. * fix insane code dealing with sg_add_sfp. * [2/4] resolve conflict with v3.11-rc7. * [3/4] remove sem_out label. * [4/4] add sdp definition. Changes from v4: * [3/4] use ERR_PTR series instead of adding another parameter in sg_add_sfp * [4/4] fix conflict for cherry-pick from v3. Changes from v3: * release o_sem in sg_release(), not in sg_remove_sfp(). * not set exclude with sfd_lock held. Vaughan Cao (4): sg: use rwsem to solve race during exclusive open sg: no need sg_open_exclusive_lock sg: checking sdp->detached isn't protected when open sg: push file descriptor list locking down to per-device locking drivers/scsi/sg.c | 176 +- 1 file changed, 81 insertions(+), 95 deletions(-) -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 2/4] sg: no need sg_open_exclusive_lock
Open exclusive check is protected by o_sem, no need sg_open_exclusive_lock.
@exclude is used to record which type of rwsem we are holding.
Signed-off-by: Vaughan Cao
---
drivers/scsi/sg.c | 34 +-
1 file changed, 5 insertions(+), 29 deletions(-)
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 4efa9b5..d4af132 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -105,8 +105,6 @@ static int scatter_elem_sz_prev = SG_SCATTER_SZ;
static int sg_add(struct device *, struct class_interface *);
static void sg_remove(struct device *, struct class_interface *);
-static DEFINE_SPINLOCK(sg_open_exclusive_lock);
-
static DEFINE_IDR(sg_index_idr);
static DEFINE_RWLOCK(sg_index_lock); /* Also used to lock
file descriptor list
for device */
@@ -176,7 +174,6 @@ typedef struct sg_device { /* holds the state of each scsi
generic device */
struct list_head sfds;
struct rw_semaphore o_sem; /* exclude open should hold this rwsem
*/
volatile char detached; /* 0->attached, 1->detached pending removal */
- /* exclude protected by sg_open_exclusive_lock */
char exclude; /* opened for exclusive access */
char sgdebug; /* 0->off, 1->sense, 9->dump dev, 10-> all devs
*/
struct gendisk *disk;
@@ -225,27 +222,6 @@ static int sg_allow_access(struct file *filp, unsigned
char *cmd)
return blk_verify_command(cmd, filp->f_mode & FMODE_WRITE);
}
-static int get_exclude(Sg_device *sdp)
-{
- unsigned long flags;
- int ret;
-
- spin_lock_irqsave(&sg_open_exclusive_lock, flags);
- ret = sdp->exclude;
- spin_unlock_irqrestore(&sg_open_exclusive_lock, flags);
- return ret;
-}
-
-static int set_exclude(Sg_device *sdp, char val)
-{
- unsigned long flags;
-
- spin_lock_irqsave(&sg_open_exclusive_lock, flags);
- sdp->exclude = val;
- spin_unlock_irqrestore(&sg_open_exclusive_lock, flags);
- return val;
-}
-
static int sfds_list_empty(Sg_device *sdp)
{
unsigned long flags;
@@ -317,7 +293,7 @@ sg_open(struct inode *inode, struct file *filp)
}
/* Since write lock is held, no need to check sfd_list */
if (flags & O_EXCL)
- set_exclude(sdp, 1);
+ sdp->exclude = 1; /* used by release lock */
if (sdp->detached) {
retval = -ENODEV;
@@ -337,7 +313,7 @@ sg_open(struct inode *inode, struct file *filp)
retval = -ENOMEM;
sem_out:
if (flags & O_EXCL) {
- set_exclude(sdp, 0);/* undo if error */
+ sdp->exclude = 0; /* undo if error */
up_write(&sdp->o_sem);
} else
up_read(&sdp->o_sem);
@@ -364,8 +340,8 @@ sg_release(struct inode *inode, struct file *filp)
return -ENXIO;
SCSI_LOG_TIMEOUT(3, printk("sg_release: %s\n", sdp->disk->disk_name));
- excl = get_exclude(sdp);
- set_exclude(sdp, 0);
+ excl = sdp->exclude;
+ sdp->exclude = 0;
if (excl)
up_write(&sdp->o_sem);
else
@@ -2622,7 +2598,7 @@ static int sg_proc_seq_show_debug(struct seq_file *s,
void *v)
scsidp->lun,
scsidp->host->hostt->emulated);
seq_printf(s, " sg_tablesize=%d excl=%d\n",
- sdp->sg_tablesize, get_exclude(sdp));
+ sdp->sg_tablesize, sdp->exclude);
sg_proc_debug_helper(s, sdp);
}
read_unlock_irqrestore(&sg_index_lock, iflags);
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 4/4] sg: push file descriptor list locking down to per-device locking
Push file descriptor list locking down to per-device locking. Let sg_index_lock
only protect device lookup.
sdp->detached is also set and checked with this lock held.
Changes from v4:
* Since I use ERR_PTR and friends in sg_add_sfp, this patch should also be
updated to resolve conflict in cherrry-pick.
Signed-off-by: Vaughan Cao
---
drivers/scsi/sg.c | 62 ++-
1 file changed, 34 insertions(+), 28 deletions(-)
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 64df1ab..5cbc4bb 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -106,8 +106,7 @@ static int sg_add(struct device *, struct class_interface
*);
static void sg_remove(struct device *, struct class_interface *);
static DEFINE_IDR(sg_index_idr);
-static DEFINE_RWLOCK(sg_index_lock); /* Also used to lock
- file descriptor list
for device */
+static DEFINE_RWLOCK(sg_index_lock);
static struct class_interface sg_interface = {
.add_dev= sg_add,
@@ -144,8 +143,7 @@ typedef struct sg_request { /* SG_MAX_QUEUE requests
outstanding per file */
} Sg_request;
typedef struct sg_fd { /* holds the state of a file descriptor */
- /* sfd_siblings is protected by sg_index_lock */
- struct list_head sfd_siblings;
+ struct list_head sfd_siblings; /* protected by sfd_lock of device */
struct sg_device *parentdp; /* owning device */
wait_queue_head_t read_wait;/* queue read until command done */
rwlock_t rq_list_lock; /* protect access to list in req_arr */
@@ -170,7 +168,7 @@ typedef struct sg_device { /* holds the state of each scsi
generic device */
struct scsi_device *device;
int sg_tablesize; /* adapter's max scatter-gather table size */
u32 index; /* device index number */
- /* sfds is protected by sg_index_lock */
+ spinlock_t sfd_lock;/* protect file descriptor list for device */
struct list_head sfds;
struct rw_semaphore o_sem; /* exclude open should hold this rwsem
*/
volatile char detached; /* 0->attached, 1->detached pending removal */
@@ -227,9 +225,9 @@ static int sfds_list_empty(Sg_device *sdp)
unsigned long flags;
int ret;
- read_lock_irqsave(&sg_index_lock, flags);
+ spin_lock_irqsave(&sdp->sfd_lock, flags);
ret = list_empty(&sdp->sfds);
- read_unlock_irqrestore(&sg_index_lock, flags);
+ spin_unlock_irqrestore(&sdp->sfd_lock, flags);
return ret;
}
@@ -1393,6 +1391,7 @@ static Sg_device *sg_alloc(struct gendisk *disk, struct
scsi_device *scsidp)
disk->first_minor = k;
sdp->disk = disk;
sdp->device = scsidp;
+ spin_lock_init(&sdp->sfd_lock);
INIT_LIST_HEAD(&sdp->sfds);
init_rwsem(&sdp->o_sem);
sdp->sg_tablesize = queue_max_segments(q);
@@ -1527,11 +1526,13 @@ static void sg_remove(struct device *cl_dev, struct
class_interface *cl_intf)
/* Need a write lock to set sdp->detached. */
write_lock_irqsave(&sg_index_lock, iflags);
+ spin_lock(&sdp->sfd_lock);
sdp->detached = 1;
list_for_each_entry(sfp, &sdp->sfds, sfd_siblings) {
wake_up_interruptible(&sfp->read_wait);
kill_fasync(&sfp->async_qp, SIGPOLL, POLL_HUP);
}
+ spin_unlock(&sdp->sfd_lock);
write_unlock_irqrestore(&sg_index_lock, iflags);
sysfs_remove_link(&scsidp->sdev_gendev.kobj, "generic");
@@ -2056,13 +2057,13 @@ sg_add_sfp(Sg_device * sdp, int dev)
sfp->cmd_q = SG_DEF_COMMAND_Q;
sfp->keep_orphan = SG_DEF_KEEP_ORPHAN;
sfp->parentdp = sdp;
- write_lock_irqsave(&sg_index_lock, iflags);
+ spin_lock_irqsave(&sdp->sfd_lock, iflags);
if (sdp->detached) {
- write_unlock_irqrestore(&sg_index_lock, iflags);
+ spin_unlock_irqrestore(&sdp->sfd_lock, iflags);
return ERR_PTR(-ENODEV);
}
list_add_tail(&sfp->sfd_siblings, &sdp->sfds);
- write_unlock_irqrestore(&sg_index_lock, iflags);
+ spin_unlock_irqrestore(&sdp->sfd_lock, iflags);
SCSI_LOG_TIMEOUT(3, printk("sg_add_sfp: sfp=0x%p\n", sfp));
if (unlikely(sg_big_buff != def_reserved_size))
sg_big_buff = def_reserved_size;
@@ -2109,11 +2110,12 @@ static void sg_remove_sfp_usercontext(struct
work_struct *work)
static void sg_remove_sfp(struct kref *kref)
{
struct sg_fd *sfp = container_of(kref, struct sg_fd, f_ref);
+ struct sg_device *sdp = sfp->parentdp;
unsigned long iflags;
- write_lock_irqsave(&sg_index_lock, iflags);
+ spin_lock_irqsave(&sdp->sfd_lock, iflags);
list_del(&sfp->sfd_siblings);
- write_unlock_irqrestore(&sg_index_lock, iflags);
+ spin_unlock_irqrestore(&sdp->sfd_lock, iflags);
INIT_WORK(&sfp->ew.work, sg_remove_sfp_
[PATCH v7 1/4] sg: use rwsem to solve race during exclusive open
A race condition may happen if two threads are both trying to open the same sg
with O_EXCL simultaneously. It's possible that they both find fsds list is
empty and get_exclude(sdp) returns 0, then they both call set_exclude() and
break out from wait_event_interruptible and resume open.
Now use rwsem to protect this process. Exclusive open gets write lock and
others get read lock. The lock will be held until file descriptor is closed.
This also leads 'exclude' only a status rather than a check mark.
Changes from v6:
* remove double if.
Changes from v5:
* remove unused variables
* fix insane code dealing with sg_add_sfp.
Signed-off-by: Vaughan Cao
---
drivers/scsi/sg.c | 79 +--
1 file changed, 41 insertions(+), 38 deletions(-)
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index df5e961..4efa9b5 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -170,11 +170,11 @@ typedef struct sg_fd {/* holds the state of a
file descriptor */
typedef struct sg_device { /* holds the state of each scsi generic device */
struct scsi_device *device;
- wait_queue_head_t o_excl_wait; /* queue open() when O_EXCL in use */
int sg_tablesize; /* adapter's max scatter-gather table size */
u32 index; /* device index number */
/* sfds is protected by sg_index_lock */
struct list_head sfds;
+ struct rw_semaphore o_sem; /* exclude open should hold this rwsem
*/
volatile char detached; /* 0->attached, 1->detached pending removal */
/* exclude protected by sg_open_exclusive_lock */
char exclude; /* opened for exclusive access */
@@ -265,7 +265,6 @@ sg_open(struct inode *inode, struct file *filp)
struct request_queue *q;
Sg_device *sdp;
Sg_fd *sfp;
- int res;
int retval;
nonseekable_open(inode, filp);
@@ -294,35 +293,35 @@ sg_open(struct inode *inode, struct file *filp)
goto error_out;
}
- if (flags & O_EXCL) {
- if (O_RDONLY == (flags & O_ACCMODE)) {
- retval = -EPERM; /* Can't lock it with read only access
*/
- goto error_out;
- }
- if (!sfds_list_empty(sdp) && (flags & O_NONBLOCK)) {
- retval = -EBUSY;
- goto error_out;
- }
- res = wait_event_interruptible(sdp->o_excl_wait,
- ((!sfds_list_empty(sdp) ||
get_exclude(sdp)) ? 0 : set_exclude(sdp, 1)));
- if (res) {
- retval = res; /* -ERESTARTSYS because signal hit
process */
- goto error_out;
- }
- } else if (get_exclude(sdp)) { /* some other fd has an exclusive lock
on dev */
- if (flags & O_NONBLOCK) {
- retval = -EBUSY;
- goto error_out;
- }
- res = wait_event_interruptible(sdp->o_excl_wait,
!get_exclude(sdp));
- if (res) {
- retval = res; /* -ERESTARTSYS because signal hit
process */
- goto error_out;
+ if ((flags & O_EXCL) && (O_RDONLY == (flags & O_ACCMODE))) {
+ retval = -EPERM; /* Can't lock it with read only access */
+ goto error_out;
+ }
+ if (flags & O_NONBLOCK) {
+ if (flags & O_EXCL) {
+ if (!down_write_trylock(&sdp->o_sem)) {
+ retval = -EBUSY;
+ goto error_out;
+ }
+ } else {
+ if (!down_read_trylock(&sdp->o_sem)) {
+ retval = -EBUSY;
+ goto error_out;
+ }
}
+ } else {
+ if (flags & O_EXCL)
+ down_write(&sdp->o_sem);
+ else
+ down_read(&sdp->o_sem);
}
+ /* Since write lock is held, no need to check sfd_list */
+ if (flags & O_EXCL)
+ set_exclude(sdp, 1);
+
if (sdp->detached) {
retval = -ENODEV;
- goto error_out;
+ goto sem_out;
}
if (sfds_list_empty(sdp)) { /* no existing opens on this device */
sdp->sgdebug = 0;
@@ -331,17 +330,18 @@ sg_open(struct inode *inode, struct file *filp)
}
if ((sfp = sg_add_sfp(sdp, dev)))
filp->private_data = sfp;
+ /* retval is already provably zero at this point because of the
+* check after retval = scsi_autopm_get_device(sdp->device))
+*/
else {
+ retval = -ENOMEM;
+sem_out:
if (flags & O_EXCL) {
set_exclude(sdp, 0);
[PATCH v7 3/4] sg: checking sdp->detached isn't protected when open
@detached is set under the protection of sg_index_lock. Without getting the
lock, new sfp will be added during sg removal and there is no chance for it
to be picked out. So check with sg_index_lock held in sg_add_sfp().
Changes from v6:
* fix via IS_ERR
Changes from v5:
* remove sem_out label.
Changes from v4:
* use ERR_PTR series instead of adding another parameter in sg_add_sfp
Signed-off-by: Vaughan Cao
---
drivers/scsi/sg.c | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index d4af132..64df1ab 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -295,23 +295,20 @@ sg_open(struct inode *inode, struct file *filp)
if (flags & O_EXCL)
sdp->exclude = 1; /* used by release lock */
- if (sdp->detached) {
- retval = -ENODEV;
- goto sem_out;
- }
if (sfds_list_empty(sdp)) { /* no existing opens on this device */
sdp->sgdebug = 0;
q = sdp->device->request_queue;
sdp->sg_tablesize = queue_max_segments(q);
}
- if ((sfp = sg_add_sfp(sdp, dev)))
+ sfp = sg_add_sfp(sdp, dev);
+ if (!IS_ERR(sfp))
filp->private_data = sfp;
/* retval is already provably zero at this point because of the
* check after retval = scsi_autopm_get_device(sdp->device))
*/
else {
- retval = -ENOMEM;
-sem_out:
+ retval = PTR_ERR(sfp);
+
if (flags & O_EXCL) {
sdp->exclude = 0; /* undo if error */
up_write(&sdp->o_sem);
@@ -2045,7 +2042,7 @@ sg_add_sfp(Sg_device * sdp, int dev)
sfp = kzalloc(sizeof(*sfp), GFP_ATOMIC | __GFP_NOWARN);
if (!sfp)
- return NULL;
+ return ERR_PTR(-ENOMEM);
init_waitqueue_head(&sfp->read_wait);
rwlock_init(&sfp->rq_list_lock);
@@ -2060,6 +2057,10 @@ sg_add_sfp(Sg_device * sdp, int dev)
sfp->keep_orphan = SG_DEF_KEEP_ORPHAN;
sfp->parentdp = sdp;
write_lock_irqsave(&sg_index_lock, iflags);
+ if (sdp->detached) {
+ write_unlock_irqrestore(&sg_index_lock, iflags);
+ return ERR_PTR(-ENODEV);
+ }
list_add_tail(&sfp->sfd_siblings, &sdp->sfds);
write_unlock_irqrestore(&sg_index_lock, iflags);
SCSI_LOG_TIMEOUT(3, printk("sg_add_sfp: sfp=0x%p\n", sfp));
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH -next] target: remove unused including
From: Wei Yongjun Remove including that don't need it. Signed-off-by: Wei Yongjun --- drivers/target/target_core_xcopy.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/target/target_core_xcopy.c b/drivers/target/target_core_xcopy.c index 7e12286..fb4b0c5 100644 --- a/drivers/target/target_core_xcopy.c +++ b/drivers/target/target_core_xcopy.c @@ -21,7 +21,6 @@ * **/ -#include #include #include #include -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 26/29] qla2xxx: Correctly print out/in mailbox registers.
From: Joe Carnuccio
At mailbox/buffer debug level, print the correct values of the
outgoing and incoming mailbox registers.
Signed-off-by: Joe Carnuccio
Signed-off-by: Saurav Kashyap
---
drivers/scsi/qla2xxx/qla_mbx.c | 28 +---
1 files changed, 13 insertions(+), 15 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_mbx.c b/drivers/scsi/qla2xxx/qla_mbx.c
index c825d1d..a9aae50 100644
--- a/drivers/scsi/qla2xxx/qla_mbx.c
+++ b/drivers/scsi/qla2xxx/qla_mbx.c
@@ -117,33 +117,25 @@ qla2x00_mailbox_command(scsi_qla_host_t *vha, mbx_cmd_t
*mcp)
command = mcp->mb[0];
mboxes = mcp->out_mb;
+ ql_dbg(ql_dbg_mbx + ql_dbg_buffer, vha, 0x,
+ "Mailbox registers (OUT):\n");
for (cnt = 0; cnt < ha->mbx_count; cnt++) {
if (IS_QLA2200(ha) && cnt == 8)
optr =
(uint16_t __iomem *)MAILBOX_REG(ha, ®->isp, 8);
- if (mboxes & BIT_0)
+ if (mboxes & BIT_0) {
+ ql_dbg(ql_dbg_mbx, vha, 0x1112,
+ "mbox[%d]<-0x%04x\n", cnt, *iptr);
WRT_REG_WORD(optr, *iptr);
+ }
mboxes >>= 1;
optr++;
iptr++;
}
- ql_dbg(ql_dbg_mbx + ql_dbg_buffer, vha, 0x,
- "Loaded MBX registers (displayed in bytes) =.\n");
- ql_dump_buffer(ql_dbg_mbx + ql_dbg_buffer, vha, 0x1112,
- (uint8_t *)mcp->mb, 16);
- ql_dbg(ql_dbg_mbx + ql_dbg_buffer, vha, 0x1113,
- ".\n");
- ql_dump_buffer(ql_dbg_mbx + ql_dbg_buffer, vha, 0x1114,
- ((uint8_t *)mcp->mb + 0x10), 16);
- ql_dbg(ql_dbg_mbx + ql_dbg_buffer, vha, 0x1115,
- ".\n");
- ql_dump_buffer(ql_dbg_mbx + ql_dbg_buffer, vha, 0x1116,
- ((uint8_t *)mcp->mb + 0x20), 8);
ql_dbg(ql_dbg_mbx + ql_dbg_buffer, vha, 0x1117,
"I/O Address = %p.\n", optr);
- ql_dump_regs(ql_dbg_mbx + ql_dbg_buffer, vha, 0x100e);
/* Issue set host interrupt command to send cmd out. */
ha->flags.mbox_int = 0;
@@ -254,9 +246,15 @@ qla2x00_mailbox_command(scsi_qla_host_t *vha, mbx_cmd_t
*mcp)
iptr2 = mcp->mb;
iptr = (uint16_t *)&ha->mailbox_out[0];
mboxes = mcp->in_mb;
+
+ ql_dbg(ql_dbg_mbx, vha, 0x1113,
+ "Mailbox registers (IN):\n");
for (cnt = 0; cnt < ha->mbx_count; cnt++) {
- if (mboxes & BIT_0)
+ if (mboxes & BIT_0) {
*iptr2 = *iptr;
+ ql_dbg(ql_dbg_mbx, vha, 0x1114,
+ "mbox[%d]->0x%04x\n", cnt, *iptr2);
+ }
mboxes >>= 1;
iptr2++;
--
1.7.7
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] PCI: add quirk for 3ware 9650SE controller
[+cc another email addr for Adam from git logs]
On Wed, Aug 28, 2013 at 9:46 AM, Jiri Kosina wrote:
> On Tue, 27 Aug 2013, Jiri Kosina wrote:
>
>> On Tue, 27 Aug 2013, Jiri Kosina wrote:
>>
>> > Commit d5dea7d95 ("PCI: msi: Disable msi interrupts when we initialize a
>> > pci device") makes MSIs be forcibly disabled at boot time.
>> >
>> > It turns out that this breaks 3ware controller -- if MSIs are disabled
>> > during PCI discovery of this controller, the device doesn't work properly
>> > (it doesn't respond to any commands that are being sent to it after
>> > initialization).
>> >
>> > Reverting d5dea7d95 or not force-disabling MSIs in pci_msi_init_pci_dev()
>> > makes the device work properly again.
>> >
>> > Signed-off-by: Jiri Kosina
>> >
>> > ---
>> >
>> > I am adding Adam Radford as a recepient as well, to see whether he is able
>> > to provide some more explanation why this device would expose this
>> > behavior.
>
> OK, so Adam Radford's lsi.com address is bouncing, hence I guess we can't
> expect any feedback from him.
>
> Bjorn, Jesse, any word on this please?
It's on my list to look at. It's too late to put it in for v3.11, and
it's doubtful that it will even make the v3.12 merge window (though
possibly it could go in post-merge window). d5dea7d95 is several
years old, so hopefully this issue isn't super-urgent. Let me know if
otherwise.
Bjorn
>> > drivers/pci/msi.c|3 +++
>> > drivers/pci/quirks.c | 10 ++
>> > include/linux/pci.h |1 +
>> > 3 files changed, 14 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
>> > index aca7578..4f36b8b 100644
>> > --- a/drivers/pci/msi.c
>> > +++ b/drivers/pci/msi.c
>> > @@ -1040,6 +1040,9 @@ void pci_msi_init_pci_dev(struct pci_dev *dev)
>> > {
>> > INIT_LIST_HEAD(&dev->msi_list);
>> >
>> > + if (dev->broken_msi_disable)
>> > + return;
>> > +
>> > /* Disable the msi hardware to avoid screaming interrupts
>> > * during boot. This is the power on reset default so
>> > * usually this should be a noop.
>> > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
>> > index e85d230..4ba3400 100644
>> > --- a/drivers/pci/quirks.c
>> > +++ b/drivers/pci/quirks.c
>> > @@ -2890,6 +2890,16 @@ static void quirk_intel_ntb(struct pci_dev *dev)
>> > DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x0e08, quirk_intel_ntb);
>> > DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x0e0d, quirk_intel_ntb);
>> >
>> > +/*
>> > + * 3ware 9650SE controller doesn't properly initialize if MSI are
>> > + * disabled on it during PCI device discovery
>> > + */
>> > +static void quirk_broken_msi_disable(struct pci_dev *dev)
>> > +{
>> > + dev->broken_msi_disable = 1;
>> > +}
>> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_3WARE, 0x1004,
>> > quirk_broken_msi_disable);
>> > +
>> > static ktime_t fixup_debug_start(struct pci_dev *dev,
>> > void (*fn)(struct pci_dev *dev))
>> > {
>> > diff --git a/include/linux/pci.h b/include/linux/pci.h
>> > index 0fd1f15..c327d74 100644
>> > --- a/include/linux/pci.h
>> > +++ b/include/linux/pci.h
>> > @@ -341,6 +341,7 @@ struct pci_dev {
>> > #ifdef CONFIG_PCI_MSI
>> > struct list_head msi_list;
>> > struct kset *msi_kset;
>> > + unsigned intbroken_msi_disable:1;
>> > #endif
>> > struct pci_vpd *vpd;
>> > #ifdef CONFIG_PCI_ATS
>> >
>> > --
>> > Jiri Kosina
>> > SUSE Labs
>> >
>>
>> --
>> Jiri Kosina
>> SUSE Labs
>>
>
> --
> Jiri Kosina
> SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
re: [SCSI] arcmsr: Support Areca new SATA Raid Adapter ARC1214/1224/1264/1284
Dan Carpenter wrote:
>Hello 黃清隆,
>
>The patch 17628f3a062b: "[SCSI] arcmsr: Support Areca new SATA Raid
>Adapter ARC1214/1224/1264/1284" from Aug 26, 2013, leads to the
>following Smatch warning:
>"drivers/scsi/arcmsr/arcmsr_hba.c:3580 arcmsr_hbaD_get_config()
>warn: signedness bug returning '(-12)'"
>
>drivers/scsi/arcmsr/arcmsr_hba.c
>3576 dma_coherent = dma_alloc_coherent(&pdev->dev,
>acb->uncache_size,
> 3577 &dma_coherent_handle, GFP_KERNEL);
> 3578 if (!dma_coherent) {
> 3579 pr_notice("DMA allocation failed...\n");
> 3580 return -ENOMEM;
>^^
>This should be returning false.
>
> 3581 }
>
>Line 3577 has messed up indenting.
>
>Also this patch says it adds support for new hardware but almost 900
>lines out of this 3605 line patch are white space changes. Do the
>unrelated white space changes in a separate patch.
>
>This patch also re-introduces a bug which I fixed in the mainline
>kernel
>a year ago.
>
>drivers/scsi/arcmsr/arcmsr_hba.c
> 4525 writel(0xD, &pmuC->write_sequence);
>4526 } while temp = readl(&pmuC->host_diagnostic))
>|
> ^
>This should be a '&' not a '|'. Please fix this again back to the way
>it was.
>
> 4527 ARCMSR_ARC1880_DiagWrite_ENABLE) == 0) &&
> 4528 (count < 5));
>
>The indenting here is messed up as well. This is a very low quality
>patch.
>
>I think you are not using git internally in your company and that is
>why
>you are messing up so badly. Please learn to use it. Keep track of
>the
>fixes which go into the mainline kernel. Separate the white space
>cleanups from the new features.
OK based on this I'll drop the arcmsr updates pending a rewrite.
James
--
Sent from my Android phone with K-9 Mail. Please excuse my brevity.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC][PATCH] Add a flight data recorder for scsi commands
On Wed, 28 August 2013 14:45:14 +0200, Bart Van Assche wrote: > > Isn't this facility something that overlaps with what Wireshark can > already do today ? Wireshark can already capture SCSI traces for > several transport types (iSCSI, USB, ...). For other transports, > e.g. FC, I'd prefer to see Wireshark being extended instead of > adding more SCSI tracing mechanisms in the SCSI core. Wireshark would be great if there was a reproducable testcase and a developer could sit down for an afternoon and collect some traces. But how would you deal with rare cases that require around a year to reproduce per device and only matter when dealing with many devices? Jörn -- Data expands to fill the space available for storage. -- Parkinson's Law -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RESEND 0/1] AHCI: Optimize interrupt processing
On Thu, Aug 15, 2013 at 07:19:29PM -0700, Nicholas A. Bellinger wrote: > Anyways, before digging further into reserved tags logic, Jens, what are > your thoughts for addressing this special queue_depth=1 case for libata > + the like..? Hi Jens, Have some comments? -- Regards, Alexander Gordeev agord...@redhat.com -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC][PATCH] Add a flight data recorder for scsi commands
On Tue, 27 August 2013 22:09:19 -0700, Nicholas A. Bellinger wrote: > On Tue, 2013-08-27 at 18:17 -0400, Jörn Engel wrote: > > Here is a fun patch in an early state. Essentially I want to trace > > scsi commands, which has already been done long ago. The problem I > > have is that I care about all the scsi commands for one particular > > device - without knowing in advance which device it will be. Once I > > know the device in question, I want to dump the last X commands. > > > > The existing tracing is covering all commands to all devices. So > > either I have to oversize my trace buffer and dump way too much, or I > > will miss the stuff I am interested in most of the time. Hence my > > per-device trace buffer (called fdr to avoid namespace collisions). > > > > Sounds like a useful idea.. > > However, it adds another kmalloc + spin_lock -> spin_unlock in the fast > path for each struct scsi_cmnd I/O dispatch, which is exactly what the > scsi-mq prototype code is trying to avoid at all costs. Agreed. It shouldn't be too hard to replace the spin_lock with cmpxchg, maybe even rcu. And for the common case, one could reuse the old buffer, getting rid of kmalloc overhead as well. For the time being I will likely skip the work and see whether the existing patch actually catches something useful. First prove the quick hack to be useful, then optimize the hell out of it. Jörn -- No art, however minor, demands less than total dedication if you want to excel in it. -- Leon Battista Alberti -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC][PATCH] Add a flight data recorder for scsi commands
On Tue, 27 August 2013 20:34:43 -0400, Steven Rostedt wrote:
> On Tue, 27 Aug 2013 18:41:47 -0400
> Jörn Engel wrote:
> >
> > "without knowing in advance which device it will be"
> >
> > I have to trace all devices, because I don't know the interesting one
> > ahead of time. And after the fact, I only care about one device. So
> > having per-device trace buffers seems to be The Right Approach(tm).
> >
> > And having multiple trace buffers is where I cannot easily make my
> > problem fit your infrastructure.
>
> Are you sure about that?
When it comes to my mental shortcomings, I am quite sure about their
existence. ;)
> Note, I'm not sure you want this as something
> in production systems to always have on boot, for that, we can modify
> things a little to help you there. But other than that you can do:
>
> for each device d
> mkdir /sys/kernel/debug/tracing/instances/scsi-$d
> cd /sys/kernel/debug/tracing/instances/scsi-$d
> echo $filter > events/scsi/*/filter
> echo 1 > events/scsi/*/enable
> done
>
> The above is a script like pseudo code, just to convey the idea, not
> something to put in verbatim.
That does look fairly neat and doable. I am not sure about the
performance overhead, though. Would this translate into one big
switch or, for large systems, hundreds of "if (some_filter)" lines.
The latter would likely be noticeable overhead.
> Now, this may still not be what you want, but at a minimum, I would not
> add another hook like you did with the fdr_scsi_cmd(cmd). You know you
> can hook to the tracepoint directly.
>
>
> register_trace_scsi_dispatch_cmd_start(fdr_scsi_cmd, NULL);
>
> static void fdr_scsi_cmd(void *data, struct scsi_cmnd *cmd)
> {
> [...]
> }
>
> Once you call that register function, the tracepoint will call the
> function you registered when the tracepoint is hit.
Hmm. I will ponder that idea for a bit. If we can come up with
something merge-worthy, that would be great. If not, I will simply
carry my existing patch and not make anyone else pay the cost for it.
Jörn
--
The wise man seeks everything in himself; the ignorant man tries to get
everything from somebody else.
-- unknown
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
re: [SCSI] arcmsr: Support Areca new SATA Raid Adapter ARC1214/1224/1264/1284
Hello 黃清隆,
The patch 17628f3a062b: "[SCSI] arcmsr: Support Areca new SATA Raid
Adapter ARC1214/1224/1264/1284" from Aug 26, 2013, leads to the
following Smatch warning:
"drivers/scsi/arcmsr/arcmsr_hba.c:3580 arcmsr_hbaD_get_config()
warn: signedness bug returning '(-12)'"
drivers/scsi/arcmsr/arcmsr_hba.c
3576 dma_coherent = dma_alloc_coherent(&pdev->dev, acb->uncache_size,
3577 &dma_coherent_handle, GFP_KERNEL);
3578 if (!dma_coherent) {
3579 pr_notice("DMA allocation failed...\n");
3580 return -ENOMEM;
^^
This should be returning false.
3581 }
Line 3577 has messed up indenting.
Also this patch says it adds support for new hardware but almost 900
lines out of this 3605 line patch are white space changes. Do the
unrelated white space changes in a separate patch.
This patch also re-introduces a bug which I fixed in the mainline kernel
a year ago.
drivers/scsi/arcmsr/arcmsr_hba.c
4525 writel(0xD, &pmuC->write_sequence);
4526 } while temp = readl(&pmuC->host_diagnostic)) |
^
This should be a '&' not a '|'. Please fix this again back to the way
it was.
4527 ARCMSR_ARC1880_DiagWrite_ENABLE) == 0) &&
4528 (count < 5));
The indenting here is messed up as well. This is a very low quality
patch.
I think you are not using git internally in your company and that is why
you are messing up so badly. Please learn to use it. Keep track of the
fixes which go into the mainline kernel. Separate the white space
cleanups from the new features.
regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] PCI: add quirk for 3ware 9650SE controller
On Tue, 27 Aug 2013, Jiri Kosina wrote:
> On Tue, 27 Aug 2013, Jiri Kosina wrote:
>
> > Commit d5dea7d95 ("PCI: msi: Disable msi interrupts when we initialize a
> > pci device") makes MSIs be forcibly disabled at boot time.
> >
> > It turns out that this breaks 3ware controller -- if MSIs are disabled
> > during PCI discovery of this controller, the device doesn't work properly
> > (it doesn't respond to any commands that are being sent to it after
> > initialization).
> >
> > Reverting d5dea7d95 or not force-disabling MSIs in pci_msi_init_pci_dev()
> > makes the device work properly again.
> >
> > Signed-off-by: Jiri Kosina
> >
> > ---
> >
> > I am adding Adam Radford as a recepient as well, to see whether he is able
> > to provide some more explanation why this device would expose this
> > behavior.
OK, so Adam Radford's lsi.com address is bouncing, hence I guess we can't
expect any feedback from him.
Bjorn, Jesse, any word on this please?
> > Thanks.
> >
> > drivers/pci/msi.c|3 +++
> > drivers/pci/quirks.c | 10 ++
> > include/linux/pci.h |1 +
> > 3 files changed, 14 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> > index aca7578..4f36b8b 100644
> > --- a/drivers/pci/msi.c
> > +++ b/drivers/pci/msi.c
> > @@ -1040,6 +1040,9 @@ void pci_msi_init_pci_dev(struct pci_dev *dev)
> > {
> > INIT_LIST_HEAD(&dev->msi_list);
> >
> > + if (dev->broken_msi_disable)
> > + return;
> > +
> > /* Disable the msi hardware to avoid screaming interrupts
> > * during boot. This is the power on reset default so
> > * usually this should be a noop.
> > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> > index e85d230..4ba3400 100644
> > --- a/drivers/pci/quirks.c
> > +++ b/drivers/pci/quirks.c
> > @@ -2890,6 +2890,16 @@ static void quirk_intel_ntb(struct pci_dev *dev)
> > DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x0e08, quirk_intel_ntb);
> > DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x0e0d, quirk_intel_ntb);
> >
> > +/*
> > + * 3ware 9650SE controller doesn't properly initialize if MSI are
> > + * disabled on it during PCI device discovery
> > + */
> > +static void quirk_broken_msi_disable(struct pci_dev *dev)
> > +{
> > + dev->broken_msi_disable = 1;
> > +}
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_3WARE, 0x1004,
> > quirk_broken_msi_disable);
> > +
> > static ktime_t fixup_debug_start(struct pci_dev *dev,
> > void (*fn)(struct pci_dev *dev))
> > {
> > diff --git a/include/linux/pci.h b/include/linux/pci.h
> > index 0fd1f15..c327d74 100644
> > --- a/include/linux/pci.h
> > +++ b/include/linux/pci.h
> > @@ -341,6 +341,7 @@ struct pci_dev {
> > #ifdef CONFIG_PCI_MSI
> > struct list_head msi_list;
> > struct kset *msi_kset;
> > + unsigned intbroken_msi_disable:1;
> > #endif
> > struct pci_vpd *vpd;
> > #ifdef CONFIG_PCI_ATS
> >
> > --
> > Jiri Kosina
> > SUSE Labs
> >
>
> --
> Jiri Kosina
> SUSE Labs
>
--
Jiri Kosina
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH -next] ufs: fix source address of the read descriptor
When the query request with read descriptor opcode is completed, the
descriptor is copied from response UPIU to the buffer that the caller
has specified. Unfortunately the source address of the descriptor is
broken due to the unnecessary address-of operator.
Signed-off-by: Akinobu Mita
Cc: Dolev Raviv
Cc: Sujit Reddy Thumma
Cc: Vinayak Holikatti
Cc: Santosh Y
Cc: James Bottomley
Cc: linux-scsi@vger.kernel.org
---
drivers/scsi/ufs/ufshcd.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index a0f5ac2..7a319c6 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -454,8 +454,7 @@ void ufshcd_copy_query_response(struct ufs_hba *hba, struct
ufshcd_lrb *lrbp)
/* Get the descriptor */
if (lrbp->ucd_rsp_ptr->qr.opcode == UPIU_QUERY_OPCODE_READ_DESC) {
- u8 *descp = (u8 *)&lrbp->ucd_rsp_ptr +
- GENERAL_UPIU_REQUEST_SIZE;
+ u8 *descp = (u8 *)lrbp->ucd_rsp_ptr + GENERAL_UPIU_REQUEST_SIZE;
u16 len;
/* data segment length */
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/29] qla2xxx: Add support for ISP8044.
Hi James, Comments inline.. >On Tue, 2013-08-27 at 01:37 -0400, Saurav Kashyap wrote: >> From: Atul Deshmukh > >You seem to have stopped running your stuff through checkpatch: We did run all the patches through checkpatch before submission, did see the warnings and was trying to keep with the formatting in the rest of the file(s). We will take care of this in future. Thanks, ~Saurav > >WARNING: please, no spaces at the start of a line >#317: FILE: drivers/scsi/qla2xxx/qla_gbl.h:479: >+uint8_t *, uint32_t, uint32_t);$ > >WARNING: please, no spaces at the start of a line >#341: FILE: drivers/scsi/qla2xxx/qla_gbl.h:713: >+const uint32_t crb_reg, const uint32_t value);$ > >WARNING: please, no spaces at the start of a line >#351: FILE: drivers/scsi/qla2xxx/qla_gbl.h:723: >+uint32_t, uint32_t);$ > >WARNING: please, no spaces at the start of a line >#3175: FILE: drivers/scsi/qla2xxx/qla_nx2.c:2084: >+struct qla8044_minidump_entry_hdr *entry_hdr)$ > >WARNING: please, no spaces at the start of a line >#4258: FILE: drivers/scsi/qla2xxx/qla_nx2.c:3167: >+uint32_t data)$ > >WARNING: please, no spaces at the start of a line >#4334: FILE: drivers/scsi/qla2xxx/qla_nx2.c:3243: >+uint32_t sector_start_addr)$ > >WARNING: please, no spaces at the start of a line >#4390: FILE: drivers/scsi/qla2xxx/qla_nx2.c:3299: >+uint32_t *p_data)$ > >WARNING: please, no spaces at the start of a line >#4425: FILE: drivers/scsi/qla2xxx/qla_nx2.c:3334: >+uint32_t faddr, uint32_t dwords)$ > >WARNING: please, no spaces at the start of a line >#4508: FILE: drivers/scsi/qla2xxx/qla_nx2.c:3417: >+uint32_t faddr, uint32_t dwords)$ > >WARNING: please, no spaces at the start of a line >#4528: FILE: drivers/scsi/qla2xxx/qla_nx2.c:3437: >+uint32_t offset, uint32_t length)$ > >total: 0 errors, 10 warnings, 5781 lines checked > >Please don't; those are all legitimate problems. > >James > <>
[Bug 60644] MPT2SAS drops all HDDs when under high I/O
https://bugzilla.kernel.org/show_bug.cgi?id=60644 --- Comment #26 from livea...@live.com --- Hi . Thanks for your kind help , Bernd . setting /sys/block/sdX(c~j)/device/queue_depth to 1 unfortuantely didn't solve the issue . I put the following in the end of the boot line : mpt2sas.disable_watchdog=1 mpt2sas.max_queue_depth=1 mpt2sas.max_sgl_entries=64 mpt2sas.max_sectors=64 However , the OS doesn't see the controllers anymore . So I had to remove these entries from the boot line . Thank you . -- You are receiving this mail because: You are watching the assignee of the bug. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
hallo!
Wir bieten persönliche und geschäftliche Kredite ohne Sicherheiten (nur Identifikation) mit 3 % Zinssatz, von 20.000 bis 90,000,000, in 1 Jahr auf 20 Jahre Laufzeit. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch|rfc] block: fix race between request completion and timeout handling
Hannes Reinecke writes: >> So, looked into things a bit more. >> It looks as if you're on the right track, although I doubt your >> patch will fix the issue for me :-( >> >> Thing is, you're right there is a race window between requeuing >> and softirq triggering, which could well be fixed by your patch. >> So for that reason alone I would like to take it. >> >> However, including your patch will end up opening another can of >> worms: the softirq might now be triggering _while the request is >> queued on the request queue_. >> blk_requeue_request will be putting the request back on the request >> queue, where it'll be stuck until being pulled off from >> scsi_request_fn(). >> So if the softirq triggers during that condition we'll end up >> calling the BUG_ON((!list_empty(&req->queuelist)) in >> __blk_put_request(). >> >> Guess we'd need to fix that one, too ... >> > Ah. Now I see it. > > We're requeuing from the softirq context, ie after the completion > has triggered. So the above scenario can't actually happen and the > patch is valid. Do you still think it won't solve the issue you're seeing? What issue is that, btw? > > So: > > Acked-by: Hannes Reinecke Thanks, I guess I'll have to send a properly signed-off patch, now. ;-) Cheers, Jeff -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v3 4/6] scsi: ufs: add unipro attribute IDs
Tested-by: Yaniv Gardi QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation = > -Original Message- = > From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi- = > ow...@vger.kernel.org] On Behalf Of Subhash Jadavani = > Sent: Tuesday, August 27, 2013 12:15 PM = > To: Seungwon Jeon = > Cc: linux-scsi@vger.kernel.org; 'Vinayak Holikatti'; 'Santosh Y'; 'James E.J. = > Bottomley' = > Subject: Re: [PATCH v3 4/6] scsi: ufs: add unipro attribute IDs = > = > = > Looks good to me. = > Reviewed-by: Subhash Jadavani = > = > On 8/26/2013 8:10 PM, Seungwon Jeon wrote: = > > 'drivers/scsi/ufs/unipro.h' is added. = > > Attributes in the layers of the UNIPRO stack can be read and written = > > via the DME. = > > = > > Signed-off-by: Seungwon Jeon = > > Reviewed-by: Subhash Jadavani = > > --- = > > drivers/scsi/ufs/unipro.h | 130 = > + = > > 1 files changed, 130 insertions(+), 0 deletions(-) = > > create mode 100644 drivers/scsi/ufs/unipro.h = > > = > > diff --git a/drivers/scsi/ufs/unipro.h b/drivers/scsi/ufs/unipro.h new = > > file mode 100644 index 000..3a710eb = > > --- /dev/null = > > +++ b/drivers/scsi/ufs/unipro.h = > > @@ -0,0 +1,130 @@ = > > +/* = > > + * drivers/scsi/ufs/unipro.h = > > + * = > > + * Copyright (C) 2013 Samsung Electronics Co., Ltd. = > > + * = > > + * This program is free software; you can redistribute it and/or = > > +modify = > > + * it under the terms of the GNU General Public License as published = > > +by = > > + * the Free Software Foundation; either version 2 of the License, or = > > + * (at your option) any later version. = > > + */ = > > + = > > +#ifndef _UNIPRO_H_ = > > +#define _UNIPRO_H_ = > > + = > > +/* = > > + * PHY Adpater attributes = > > + */ = > > +#define PA_ACTIVETXDATALANES 0x1560 = > > +#define PA_ACTIVERXDATALANES 0x1580 = > > +#define PA_TXTRAILINGCLOCKS 0x1564 = > > +#define PA_PHY_TYPE 0x1500 = > > +#define PA_AVAILTXDATALANES 0x1520 = > > +#define PA_AVAILRXDATALANES 0x1540 = > > +#define PA_MINRXTRAILINGCLOCKS 0x1543 = > > +#define PA_TXPWRSTATUS 0x1567 = > > +#define PA_RXPWRSTATUS 0x1582 = > > +#define PA_TXFORCECLOCK 0x1562 = > > +#define PA_TXPWRMODE 0x1563 = > > +#define PA_LEGACYDPHYESCDL 0x1570 = > > +#define PA_MAXTXSPEEDFAST0x1521 = > > +#define PA_MAXTXSPEEDSLOW0x1522 = > > +#define PA_MAXRXSPEEDFAST0x1541 = > > +#define PA_MAXRXSPEEDSLOW0x1542 = > > +#define PA_TXLINKSTARTUPHS 0x1544 = > > +#define PA_TXSPEEDFAST 0x1565 = > > +#define PA_TXSPEEDSLOW 0x1566 = > > +#define PA_REMOTEVERINFO 0x15A0 = > > +#define PA_TXGEAR0x1568 = > > +#define PA_TXTERMINATION 0x1569 = > > +#define PA_HSSERIES 0x156A = > > +#define PA_PWRMODE 0x1571 = > > +#define PA_RXGEAR0x1583 = > > +#define PA_RXTERMINATION 0x1584 = > > +#define PA_MAXRXPWMGEAR 0x1586 = > > +#define PA_MAXRXHSGEAR 0x1587 = > > +#define PA_RXHSUNTERMCAP 0x15A5 = > > +#define PA_RXLSTERMCAP 0x15A6 = > > +#define PA_PACPREQTIMEOUT0x1590 = > > +#define PA_PACPREQEOBTIMEOUT 0x1591 = > > +#define PA_HIBERN8TIME 0x15A7 = > > +#define PA_LOCALVERINFO 0x15A9 = > > +#define PA_TACTIVATE 0x15A8 = > > +#define PA_PACPFRAMECOUNT0x15C0 = > > +#define PA_PACPERRORCOUNT0x15C1 = > > +#define PA_PHYTESTCONTROL0x15C2 = > > +#define PA_PWRMODEUSERDATA0 0x15B0 = > > +#define PA_PWRMODEUSERDATA1 0x15B1 = > > +#define PA_PWRMODEUSERDATA2 0x15B2 = > > +#define PA_PWRMODEUSERDATA3 0x15B3 = > > +#define PA_PWRMODEUSERDATA4 0x15B4 = > > +#define PA_PWRMODEUSERDATA5 0x15B5 = > > +#define PA_PWRMODEUSERDATA6 0x15B6 = > > +#define PA_PWRMODEUSERDATA7 0x15B7 = > > +#define PA_PWRMODEUSERDATA8 0x15B8 = > > +#define PA_PWRMODEUSERDATA9 0x15B9 = > > +#define PA_PWRMODEUSERDATA10 0x15BA = > > +#define PA_PWRMODEUSERDATA11 0x15BB = > > +#define PA_CONNECTEDTXDATALANES 0x1561 = > > +#define PA_CONNECTEDRXDATALANES 0x1581 = > > +#define PA_LOGICALLANEMAP0x15A1 = > > +#define PA_SLEEPNOCONFIGTIME 0x15A2 = > > +#define PA_STALLNOCONFIGTIME 0x15A3 = > > +#define PA_SAVECONFIGTIME0x15A4 = > > + = > > +/* = > > + * Data Link Layer Attributes = > > + */ = > > +#define DL_TC0TXFCTHRESHOLD 0x2040 = > > +#define DL_FC0PROTTIMEOUTVAL 0x2041 = > > +#define DL_TC0REPLAYTIMEOUTVAL 0x2042 = > > +#define DL_AFC0REQTIMEOUTVAL 0x2043 = > > +#define DL_AFC0CREDITTHRESHOLD 0x2044 = > > +#define DL_TC0OUTACKTHRESHOLD0x2045 = > > +#define DL_TC1TXFCTHRESHOLD 0x2060 = > > +#define DL_FC1PROTTIMEOUTVAL 0x2061 = > > +#define DL_TC1REPLAYTIMEOUTVAL 0x2062 = > > +#define DL_AFC1REQTIMEOUTVAL 0x2063 = > > +#defi
RE: [PATCH v3 5/6] scsi: ufs: add operation for the uic power mode change
Tested-by: Yaniv Gardi
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member of
Code Aurora Forum, hosted by The Linux Foundation
= > -Original Message-
= > From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi-
= > ow...@vger.kernel.org] On Behalf Of Seungwon Jeon
= > Sent: Tuesday, August 27, 2013 2:58 PM
= > To: 'Subhash Jadavani'
= > Cc: linux-scsi@vger.kernel.org; 'Vinayak Holikatti'; 'Santosh Y'; 'James
E.J.
= > Bottomley'
= > Subject: RE: [PATCH v3 5/6] scsi: ufs: add operation for the uic power
= > mode change
= >
= > On Tue, August 27, 2013, Subhash Jadavani wrote:
= > > On 8/27/2013 4:58 PM, Seungwon Jeon wrote:
= > > > On Tue, August 27, 2013, Subhash Jadavani wrote:
= > > >> On 8/26/2013 8:10 PM, Seungwon Jeon wrote:
= > > >>> Setting PA_PWRMode using DME_SET triggers the power mode
= > change.
= > > >>> And then the result will be given by the HCS.UPMCRS.
= > > >>> This operation should be done atomically.
= > > >>>
= > > >>> Signed-off-by: Seungwon Jeon
= > > >>> ---
= > > >>>drivers/scsi/ufs/ufshcd.c | 84
= > ++--
= > > >>>drivers/scsi/ufs/ufshcd.h |3 ++
= > > >>>drivers/scsi/ufs/ufshci.h | 12 ++
= > > >>>3 files changed, 95 insertions(+), 4 deletions(-)
= > > >>>
= > > >>> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
= > > >>> index 00bfc1a..c67118d 100644
= > > >>> --- a/drivers/scsi/ufs/ufshcd.c
= > > >>> +++ b/drivers/scsi/ufs/ufshcd.c
= > > >>> @@ -36,9 +36,11 @@
= > > >>>#include
= > > >>>
= > > >>>#include "ufshcd.h"
= > > >>> +#include "unipro.h"
= > > >>>
= > > >>>#define UFSHCD_ENABLE_INTRS
= > (UTP_TRANSFER_REQ_COMPL |\
= > > >>>UTP_TASK_REQ_COMPL |\
= > > >>> + UIC_POWER_MODE |\
= > > >>>UFSHCD_ERROR_MASK)
= > > >>>/* UIC command timeout, unit: ms */
= > > >>>#define UIC_CMD_TIMEOUT500
= > > >>> @@ -520,6 +522,18 @@ static inline bool
= > ufshcd_ready_for_uic_cmd(struct ufs_hba *hba)
= > > >>>}
= > > >>>
= > > >>>/**
= > > >>> + * ufshcd_get_upmcrs - Get the power mode change request status
= > > >>> + * @hba: Pointer to adapter instance
= > > >>> + *
= > > >>> + * This function gets the UPMCRS field of HCS register
= > > >>> + * Returns value of UPMCRS field
= > > >>> + */
= > > >>> +static inline u8 ufshcd_get_upmcrs(struct ufs_hba *hba) {
= > > >>> + return (ufshcd_readl(hba, REG_CONTROLLER_STATUS) >> 8) & 0x7;
= > }
= > > >>> +
= > > >>> +/**
= > > >>> * ufshcd_dispatch_uic_cmd - Dispatch UIC commands to unipro
= > layers
= > > >>> * @hba: per adapter instance
= > > >>> * @uic_cmd: UIC command
= > > >>> @@ -1526,6 +1540,64 @@ out:
= > > >>>EXPORT_SYMBOL_GPL(ufshcd_dme_get_attr);
= > > >>>
= > > >>>/**
= > > >>> + * ufshcd_uic_change_pwr_mode - Perform the UIC power mode
= > chage
= > > >>> + * using DME_SET primitives.
= > > >>> + * @hba: per adapter instance
= > > >>> + * @mode: powr mode value
= > > >>> + *
= > > >>> + * Returns 0 on success, non-zero value on failure */ int
= > > >>> +ufshcd_uic_change_pwr_mode(struct ufs_hba *hba, u8 mode) {
= > > >>> + struct uic_command uic_cmd = {0};
= > > >>> + struct completion pwr_done;
= > > >>> + unsigned long flags;
= > > >>> + u8 status;
= > > >>> + int ret;
= > > >>> +
= > > >>> + uic_cmd.command = UIC_CMD_DME_SET;
= > > >>> + uic_cmd.argument1 = UIC_ARG_MIB(PA_PWRMODE);
= > > >>> + uic_cmd.argument3 = mode;
= > > >>> + init_completion(&pwr_done);
= > > >>> +
= > > >>> + mutex_lock(&hba->uic_cmd_mutex);
= > > >>> +
= > > >>> + spin_lock_irqsave(hba->host->host_lock, flags);
= > > >>> + hba->pwr_done = &pwr_done;
= > > >>> + spin_unlock_irqrestore(hba->host->host_lock, flags);
= > > >>> + ret = __ufshcd_send_uic_cmd(hba, &uic_cmd);
= > > >>> + if (ret) {
= > > >>> + dev_err(hba->dev,
= > > >>> + "pwr mode change with mode 0x%x uic error
= > %d\n",
= > > >>> + mode, ret);
= > > >>> + goto out;
= > > >>> + }
= > > >>> +
= > > >>> + if (!wait_for_completion_timeout(hba->pwr_done,
= > > >>> +
= > msecs_to_jiffies(UIC_CMD_TIMEOUT))) {
= > > >>> + dev_err(hba->dev,
= > > >>> + "pwr mode change with mode 0x%x completion
= > timeout\n",
= > > >>> + mode);
= > > >>> + ret = -ETIMEDOUT;
= > > >>> + goto out;
= > > >>> + }
= > > >>> +
= > > >>> + status = ufshcd_get_upmcrs(hba);
= > > >> I have a doubt on the relation between UIC Power Mode Status
= > (UPMS)
= > > >> bit & UIC Power Mode Change Request Status (UPMCRS) field.
= > > >>
= > > >> Once we send the DME_SET(PA_PWRMODE) command, we will get
= > the UIC
= > > >> command completion interrupt (with IS.UCCS bit set) which just
= > > >> indicate that the attribute is
Re: [RFC][PATCH] Add a flight data recorder for scsi commands
On 08/28/13 00:17, Jörn Engel wrote: Here is a fun patch in an early state. Essentially I want to trace scsi commands, which has already been done long ago. The problem I have is that I care about all the scsi commands for one particular device - without knowing in advance which device it will be. Once I know the device in question, I want to dump the last X commands. The existing tracing is covering all commands to all devices. So either I have to oversize my trace buffer and dump way too much, or I will miss the stuff I am interested in most of the time. Hence my per-device trace buffer (called fdr to avoid namespace collisions). Isn't this facility something that overlaps with what Wireshark can already do today ? Wireshark can already capture SCSI traces for several transport types (iSCSI, USB, ...). For other transports, e.g. FC, I'd prefer to see Wireshark being extended instead of adding more SCSI tracing mechanisms in the SCSI core. Best regards, Bart. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v3 3/6] scsi: ufs: add dme configuration primitives
Tested-by: Yaniv Gardi
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member of
Code Aurora Forum, hosted by The Linux Foundation
= > -Original Message-
= > From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi-
= > ow...@vger.kernel.org] On Behalf Of Subhash Jadavani
= > Sent: Tuesday, August 27, 2013 12:16 PM
= > To: Seungwon Jeon
= > Cc: linux-scsi@vger.kernel.org; 'Vinayak Holikatti'; 'Santosh Y'; 'James
E.J.
= > Bottomley'
= > Subject: Re: [PATCH v3 3/6] scsi: ufs: add dme configuration primitives
= >
= >
= > Looks good to me.
= > Reviewed-by: Subhash Jadavani
= >
= > On 8/26/2013 8:10 PM, Seungwon Jeon wrote:
= > > Implements to support GET and SET operations of the DME.
= > > These operations are used to configure the behavior of the UNIPRO.
= > > Along with basic operation, {Peer/AttrSetType} can be mixed.
= > >
= > > Signed-off-by: Seungwon Jeon
= > > Reviewed-by: Subhash Jadavani
= > > ---
= > > drivers/scsi/ufs/ufshcd.c | 88
= > +
= > > drivers/scsi/ufs/ufshcd.h | 51 ++
= > > drivers/scsi/ufs/ufshci.h |6 +++
= > > 3 files changed, 145 insertions(+), 0 deletions(-)
= > >
= > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
= > > index c90b88a..00bfc1a 100644
= > > --- a/drivers/scsi/ufs/ufshcd.c
= > > +++ b/drivers/scsi/ufs/ufshcd.c
= > > @@ -285,6 +285,18 @@ static inline int
= > ufshcd_get_uic_cmd_result(struct ufs_hba *hba)
= > > }
= > >
= > > /**
= > > + * ufshcd_get_dme_attr_val - Get the value of attribute returned by
= > > +UIC command
= > > + * @hba: Pointer to adapter instance
= > > + *
= > > + * This function gets UIC command argument3
= > > + * Returns 0 on success, non zero value on error */ static inline
= > > +u32 ufshcd_get_dme_attr_val(struct ufs_hba *hba) {
= > > + return ufshcd_readl(hba, REG_UIC_COMMAND_ARG_3); }
= > > +
= > > +/**
= > >* ufshcd_get_req_rsp - returns the TR response transaction type
= > >* @ucd_rsp_ptr: pointer to response UPIU
= > >*/
= > > @@ -1440,6 +1452,80 @@ static int ufshcd_dme_link_startup(struct
= > ufs_hba *hba)
= > > }
= > >
= > > /**
= > > + * ufshcd_dme_set_attr - UIC command for DME_SET, DME_PEER_SET
= > > + * @hba: per adapter instance
= > > + * @attr_sel: uic command argument1
= > > + * @attr_set: attribute set type as uic command argument2
= > > + * @mib_val: setting value as uic command argument3
= > > + * @peer: indicate whether peer or local
= > > + *
= > > + * Returns 0 on success, non-zero value on failure */ int
= > > +ufshcd_dme_set_attr(struct ufs_hba *hba, u32 attr_sel,
= > > + u8 attr_set, u32 mib_val, u8 peer) {
= > > + struct uic_command uic_cmd = {0};
= > > + static const char *const action[] = {
= > > + "dme-set",
= > > + "dme-peer-set"
= > > + };
= > > + const char *set = action[!!peer];
= > > + int ret;
= > > +
= > > + uic_cmd.command = peer ?
= > > + UIC_CMD_DME_PEER_SET : UIC_CMD_DME_SET;
= > > + uic_cmd.argument1 = attr_sel;
= > > + uic_cmd.argument2 = UIC_ARG_ATTR_TYPE(attr_set);
= > > + uic_cmd.argument3 = mib_val;
= > > +
= > > + ret = ufshcd_send_uic_cmd(hba, &uic_cmd);
= > > + if (ret)
= > > + dev_err(hba->dev, "%s: attr-id 0x%x val 0x%x error code
= > %d\n",
= > > + set, UIC_GET_ATTR_ID(attr_sel), mib_val, ret);
= > > +
= > > + return ret;
= > > +}
= > > +EXPORT_SYMBOL_GPL(ufshcd_dme_set_attr);
= > > +
= > > +/**
= > > + * ufshcd_dme_get_attr - UIC command for DME_GET, DME_PEER_GET
= > > + * @hba: per adapter instance
= > > + * @attr_sel: uic command argument1
= > > + * @mib_val: the value of the attribute as returned by the UIC
= > > +command
= > > + * @peer: indicate whether peer or local
= > > + *
= > > + * Returns 0 on success, non-zero value on failure */ int
= > > +ufshcd_dme_get_attr(struct ufs_hba *hba, u32 attr_sel,
= > > + u32 *mib_val, u8 peer)
= > > +{
= > > + struct uic_command uic_cmd = {0};
= > > + static const char *const action[] = {
= > > + "dme-get",
= > > + "dme-peer-get"
= > > + };
= > > + const char *get = action[!!peer];
= > > + int ret;
= > > +
= > > + uic_cmd.command = peer ?
= > > + UIC_CMD_DME_PEER_GET : UIC_CMD_DME_GET;
= > > + uic_cmd.argument1 = attr_sel;
= > > +
= > > + ret = ufshcd_send_uic_cmd(hba, &uic_cmd);
= > > + if (ret) {
= > > + dev_err(hba->dev, "%s: attr-id 0x%x error code %d\n",
= > > + get, UIC_GET_ATTR_ID(attr_sel), ret);
= > > + goto out;
= > > + }
= > > +
= > > + if (mib_val)
= > > + *mib_val = uic_cmd.argument3;
= > > +out:
= > > + return ret;
= > > +}
= > > +EXPORT_SYMBOL_GPL(ufshcd_dme_get_attr);
= > > +
= > > +/**
= > >* ufshcd_complete_dev_init() - checks device readiness
= > >* hba: per-adapter instance
= > >*
= > > @@ -1912,6 +1998,8 @@ static void ufshcd_uic_cmd_compl(struct
= > ufs_hba *hba)
= > > if (hba->active_uic_cmd) {
= > >
RE: [PATCH v3 2/6] scsi: ufs: fix the setting interrupt aggregation counter
Tested-by: Yaniv Gardi
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member of
Code Aurora Forum, hosted by The Linux Foundation
= > -Original Message-
= > From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi-
= > ow...@vger.kernel.org] On Behalf Of Subhash Jadavani
= > Sent: Tuesday, August 27, 2013 12:02 PM
= > To: Seungwon Jeon
= > Cc: linux-scsi@vger.kernel.org; 'Vinayak Holikatti'; 'Santosh Y'; 'James
E.J.
= > Bottomley'
= > Subject: Re: [PATCH v3 2/6] scsi: ufs: fix the setting interrupt aggregation
= > counter
= >
= >
= > Looks good to me.
= > Reviewed-by: Subhash Jadavani
= >
= > On 8/26/2013 8:10 PM, Seungwon Jeon wrote:
= > > IACTH(Interrupt aggregation counter threshold) value is allowed up to
= > > 0x1F and current setting value is the maximum.
= > > This value is related with NUTRS(max:0x20) of HCI's capability.
= > > Considering HCI controller doesn't support the maximum, IACTH setting
= > > should be adjusted with possible value.
= > > For that, existing 'ufshcd_config_int_aggr' is split into two part
= > > [reset, configure].
= > >
= > > Signed-off-by: Seungwon Jeon
= > > Reviewed-by: Subhash Jadavani
= > > ---
= > > drivers/scsi/ufs/ufshcd.c | 53 +--
= > -
= > > drivers/scsi/ufs/ufshci.h |4 +-
= > > 2 files changed, 27 insertions(+), 30 deletions(-)
= > >
= > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
= > > index 6ff16c9..c90b88a 100644
= > > --- a/drivers/scsi/ufs/ufshcd.c
= > > +++ b/drivers/scsi/ufs/ufshcd.c
= > > @@ -59,6 +59,9 @@
= > > /* Expose the flag value from utp_upiu_query.value */
= > > #define MASK_QUERY_UPIU_FLAG_LOC 0xFF
= > >
= > > +/* Interrupt aggregation default timeout, unit: 40us */
= > > +#define INT_AGGR_DEF_TO 0x02
= > > +
= > > enum {
= > > UFSHCD_MAX_CHANNEL = 0,
= > > UFSHCD_MAX_ID = 1,
= > > @@ -94,12 +97,6 @@ enum {
= > > UFSHCD_INT_CLEAR,
= > > };
= > >
= > > -/* Interrupt aggregation options */
= > > -enum {
= > > - INT_AGGR_RESET,
= > > - INT_AGGR_CONFIG,
= > > -};
= > > -
= > > #define ufshcd_set_eh_in_progress(h) \
= > > (h->eh_flags |= UFSHCD_EH_IN_PROGRESS)
= > > #define ufshcd_eh_in_progress(h) \
= > > @@ -340,30 +337,30 @@ static inline bool
= > ufshcd_is_exception_event(struct utp_upiu_rsp *ucd_rsp_ptr)
= > > }
= > >
= > > /**
= > > - * ufshcd_config_int_aggr - Configure interrupt aggregation values.
= > > - * Currently there is no use case where we want to
configure
= > > - * interrupt aggregation dynamically. So to configure
interrupt
= > > - * aggregation, #define
= > INT_AGGR_COUNTER_THRESHOLD_VALUE and
= > > - * INT_AGGR_TIMEOUT_VALUE are used.
= > > + * ufshcd_reset_intr_aggr - Reset interrupt aggregation values.
= > >* @hba: per adapter instance
= > > - * @option: Interrupt aggregation option
= > >*/
= > > static inline void
= > > -ufshcd_config_int_aggr(struct ufs_hba *hba, int option)
= > > +ufshcd_reset_intr_aggr(struct ufs_hba *hba)
= > > {
= > > - switch (option) {
= > > - case INT_AGGR_RESET:
= > > - ufshcd_writel(hba, INT_AGGR_ENABLE |
= > > - INT_AGGR_COUNTER_AND_TIMER_RESET,
= > > -
= > REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL);
= > > - break;
= > > - case INT_AGGR_CONFIG:
= > > - ufshcd_writel(hba, INT_AGGR_ENABLE |
= > INT_AGGR_PARAM_WRITE |
= > > - INT_AGGR_COUNTER_THRESHOLD_VALUE |
= > > - INT_AGGR_TIMEOUT_VALUE,
= > > -
= > REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL);
= > > - break;
= > > - }
= > > + ufshcd_writel(hba, INT_AGGR_ENABLE |
= > > + INT_AGGR_COUNTER_AND_TIMER_RESET,
= > > + REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL);
= > > +}
= > > +
= > > +/**
= > > + * ufshcd_config_intr_aggr - Configure interrupt aggregation values.
= > > + * @hba: per adapter instance
= > > + * @cnt: Interrupt aggregation counter threshold
= > > + * @tmout: Interrupt aggregation timeout value */ static inline void
= > > +ufshcd_config_intr_aggr(struct ufs_hba *hba, u8 cnt, u8 tmout) {
= > > + ufshcd_writel(hba, INT_AGGR_ENABLE |
= > INT_AGGR_PARAM_WRITE |
= > > + INT_AGGR_COUNTER_THLD_VAL(cnt) |
= > > + INT_AGGR_TIMEOUT_VAL(tmout),
= > > + REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL);
= > > }
= > >
= > > /**
= > > @@ -1523,7 +1520,7 @@ static int ufshcd_make_hba_operational(struct
= > ufs_hba *hba)
= > > ufshcd_enable_intr(hba, UFSHCD_ENABLE_INTRS);
= > >
= > > /* Configure interrupt aggregation */
= > > - ufshcd_config_int_aggr(hba, INT_AGGR_CONFIG);
= > > + ufshcd_config_intr_aggr(hba, hba->nutrs - 1, INT_AGGR_DEF_TO);
= > >
= > > /* Configure UTRL and UTMRL base address registers */
= > > ufshcd_writel(hba, lower_32_bits(hba->utrdl_dma_addr),
= > > @@ -1971,7 +1968,7 @@ static void ufshcd_transf
RE: [PATCH v3 1/6] scsi: ufs: find out sense data over scsi status values
Tested-by: Yaniv Gardi
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member of
Code Aurora Forum, hosted by The Linux Foundation
= > -Original Message-
= > From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi-
= > ow...@vger.kernel.org] On Behalf Of Subhash Jadavani
= > Sent: Tuesday, August 27, 2013 11:53 AM
= > To: Seungwon Jeon
= > Cc: linux-scsi@vger.kernel.org; 'Vinayak Holikatti'; 'Santosh Y'; 'James
E.J.
= > Bottomley'
= > Subject: Re: [PATCH v3 1/6] scsi: ufs: find out sense data over scsi status
= > values
= >
= >
= > Looks good to me.
= > Reviewed-by: Subhash Jadavani
= >
= > On 8/26/2013 8:10 PM, Seungwon Jeon wrote:
= > > Unlike 'GOOD' and 'CHECK CONDITION', other status values in Response
= > > UPIU may or may not contain sense data. That is returning sense data
= > > isn't obvious. So, in this case the Data Segment Length field should
= > > be checked. If a non-zero value, it means that UPIU has Sense Data in
= > > the Data Segment area.
= > >
= > > Signed-off-by: Seungwon Jeon
= > > Reviewed-by: Subhash Jadavani
= > > ---
= > > drivers/scsi/ufs/ufs.h|1 +
= > > drivers/scsi/ufs/ufshcd.c | 37 ++---
= > > 2 files changed, 23 insertions(+), 15 deletions(-)
= > >
= > > diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h index
= > > bce09a6..7210500 100644
= > > --- a/drivers/scsi/ufs/ufs.h
= > > +++ b/drivers/scsi/ufs/ufs.h
= > > @@ -177,6 +177,7 @@ enum {
= > > MASK_TASK_RESPONSE = 0xFF00,
= > > MASK_RSP_UPIU_RESULT= 0x,
= > > MASK_QUERY_DATA_SEG_LEN = 0x,
= > > + MASK_RSP_UPIU_DATA_SEG_LEN = 0x,
= > > MASK_RSP_EXCEPTION_EVENT= 0x1,
= > > };
= > >
= > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
= > > index 1b99c0a..6ff16c9 100644
= > > --- a/drivers/scsi/ufs/ufshcd.c
= > > +++ b/drivers/scsi/ufs/ufshcd.c
= > > @@ -310,6 +310,20 @@ ufshcd_get_rsp_upiu_result(struct utp_upiu_rsp
= > *ucd_rsp_ptr)
= > > return be32_to_cpu(ucd_rsp_ptr->header.dword_1) &
= > MASK_RSP_UPIU_RESULT;
= > > }
= > >
= > > +/*
= > > + * ufshcd_get_rsp_upiu_data_seg_len - Get the data segment length
= > > + * from response UPIU
= > > + * @ucd_rsp_ptr: pointer to response UPIU
= > > + *
= > > + * Return the data segment length.
= > > + */
= > > +static inline unsigned int
= > > +ufshcd_get_rsp_upiu_data_seg_len(struct utp_upiu_rsp *ucd_rsp_ptr) {
= > > + return be32_to_cpu(ucd_rsp_ptr->header.dword_2) &
= > > + MASK_RSP_UPIU_DATA_SEG_LEN;
= > > +}
= > > +
= > > /**
= > >* ufshcd_is_exception_event - Check if the device raised an exception
= > event
= > >* @ucd_rsp_ptr: pointer to response UPIU @@ -405,7 +419,8 @@ void
= > > ufshcd_send_command(struct ufs_hba *hba, unsigned int task_tag)
= > > static inline void ufshcd_copy_sense_data(struct ufshcd_lrb *lrbp)
= > > {
= > > int len;
= > > - if (lrbp->sense_buffer) {
= > > + if (lrbp->sense_buffer &&
= > > + ufshcd_get_rsp_upiu_data_seg_len(lrbp->ucd_rsp_ptr)) {
= > > len = be16_to_cpu(lrbp->ucd_rsp_ptr->sr.sense_data_len);
= > > memcpy(lrbp->sense_buffer,
= > > lrbp->ucd_rsp_ptr->sr.sense_data, @@ -1789,32
= > +1804,24 @@
= > > ufshcd_scsi_cmd_status(struct ufshcd_lrb *lrbp, int scsi_status)
= > > int result = 0;
= > >
= > > switch (scsi_status) {
= > > - case SAM_STAT_GOOD:
= > > - result |= DID_OK << 16 |
= > > - COMMAND_COMPLETE << 8 |
= > > - SAM_STAT_GOOD;
= > > - break;
= > > case SAM_STAT_CHECK_CONDITION:
= > > + ufshcd_copy_sense_data(lrbp);
= > > + case SAM_STAT_GOOD:
= > > result |= DID_OK << 16 |
= > > COMMAND_COMPLETE << 8 |
= > > - SAM_STAT_CHECK_CONDITION;
= > > - ufshcd_copy_sense_data(lrbp);
= > > - break;
= > > - case SAM_STAT_BUSY:
= > > - result |= SAM_STAT_BUSY;
= > > + scsi_status;
= > > break;
= > > case SAM_STAT_TASK_SET_FULL:
= > > -
= > > /*
= > >* If a LUN reports SAM_STAT_TASK_SET_FULL, then the
= > LUN queue
= > >* depth needs to be adjusted to the exact number of
= > >* outstanding commands the LUN can handle at any given
= > time.
= > >*/
= > > ufshcd_adjust_lun_qdepth(lrbp->cmd);
= > > - result |= SAM_STAT_TASK_SET_FULL;
= > > - break;
= > > + case SAM_STAT_BUSY:
= > > case SAM_STAT_TASK_ABORTED:
= > > - result |= SAM_STAT_TASK_ABORTED;
= > > + ufshcd_copy_sense_data(lrbp);
= > > + result |= scsi_status;
= > > break;
= > > default:
= > > result |= DID_ER
Re: [PATCH v3 0/6] scsi: ufs: some fixes and updates
Hi Santosh, These patches looks good to me (Have already gave Reviewed-by to all patches). Please check if its fine to be pushed to scsi-next? Regards, Subhash On 8/26/2013 8:10 PM, Seungwon Jeon wrote: This path series contain driver's fixes and updates. Changes in v3: - Rebased with scsi-next Changes in v2: - [scsi: ufs: amend the ocs handling with fatal error] is excluded. Host behavior needs to be defined clearly in UFSHCI specification. - Some minor changes are applied with comments from Subhash, Sujit and Santosh. Seungwon Jeon (6): scsi: ufs: find out sense data over scsi status values scsi: ufs: fix the setting interrupt aggregation counter scsi: ufs: add dme configuration primitives scsi: ufs: add unipro attribute IDs scsi: ufs: add operation for the uic power mode change scsi: ufs: configure the attribute for power mode drivers/scsi/ufs/ufs.h|1 + drivers/scsi/ufs/ufshcd.c | 328 ++--- drivers/scsi/ufs/ufshcd.h | 54 drivers/scsi/ufs/ufshci.h | 22 +++- drivers/scsi/ufs/unipro.h | 151 + 5 files changed, 507 insertions(+), 49 deletions(-) Thanks, Seungwon Jeon -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 1/4] sg: use rwsem to solve race during exclusive open
On Wed, 2013-08-28 at 18:07 +0800, Vaughan Cao wrote:
> @@ -331,17 +330,20 @@ sg_open(struct inode *inode, struct file *filp)
> }
> if ((sfp = sg_add_sfp(sdp, dev)))
> filp->private_data = sfp;
> - else {
> + /* retval is already provably zero at this point
> because of the
> +* check after retval =
> scsi_autopm_get_device(sdp->device))
> +*/
> + else
> + retval = -ENOMEM;
> +
> + if (retval) {
> +sem_out:
There's still no need for the double if. You know the value of retval
in each of the legs of the first if; its only non zero for the else leg,
so you can combine them thus:
if ((sfp = sg_add_sfp(sdp, dev)))
filp->private_data = sfp;
/* retval is already provably zero at this point because of the
* check after retval scsi_autopm_get_device(sdp->device))
*/
else {
retval = -ENOMEM;
sem_out:
...
James
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 0/4][SCSI] sg: fix race condition in sg_open
There is a race when open sg with O_EXCL flag. Also a race may happen between sg_open and sg_remove. Changes from v5: Patches based on v3.11-rc7 and passed sg_tst_excl test. * [1/4] * remove unused variables - res,sdp. * fix insane code dealing with sg_add_sfp. * [2/4] resolve conflict with v3.11-rc7. * [3/4] remove sem_out label. * [4/4] add sdp definition. Changes from v4: * [3/4] use ERR_PTR series instead of adding another parameter in sg_add_sfp * [4/4] fix conflict for cherry-pick from v3. Changes from v3: * release o_sem in sg_release(), not in sg_remove_sfp(). * not set exclude with sfd_lock held. Vaughan Cao (4): sg: use rwsem to solve race during exclusive open sg: no need sg_open_exclusive_lock sg: checking sdp->detached isn't protected when open sg: push file descriptor list locking down to per-device locking drivers/scsi/sg.c | 173 +- 1 file changed, 80 insertions(+), 93 deletions(-) -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 3/4] sg: checking sdp->detached isn't protected when open
@detached is set under the protection of sg_index_lock. Without getting the
lock, new sfp will be added during sg removal and there is no chance for it
to be picked out. So check with sg_index_lock held in sg_add_sfp().
Changes from v5:
* remove sem_out label.
Changes from v4:
* use ERR_PTR series instead of adding another parameter in sg_add_sfp
Signed-off-by: Vaughan Cao
---
drivers/scsi/sg.c | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index dcbd95f..6bffe52 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -295,10 +295,6 @@ sg_open(struct inode *inode, struct file *filp)
if (flags & O_EXCL)
sdp->exclude = 1; /* used by release lock */
- if (sdp->detached) {
- retval = -ENODEV;
- goto sem_out;
- }
if (sfds_list_empty(sdp)) { /* no existing opens on this device */
sdp->sgdebug = 0;
q = sdp->device->request_queue;
@@ -309,16 +305,16 @@ sg_open(struct inode *inode, struct file *filp)
/* retval is already provably zero at this point because of the
* check after retval = scsi_autopm_get_device(sdp->device))
*/
- else
- retval = -ENOMEM;
-
- if (retval) {
-sem_out:
+ else {
+ retval = PTR_ERR(sfp);
if (flags & O_EXCL) {
sdp->exclude = 0; /* undo if error */
up_write(&sdp->o_sem);
} else
up_read(&sdp->o_sem);
+ }
+
+ if (retval) {
error_out:
scsi_autopm_put_device(sdp->device);
sdp_put:
@@ -2047,7 +2043,7 @@ sg_add_sfp(Sg_device * sdp, int dev)
sfp = kzalloc(sizeof(*sfp), GFP_ATOMIC | __GFP_NOWARN);
if (!sfp)
- return NULL;
+ return ERR_PTR(-ENOMEM);
init_waitqueue_head(&sfp->read_wait);
rwlock_init(&sfp->rq_list_lock);
@@ -2062,6 +2058,10 @@ sg_add_sfp(Sg_device * sdp, int dev)
sfp->keep_orphan = SG_DEF_KEEP_ORPHAN;
sfp->parentdp = sdp;
write_lock_irqsave(&sg_index_lock, iflags);
+ if (sdp->detached) {
+ write_unlock_irqrestore(&sg_index_lock, iflags);
+ return ERR_PTR(-ENODEV);
+ }
list_add_tail(&sfp->sfd_siblings, &sdp->sfds);
write_unlock_irqrestore(&sg_index_lock, iflags);
SCSI_LOG_TIMEOUT(3, printk("sg_add_sfp: sfp=0x%p\n", sfp));
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 4/4] sg: push file descriptor list locking down to per-device locking
Push file descriptor list locking down to per-device locking. Let sg_index_lock
only protect device lookup.
sdp->detached is also set and checked with this lock held.
Changes from v4:
* Since I use ERR_PTR and friends in sg_add_sfp, this patch should also be
updated to resolve conflict in cherrry-pick.
Signed-off-by: Vaughan Cao
---
drivers/scsi/sg.c | 62 ++-
1 file changed, 34 insertions(+), 28 deletions(-)
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 6bffe52..10d6943 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -106,8 +106,7 @@ static int sg_add(struct device *, struct class_interface
*);
static void sg_remove(struct device *, struct class_interface *);
static DEFINE_IDR(sg_index_idr);
-static DEFINE_RWLOCK(sg_index_lock); /* Also used to lock
- file descriptor list
for device */
+static DEFINE_RWLOCK(sg_index_lock);
static struct class_interface sg_interface = {
.add_dev= sg_add,
@@ -144,8 +143,7 @@ typedef struct sg_request { /* SG_MAX_QUEUE requests
outstanding per file */
} Sg_request;
typedef struct sg_fd { /* holds the state of a file descriptor */
- /* sfd_siblings is protected by sg_index_lock */
- struct list_head sfd_siblings;
+ struct list_head sfd_siblings; /* protected by sfd_lock of device */
struct sg_device *parentdp; /* owning device */
wait_queue_head_t read_wait;/* queue read until command done */
rwlock_t rq_list_lock; /* protect access to list in req_arr */
@@ -170,7 +168,7 @@ typedef struct sg_device { /* holds the state of each scsi
generic device */
struct scsi_device *device;
int sg_tablesize; /* adapter's max scatter-gather table size */
u32 index; /* device index number */
- /* sfds is protected by sg_index_lock */
+ spinlock_t sfd_lock;/* protect file descriptor list for device */
struct list_head sfds;
struct rw_semaphore o_sem; /* exclude open should hold this rwsem
*/
volatile char detached; /* 0->attached, 1->detached pending removal */
@@ -227,9 +225,9 @@ static int sfds_list_empty(Sg_device *sdp)
unsigned long flags;
int ret;
- read_lock_irqsave(&sg_index_lock, flags);
+ spin_lock_irqsave(&sdp->sfd_lock, flags);
ret = list_empty(&sdp->sfds);
- read_unlock_irqrestore(&sg_index_lock, flags);
+ spin_unlock_irqrestore(&sdp->sfd_lock, flags);
return ret;
}
@@ -1394,6 +1392,7 @@ static Sg_device *sg_alloc(struct gendisk *disk, struct
scsi_device *scsidp)
disk->first_minor = k;
sdp->disk = disk;
sdp->device = scsidp;
+ spin_lock_init(&sdp->sfd_lock);
INIT_LIST_HEAD(&sdp->sfds);
init_rwsem(&sdp->o_sem);
sdp->sg_tablesize = queue_max_segments(q);
@@ -1528,11 +1527,13 @@ static void sg_remove(struct device *cl_dev, struct
class_interface *cl_intf)
/* Need a write lock to set sdp->detached. */
write_lock_irqsave(&sg_index_lock, iflags);
+ spin_lock(&sdp->sfd_lock);
sdp->detached = 1;
list_for_each_entry(sfp, &sdp->sfds, sfd_siblings) {
wake_up_interruptible(&sfp->read_wait);
kill_fasync(&sfp->async_qp, SIGPOLL, POLL_HUP);
}
+ spin_unlock(&sdp->sfd_lock);
write_unlock_irqrestore(&sg_index_lock, iflags);
sysfs_remove_link(&scsidp->sdev_gendev.kobj, "generic");
@@ -2057,13 +2058,13 @@ sg_add_sfp(Sg_device * sdp, int dev)
sfp->cmd_q = SG_DEF_COMMAND_Q;
sfp->keep_orphan = SG_DEF_KEEP_ORPHAN;
sfp->parentdp = sdp;
- write_lock_irqsave(&sg_index_lock, iflags);
+ spin_lock_irqsave(&sdp->sfd_lock, iflags);
if (sdp->detached) {
- write_unlock_irqrestore(&sg_index_lock, iflags);
+ spin_unlock_irqrestore(&sdp->sfd_lock, iflags);
return ERR_PTR(-ENODEV);
}
list_add_tail(&sfp->sfd_siblings, &sdp->sfds);
- write_unlock_irqrestore(&sg_index_lock, iflags);
+ spin_unlock_irqrestore(&sdp->sfd_lock, iflags);
SCSI_LOG_TIMEOUT(3, printk("sg_add_sfp: sfp=0x%p\n", sfp));
if (unlikely(sg_big_buff != def_reserved_size))
sg_big_buff = def_reserved_size;
@@ -2110,11 +2111,12 @@ static void sg_remove_sfp_usercontext(struct
work_struct *work)
static void sg_remove_sfp(struct kref *kref)
{
struct sg_fd *sfp = container_of(kref, struct sg_fd, f_ref);
+ struct sg_device *sdp = sfp->parentdp;
unsigned long iflags;
- write_lock_irqsave(&sg_index_lock, iflags);
+ spin_lock_irqsave(&sdp->sfd_lock, iflags);
list_del(&sfp->sfd_siblings);
- write_unlock_irqrestore(&sg_index_lock, iflags);
+ spin_unlock_irqrestore(&sdp->sfd_lock, iflags);
INIT_WORK(&sfp->ew.work, sg_remove_sfp_
[PATCH v6 2/4] sg: no need sg_open_exclusive_lock
Open exclusive check is protected by o_sem, no need sg_open_exclusive_lock.
@exclude is used to record which type of rwsem we are holding.
Signed-off-by: Vaughan Cao
---
drivers/scsi/sg.c | 34 +-
1 file changed, 5 insertions(+), 29 deletions(-)
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 7a54c92..dcbd95f 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -105,8 +105,6 @@ static int scatter_elem_sz_prev = SG_SCATTER_SZ;
static int sg_add(struct device *, struct class_interface *);
static void sg_remove(struct device *, struct class_interface *);
-static DEFINE_SPINLOCK(sg_open_exclusive_lock);
-
static DEFINE_IDR(sg_index_idr);
static DEFINE_RWLOCK(sg_index_lock); /* Also used to lock
file descriptor list
for device */
@@ -176,7 +174,6 @@ typedef struct sg_device { /* holds the state of each scsi
generic device */
struct list_head sfds;
struct rw_semaphore o_sem; /* exclude open should hold this rwsem
*/
volatile char detached; /* 0->attached, 1->detached pending removal */
- /* exclude protected by sg_open_exclusive_lock */
char exclude; /* opened for exclusive access */
char sgdebug; /* 0->off, 1->sense, 9->dump dev, 10-> all devs
*/
struct gendisk *disk;
@@ -225,27 +222,6 @@ static int sg_allow_access(struct file *filp, unsigned
char *cmd)
return blk_verify_command(cmd, filp->f_mode & FMODE_WRITE);
}
-static int get_exclude(Sg_device *sdp)
-{
- unsigned long flags;
- int ret;
-
- spin_lock_irqsave(&sg_open_exclusive_lock, flags);
- ret = sdp->exclude;
- spin_unlock_irqrestore(&sg_open_exclusive_lock, flags);
- return ret;
-}
-
-static int set_exclude(Sg_device *sdp, char val)
-{
- unsigned long flags;
-
- spin_lock_irqsave(&sg_open_exclusive_lock, flags);
- sdp->exclude = val;
- spin_unlock_irqrestore(&sg_open_exclusive_lock, flags);
- return val;
-}
-
static int sfds_list_empty(Sg_device *sdp)
{
unsigned long flags;
@@ -317,7 +293,7 @@ sg_open(struct inode *inode, struct file *filp)
}
/* Since write lock is held, no need to check sfd_list */
if (flags & O_EXCL)
- set_exclude(sdp, 1);
+ sdp->exclude = 1; /* used by release lock */
if (sdp->detached) {
retval = -ENODEV;
@@ -339,7 +315,7 @@ sg_open(struct inode *inode, struct file *filp)
if (retval) {
sem_out:
if (flags & O_EXCL) {
- set_exclude(sdp, 0);/* undo if error */
+ sdp->exclude = 0; /* undo if error */
up_write(&sdp->o_sem);
} else
up_read(&sdp->o_sem);
@@ -366,8 +342,8 @@ sg_release(struct inode *inode, struct file *filp)
return -ENXIO;
SCSI_LOG_TIMEOUT(3, printk("sg_release: %s\n", sdp->disk->disk_name));
- excl = get_exclude(sdp);
- set_exclude(sdp, 0);
+ excl = sdp->exclude;
+ sdp->exclude = 0;
if (excl)
up_write(&sdp->o_sem);
else
@@ -2624,7 +2600,7 @@ static int sg_proc_seq_show_debug(struct seq_file *s,
void *v)
scsidp->lun,
scsidp->host->hostt->emulated);
seq_printf(s, " sg_tablesize=%d excl=%d\n",
- sdp->sg_tablesize, get_exclude(sdp));
+ sdp->sg_tablesize, sdp->exclude);
sg_proc_debug_helper(s, sdp);
}
read_unlock_irqrestore(&sg_index_lock, iflags);
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 1/4] sg: use rwsem to solve race during exclusive open
A race condition may happen if two threads are both trying to open the same sg
with O_EXCL simultaneously. It's possible that they both find fsds list is
empty and get_exclude(sdp) returns 0, then they both call set_exclude() and
break out from wait_event_interruptible and resume open.
Now use rwsem to protect this process. Exclusive open gets write lock and
others get read lock. The lock will be held until file descriptor is closed.
This also leads 'exclude' only a status rather than a check mark.
Changes from v5:
* remove unused variables
* fix insane code dealing with sg_add_sfp.
Signed-off-by: Vaughan Cao
---
drivers/scsi/sg.c | 83 +--
1 file changed, 44 insertions(+), 39 deletions(-)
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index df5e961..7a54c92 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -170,11 +170,11 @@ typedef struct sg_fd {/* holds the state of a
file descriptor */
typedef struct sg_device { /* holds the state of each scsi generic device */
struct scsi_device *device;
- wait_queue_head_t o_excl_wait; /* queue open() when O_EXCL in use */
int sg_tablesize; /* adapter's max scatter-gather table size */
u32 index; /* device index number */
/* sfds is protected by sg_index_lock */
struct list_head sfds;
+ struct rw_semaphore o_sem; /* exclude open should hold this rwsem
*/
volatile char detached; /* 0->attached, 1->detached pending removal */
/* exclude protected by sg_open_exclusive_lock */
char exclude; /* opened for exclusive access */
@@ -265,7 +265,6 @@ sg_open(struct inode *inode, struct file *filp)
struct request_queue *q;
Sg_device *sdp;
Sg_fd *sfp;
- int res;
int retval;
nonseekable_open(inode, filp);
@@ -294,35 +293,35 @@ sg_open(struct inode *inode, struct file *filp)
goto error_out;
}
- if (flags & O_EXCL) {
- if (O_RDONLY == (flags & O_ACCMODE)) {
- retval = -EPERM; /* Can't lock it with read only access
*/
- goto error_out;
- }
- if (!sfds_list_empty(sdp) && (flags & O_NONBLOCK)) {
- retval = -EBUSY;
- goto error_out;
- }
- res = wait_event_interruptible(sdp->o_excl_wait,
- ((!sfds_list_empty(sdp) ||
get_exclude(sdp)) ? 0 : set_exclude(sdp, 1)));
- if (res) {
- retval = res; /* -ERESTARTSYS because signal hit
process */
- goto error_out;
- }
- } else if (get_exclude(sdp)) { /* some other fd has an exclusive lock
on dev */
- if (flags & O_NONBLOCK) {
- retval = -EBUSY;
- goto error_out;
- }
- res = wait_event_interruptible(sdp->o_excl_wait,
!get_exclude(sdp));
- if (res) {
- retval = res; /* -ERESTARTSYS because signal hit
process */
- goto error_out;
+ if ((flags & O_EXCL) && (O_RDONLY == (flags & O_ACCMODE))) {
+ retval = -EPERM; /* Can't lock it with read only access */
+ goto error_out;
+ }
+ if (flags & O_NONBLOCK) {
+ if (flags & O_EXCL) {
+ if (!down_write_trylock(&sdp->o_sem)) {
+ retval = -EBUSY;
+ goto error_out;
+ }
+ } else {
+ if (!down_read_trylock(&sdp->o_sem)) {
+ retval = -EBUSY;
+ goto error_out;
+ }
}
+ } else {
+ if (flags & O_EXCL)
+ down_write(&sdp->o_sem);
+ else
+ down_read(&sdp->o_sem);
}
+ /* Since write lock is held, no need to check sfd_list */
+ if (flags & O_EXCL)
+ set_exclude(sdp, 1);
+
if (sdp->detached) {
retval = -ENODEV;
- goto error_out;
+ goto sem_out;
}
if (sfds_list_empty(sdp)) { /* no existing opens on this device */
sdp->sgdebug = 0;
@@ -331,17 +330,20 @@ sg_open(struct inode *inode, struct file *filp)
}
if ((sfp = sg_add_sfp(sdp, dev)))
filp->private_data = sfp;
- else {
+ /* retval is already provably zero at this point because of the
+* check after retval = scsi_autopm_get_device(sdp->device))
+*/
+ else
+ retval = -ENOMEM;
+
+ if (retval) {
+sem_out:
if (flags & O_EXCL) {
set_exclude(sdp, 0);
[Bug 60644] MPT2SAS drops all HDDs when under high I/O
https://bugzilla.kernel.org/show_bug.cgi?id=60644 Bernd Schubert changed: What|Removed |Added CC||bernd.schub...@fastmail.fm --- Comment #25 from Bernd Schubert --- Have you already tried to give the controller less work, i.e. by setting /sys/block/sdX/device/queue_depth to 1? If you can't set it, use the mpt2sas option max_queue_depth=1. A low value of max_sgl_entries and max_sectors also might help. Lowering all of that is not good for performance, but might increase stability. -- You are receiving this mail because: You are watching the assignee of the bug. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[Bug 60644] MPT2SAS drops all HDDs when under high I/O
https://bugzilla.kernel.org/show_bug.cgi?id=60644 --- Comment #24 from livea...@live.com --- Hello . I used to have the same issues with MD . yes . I'm using BTRFS , I don't know if BTRFS RAID code was ported from MD , but the issue is the same . I didn't try anything other than BTRFS and MD . Maybe I should give ZFS a try , although it is still slower on linux . I'll report back as soon as possible . Thank you . -- You are receiving this mail because: You are watching the assignee of the bug. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[Bug 60644] MPT2SAS drops all HDDs when under high I/O
https://bugzilla.kernel.org/show_bug.cgi?id=60644 --- Comment #23 from Hannes Reinecke --- So the firmware does indeed wedge under high load. Given the issues I've had so far with LSI SATL I'm not surprised. Does the same thing happen when running on a single disk, ie without MD? There have been issues with MD dropping any queue limitations (ie the 4k physical / 512 logical block sizes you're having) so MD might end up spitting out non-aligned requests. Which in turn might trigger issues in the firmware translation. Using the devices directly without MD would eliminate this problem. -- You are receiving this mail because: You are watching the assignee of the bug. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch|rfc] block: fix race between request completion and timeout handling
On 08/28/2013 09:04 AM, Hannes Reinecke wrote:
> On 08/27/2013 04:28 PM, Jeff Moyer wrote:
>> Hi,
>>
>> We have several reports (against a distro kernel) of panics in
>> blk_requeue_request that look like this:
>>
>> kernel BUG at block/blk-core.c:1045!
>> invalid opcode: [#1] SMP
>> last sysfs file:
>> /sys/devices/pci:40/:40:03.0/:55:00.0/infiniband_mad/umad0/port
>> CPU 0
>> Modules linked in: ipmi_si ipmi_devintf ipmi_msghandler bonding rdma_ucm(U)
>> rdma_cm(U) iw_cm(U) ib_addr(U) ib_ipoib(U) ib_cm(U) ib_sa(U) ipv6
>> ib_uverbs(U) ib_umad(U) iw_nes(U) libcrc32c mlx4_ib(U) mlx4_en(U)
>> mlx4_core(U) ib_mthca(U) ib_mad(U) ib_core(U) cdc_ether usbnet mii microcode
>> i2c_i801 i2c_core iTCO_wdt iTCO_vendor_support shpchp ioatdma dca be2net sg
>> ses enclosure ext4 mbcache jbd2 sd_mod crc_t10dif ahci megaraid_sas(U)
>> dm_mirror dm_region_hash dm_log dm_mod [last unloaded: scsi_wait_scan]
>>
>> Pid: 491, comm: scsi_eh_0 Tainted: GW
>> 2.6.32-220.13.1.el6.x86_64 #1 IBM -[8722PAX]-/00D1461
>> RIP: 0010:[] []
>> blk_requeue_request+0x94/0xa0
>> RSP: 0018:881057eefd60 EFLAGS: 00010012
>> RAX: 881d99e3e8a8 RBX: 881d99e3e780 RCX: 881d99e3e8a8
>> RDX: 881d99e3e8a8 RSI: 881d99e3e780 RDI: 881d99e3e780
>> RBP: 881057eefd80 R08: 881057eefe90 R09:
>> R10: R11: R12: 881057f92338
>> R13: R14: 881057f92338 R15: 883058188000
>> FS: () GS:88004020() knlGS:
>> CS: 0010 DS: 0018 ES: 0018 CR0: 8005003b
>> CR2: 006d3ec0 CR3: 00302cd7d000 CR4: 000406b0
>> DR0: DR1: DR2:
>> DR3: DR6: 0ff0 DR7: 0400
>> Process scsi_eh_0 (pid: 491, threadinfo 881057eee000, task
>> 881057e29540)
>> Stack:
>> 1057 0286 8810275efdc0 881057f16000
>> <0> 881057eefdd0 81362323 881057eefe20 8135f393
>> <0> 881057e29af8 8810275efdc0 881057eefe78 881057eefe90
>> Call Trace:
>> [] __scsi_queue_insert+0xa3/0x150
>> [] ? scsi_eh_ready_devs+0x5e3/0x850
>> [] scsi_queue_insert+0x13/0x20
>> [] scsi_eh_flush_done_q+0x104/0x160
>> [] scsi_error_handler+0x35b/0x660
>> [] ? scsi_error_handler+0x0/0x660
>> [] kthread+0x96/0xa0
>> [] child_rip+0xa/0x20
>> [] ? kthread+0x0/0xa0
>> [] ? child_rip+0x0/0x20
>> Code: 00 00 eb d1 4c 8b 2d 3c 8f 97 00 4d 85 ed 74 bf 49 8b 45 00 49 83 c5
>> 08 48 89 de 4c 89 e7 ff d0 49 8b 45 00 48 85 c0 75 eb eb a4 <0f> 0b eb fe 0f
>> 1f 84 00 00 00 00 00 55 48 89 e5 0f 1f 44 00 00
>> RIP [] blk_requeue_request+0x94/0xa0
>> RSP
>>
>> The RIP is this line:
>> BUG_ON(blk_queued_rq(rq));
>>
>> After digging through the code, I think there may be a race between the
>> request completion and the timer handler running.
>>
>> A timer is started for each request put on the device's queue (see
>> blk_start_request->blk_add_timer). If the request does not complete
>> before the timer expires, the timer handler (blk_rq_timed_out_timer)
>> will mark the request complete atomically:
>>
>> static inline int blk_mark_rq_complete(struct request *rq)
>> {
>> return test_and_set_bit(REQ_ATOM_COMPLETE, &rq->atomic_flags);
>> }
>>
>> and then call blk_rq_timed_out. The latter function will call
>> scsi_times_out, which will return one of BLK_EH_HANDLED,
>> BLK_EH_RESET_TIMER or BLK_EH_NOT_HANDLED. If BLK_EH_RESET_TIMER is
>> returned, blk_clear_rq_complete is called, and blk_add_timer is again
>> called to simply wait longer for the request to complete.
>>
>> Now, if the request happens to complete while this is going on, what
>> happens? Given that we know the completion handler will bail if it
>> finds the REQ_ATOM_COMPLETE bit set, we need to focus on the completion
>> handler running after that bit is cleared. So, from the above
>> paragraph, after the call to blk_clear_rq_complete. If the completion
>> sets REQ_ATOM_COMPLETE before the BUG_ON in blk_add_timer, we go boom
>> there (I haven't seen this in the cores). Next, if we get the
>> completion before the call to list_add_tail, then the timer will
>> eventually fire for an old req, which may either be freed or reallocated
>> (there is evidence that this might be the case). Finally, if the
>> completion comes in *after* the addition to the timeout list, I think
>> it's harmless. The request will be removed from the timeout list,
>> req_atom_complete will be set, and all will be well.
>>
>> This RFC patch moves the BUG_ON(test_bit(REQ_ATOM_COMPLETE,
>> &req->atomic_flags)); from blk_add_timer to the only caller that could
>> trip over it (blk_start_request). It then inverts the calls to
>> blk_clear_rq_complete and blk_add_timer in blk_rq_timed_out to address
>> the race. I've boot tested this patch, but nothing more.
>>
>> Jens, James, others, w
Re: [PATCH 02/29] qla2xxx: Add support for ISP8044.
On Tue, 2013-08-27 at 01:37 -0400, Saurav Kashyap wrote: > From: Atul Deshmukh You seem to have stopped running your stuff through checkpatch: WARNING: please, no spaces at the start of a line #317: FILE: drivers/scsi/qla2xxx/qla_gbl.h:479: +uint8_t *, uint32_t, uint32_t);$ WARNING: please, no spaces at the start of a line #341: FILE: drivers/scsi/qla2xxx/qla_gbl.h:713: +const uint32_t crb_reg, const uint32_t value);$ WARNING: please, no spaces at the start of a line #351: FILE: drivers/scsi/qla2xxx/qla_gbl.h:723: +uint32_t, uint32_t);$ WARNING: please, no spaces at the start of a line #3175: FILE: drivers/scsi/qla2xxx/qla_nx2.c:2084: +struct qla8044_minidump_entry_hdr *entry_hdr)$ WARNING: please, no spaces at the start of a line #4258: FILE: drivers/scsi/qla2xxx/qla_nx2.c:3167: +uint32_t data)$ WARNING: please, no spaces at the start of a line #4334: FILE: drivers/scsi/qla2xxx/qla_nx2.c:3243: +uint32_t sector_start_addr)$ WARNING: please, no spaces at the start of a line #4390: FILE: drivers/scsi/qla2xxx/qla_nx2.c:3299: +uint32_t *p_data)$ WARNING: please, no spaces at the start of a line #4425: FILE: drivers/scsi/qla2xxx/qla_nx2.c:3334: +uint32_t faddr, uint32_t dwords)$ WARNING: please, no spaces at the start of a line #4508: FILE: drivers/scsi/qla2xxx/qla_nx2.c:3417: +uint32_t faddr, uint32_t dwords)$ WARNING: please, no spaces at the start of a line #4528: FILE: drivers/scsi/qla2xxx/qla_nx2.c:3437: +uint32_t offset, uint32_t length)$ total: 0 errors, 10 warnings, 5781 lines checked Please don't; those are all legitimate problems. James -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch|rfc] block: fix race between request completion and timeout handling
On 08/27/2013 04:28 PM, Jeff Moyer wrote:
> Hi,
>
> We have several reports (against a distro kernel) of panics in
> blk_requeue_request that look like this:
>
> kernel BUG at block/blk-core.c:1045!
> invalid opcode: [#1] SMP
> last sysfs file:
> /sys/devices/pci:40/:40:03.0/:55:00.0/infiniband_mad/umad0/port
> CPU 0
> Modules linked in: ipmi_si ipmi_devintf ipmi_msghandler bonding rdma_ucm(U)
> rdma_cm(U) iw_cm(U) ib_addr(U) ib_ipoib(U) ib_cm(U) ib_sa(U) ipv6
> ib_uverbs(U) ib_umad(U) iw_nes(U) libcrc32c mlx4_ib(U) mlx4_en(U)
> mlx4_core(U) ib_mthca(U) ib_mad(U) ib_core(U) cdc_ether usbnet mii microcode
> i2c_i801 i2c_core iTCO_wdt iTCO_vendor_support shpchp ioatdma dca be2net sg
> ses enclosure ext4 mbcache jbd2 sd_mod crc_t10dif ahci megaraid_sas(U)
> dm_mirror dm_region_hash dm_log dm_mod [last unloaded: scsi_wait_scan]
>
> Pid: 491, comm: scsi_eh_0 Tainted: GW
> 2.6.32-220.13.1.el6.x86_64 #1 IBM -[8722PAX]-/00D1461
> RIP: 0010:[] []
> blk_requeue_request+0x94/0xa0
> RSP: 0018:881057eefd60 EFLAGS: 00010012
> RAX: 881d99e3e8a8 RBX: 881d99e3e780 RCX: 881d99e3e8a8
> RDX: 881d99e3e8a8 RSI: 881d99e3e780 RDI: 881d99e3e780
> RBP: 881057eefd80 R08: 881057eefe90 R09:
> R10: R11: R12: 881057f92338
> R13: R14: 881057f92338 R15: 883058188000
> FS: () GS:88004020() knlGS:
> CS: 0010 DS: 0018 ES: 0018 CR0: 8005003b
> CR2: 006d3ec0 CR3: 00302cd7d000 CR4: 000406b0
> DR0: DR1: DR2:
> DR3: DR6: 0ff0 DR7: 0400
> Process scsi_eh_0 (pid: 491, threadinfo 881057eee000, task
> 881057e29540)
> Stack:
> 1057 0286 8810275efdc0 881057f16000
> <0> 881057eefdd0 81362323 881057eefe20 8135f393
> <0> 881057e29af8 8810275efdc0 881057eefe78 881057eefe90
> Call Trace:
> [] __scsi_queue_insert+0xa3/0x150
> [] ? scsi_eh_ready_devs+0x5e3/0x850
> [] scsi_queue_insert+0x13/0x20
> [] scsi_eh_flush_done_q+0x104/0x160
> [] scsi_error_handler+0x35b/0x660
> [] ? scsi_error_handler+0x0/0x660
> [] kthread+0x96/0xa0
> [] child_rip+0xa/0x20
> [] ? kthread+0x0/0xa0
> [] ? child_rip+0x0/0x20
> Code: 00 00 eb d1 4c 8b 2d 3c 8f 97 00 4d 85 ed 74 bf 49 8b 45 00 49 83 c5 08
> 48 89 de 4c 89 e7 ff d0 49 8b 45 00 48 85 c0 75 eb eb a4 <0f> 0b eb fe 0f 1f
> 84 00 00 00 00 00 55 48 89 e5 0f 1f 44 00 00
> RIP [] blk_requeue_request+0x94/0xa0
> RSP
>
> The RIP is this line:
> BUG_ON(blk_queued_rq(rq));
>
> After digging through the code, I think there may be a race between the
> request completion and the timer handler running.
>
> A timer is started for each request put on the device's queue (see
> blk_start_request->blk_add_timer). If the request does not complete
> before the timer expires, the timer handler (blk_rq_timed_out_timer)
> will mark the request complete atomically:
>
> static inline int blk_mark_rq_complete(struct request *rq)
> {
> return test_and_set_bit(REQ_ATOM_COMPLETE, &rq->atomic_flags);
> }
>
> and then call blk_rq_timed_out. The latter function will call
> scsi_times_out, which will return one of BLK_EH_HANDLED,
> BLK_EH_RESET_TIMER or BLK_EH_NOT_HANDLED. If BLK_EH_RESET_TIMER is
> returned, blk_clear_rq_complete is called, and blk_add_timer is again
> called to simply wait longer for the request to complete.
>
> Now, if the request happens to complete while this is going on, what
> happens? Given that we know the completion handler will bail if it
> finds the REQ_ATOM_COMPLETE bit set, we need to focus on the completion
> handler running after that bit is cleared. So, from the above
> paragraph, after the call to blk_clear_rq_complete. If the completion
> sets REQ_ATOM_COMPLETE before the BUG_ON in blk_add_timer, we go boom
> there (I haven't seen this in the cores). Next, if we get the
> completion before the call to list_add_tail, then the timer will
> eventually fire for an old req, which may either be freed or reallocated
> (there is evidence that this might be the case). Finally, if the
> completion comes in *after* the addition to the timeout list, I think
> it's harmless. The request will be removed from the timeout list,
> req_atom_complete will be set, and all will be well.
>
> This RFC patch moves the BUG_ON(test_bit(REQ_ATOM_COMPLETE,
> &req->atomic_flags)); from blk_add_timer to the only caller that could
> trip over it (blk_start_request). It then inverts the calls to
> blk_clear_rq_complete and blk_add_timer in blk_rq_timed_out to address
> the race. I've boot tested this patch, but nothing more.
>
> Jens, James, others, what do you think?
>
> Cheers,
> Jeff
>
> diff --git a/block/blk-core.c b/block/blk-core.c
> index 93a18d1..236ae0a 100644
> --- a/block
