[openib-general] Re: RE: Re: [PATCH] ipoib_flush_paths

[Michael S. Tsirkin]

Quoting r. Fabian Tillier <ftillier at silverstorm.com>:
> Subject: Re: [openib-general] Re: RE: Re: [PATCH] ipoib_flush_paths
> 
> On 4/6/06, Michael S. Tsirkin <mst at mellanox.co.il> wrote:
> > Quoting r. Muli Ben-Yehuda <mulix at mulix.org>:
> > > Subject: Re: [openib-general] Re: RE: Re: [PATCH] ipoib_flush_paths
> > >
> > > On Thu, Apr 06, 2006 at 04:38:33PM +0300, Michael S. Tsirkin wrote:
> > >
> > > > No, since we are keeping a callback pointer into that module.
> > >
> > > Sorry if I'm being dense but I don't see it in this patch. Point me at
> > > it?
> >
> > You don't see it in the patch because SA already kept a callback pointer -
> > that's the race I'm solving. Look in sa_query.c
> >
> >
> > If I have
> >
> > struct query {
> >        void (*callback)();
> >        struct module *owner;
> > }
> >
> > Then it is always safe to do
> >
> >        __get_module(query->owner);
> >        query->callback();
> >        put_module(query->owner);
> >
> > since it is the called module's responsibility to invalidate
> > all such query objects before its unloaded.
> 
> Wait, why are you doing __get_module just before the callback?  This
> leaves the possibility of crashing - sure, you'll detect that things
> went wrong, but you haven't solved the issue.  The whole point of the
> reference is to prevent the crash.

No, this problem is solved today in all ULPs by caller polling on flag
(completion) that callback sets: all ULPs do this already, since the need to
track resources irrespective of module being unloaded.

> You need to call __get_module from the context of teh caller making the request.

No, this would prevent module from being unloaded for extended periods
of time - we don't want this.

All I must prevent is problem we missed previously: module being unloaded
while callback is in progress.

-- 
MST