Re: [PATCH for-next 09/16] IB/hfi1: Clean up pin_vector_pages() function

On Tue, Aug 22, 2017 at 11:39:48AM -0700, Harish Chegondi wrote:

On 08/22/2017 08:46 AM, Leon Romanovsky wrote:

quoted

On Mon, Aug 21, 2017 at 06:27:03PM -0700, Dennis Dalessandro wrote:

quoted

From: Harish Chegondi <redacted>

Clean up pin_vector_pages() function by moving page pinning related code
to a separate function since it really stands on its own.

Reviewed-by: Dennis Dalessandro <redacted>
Signed-off-by: Harish Chegondi <redacted>
Signed-off-by: Dennis Dalessandro <redacted>
---
 drivers/infiniband/hw/hfi1/user_sdma.c |   79 ++++++++++++++++++--------------
 1 files changed, 45 insertions(+), 34 deletions(-)

diff --git a/drivers/infiniband/hw/hfi1/user_sdma.c b/drivers/infiniband/hw/hfi1/user_sdma.c
index d5a2572..6f26253 100644
--- a/drivers/infiniband/hw/hfi1/user_sdma.c
+++ b/drivers/infiniband/hw/hfi1/user_sdma.c

@@ -1124,11 +1124,53 @@ static u32 sdma_cache_evict(struct hfi1_user_sdma_pkt_q *pq, u32 npages)
 	return evict_data.cleared;
 }

+static int pin_sdma_pages(struct user_sdma_request *req,
+			  struct user_sdma_iovec *iovec,
+			  struct sdma_mmu_node *node,
+			  int npages)
+{
+	int pinned, cleared;
+	struct page **pages;
+	struct hfi1_user_sdma_pkt_q *pq = req->pq;
+
+	pages = kcalloc(npages, sizeof(*pages), GFP_KERNEL);
+	if (!pages) {
+		SDMA_DBG(req, "Failed page array alloc");

Please don't add prints after k[c|m|z]alloc failures, despite the fact
that it was before.

Thanks

SDMA_DBG MACRO is a trace for development debugging. It's not a print.

No problem, I'll rephrase it for you.

Please don't add prints, tracepoints, debug output and SDMA_DBG after k[c|m|z]alloc failures,
despite the fact that it was before.

Tracepoints output will be less worry for you if your kcalloc starts to fail.

Is it clear enough now?

Thanks,