[PATCH] libfc: add stats info against memory leakage in exchange pool

[PATCH] libfc: add stats info against memory leakage in exchange pool

[Hillf Danton]

There seems stats info needed if mem leak is concerned.

Signed-off-by: Hillf Danton <dhillf@gmail.com>
---

--- a/drivers/scsi/libfc/fc_exch.c	2010-09-13 07:07:38.000000000 +0800
+++ b/drivers/scsi/libfc/fc_exch.c	2010-10-24 17:10:08.000000000 +0800
@@ -67,6 +67,11 @@ struct workqueue_struct *fc_exch_workque
 struct fc_exch_pool {
 	u16		 next_index;
 	u16		 total_exches;
+
+	/* stats info against memory leakage */
+	atomic_t	 allocated;
+	atomic_t	 released;
+
 	spinlock_t	 lock;
 	struct list_head ex_list;
 };
@@ -405,6 +410,8 @@ static void fc_exch_delete(struct fc_exc
 	fc_exch_ptr_set(pool, (ep->xid - ep->em->min_xid) >> fc_cpu_order,
 			NULL);
 	list_del(&ep->ex_list);
+	if (1 == atomic_read(&ep->ex_refcnt))
+		atomic_inc(&pool->released);
 	spin_unlock_bh(&pool->lock);
 	fc_exch_release(ep);	/* drop hold for exch in mp */
 }
@@ -689,6 +696,7 @@ static struct fc_exch *fc_exch_em_alloc(
 	pool->next_index = index == mp->pool_max_index ? 0 : index + 1;

 	fc_exch_hold(ep);	/* hold for exch in mp */
+	atomic_inc(&pool->allocated);
 	spin_lock_init(&ep->ex_lock);
 	/*
 	 * Hold exch lock for caller to prevent fc_exch_reset()
@@ -1699,6 +1707,12 @@ restart:
 			goto restart;
 		}
 	}
+	if (atomic_read(&pool->allocated) != atomic_read(&pool->released))
+		printk(KERN_WARNING "libfc: host %u exch pool %p "
+			"allocated %d released %d\n",
+			lport->host->host_no, pool,
+			atomic_read(&pool->allocated),
+			atomic_read(&pool->released));
 	spin_unlock_bh(&pool->lock);
 }

Re: [PATCH] libfc: add stats info against memory leakage in exchange pool

[Hillf Danton]

This is the second version, with a fault in the previous version fixed.

Signed-off-by: Hillf Danton <dhillf@gmail.com>
---

--- a/drivers/scsi/libfc/fc_exch.c	2010-09-13 07:07:38.000000000 +0800
+++ b/drivers/scsi/libfc/fc_exch.c	2010-10-26 21:24:22.000000000 +0800
@@ -67,6 +67,11 @@ struct workqueue_struct *fc_exch_workque
 struct fc_exch_pool {
 	u16		 next_index;
 	u16		 total_exches;
+
+	/* stats info against memory leakage */
+	atomic_t         allocated;
+	atomic_t         released;
+
 	spinlock_t	 lock;
 	struct list_head ex_list;
 };
@@ -326,6 +331,7 @@ static void fc_exch_release(struct fc_ex
 	struct fc_exch_mgr *mp;

 	if (atomic_dec_and_test(&ep->ex_refcnt)) {
+		atomic_inc(&ep->pool->released);
 		mp = ep->em;
 		if (ep->destructor)
 			ep->destructor(&ep->seq, ep->arg);
@@ -688,6 +694,7 @@ static struct fc_exch *fc_exch_em_alloc(
 	}
 	pool->next_index = index == mp->pool_max_index ? 0 : index + 1;

+	atomic_inc(&pool->allocated);
 	fc_exch_hold(ep);	/* hold for exch in mp */
 	spin_lock_init(&ep->ex_lock);
 	/*
@@ -1699,6 +1706,14 @@ restart:
 			goto restart;
 		}
 	}
+
+	if (atomic_read(&pool->allocated) != atomic_read(&pool->released))
+		printk(KERN_WARNING "libfc: host %u exch pool %p "
+			"allocated %d released %d\n",
+			lport->host->host_no, pool,
+			atomic_read(&pool->allocated),
+			atomic_read(&pool->released));
+
 	spin_unlock_bh(&pool->lock);
 }

Re: [Open-FCoE] [PATCH] libfc: add stats info against memory leakage in exchange pool

[Robert Love]

On Tue, 2010-10-26 at 21:24 +0800, Hillf Danton wrote:
> This is the second version, with a fault in the previous version fixed.
> 
> Signed-off-by: Hillf Danton <dhillf@gmail.com>
> ---
> 

This patch fails to apply on top of your other patch that adds a
exchange cache. I have fixed it locally and will add the fixed patch to
fcoe-next. You also omitted any patch description in this patch other
than it being the second version.

Can you please add more verbose descriptions to your patches. The best
advice given to me was to have a problem statement, a statement on what
the solution is and then any technical details. I realize that some of
your patches are very straight-forward and don't require that degree of
detail, but some have been a bit more involved and "seems like there is
a problem" doesn't give much context before looking at the changes
themselves. It would help me when reviewing your changes.

Thanks, //Rob

Re: [Open-FCoE] [PATCH] libfc: add stats info against memory leakage in exchange pool

[Hillf Danton]

On Thu, Oct 28, 2010 at 8:49 AM, Robert Love <robert.w.love@intel.com> wrote:
> On Tue, 2010-10-26 at 21:24 +0800, Hillf Danton wrote:
>> This is the second version, with a fault in the previous version fixed.
>>
>> Signed-off-by: Hillf Danton <dhillf@gmail.com>
>> ---
>>
>
> This patch fails to apply on top of your other patch that adds a
> exchange cache. I have fixed it locally and will add the fixed patch to
> fcoe-next. You also omitted any patch description in this patch other
> than it being the second version.

The related patches were prepared without consideration of each other,
and it is my fault.

>
> Can you please add more verbose descriptions to your patches. The best
> advice given to me was to have a problem statement, a statement on what
> the solution is and then any technical details. I realize that some of

I will try as hard as I can for better patch, thanks. //Hillf

> your patches are very straight-forward and don't require that degree of
> detail, but some have been a bit more involved and "seems like there is
> a problem" doesn't give much context before looking at the changes
> themselves. It would help me when reviewing your changes.
>
> Thanks, //Rob
>
>