On Wed, 2007-01-31 at 12:24 +0200, Michael S. Tsirkin wrote: > > Quoting Roland Dreier <[EMAIL PROTECTED]>: > > Subject: Re: [PATCH] The ibv_cmd_* create functions need to set the context. > > > > Thanks, applied to master and stable branches. > > Did you test it? > This patch (8b3d225476c99ea29a68109a7d40e5ef353d4388) causes ibv_ud_pingpong > to segfault on libmthca: libmthca never calls ibv_cmd_create_ah to context is > now > never set. > >
I didn't test UD. > Starting program: /usr/local/ofed/bin/ibv_ud_pingpong sw069 > [Thread debugging using libthread_db enabled] > [New Thread 47299578320592 (LWP 5085)] > local address: LID 0x0002, QPN 0x090406, PSN 0x71bffb > remote address: LID 0x0001, QPN 0x040406, PSN 0x92316a > 4096000 bytes in 0.02 seconds = 1893.99 Mbit/sec > 1000 iters in 0.02 seconds = 17.30 usec/iter > > Program received signal SIGSEGV, Segmentation fault. > [Switching to Thread 47299578320592 (LWP 5085)] > 0x00002b04ca3b7263 in __ibv_destroy_ah (ah=0x5050b0) at src/verbs.c:475 > 475 return ah->context->ops.destroy_ah(ah); > (gdb) p ah->context > $1 = (struct ibv_context *) 0x0 > > I actually think this approach is a wrong one: context should be > set in common code like ibv_create_ah, not in ibv_cmd_ which is > a library function low level driver might or might not call. > And certainly this kind of change does not seem appropriate for stable branch. > > I think the proper thing is for low level driver not to assume that > fields such as contex are intialized until create functions have returned. > Steve, pls fix your low level driver not to rely on this. > The issue is that the provider lib calls ibv_cmd_create_blah to create the object, then some failure happens (like a failure mmap()ing the object's DMA area to the process). At this point the provider lib must destroy this object that is created from the perspective of the ibv_cmd* interface. The only way to do that is to call the ibv_cmd_destroy_blah call, which needs the context field. So I don't think solving this in the provider lib is the right thing to do. > Roland, I have reverted this in OFED, please revert on master and stable. > I think we should fix the bug introduced: set the context field in the ibv_create_blah service if its not set after calling the provider method. Steve. _______________________________________________ openib-general mailing list [email protected] http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
