[Openib-windows] [IBAL][Patch] reference counter for IOC obj in IOC PnP manager
Yossi Leybovich
sleybo at mellanox.co.il
Tue Jan 2 06:47:58 PST 2007
Alex
I notice that my ioc_pool is always growing (for each sweep).
I think we miss decrement of the ref count in case the svc_entries query
returned.
This patch fix the problem.
Pls review.
10x
Yossi
Index: al_ioc_pnp.c
===================================================================
--- al_ioc_pnp.c (revision 1890)
+++ al_ioc_pnp.c (working copy)
@@ -2151,7 +2151,7 @@
/* Update the number of entries received so far. */
p_ioc->num_valid_entries += (hi - lo) + 1;
-
+ cl_atomic_dec(&p_ioc->ref_cnt);
AL_EXIT( AL_DBG_PNP );
}
> -----Original Message-----
> From: Alex Estrin [mailto:alex.estrin at qlogic.com]
> Sent: Tuesday, November 28, 2006 1:10 AM
> To: Yossi Leybovich
> Cc: openib-windows at openib.org
> Subject: RE: [Openib-windows] [IBAL][Patch] reference counter
> for IOC obj in IOC PnP manager
>
> Hi Yossi,
>
> Please review the updated patch.
>
> Thanks,
> Alex
>
> Index: al_ioc_pnp.c
> ===================================================================
> --- al_ioc_pnp.c (revision 548)
> +++ al_ioc_pnp.c (working copy)
> @@ -272,6 +272,7 @@
> ib_ioc_profile_t profile;
> uint8_t num_valid_entries;
> ib_svc_entry_t *p_svc_entries;
> + atomic32_t ref_cnt;
>
> } iou_ioc_t;
> #pragma warning(default:4324)
> @@ -1052,13 +1053,15 @@
>
> p_ioc = PARENT_STRUCT( PARENT_STRUCT( p_item,
> cl_map_item_t, pool_item ),
> iou_ioc_t, map_item );
> +
> + CL_ASSERT( !p_ioc->ref_cnt );
>
> CL_ASSERT( !(ioc_slot >> 8) );
> p_ioc->slot = (uint8_t)ioc_slot;
> p_ioc->profile = *p_profile;
> p_ioc->num_valid_entries = 0;
> p_ioc->p_svc_entries = p_svc_entries;
> -
> + cl_atomic_inc( &p_ioc->ref_cnt );
> return p_ioc;
> }
>
> @@ -1068,12 +1071,14 @@
> IN ioc_pnp_mgr_t* const
> p_ioc_mgr,
> IN iou_ioc_t* const
> p_ioc )
> {
> - CL_ASSERT( p_ioc->p_svc_entries );
> - cl_free( p_ioc->p_svc_entries );
> + if( cl_atomic_dec( &p_ioc->ref_cnt ) == 0 )
> + {
> + cl_free( p_ioc->p_svc_entries );
>
> - cl_spinlock_acquire( &p_ioc_mgr->ioc_pool_lock );
> - cl_qpool_put( &p_ioc_mgr->ioc_pool,
> &p_ioc->map_item.pool_item );
> - cl_spinlock_release( &p_ioc_mgr->ioc_pool_lock );
> + cl_spinlock_acquire( &p_ioc_mgr->ioc_pool_lock );
> + cl_qpool_put( &p_ioc_mgr->ioc_pool,
> &p_ioc->map_item.pool_item );
> + cl_spinlock_release( &p_ioc_mgr->ioc_pool_lock );
> + }
> }
>
>
> @@ -1096,9 +1101,12 @@
> p_ioc = PARENT_STRUCT(
> PARENT_STRUCT( p_item, cl_map_item_t,
> pool_item ),
> iou_ioc_t, map_item );
> -
> - cl_free( p_ioc->p_svc_entries );
> - cl_qlist_insert_head( &list,
> &p_item->pool_item.list_item );
> +
> + if( cl_atomic_dec( &p_ioc->ref_cnt ) == 0 )
> + {
> + cl_free( p_ioc->p_svc_entries );
> + cl_qlist_insert_head( &list,
> &p_item->pool_item.list_item );
> + }
> p_item = cl_qmap_head( p_ioc_map );
> }
> cl_spinlock_acquire( &p_ioc_mgr->ioc_pool_lock ); @@
> -2479,6 +2487,7 @@
> p_mad->p_next = p_mad_list;
> p_mad_list = p_mad;
>
> + cl_atomic_inc( &p_ioc->ref_cnt );
> cl_atomic_inc(
> &p_results->p_svc->query_cnt );
> }
> }
> @@ -2502,6 +2511,8 @@
> {
> p_mad_list = p_mad->p_next;
> p_mad->p_next = NULL;
> + p_ioc = (iou_ioc_t* __ptr64)p_mad->context2;
> + cl_atomic_dec( &p_ioc->ref_cnt );
> ib_put_mad( p_mad );
> if( !cl_atomic_dec(
> &p_results->p_svc->query_cnt ) &&
> status == IB_SUCCESS )
>
> > -----Original Message-----
> > From: openib-windows-bounces at openib.org
> > [mailto:openib-windows-bounces at openib.org]On Behalf Of
> Yossi Leybovich
> > Sent: Monday, November 27, 2006 2:01 PM
> > To: Estrin, Alex
> > Cc: openib-windows at openib.org
> > Subject: Re: [Openib-windows] [IBAL][Patch] reference
> counter for IOC
> > obj in IOC PnP manager
> >
> >
> > Hi
> > See my comments
> >
> > > -----Original Message-----
> > > From: Estrin, Alex [mailto:aestrin at silverstorm.com]
> > > Sent: Monday, October 30, 2006 4:28 PM
> > > To: Yossi Leybovich
> > > Cc: openib-windows at openib.org
> > > Subject: RE: [Openib-windows] [IBAL][Patch] reference counter for
> > > IOC obj in IOC PnP manager
> > >
> > > Yossi,
> > > Please see comments below.
> > >
> > > > -----Original Message-----
> > > > From: Yossi Leybovich [mailto:sleybo at mellanox.co.il]
> > > > Sent: Sunday, October 29, 2006 8:07 AM
> > > > To: Estrin, Alex; Yossi Leybovich
> > > > Cc: openib-windows at openib.org
> > > > Subject: RE: [Openib-windows] [IBAL][Patch] reference
> > > counter for IOC
> > > > obj in IOC PnP manager
> > > >
> > > >
> > > > Hi
> > > >
> > > > Thanks for the patch I am reviewing it to check if the
> > > query_cnt field
> > > > can be use to track the outstanding queries.
> > > > Any way see below my comment
> > > >
> > > > > -----Original Message-----
> > > > > From: openib-windows-bounces at openib.org
> > > > > [mailto:openib-windows-bounces at openib.org] On Behalf Of
> > > Estrin, Alex
> > > > > Sent: Friday, October 27, 2006 1:43 AM
> > > > > To: Yossi Leybovich
> > > > > Cc: openib-windows at openib.org
> > > > > Subject: [Openib-windows] [IBAL][Patch] reference
> > counter for IOC
> > > > > obj in IOC PnP manager
> > > > >
> > > > > Hi Yossi,
> > > > >
> > > > > This patch introduces a reference counter to protect
> > ioc obj from
> > > > > double memory free.
> > > > >
> > > > > Possible scenario - IOC Manager handles PORT_DOWN event,
> > > frees ioc
> > > > > obj and put it back to the pool, while there are
> > outstanding send
> > > > > MAD svc that has reference to that ioc obj.
> > > > > On timeout send callback will try to free ioc obj again.
> > > > >
> > > > > By using this counter we also protect ioc obj from
> > return to the
> > > > > pool and possible reusage while outstanding send mad
> keeps it's
> > > > > reference.
> > > > > Please review.
> > > > >
> > > > > Thanks,
> > > > > Alex
> > > > >
> > > > ...
> > > > >
> > > > > cl_atomic_inc(
> > > > > &p_results->p_svc->query_cnt );
> > > > > }
> > > > > + if( status == IB_SUCCESS )
> > > > > + cl_atomic_inc(
> > &p_ioc->ref_cnt );
> > > >
> > > > The AL create mad for each chunk of 4 service entries it
> > > create list
> > > > from the mad and post it.
> > > > I think that we need to take reference for each mad we
> > send and not
> > > > for each mad list we post ( same as the query_cnt) What do
> > > you think ?
> > > >
> > > You are right.
> > > We should increment counter for every mad element that holds
> > > reference to ioc object.
> > > Thanks.
> >
> > What about decrement in case the we did not success to send all the
> > MADs ( similar to the query_cnt counter) I think we should
> decrement
> > the ref_cnt for each mad we tried to send but failed.
> > Otherwise we will never will decrement the counter ,
> because we will
> > never get complition.
> > What do you think ?
> >
> >
> >
> >
> > >
> >
> > _______________________________________________
> > openib-windows mailing list
> > openib-windows at openib.org
> > http://openib.org/mailman/listinfo/openib-windows
> >
> >
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: al_ioc_pnp_3.patch
Type: application/octet-stream
Size: 355 bytes
Desc: al_ioc_pnp_3.patch
URL: <http://lists.openfabrics.org/pipermail/ofw/attachments/20070102/c4b393bb/attachment.obj>
More information about the ofw
mailing list