Mercurial > illumos > illumos-gate

changeset 10027:c95796758397

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
6814751 px_dma_allocmp and hermon_dbr_page_alloc leak
author agiri <Giri.Adari@Sun.COM>
date Thu, 02 Jul 2009 14:17:43 -0700
parents 2659dc230676
children 2de34ea54ad5
files usr/src/uts/common/io/ib/adapters/hermon/hermon.c usr/src/uts/common/io/ib/adapters/hermon/hermon_misc.c usr/src/uts/common/sys/ib/adapters/hermon/hermon.h usr/src/uts/common/sys/ib/adapters/hermon/hermon_misc.h
diffstat 4 files changed, 55 insertions(+), 106 deletions(-) [+]
line wrap: on
line diff
--- a/usr/src/uts/common/io/ib/adapters/hermon/hermon.c	Thu Jul 02 14:05:51 2009 -0700
+++ b/usr/src/uts/common/io/ib/adapters/hermon/hermon.c	Thu Jul 02 14:17:43 2009 -0700
@@ -3494,7 +3494,6 @@
 	int	status;
 	hermon_dbr_info_t 	*info;
 
-
 	/*
 	 * Allocate the UAR page for kernel use. This UAR page is
 	 * the privileged UAR page through which all kernel generated
@@ -3519,9 +3518,7 @@
 	if (status != DDI_SUCCESS) {
 		return (DDI_FAILURE);
 	}
-
-	/* store the page pointer in the private area - the rest s/b done */
-	state->hs_kern_dbr = info->dbr_page;
+	state->hs_kern_dbr = info;
 	return (DDI_SUCCESS);
 }
 
@@ -3533,10 +3530,8 @@
 static void
 hermon_internal_uarpg_fini(hermon_state_t *state)
 {
-
 	/* Free up Hermon UAR page #1 (kernel driver doorbells) */
 	hermon_rsrc_free(state, &state->hs_uarkpg_rsrc);
-
 }
 
 
@@ -3550,7 +3545,6 @@
 	hermon_rsrc_t	*qp0_rsrc, *qp1_rsrc, *qp_resvd;
 	int		status;
 
-
 	/* Initialize the lock used for special QP rsrc management */
 	mutex_init(&state->hs_spec_qplock, NULL, MUTEX_DRIVER,
 	    DDI_INTR_PRI(state->hs_intrmsi_pri));
--- a/usr/src/uts/common/io/ib/adapters/hermon/hermon_misc.c	Thu Jul 02 14:05:51 2009 -0700
+++ b/usr/src/uts/common/io/ib/adapters/hermon/hermon_misc.c	Thu Jul 02 14:17:43 2009 -0700
@@ -237,7 +237,6 @@
  *	in this case, we want exactly one page per call, and aligned on a
  *	page - and may need to be mapped to the user for access
  */
