[Openib-windows] [IBAL][Patch] reference counter for IOC obj in IOC PnP manager
Yossi Leybovich
sleybo at dev.mellanox.co.il
Tue Nov 28 11:42:08 PST 2006
> -----Original Message-----
> From: Alex Estrin [mailto:alex.estrin at qlogic.com]
> Sent: Tuesday, November 28, 2006 9:01 PM
> To: Yossi Leybovich
> Subject: RE: [Openib-windows] [IBAL][Patch] reference counter
> for IOC obj in IOC PnP manager
>
> Hi Yossi,
>
> Thank you for applying patch.
> Yes, maybe it make sense to add ref counter for iou as well
> although it only be able to protect against early reusage.
If we create IOU and send query we need to
be sure the query return before we return the IOU to the pool.
There also might be case that we will destroy the pool and than
when we will get the query CB we will end up with blue screen.
> I consider it theoretical, since I couldn't make it happend
> in my environment, probably because scale of ious is much
> less than iocs+svc entries.
How many IOC do you have in each IOU ?
In my system I have 1 IOC on each IOU
> Any thoughts?
>
I think that we should add this
> Thanks,
> Alex
>
> > -----Original Message-----
> > From: Yossi Leybovich [mailto:sleybo at dev.mellanox.co.il]
> > Sent: Tuesday, November 28, 2006 1:36 PM
> > To: Alex Estrin
> > Subject: RE: [Openib-windows] [IBAL][Patch] reference
> counter for IOC
> > obj in IOC PnP manager
> >
> >
> > Hi
> >
> > Apply in rev 535
> >
> > Do you think we will need ref count for the ious?
> >
> > Yossi
> >
> >
> > > -----Original Message-----
> > > From: openib-windows-bounces at openib.org
> > > [mailto:openib-windows-bounces at openib.org] On Behalf Of
> Alex Estrin
> > > 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