The following series implements mostly minor fixes.
Hoisted a tag masking operation out of a loop. Inform
the controller that we're using 32-bit tags. Mask off
low error bits prior to calling pci_free_consistent.
Removed an unnecessary cast of kmalloc return value.
Fixed a missing command status of CMD_UNABORTABLE.
Added cciss_tape_cmds module parameter to allow reserving
more commands for use with SCSI tape drives.
---
Stephen M. Cameron (6):
cciss: host tag masking out of loop
cciss: Inform controller we are using 32-bit tags.
cciss: Mask off error bits of c->busaddr in cmd_special_free when calling pci_free_consistent
cciss: remove unnecessary casts
cciss: fix missed command status value CMD_UNABORTABLE
cciss: add cciss_tape_cmds module paramter
Documentation/blockdev/cciss.txt | 16 ++++++++++
drivers/block/cciss.c | 58 ++++++++++++++++++++++++++------------
drivers/block/cciss_cmd.h | 1 +
drivers/block/cciss_scsi.c | 45 +++++++++++++++++------------
drivers/block/cciss_scsi.h | 4 ---
5 files changed, 83 insertions(+), 41 deletions(-)
--
-- steve
From: Stephen M. Cameron <[email protected]>
In process_nonindexed_cmd, hoist figuring of masked tag out of loop since
it is the same throughout.
Signed-off-by: Stephen M. Cameron <[email protected]>
---
drivers/block/cciss.c | 4 +---
1 files changed, 1 insertions(+), 3 deletions(-)
diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index 9279272..bbf82e5 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -3400,14 +3400,12 @@ static inline u32 process_indexed_cmd(ctlr_info_t *h, u32 raw_tag)
/* process completion of a non-indexed command */
static inline u32 process_nonindexed_cmd(ctlr_info_t *h, u32 raw_tag)
{
- u32 tag;
CommandList_struct *c = NULL;
__u32 busaddr_masked, tag_masked;
- tag = cciss_tag_discard_error_bits(raw_tag);
+ tag_masked = cciss_tag_discard_error_bits(raw_tag);
list_for_each_entry(c, &h->cmpQ, list) {
busaddr_masked = cciss_tag_discard_error_bits(c->busaddr);
- tag_masked = cciss_tag_discard_error_bits(tag);
if (busaddr_masked == tag_masked) {
finish_cmd(h, c, raw_tag);
return next_command(h);
From: Stephen M. Cameron <[email protected]>
Controller will transfer only 32-bits on completion if it
knows we are only using 32-bit tags. Also, some newer controllers
apparently (and erroneously) require that we only use 32-bit tags,
and that we inform the controller of this.
Signed-off-by: Stephen M. Cameron <[email protected]>
---
drivers/block/cciss.c | 25 +++++++++++++++----------
drivers/block/cciss_cmd.h | 1 +
2 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index bbf82e5..2e832b0 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -231,7 +231,7 @@ static const struct block_device_operations cciss_fops = {
*/
static void set_performant_mode(ctlr_info_t *h, CommandList_struct *c)
{
- if (likely(h->transMethod == CFGTBL_Trans_Performant))
+ if (likely(h->transMethod & CFGTBL_Trans_Performant))
c->busaddr |= 1 | (h->blockFetchTable[c->Header.SGList] << 1);
}
@@ -3136,10 +3136,13 @@ static inline u32 cciss_tag_to_index(u32 tag)
return tag >> DIRECT_LOOKUP_SHIFT;
}
-static inline u32 cciss_tag_discard_error_bits(u32 tag)
+static inline u32 cciss_tag_discard_error_bits(ctlr_info_t *h, u32 tag)
{
-#define CCISS_ERROR_BITS 0x03
- return tag & ~CCISS_ERROR_BITS;
+#define CCISS_PERF_ERROR_BITS ((1 << DIRECT_LOOKUP_SHIFT) - 1)
+#define CCISS_SIMPLE_ERROR_BITS 0x03
+ if (likely(h->transMethod & CFGTBL_Trans_Performant))
+ return tag & ~CCISS_PERF_ERROR_BITS;
+ return tag & ~CCISS_SIMPLE_ERROR_BITS;
}
static inline void cciss_mark_tag_indexed(u32 *tag)
@@ -3365,7 +3368,7 @@ static inline u32 next_command(ctlr_info_t *h)
{
u32 a;
- if (unlikely(h->transMethod != CFGTBL_Trans_Performant))
+ if (unlikely(!(h->transMethod & CFGTBL_Trans_Performant)))
return h->access.command_completed(h);
if ((*(h->reply_pool_head) & 1) == (h->reply_pool_wraparound)) {
@@ -3403,9 +3406,9 @@ static inline u32 process_nonindexed_cmd(ctlr_info_t *h, u32 raw_tag)
CommandList_struct *c = NULL;
__u32 busaddr_masked, tag_masked;
- tag_masked = cciss_tag_discard_error_bits(raw_tag);
+ tag_masked = cciss_tag_discard_error_bits(h, raw_tag);
list_for_each_entry(c, &h->cmpQ, list) {
- busaddr_masked = cciss_tag_discard_error_bits(c->busaddr);
+ busaddr_masked = cciss_tag_discard_error_bits(h, c->busaddr);
if (busaddr_masked == tag_masked) {
finish_cmd(h, c, raw_tag);
return next_command(h);
@@ -3757,7 +3760,8 @@ static void __devinit cciss_wait_for_mode_change_ack(ctlr_info_t *h)
}
}
-static __devinit void cciss_enter_performant_mode(ctlr_info_t *h)
+static __devinit void cciss_enter_performant_mode(ctlr_info_t *h,
+ u32 use_short_tags)
{
/* This is a bit complicated. There are 8 registers on
* the controller which we write to to tell it 8 different
@@ -3812,7 +3816,7 @@ static __devinit void cciss_enter_performant_mode(ctlr_info_t *h)
writel(0, &h->transtable->RepQCtrAddrHigh32);
writel(h->reply_pool_dhandle, &h->transtable->RepQAddr0Low32);
writel(0, &h->transtable->RepQAddr0High32);
- writel(CFGTBL_Trans_Performant,
+ writel(CFGTBL_Trans_Performant | use_short_tags,
&(h->cfgtable->HostWrite.TransportRequest));
writel(CFGTBL_ChangeReq, h->vaddr + SA5_DOORBELL);
@@ -3859,7 +3863,8 @@ static void __devinit cciss_put_controller_into_performant_mode(ctlr_info_t *h)
if ((h->reply_pool == NULL) || (h->blockFetchTable == NULL))
goto clean_up;
- cciss_enter_performant_mode(h);
+ cciss_enter_performant_mode(h,
+ trans_support & CFGTBL_Trans_use_short_tags);
/* Change the access methods to the performant access methods */
h->access = SA5_performant_access;
diff --git a/drivers/block/cciss_cmd.h b/drivers/block/cciss_cmd.h
index 35463d2..cd441be 100644
--- a/drivers/block/cciss_cmd.h
+++ b/drivers/block/cciss_cmd.h
@@ -56,6 +56,7 @@
#define CFGTBL_Trans_Simple 0x00000002l
#define CFGTBL_Trans_Performant 0x00000004l
+#define CFGTBL_Trans_use_short_tags 0x20000000l
#define CFGTBL_BusType_Ultra2 0x00000001l
#define CFGTBL_BusType_Ultra3 0x00000002l
From: Stephen M. Cameron <[email protected]>
Signed-off-by: Stephen M. Cameron <[email protected]>
---
drivers/block/cciss.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index 2e832b0..03cead6 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -193,7 +193,7 @@ static int __devinit cciss_find_cfg_addrs(struct pci_dev *pdev,
u64 *cfg_offset);
static int __devinit cciss_pci_find_memory_BAR(struct pci_dev *pdev,
unsigned long *memory_bar);
-
+static inline u32 cciss_tag_discard_error_bits(ctlr_info_t *h, u32 tag);
/* performant mode helper functions */
static void calc_bucket_map(int *bucket, int num_buckets, int nsgs,
@@ -973,8 +973,8 @@ static void cmd_special_free(ctlr_info_t *h, CommandList_struct *c)
temp64.val32.upper = c->ErrDesc.Addr.upper;
pci_free_consistent(h->pdev, sizeof(ErrorInfo_struct),
c->err_info, (dma_addr_t) temp64.val);
- pci_free_consistent(h->pdev, sizeof(CommandList_struct),
- c, (dma_addr_t) c->busaddr);
+ pci_free_consistent(h->pdev, sizeof(CommandList_struct), c,
+ (dma_addr_t) cciss_tag_discard_error_bits(h, (u32) c->busaddr));
}
static inline ctlr_info_t *get_host(struct gendisk *disk)
From: Stephen M. Cameron <[email protected]>
This is to allow number of commands reserved for use by SCSI tape drives
and medium changers to be adjusted at driver load time via the kernel
parameter cciss_tape_cmds, with a default value of 6, and a range
of 2 - 16 inclusive. Previously, the driver limited the number of
commands which could be queued to the SCSI half of the the driver
to only 2. This is to fix the problem that if you had more than
two tape drives, you couldn't, for example, erase or rewind them all
at the same time.
Signed-off-by: Stephen M. Cameron <[email protected]>
---
Documentation/blockdev/cciss.txt | 16 ++++++++++++++++
drivers/block/cciss.c | 11 ++++++++++-
drivers/block/cciss_scsi.c | 32 +++++++++++++++-----------------
drivers/block/cciss_scsi.h | 4 ----
4 files changed, 41 insertions(+), 22 deletions(-)
diff --git a/Documentation/blockdev/cciss.txt b/Documentation/blockdev/cciss.txt
index 89698e8..ce6e83c 100644
--- a/Documentation/blockdev/cciss.txt
+++ b/Documentation/blockdev/cciss.txt
@@ -169,3 +169,19 @@ is issued which positions the tape to a known position. Typically you
must rewind the tape (by issuing "mt -f /dev/st0 rewind" for example)
before i/o can proceed again to a tape drive which was reset.
+There is a cciss_tape_cmds module parameter which can be used to make cciss
+allocate more commands for use by tape drives. Ordinarily only a few commands
+(6) are allocated for tape drives because tape drives are slow and
+infrequently used and the primary purpose of Smart Array controllers is to
+act as a RAID controller for disk drives, so the vast majority of commands
+are allocated for disk devices. However, if you have more than a few tape
+drives attached to a smart array, the default number of commands may not be
+enought (for example, if you have 8 tape drives, you could only rewind 6
+at one time with the default number of commands.) The cciss_tape_cmds module
+parameter allows more commands (up to 16 more) to be allocated for use by
+tape drives. For example:
+
+ insmod cciss.ko cciss_tape_cmds=16
+
+Or, as a kernel boot parameter passed in via grub: cciss.cciss_tape_cmds=8
+
diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index c7f073b..8e52e63 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -64,6 +64,10 @@ MODULE_DESCRIPTION("Driver for HP Smart Array Controllers");
MODULE_SUPPORTED_DEVICE("HP Smart Array Controllers");
MODULE_VERSION("3.6.26");
MODULE_LICENSE("GPL");
+static int cciss_tape_cmds = 6;
+module_param(cciss_tape_cmds, int, 0644);
+MODULE_PARM_DESC(cciss_tape_cmds,
+ "number of commands to allocate for tape devices (default: 6)");
static DEFINE_MUTEX(cciss_mutex);
static struct proc_dir_entry *proc_cciss;
@@ -4079,7 +4083,7 @@ static void __devinit cciss_get_max_perf_mode_cmds(struct ctlr_info *h)
static void __devinit cciss_find_board_params(ctlr_info_t *h)
{
cciss_get_max_perf_mode_cmds(h);
- h->nr_cmds = h->max_commands - 4; /* Allow room for some ioctls */
+ h->nr_cmds = h->max_commands - 4 - cciss_tape_cmds;
h->maxsgentries = readl(&(h->cfgtable->MaxSGElements));
/*
* Limit in-command s/g elements to 32 save dma'able memory.
@@ -4594,6 +4598,11 @@ static int __devinit cciss_init_one(struct pci_dev *pdev,
sprintf(h->devname, "cciss%d", i);
h->ctlr = i;
+ if (cciss_tape_cmds < 2)
+ cciss_tape_cmds = 2;
+ if (cciss_tape_cmds > 16)
+ cciss_tape_cmds = 16;
+
init_completion(&h->scan_wait);
if (cciss_create_hba_sysfs_entry(h))
diff --git a/drivers/block/cciss_scsi.c b/drivers/block/cciss_scsi.c
index df79380..514470d 100644
--- a/drivers/block/cciss_scsi.c
+++ b/drivers/block/cciss_scsi.c
@@ -84,7 +84,6 @@ static struct scsi_host_template cciss_driver_template = {
.proc_name = "cciss",
.proc_info = cciss_scsi_proc_info,
.queuecommand = cciss_scsi_queue_command,
- .can_queue = SCSI_CCISS_CAN_QUEUE,
.this_id = 7,
.cmd_per_lun = 1,
.use_clustering = DISABLE_CLUSTERING,
@@ -108,16 +107,13 @@ struct cciss_scsi_cmd_stack_elem_t {
#pragma pack()
-#define CMD_STACK_SIZE (SCSI_CCISS_CAN_QUEUE * \
- CCISS_MAX_SCSI_DEVS_PER_HBA + 2)
- // plus two for init time usage
-
#pragma pack(1)
struct cciss_scsi_cmd_stack_t {
struct cciss_scsi_cmd_stack_elem_t *pool;
- struct cciss_scsi_cmd_stack_elem_t *elem[CMD_STACK_SIZE];
+ struct cciss_scsi_cmd_stack_elem_t **elem;
dma_addr_t cmd_pool_handle;
int top;
+ int nelems;
};
#pragma pack()
@@ -191,7 +187,7 @@ scsi_cmd_free(ctlr_info_t *h, CommandList_struct *c)
sa = h->scsi_ctlr;
stk = &sa->cmd_stack;
stk->top++;
- if (stk->top >= CMD_STACK_SIZE) {
+ if (stk->top >= stk->nelems) {
dev_err(&h->pdev->dev,
"scsi_cmd_free called too many times.\n");
BUG();
@@ -206,13 +202,14 @@ scsi_cmd_stack_setup(ctlr_info_t *h, struct cciss_scsi_adapter_data_t *sa)
struct cciss_scsi_cmd_stack_t *stk;
size_t size;
+ stk = &sa->cmd_stack;
+ stk->nelems = cciss_tape_cmds + 2;
sa->cmd_sg_list = cciss_allocate_sg_chain_blocks(h,
- h->chainsize, CMD_STACK_SIZE);
+ h->chainsize, stk->nelems);
if (!sa->cmd_sg_list && h->chainsize > 0)
return -ENOMEM;
- stk = &sa->cmd_stack;
- size = sizeof(struct cciss_scsi_cmd_stack_elem_t) * CMD_STACK_SIZE;
+ size = sizeof(struct cciss_scsi_cmd_stack_elem_t) * stk->nelems;
/* Check alignment, see cciss_cmd.h near CommandList_struct def. */
BUILD_BUG_ON((sizeof(*stk->pool) % COMMANDLIST_ALIGNMENT) != 0);
@@ -221,18 +218,18 @@ scsi_cmd_stack_setup(ctlr_info_t *h, struct cciss_scsi_adapter_data_t *sa)
pci_alloc_consistent(h->pdev, size, &stk->cmd_pool_handle);
if (stk->pool == NULL) {
- cciss_free_sg_chain_blocks(sa->cmd_sg_list, CMD_STACK_SIZE);
+ cciss_free_sg_chain_blocks(sa->cmd_sg_list, stk->nelems);
sa->cmd_sg_list = NULL;
return -ENOMEM;
}
- for (i=0; i<CMD_STACK_SIZE; i++) {
+ for (i = 0; i < stk->nelems; i++) {
stk->elem[i] = &stk->pool[i];
stk->elem[i]->busaddr = (__u32) (stk->cmd_pool_handle +
(sizeof(struct cciss_scsi_cmd_stack_elem_t) * i));
stk->elem[i]->cmdindex = i;
}
- stk->top = CMD_STACK_SIZE-1;
+ stk->top = stk->nelems-1;
return 0;
}
@@ -245,16 +242,16 @@ scsi_cmd_stack_free(ctlr_info_t *h)
sa = h->scsi_ctlr;
stk = &sa->cmd_stack;
- if (stk->top != CMD_STACK_SIZE-1) {
+ if (stk->top != stk->nelems-1) {
dev_warn(&h->pdev->dev,
"bug: %d scsi commands are still outstanding.\n",
- CMD_STACK_SIZE - stk->top);
+ stk->nelems - stk->top);
}
- size = sizeof(struct cciss_scsi_cmd_stack_elem_t) * CMD_STACK_SIZE;
+ size = sizeof(struct cciss_scsi_cmd_stack_elem_t) * stk->nelems;
pci_free_consistent(h->pdev, size, stk->pool, stk->cmd_pool_handle);
stk->pool = NULL;
- cciss_free_sg_chain_blocks(sa->cmd_sg_list, CMD_STACK_SIZE);
+ cciss_free_sg_chain_blocks(sa->cmd_sg_list, stk->nelems);
}
#if 0
@@ -859,6 +856,7 @@ cciss_scsi_detect(ctlr_info_t *h)
sh->io_port = 0; // good enough? FIXME,
sh->n_io_port = 0; // I don't think we use these two...
sh->this_id = SELF_SCSI_ID;
+ sh->can_queue = cciss_tape_cmds;
sh->sg_tablesize = h->maxsgentries;
sh->max_cmd_len = MAX_COMMAND_SIZE;
diff --git a/drivers/block/cciss_scsi.h b/drivers/block/cciss_scsi.h
index 6d5822f..e71d986 100644
--- a/drivers/block/cciss_scsi.h
+++ b/drivers/block/cciss_scsi.h
@@ -36,13 +36,9 @@
addressible natively, and may in fact turn
out to be not scsi at all. */
-#define SCSI_CCISS_CAN_QUEUE 2
/*
-Note, cmd_per_lun could give us some trouble, so I'm setting it very low.
-Likewise, SCSI_CCISS_CAN_QUEUE is set very conservatively.
-
If the upper scsi layer tries to track how many commands we have
outstanding, it will be operating under the misapprehension that it is
the only one sending us requests. We also have the block interface,
From: Stephen M. Cameron <[email protected]>
Signed-off-by: Stephen M. Cameron <[email protected]>
---
drivers/block/cciss.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index 03cead6..6110918 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -1490,8 +1490,7 @@ static int cciss_bigpassthru(ctlr_info_t *h, void __user *argp)
return -EINVAL;
if (!capable(CAP_SYS_RAWIO))
return -EPERM;
- ioc = (BIG_IOCTL_Command_struct *)
- kmalloc(sizeof(*ioc), GFP_KERNEL);
+ ioc = kmalloc(sizeof(*ioc), GFP_KERNEL);
if (!ioc) {
status = -ENOMEM;
goto cleanup1;
From: Stephen M. Cameron <[email protected]>
and fix a nearby typo, "do" that should have been "due"
Signed-off-by: Stephen M. Cameron <[email protected]>
---
drivers/block/cciss.c | 11 +++++++++++
drivers/block/cciss_scsi.c | 13 +++++++++++--
2 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index 6110918..c7f073b 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -2652,6 +2652,10 @@ static int process_sendcmd_error(ctlr_info_t *h, CommandList_struct *c)
c->Request.CDB[0]);
return_status = IO_NEEDS_RETRY;
break;
+ case CMD_UNABORTABLE:
+ dev_warn(&h->pdev->dev, "cmd unabortable\n");
+ return_status = IO_ERROR;
+ break;
default:
dev_warn(&h->pdev->dev, "cmd 0x%02x returned "
"unknown status %x\n", c->Request.CDB[0],
@@ -3102,6 +3106,13 @@ static inline void complete_command(ctlr_info_t *h, CommandList_struct *cmd,
(cmd->rq->cmd_type == REQ_TYPE_BLOCK_PC) ?
DID_PASSTHROUGH : DID_ERROR);
break;
+ case CMD_UNABORTABLE:
+ dev_warn(&h->pdev->dev, "cmd %p unabortable\n", cmd);
+ rq->errors = make_status_bytes(SAM_STAT_GOOD,
+ cmd->err_info->CommandStatus, DRIVER_OK,
+ cmd->rq->cmd_type == REQ_TYPE_BLOCK_PC ?
+ DID_PASSTHROUGH : DID_ERROR);
+ break;
default:
dev_warn(&h->pdev->dev, "cmd %p returned "
"unknown status %x\n", cmd,
diff --git a/drivers/block/cciss_scsi.c b/drivers/block/cciss_scsi.c
index 727d022..df79380 100644
--- a/drivers/block/cciss_scsi.c
+++ b/drivers/block/cciss_scsi.c
@@ -824,13 +824,18 @@ static void complete_scsi_command(CommandList_struct *c, int timeout,
break;
case CMD_UNSOLICITED_ABORT:
cmd->result = DID_ABORT << 16;
- dev_warn(&h->pdev->dev, "%p aborted do to an "
+ dev_warn(&h->pdev->dev, "%p aborted due to an "
"unsolicited abort\n", c);
break;
case CMD_TIMEOUT:
cmd->result = DID_TIME_OUT << 16;
dev_warn(&h->pdev->dev, "%p timedout\n", c);
break;
+ case CMD_UNABORTABLE:
+ cmd->result = DID_ERROR << 16;
+ dev_warn(&h->pdev->dev, "c %p command "
+ "unabortable\n", c);
+ break;
default:
cmd->result = DID_ERROR << 16;
dev_warn(&h->pdev->dev,
@@ -1007,11 +1012,15 @@ cciss_scsi_interpret_error(ctlr_info_t *h, CommandList_struct *c)
break;
case CMD_UNSOLICITED_ABORT:
dev_warn(&h->pdev->dev,
- "%p aborted do to an unsolicited abort\n", c);
+ "%p aborted due to an unsolicited abort\n", c);
break;
case CMD_TIMEOUT:
dev_warn(&h->pdev->dev, "%p timedout\n", c);
break;
+ case CMD_UNABORTABLE:
+ dev_warn(&h->pdev->dev,
+ "%p unabortable\n", c);
+ break;
default:
dev_warn(&h->pdev->dev,
"%p returned unknown status %x\n",
On 22/02/2011 20:14, Stephen M. Cameron wrote:
> From: Stephen M. Cameron <[email protected]>
>
> Signed-off-by: Stephen M. Cameron <[email protected]>
> ---
> drivers/block/cciss.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
> index 2e832b0..03cead6 100644
> --- a/drivers/block/cciss.c
> +++ b/drivers/block/cciss.c
> @@ -193,7 +193,7 @@ static int __devinit cciss_find_cfg_addrs(struct pci_dev *pdev,
> u64 *cfg_offset);
> static int __devinit cciss_pci_find_memory_BAR(struct pci_dev *pdev,
> unsigned long *memory_bar);
> -
> +static inline u32 cciss_tag_discard_error_bits(ctlr_info_t *h, u32 tag);
Is there any reason we cannot just reorder the functions rather than
have this forward declaration?
Thanks,
Jack