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

[Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 2155 bytes --]

On Tue, Aug 22, 2017 at 11:39:48AM -0700, Harish Chegondi wrote:
>
>
> On 08/22/2017 08:46 AM, Leon Romanovsky wrote:
> > On Mon, Aug 21, 2017 at 06:27:03PM -0700, Dennis Dalessandro wrote:
> >> From: Harish Chegondi <harish.chegondi-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> >>
> >> 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 <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> >> Signed-off-by: Harish Chegondi <harish.chegondi-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> >> Signed-off-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> >> ---
> >>  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,

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]