Re: [PATCH 02/10] svcrdma: Add FRMR get/put services

[Tom Tucker <tom@opengridcomputing.com>]

J. Bruce Fields wrote:
> On Thu, Sep 25, 2008 at 09:25:30AM -0500, Tom Tucker wrote:
>> J. Bruce Fields wrote:
>>> On Tue, Sep 16, 2008 at 06:34:31AM -0500, Tom Tucker wrote:
>>>> +	}
>>>> +	frmr->map_len = 0;
>>>> +	frmr->page_list_len = 0;
>>>> +
>>>> +	return frmr;
>>>> +}
>>>> +
>>>> +static void frmr_unmap_dma(struct svcxprt_rdma *xprt,
>>>> +			   struct svc_rdma_fastreg_mr *frmr)
>>>> +{
>>>> +	int page_no;
>>>> +	dprintk("svcrdma:%s: xprt %p page_list_len %d\n",
>>>> +		__func__, xprt, frmr->page_list_len);
>>>> +	for (page_no = 0; page_no < frmr->page_list_len; page_no++) {
>>>> +		dma_addr_t addr = frmr->page_list->page_list[page_no];
>>>> +		dprintk("svcrdma:      %08x %llx\n", frmr->mr->lkey, addr);
>>> Are these dprintk's going to be useful for debugging user issues
>>> remotely, or were they just for your personal use while writing the
>>> code?
>>>
>>> We saw recently that we may already have too many dprintk's for them to
>>> be useful in production, and the above seem likely to be rather
>>> frequent.
>> Agreed. It needs to go.
> 
> OK!  That being the case: I ignored other additions of dprintk's in this
> patch set; would you mind scanning through them to see if there's others
> that should also go?  Stuff to look for, off the top of my head:
> 
> 	- How frequently is a dprintk going to be called?
> 	- Is this dprintk redundant with some other dprintk?  (E.g. are
> 	  a function and its caller both dprintk'ing the same
> 	  information?)
> 	- Is this going to be help debug problems with users in the
> 	  field?
> 

Yes, I will remove all pointless chatter :-)

> --b.