Michael S. Tsirkin wrote:
> Adding registration at module start/stop seems simple enough and overhead is
> minimal. We already have this for other modules (e.g. ib_sa). I don't really
> unerstand why is there such a resistance to this simple fix for unload race?
There's no resistance if someone i
Quoting r. Sean Hefty <[EMAIL PROTECTED]>:
> Does SDP use this feature for events other than for connection requests?
Yes, it does. But as I said, since SDP is out of tree, for now we
can just ignore the module unloading race.
--
MST
___
openib-gener
Quoting r. Sean Hefty <[EMAIL PROTECTED]>:
> I use the callback method of destruction for new cm_id's in the ucm and ucma
> modules, so I want to keep this feature myself. However, this method is
> unused, and likely unneeded, for events other than connection requests. If
> this is the case, we c
>Another case is a request and then a reject.
Yes - I considered reject, disconnect, and device removal as good candidates to
make use of this.
It's just that in these cases, the user has had the option of allocating
resources with the cm_id that it can use to queue for destruction. With a new
c
Quoting r. Sean Hefty <[EMAIL PROTECTED]>:
> Subject: Re: [PATCH] for 2-6-19 rdma/addr: use client registration to fix
> module unload race
>
> > All active side users are fine I think. But any client on the passive side
> > currently might destroy the new ID by returning error from the callback
> All active side users are fine I think. But any client on the passive side
> currently might destroy the new ID by returning error from the callback, and I
> like this interface since it frees the resources immediately.
As long as only *newly* created (i.e. associated with a connection request)
Roland Dreier wrote:
> Unfortunately I don't think this solves the module unloading race at
> all: there is still a window where code in the client module callback
> is running, but the callback has dropped all references etc. so the
> client module will happily proceed to unload.
>
> > At the bo
Quoting r. Sean Hefty <[EMAIL PROTECTED]>:
> Subject: Re: [PATCH] for 2-6-19 rdma/addr: use client registration to fix
> module unload race
>
> >>I think this is actually a good point for the CM case at least.
> >>Clients already have something registered with the CM (namely the CM
> >>ID itself)
>>I think this is actually a good point for the CM case at least.
>>Clients already have something registered with the CM (namely the CM
>>ID itself), so if we required all consumers to destroy their IDs
>>explicitly, then there's no reason to add additional client
>>registration.
>
> The issue is
Roland Dreier wrote:
> I think this is actually a good point for the CM case at least.
> Clients already have something registered with the CM (namely the CM
> ID itself), so if we required all consumers to destroy their IDs
> explicitly, then there's no reason to add additional client
> registrati
> what about enhancing xxx_destory_id() to sense that it was called from
> this id callback context so the xxx module code defers the
> destory_id() execution to run after the callback is over. This can be
> done by writing at the id the pid of the thread running the callback
> before going to
Quoting r. Or Gerlitz <[EMAIL PROTECTED]>:
> what about enhancing xxx_destory_id() to sense that it was called from
> this id callback context so the xxx module code defers the destory_id()
> execution to run after the callback is over. This can be done by
> writing at the id the pid of the th
Sean Hefty wrote:
> Or Gerlitz wrote:
>> If yes, this seems to me as one big over-doing, assuming the consumer
>> always either call XXX_destory_id() OR returns non zero from a
>> callback on this ID, there must be away to avoid the race within the
>> ID provider module, so at least the api can
Roland Dreier wrote:
> > @@ -305,6 +330,7 @@ int rdma_resolve_ip(struct sockaddr *src
> >break;
> >default:
> >ret = req->status;
> > + atomic_dec(&client->refcount);
> >kfree(req);
> >break;
> >}
>
> Doesn't this need to be
> @@ -305,6 +330,7 @@ int rdma_resolve_ip(struct sockaddr *src
> break;
> default:
> ret = req->status;
> +atomic_dec(&client->refcount);
> kfree(req);
> break;
> }
Doesn't this need to be deref_client() here too? O
Michael S. Tsirkin wrote:
> ib_cm has this bug as well. Shouldn't we patch it for 2.6.19 too?
Yes - I've started on patches for the ib_cm and rdma_cm as well. They just
aren't quite so straightforward to fix.
- Sean
___
openib-general mailing list
op
Or Gerlitz wrote:
> So the only case for which all this registration api/code at the ib_sa
> ib_cm and ib_addr (is it also in the ib_mad) protects against is where
> the consumer wants to destroy its ID by returning non zero from a
> callback and not by an explicit call to XXX_destory_id()
ib_s
Quoting r. Or Gerlitz <[EMAIL PROTECTED]>:
> there must be away to avoid the race within the ID provider module,
There's no other way - go over the archives.
> so at least the api can be saved...
See Documentation/stable_api_nonsense.txt
--
MST
___
Michael S. Tsirkin wrote:
>> Michael S. Tsirkin wrote:
>>> Quoting r. Or Gerlitz <[EMAIL PROTECTED]>:
>>> The race happens on module unload - you might be inside the cm callback when
>>> the module is unloaded. Nothing the module itself does can help here - you
>>> must
>>> synchronize with the c
Quoting r. Or Gerlitz <[EMAIL PROTECTED]>:
> Subject: Re: [openib-general] [PATCH] for 2-6-19 rdma/addr: use client
> registration to fix module unload race
>
> Michael S. Tsirkin wrote:
> > Quoting r. Or Gerlitz <[EMAIL PROTECTED]>:
> >>>> Requir
Michael S. Tsirkin wrote:
> Quoting r. Or Gerlitz <[EMAIL PROTECTED]>:
Require registration with ib_addr module to prevent caller from unloading
while a callback is in progress.
>>> ib_cm has this bug as well. Shouldn't we patch it for 2.6.19 too?
>> I know there was a similar discussion
Quoting r. Or Gerlitz <[EMAIL PROTECTED]>:
> >> Require registration with ib_addr module to prevent caller from unloading
> >> while a callback is in progress.
>
> > ib_cm has this bug as well. Shouldn't we patch it for 2.6.19 too?
>
> I know there was a similar discussion which i was not trackin
Michael S. Tsirkin wrote:
> Quoting r. Sean Hefty <[EMAIL PROTECTED]>:
>> Subject: [PATCH] for 2-6-19 rdma/addr: use client registration to fix module
>> unload race
>> Require registration with ib_addr module to prevent caller from unloading
>> while a callback is in progress.
> ib_cm has this b
Quoting r. Sean Hefty <[EMAIL PROTECTED]>:
> Subject: [PATCH] for 2-6-19 rdma/addr: use client registration to fix module
> unload race
>
> Require registration with ib_addr module to prevent caller from unloading
> while a callback is in progress.
>
> Signed-off-by: Sean Hefty <[EMAIL PROTECTED
Quoting r. Or Gerlitz <[EMAIL PROTECTED]>:
> Subject: Re: [PATCH] for 2-6-19 rdma/addr: use client registration to fix
> module unload race
>
> Sean Hefty wrote:
> > Require registration with ib_addr module to prevent caller from unloading
> > while a callback is in progress.
>
> Sean,
>
> Is t
Sean Hefty wrote:
> Require registration with ib_addr module to prevent caller from unloading
> while a callback is in progress.
Sean,
Is there a conceptual/practical difference between the ib_addr and ib_cm
modules which enforces this client registration of the cma being an addr
consumer but n
Require registration with ib_addr module to prevent caller from unloading
while a callback is in progress.
Signed-off-by: Sean Hefty <[EMAIL PROTECTED]>
---
diff --git a/drivers/infiniband/core/addr.c b/drivers/infiniband/core/addr.c
index 60d3fbd..894d856 100644
--- a/drivers/infiniband/core/addr
27 matches
Mail list logo
