RE: [openib-general] [PATCH 1/2] kDAPL: remove inline functions
On Thu, 4 Aug 2005, Tom Duffy wrote: On Thu, 2005-08-04 at 20:30 +0300, Or Gerlitz wrote: This is almost exactly what is there now. Not really a change. I don't like the multiple indirections to get to the function table, but this can be simplified later. Indeed, it can be simplified a little, but why not having this simplification in the kdapl registry level ("dat") as it is in ib_core calling ib_verbs and libibverbs calling libmthca, while the verbs consumer does not have to go into those indirections at all but rather just call a well understood/defined api? are you suggesting to apply such a chance also to the verbs? I don't understand what you mean? What change would happen in verbs? I think Or is asking if similar changes should be made to the verbs (i.e. should ib_alloc_pd() be remove and replaced in each verb's user by device->alloc_pd(..), etc.). Is that right Or? In kDAPL, the inline dat_* functions just perform a function call. In the case of the verbs, there are some operations performed by the verbs core in addition to calling the function. For example, ib_alloc_pd() intiailizes the ib_pd fields in addition to calling the device specific alloc_pd function. Due to the additional functionality, I don't think this convention would extend to the IB verbs. In terms of kDAPL, I share Or's concern that this will make the API harder for consumers to use. Picture someone who wants to use the kDAPL API for the first time. Today that person would scan the kDAPL header and see all of the functions available and get a pretty clear picture of what each functions parameters are for just by looking at the parameter names. We'd loose that "documentation" with this change. With respect to the struct file_operations analogy, is a struct file_operations embedded in any other structure besides struct file? In kDAPL, the function table is embedded in several different objects. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
RE: [openib-general] [PATCH 1/2] kDAPL: remove inline functions
On Thu, 2005-08-04 at 20:30 +0300, Or Gerlitz wrote: > >This is almost exactly what is there now. Not really a change. I don't > >like > > the multiple indirections to get to the function table, but this can be > > simplified later. > > Indeed, it can be simplified a little, but why not having this simplification > in the kdapl > registry level ("dat") as it is in ib_core calling ib_verbs and libibverbs > calling libmthca, > while the verbs consumer does not have to go into those indirections at all > but rather > just call a well understood/defined api? are you suggesting to apply such a > chance also > to the verbs? I don't understand what you mean? What change would happen in verbs? -tduffy signature.asc Description: This is a digitally signed message part ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
RE: [openib-general] [PATCH 1/2] kDAPL: remove inline functions
>This is almost exactly what is there now. Not really a change. I don't like > the multiple indirections to get to the function table, but this can be > simplified later. Indeed, it can be simplified a little, but why not having this simplification in the kdapl registry level ("dat") as it is in ib_core calling ib_verbs and libibverbs calling libmthca, while the verbs consumer does not have to go into those indirections at all but rather just call a well understood/defined api? are you suggesting to apply such a chance also to the verbs? Or. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
RE: [openib-general] [PATCH 1/2] kDAPL: remove inline functions
On Thu, 2005-08-04 at 17:13 +0300, Or Gerlitz wrote: > Tom, > > Wouldn't it be simple to come with a patch that applies only to the > "dat" code ie > (gen2/trunk/src/linux-kernel/infiniband/ulp/kdapl/kdapl.h and/or api.c) > which does > not require --any-- code change at the cosumer side? > > that is, with the patch you sent, a call to dat_pz_create at in consumer > code was changed > > from > -ret = dat_pz_create (ia, &pz); > to > + ret = ia->common.provider->pz_create (ia, &pz); > > so cant it be done with dat_pz_create being a function (or define) > calling to > > ia->common.provider->pz_create(ia, &pz); > > similar to what is done by ib_core / libibverbs calling ib_mthca / > libmthca > > This makes the consumer code simpler and no changes are need in current > consumers This is almost exactly what is there now. Not really a change. I don't like the multiple indirections to get to the function table, but this can be simplified later. Using the function table to call the function is a well established practice within the Linux kernel -- look at struct file_operations for a good example usage. -tduffy signature.asc Description: This is a digitally signed message part ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH 1/2] kDAPL: remove inline functions
On Wed, 3 Aug 2005, Tom Duffy wrote: On Tue, 2005-08-02 at 15:00 -0700, Tom Duffy wrote: This patch removes all the inline functions, instead just call the functions from the function table. It also removes the _func from the names as this is obvious by its type. James, Any chance of merging these? I was removing dapl_hca_util.[ch]. I'll go through them now. james ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
RE: [openib-general] [PATCH 1/2] kDAPL: remove inline functions
Tom, Wouldn't it be simple to come with a patch that applies only to the "dat" code ie (gen2/trunk/src/linux-kernel/infiniband/ulp/kdapl/kdapl.h and/or api.c) which does not require --any-- code change at the cosumer side? that is, with the patch you sent, a call to dat_pz_create at in consumer code was changed from -ret = dat_pz_create (ia, &pz); to + ret = ia->common.provider->pz_create (ia, &pz); so cant it be done with dat_pz_create being a function (or define) calling to ia->common.provider->pz_create(ia, &pz); similar to what is done by ib_core / libibverbs calling ib_mthca / libmthca This makes the consumer code simpler and no changes are need in current consumers Or. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH 1/2] kDAPL: remove inline functions
On Tue, 2005-08-02 at 15:00 -0700, Tom Duffy wrote: > This patch removes all the inline functions, instead just call the > functions from the function table. It also removes the _func from the > names as this is obvious by its type. James, Any chance of merging these? -tduffy signature.asc Description: This is a digitally signed message part ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general