-
 int
 hermon_dbr_page_alloc(hermon_state_t *state, hermon_dbr_info_t **dinfo)
 {
@@ -247,10 +246,9 @@
 	ddi_dma_attr_t		dma_attr;
 	ddi_dma_cookie_t	cookie;
 	uint_t			cookie_cnt;
-	hermon_dbr_header_t	*pagehdr;
 	int			i;
 	hermon_dbr_info_t 	*info;
-	uint64_t		dmaaddr;
+	caddr_t			dmaaddr;
 	uint64_t		dmalen;
 
 	info = kmem_zalloc(sizeof (hermon_dbr_info_t), KM_SLEEP);
@@ -274,7 +272,7 @@
 
 	status = ddi_dma_mem_alloc(dma_hdl, PAGESIZE,
 	    &state->hs_reg_accattr, DDI_DMA_CONSISTENT, DDI_DMA_SLEEP,
-	    NULL, (caddr_t *)&dmaaddr, (size_t *)&dmalen, &acc_hdl);
+	    NULL, &dmaaddr, (size_t *)&dmalen, &acc_hdl);
 	if (status != DDI_SUCCESS)	{
 		ddi_dma_free_handle(&dma_hdl);
 		cmn_err(CE_CONT, "dbr DMA mem alloc failed(status %d)", status);
@@ -284,7 +282,7 @@
 
 	/* this memory won't be IB registered, so do the bind here */
 	status = ddi_dma_addr_bind_handle(dma_hdl, NULL,
-	    (caddr_t)(uintptr_t)dmaaddr, (size_t)dmalen, DDI_DMA_RDWR |
+	    dmaaddr, (size_t)dmalen, DDI_DMA_RDWR |
 	    DDI_DMA_CONSISTENT, DDI_DMA_SLEEP, NULL, &cookie, &cookie_cnt);
 	if (status != DDI_SUCCESS) {
 		ddi_dma_mem_free(&acc_hdl);
@@ -299,18 +297,15 @@
 	/* init the info structure with returned info */
 	info->dbr_dmahdl = dma_hdl;
 	info->dbr_acchdl = acc_hdl;
-	info->dbr_page   = (caddr_t)(uintptr_t)dmaaddr;
+	info->dbr_page   = (hermon_dbr_t *)(void *)dmaaddr;
+	info->dbr_link = NULL;
 	/* extract the phys addr from the cookie */
 	info->dbr_paddr = cookie.dmac_laddress;
-	/* should have everything now, so do the init of the header */
-	pagehdr = (hermon_dbr_header_t *)(void *)info->dbr_page;
-	pagehdr->next = 0;
-	pagehdr->firstfree = 0;
-	pagehdr->nfree = HERMON_NUM_DBR_PER_PAGE;
-	pagehdr->dbr_info = info;
+	info->dbr_firstfree = 0;
+	info->dbr_nfree = HERMON_NUM_DBR_PER_PAGE;
 	/* link all DBrs onto the free list */
 	for (i = 0; i < HERMON_NUM_DBR_PER_PAGE; i++) {
-		pagehdr->dbr[i] = i + 1;
+		info->dbr_page[i] = i + 1;
 	}
 
 	return (DDI_SUCCESS);
@@ -328,9 +323,9 @@
 hermon_dbr_alloc(hermon_state_t *state, uint_t index, ddi_acc_handle_t *acchdl,
     hermon_dbr_t **vdbr, uint64_t *pdbr, uint64_t *mapoffset)
 {
-	hermon_dbr_header_t	*pagehdr, *lastpage;
 	hermon_dbr_t		*record = NULL;
-	hermon_dbr_info_t	*dinfo = NULL;
+	hermon_dbr_info_t	*info = NULL;
+	uint32_t		idx;
 	int			status;
 
 	if (index != state->hs_kernel_uar_index)
@@ -338,37 +333,30 @@
 		    mapoffset));
 
 	mutex_enter(&state->hs_dbr_lock);
-	/* 'pagehdr' holds pointer to first page */
-	pagehdr = (hermon_dbr_header_t *)(void *)state->hs_kern_dbr;
-	do {
-		lastpage = pagehdr; /* save pagehdr for later linking */
-		if (pagehdr->nfree == 0) {
-			pagehdr = (hermon_dbr_header_t *)(void *)pagehdr->next;
-			continue; /* page is full, go to next if there is one */
-		}
-		dinfo = pagehdr->dbr_info;
-		break;			/* found a page w/ one available */
-	} while (pagehdr != 0);
-
-	if (dinfo == NULL) {	/* did NOT find a page with one available */
-		status = hermon_dbr_page_alloc(state, &dinfo);
+	for (info = state->hs_kern_dbr; info != NULL; info = info->dbr_link)
+		if (info->dbr_nfree != 0)
+			break;		/* found a page w/ one available */
+
+	if (info == NULL) {	/* did NOT find a page with one available */
+		status = hermon_dbr_page_alloc(state, &info);
 		if (status != DDI_SUCCESS) {
 			/* do error handling */
 			mutex_exit(&state->hs_dbr_lock);
 			return (DDI_FAILURE);
 		}
 		/* got a new page, so link it in. */
-		pagehdr = (hermon_dbr_header_t *)(void *)dinfo->dbr_page;
-		lastpage->next = pagehdr;
+		info->dbr_link = state->hs_kern_dbr;
+		state->hs_kern_dbr = info;
 	}
-	record = pagehdr->dbr + pagehdr->firstfree;
-	pagehdr->firstfree = *record;
-	pagehdr->nfree--;
+	idx = info->dbr_firstfree;
+	record = info->dbr_page + idx;
+	info->dbr_firstfree = *record;
+	info->dbr_nfree--;
 	*record = 0;
 
-	*acchdl = dinfo->dbr_acchdl;
+	*acchdl = info->dbr_acchdl;
 	*vdbr = record;
-	*pdbr = ((uintptr_t)record - (uintptr_t)pagehdr + dinfo->dbr_paddr);
+	*pdbr = info->dbr_paddr + idx * sizeof (hermon_dbr_t);
 	mutex_exit(&state->hs_dbr_lock);
 	return (DDI_SUCCESS);
 }
@@ -380,22 +368,25 @@
  *	the dbr, but will NEVER free pages of dbrs - small
  *	price to pay, but userland access never will anyway
  */
-
 void
 hermon_dbr_free(hermon_state_t *state, uint_t indx, hermon_dbr_t *record)
 {
-	hermon_dbr_header_t	*pagehdr;
+	hermon_dbr_t		*page;
+	hermon_dbr_info_t	*info;
 
 	if (indx != state->hs_kernel_uar_index) {
 		hermon_user_dbr_free(state, indx, record);
 		return;
 	}
+	page = (hermon_dbr_t *)(uintptr_t)((uintptr_t)record & PAGEMASK);
 	mutex_enter(&state->hs_dbr_lock);
-	pagehdr = (hermon_dbr_header_t *)((uintptr_t)record &
-	    (uintptr_t)PAGEMASK);
-	*record = pagehdr->firstfree;
-	pagehdr->firstfree = record - pagehdr->dbr;
-	pagehdr->nfree++;		/* decr the count for this one */
+	for (info = state->hs_kern_dbr; info != NULL; info = info->dbr_link)
+		if (info->dbr_page == page)
+			break;
+	ASSERT(info != NULL);
+	*record = info->dbr_firstfree;
+	info->dbr_firstfree = record - info->dbr_page;
+	info->dbr_nfree++;
 	mutex_exit(&state->hs_dbr_lock);
 }
 
@@ -411,8 +402,7 @@
 void
 hermon_dbr_kern_free(hermon_state_t *state)
 {
-	hermon_dbr_header_t	*pagehdr, *lastpage;
-	hermon_dbr_info_t	*dinfo;
+	hermon_dbr_info_t	*info, *link;
 	hermon_user_dbr_t	*udbr, *next;
 	hermon_udbr_page_t	*pagep, *nextp;
 	hermon_umap_db_entry_t	*umapdb;
@@ -421,15 +411,12 @@
 	extern			hermon_umap_db_t hermon_userland_rsrc_db;
 
 	mutex_enter(&state->hs_dbr_lock);
-	pagehdr = (hermon_dbr_header_t *)(void *)state->hs_kern_dbr;
-	while (pagehdr != NULL) {
-		lastpage = (hermon_dbr_header_t *)(void *)pagehdr->next;
-		dinfo = pagehdr->dbr_info;
-		(void) ddi_dma_unbind_handle(dinfo->dbr_dmahdl);
-		ddi_dma_mem_free(&dinfo->dbr_acchdl);	/* free page */
-		ddi_dma_free_handle(&dinfo->dbr_dmahdl);
-		kmem_free(dinfo, sizeof (hermon_dbr_info_t));
-		pagehdr = lastpage;
+	for (info = state->hs_kern_dbr; info != NULL; info = link) {
+		(void) ddi_dma_unbind_handle(info->dbr_dmahdl);
+		ddi_dma_mem_free(&info->dbr_acchdl);	/* free page */
+		ddi_dma_free_handle(&info->dbr_dmahdl);
+		link = info->dbr_link;
+		kmem_free(info, sizeof (hermon_dbr_info_t));
 	}
 
 	udbr = state->hs_user_dbr;
--- a/usr/src/uts/common/sys/ib/adapters/hermon/hermon.h	Thu Jul 02 14:05:51 2009 -0700
+++ b/usr/src/uts/common/sys/ib/adapters/hermon/hermon.h	Thu Jul 02 14:17:43 2009 -0700
@@ -580,9 +580,11 @@
 	hermon_qphdl_t		*hs_qphdl;
 	hermon_srqhdl_t		*hs_srqhdl;
 	kmutex_t		hs_dbr_lock;	/* lock for dbr mgmt */
-	caddr_t			hs_kern_dbr;
 
-	/* XXX needs work - link list of non-kernel dbr resources */
+	/* linked list of kernel dbr resources */
+	hermon_dbr_info_t	*hs_kern_dbr;
+
+	/* linked list of non-kernel dbr resources */
 	hermon_user_dbr_t	*hs_user_dbr;
 
 	/*
--- a/usr/src/uts/common/sys/ib/adapters/hermon/hermon_misc.h	Thu Jul 02 14:05:51 2009 -0700
+++ b/usr/src/uts/common/sys/ib/adapters/hermon/hermon_misc.h	Thu Jul 02 14:17:43 2009 -0700
@@ -179,8 +179,6 @@
  * of UAR index, 3 bits of driver instance number).  This is especially true
  * for 32-bit kernels.
  */
-
-
 #define	HERMON_NUM_UAR_SHIFT		0xA
 
 /*
@@ -199,56 +197,24 @@
  * Though it may lead to minor wastage, it also means that reuse is easier since
  * any DBr can be used for either, and we don't have to play allocation games.
  *
- * The structure of a page of DBrs looks like this:
- *
- *         ------------------------------
- *         |                            |
- *         |                            |
- *         |      HEADER structure      |
- *         |                            |
- *         |----------------------------|
- *         |                            |
- *         |                            |
- *         |                            |
- *         |           DBrs             |  ((PAGESIZE >> 3) - 4) DBrs / page
- *         |                            |
- *         |                            |
- *         |                            |
- *         |                            |
- *         |                            |
- *         |----------------------------|
- *
- *
- * The state structure will hold the pointer to the start of a list of pages
- * containing DBr's.  As you allocate each page you also allocate a dbr_into
- * structure, that contains the access information about the page, to minimize
- * what needs to be in the page itself - what's there is just what's needed
- * to step through at alloc time
+ * The state structure will hold the pointer to the start of a list of struct
+ * hermon_dbr_info_s, each one containing the necessary information to manage
+ * a page of DBr's.
  */
 
 typedef uint64_t hermon_dbr_t;
 
 typedef struct hermon_dbr_info_s {
-	caddr_t			dbr_page;	/* addr of page */
-	uint64_t		dbr_paddr;	/* paddr of page for HCA */
+	struct hermon_dbr_info_s *dbr_link;
+	hermon_dbr_t		*dbr_page;	/* virtual addr of page */
+	uint64_t		dbr_paddr;	/* physical addr of page */
 	ddi_acc_handle_t	dbr_acchdl;
 	ddi_dma_handle_t	dbr_dmahdl;
-	ddi_umem_cookie_t	dbr_umemcookie;
+	uint32_t		dbr_nfree;	/* #free DBrs in this page */
+	uint32_t		dbr_firstfree;	/* idx of first free DBr */
 } hermon_dbr_info_t;
 
-typedef struct hermon_dbr_header_s {
-	struct hermon_dbr_header_s *next; /* next page in chain */
-					/* (zero means last) */
-	hermon_dbr_info_t *dbr_info;	/* info structure for this page */
-	uint32_t	nfree;		/* #free DBrs in this page */
-	uint32_t	firstfree;	/* idx of first free DBr in this page */
-	hermon_dbr_t	dbr[1];		/* rest ot he page is the DBrs */
-} hermon_dbr_header_t;
-
-#define	HERMON_DBR_HEADER_LEN	/* size in QUADWORDS/DBRs */ \
-	((sizeof (hermon_dbr_header_t) / sizeof (hermon_dbr_t)) - 1)
-
-#define	HERMON_NUM_DBR_PER_PAGE	((PAGESIZE >> 3) - HERMON_DBR_HEADER_LEN)
+#define	HERMON_NUM_DBR_PER_PAGE	(PAGESIZE / sizeof (hermon_dbr_t))
 
 
 /*