[Openib-windows] [IBAL][Patch] reference counter for IOC obj in IOC PnP manager
Alex Estrin
alex.estrin at qlogic.com
Tue Jan 2 08:15:44 PST 2007
Hi Yossi,
Yes, you are right, counter should decrement here.
Please apply.
Thank you,
Alex
> -----Original Message-----
> From: Yossi Leybovich [mailto:sleybo at mellanox.co.il]
> Sent: Tuesday, January 02, 2007 9:48 AM
> To: Alex Estrin
> Cc: openib-windows at openib.org
> Subject: RE: [Openib-windows] [IBAL][Patch] reference counter
> for IOC obj in IOC PnP manager
>
> 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
> > >
> > >
> >
> >
>
More information about the ofw
mailing list