linux-kernel - Re: [RESEND][PATCH 07/10][SCSI]mpt2sas: Added Reply Descriptor Post Queue (RDPQ) Array support

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 05 Aug 2014 14:46:38 -0400
From:	"Martin K. Petersen" <martin.petersen@...cle.com>
To:	Sreekanth Reddy <sreekanth.reddy@...gotech.com>
Cc:	"Martin K. Petersen" <martin.petersen@...cle.com>, jejb@...nel.org,
	"James E.J. Bottomley" <JBottomley@...allels.com>,
	linux-scsi@...r.kernel.org,
	Sathya Prakash <Sathya.Prakash@...gotech.com>,
	Nagalakshmi Nandigama <Nagalakshmi.Nandigama@...gotech.com>,
	linux-kernel@...r.kernel.org, Christoph Hellwig <hch@...radead.org>
Subject: Re: [RESEND][PATCH 07/10][SCSI]mpt2sas: Added Reply Descriptor Post Queue (RDPQ) Array support

>>>>> "Sreekanth" == Sreekanth Reddy <sreekanth.reddy@...gotech.com> writes:

Sreekanth,

Patch was mangled and I had to apply every single hunk by hand. Please
use git send-email.

+static int dma_mask;
+
+static int
+_base_wait_for_doorbell_int(struct MPT2SAS_ADAPTER *ioc, int timeout,
+                int sleep_flag);
+static int
+_base_wait_for_doorbell_ack(struct MPT2SAS_ADAPTER *ioc, int timeout,
+                int sleep_flag);
+static int
+_base_wait_for_doorbell_not_used(struct MPT2SAS_ADAPTER *ioc, int timeout,
+                 int sleep_flag);
+static int
+_base_handshake_req_reply_wait(struct MPT2SAS_ADAPTER *ioc, int request_bytes,
+    u32 *request, int reply_bytes, u16 *reply, int timeout, int sleep_flag);
+static int
+_base_get_ioc_facts(struct MPT2SAS_ADAPTER *ioc, int sleep_flag);

Are you sure you need all these? _base_get_ioc_facts was the only one
that needed to be declared in my original patch.

+    if (ioc->rdpq_array_enable)
+        sz = reply_post_free_sz;
+    else {
+        if (_base_is_controller_msix_enabled(ioc))
+            sz = reply_post_free_sz * ioc->reply_queue_count;
+        else
+            sz = reply_post_free_sz;
+    }

sz = reply_post_free_sz;
if (_base_is_controller_msix_enabled(ioc) && !ioc->rdpq_array_enable)
   sz *= ioc->reply_queue_count;

+    ioc->reply_post = kcalloc((ioc->rdpq_array_enable) ?
+        (ioc->reply_queue_count):1,
+        sizeof(struct reply_post_struct), GFP_KERNEL);

You're special casing the !rdpq code path again. Why don't you just make
sure reply_queue_count is always correct?

+    do {
+        ioc->reply_post[i].reply_post_free =
+            pci_pool_alloc(ioc->reply_post_free_dma_pool,
+            GFP_KERNEL,
+            &ioc->reply_post[i].reply_post_free_dma);
+        if (!ioc->reply_post[i].reply_post_free) {
+            printk(MPT2SAS_ERR_FMT
+            "reply_post_free pool: pci_pool_alloc failed\n",
+            ioc->name);
+            goto out;
+        }
+        memset(ioc->reply_post[i].reply_post_free, 0, sz);
+        dinitprintk(ioc, printk(MPT2SAS_INFO_FMT
+            "reply post free pool (0x%p): depth(%d),"
+            "element_size(%d), pool_size(%d kB)\n", ioc->name,
+            ioc->reply_post[i].reply_post_free,
+            ioc->reply_post_queue_depth, 8, sz/1024));
+        dinitprintk(ioc, printk(MPT2SAS_INFO_FMT
+            "reply_post_free_dma = (0x%llx)\n", ioc->name,
+            (unsigned long long)
+            ioc->reply_post[i].reply_post_free_dma));
+        total_sz += sz;
+    } while (ioc->rdpq_array_enable && (++i < ioc->reply_queue_count));

Same thing. I think:

     for (i = 0; i < ioc->reply_queue_count ; i++) {

was much clearer.

If reply_queue_count is ever inconsistent wrt. ioc->rdpq_array_enable
and _base_is_controller_msix_enabled(ioc) then that's an orthogonal
problem that you should address directly instead of working around it
several places in the code.

-- 
Martin K. Petersen	Oracle Linux Engineering
--
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/