| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20100604131926.GO17187@beardog.cce.hp.com>
Date: Fri, 4 Jun 2010 08:19:26 -0500
From: scameron@...rdog.cce.hp.com
To: akpm@...ux-foundation.org
Cc: mm-commits@...r.kernel.org, mike.miller@...com,
jens.axboe@...cle.com, linux-kernel@...r.kernel.org,
scameron@...rdog.cce.hp.com
Subject: Re: + cciss-add-performant-mode-support-for-stars-sirius.patch added to -mm tree
On Wed, Jun 02, 2010 at 12:58:06PM -0700, akpm@...ux-foundation.org wrote:
>
> The patch titled
> cciss: add performant mode support for Stars/Sirius
> has been added to the -mm tree. Its filename is
> cciss-add-performant-mode-support-for-stars-sirius.patch
>
[...snip...] see my comments below.
>
> ------------------------------------------------------
> Subject: cciss: add performant mode support for Stars/Sirius
> From: Mike Miller <mike.miller@...com>
>
> Add a mode of controller operation called Performant Mode. Even though
> cciss has been deprecated in favor of hpsa there are new controllers due
> out next year that HP must support in older vendor distros. Vendors
> require all fixes/features be upstream. These new controllers support
> only 16 commands in simple mode but support up to 1024 in performant mode.
> This requires us to add this support at this late date.
>
> The performant mode transport minimizes host PCI accesses by performinf
> many completions per read. PCI writes are posted so the host can write
> then immediately get off the bus not waiting for the writwe to complete to
> the target. In the context of performant mode the host read out to a
> controller pulls all posted writes into host memory ensuring the reply
> queue is coherent.
>
> Signed-off-by: Mike Miller <mike.miller@...com>
> Cc: Stephen M. Cameron <scameron@...rdog.cce.hp.com>
> Cc: Jens Axboe <jens.axboe@...cle.com>
> Signed-off-by: Andrew Morton <akpm@...ux-foundation.org>
> ---
>
> drivers/block/cciss.c | 267 ++++++++++++++++++++++++++++-------
> drivers/block/cciss.h | 109 +++++++++++++-
> drivers/block/cciss_cmd.h | 34 +++-
> drivers/block/cciss_scsi.c | 6
> 4 files changed, 353 insertions(+), 63 deletions(-)
>
> diff -puN drivers/block/cciss.c~cciss-add-performant-mode-support-for-stars-sirius drivers/block/cciss.c
> --- a/drivers/block/cciss.c~cciss-add-performant-mode-support-for-stars-sirius
> +++ a/drivers/block/cciss.c
> @@ -206,6 +206,11 @@ static void cciss_device_release(struct
> static void cciss_free_gendisk(ctlr_info_t *h, int drv_index);
> static void cciss_free_drive_info(ctlr_info_t *h, int drv_index);
>
[...snip...]
> + set_performant_mode(h, c);
> spin_lock_irqsave(&h->lock, flags);
> addQ(&h->reqQ, c);
> h->Qdepth++;
> @@ -350,6 +366,28 @@ static const char *raid_label[] = { "0",
>
> #ifdef CONFIG_PROC_FS
>
> +static inline u32 next_command(ctlr_info_t *h)
> +{
"next_command(...)" should not be inside "#ifdef CONFIG_PROC_FS"
[... snip...]
> dma_prefetch |= 0x8000;
> @@ -3944,38 +4140,8 @@ static int __devinit cciss_pci_init(ctlr
> }
>
> #ifdef CCISS_DEBUG
> return 0;
[...snip...]
>
> err_out_free_res:
> @@ -3984,6 +4150,7 @@ err_out_free_res:
> * Smart Array controllers that pci_enable_device does not undo
> */
> pci_release_regions(pdev);
> + cciss_put_controller_into_performant_mode(c);
> return err;
^^^ This is calling cciss_put_controller_into_performant_mode() in
the error code path of cciss_pci_init(), and not in the main initialization
code path. That cannot be correct.
[...snip...]
>
> static void __devexit cciss_remove_one(struct pci_dev *pdev)
> @@ -4575,7 +4741,6 @@ static int __init cciss_init(void)
> * array of them, the size must be a multiple of 8 bytes.
^^^ "8 bytes" is no longer correct in the above comment,
should be COMMANDLIST_ALIGNMENT (which is 32 now).
> */
> BUILD_BUG_ON(sizeof(CommandList_struct) % COMMANDLIST_ALIGNMENT);
> -
> printk(KERN_INFO DRIVER_NAME "\n");
>
[...snip...]
> @@ -173,12 +174,15 @@ typedef struct _SGDescriptor_struct {
> * PAD_64 can be adjusted independently as needed for 32-bit
> * and 64-bits systems.
> */
> -#define COMMANDLIST_ALIGNMENT (8)
> +#define COMMANDLIST_ALIGNMENT (32)
> #define IS_64_BIT ((sizeof(long) - 4)/4)
> #define IS_32_BIT (!IS_64_BIT)
> -#define PAD_32 (0)
> +#define PAD_32 (32)
^^^ if it's supposed to be aligned at 32 bit boundary
(which it is) then PAD_32 should be 0, not 32, if 32 works.
[...snip...]
> @@ -213,6 +213,8 @@ scsi_cmd_stack_setup(int ctlr, struct cc
>
> /* Check alignment, see cciss_cmd.h near CommandList_struct def. */
> BUILD_BUG_ON((sizeof(*stk->pool) % COMMANDLIST_ALIGNMENT) != 0);
> + /* printk(KERN_WARNING "cciss_scsi.c: 0x%08x 0x%08x 0x%08x\n",
> + 0xdeadbeef, sizeof(*stk->pool), 0xbeefdead); */
^^^ this looks like debug code left in by mistake.
> /* pci_alloc_consistent guarantees 32-bit DMA address will be used */
> stk->pool = (struct cciss_scsi_cmd_stack_elem_t *)
> pci_alloc_consistent(hba[ctlr]->pdev, size, &stk->cmd_pool_handle);
> _
>
-- steve
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists