[openib-general] Re: [PATCH][kdapl]update dat_rmr_bind API & delete dapl_hash

Itamar Rabenstein itamar at mellanox.co.il
Mon Jun 20 02:01:32 PDT 2005


O.K 
i am resending this patch and spliting it to 5 small patches

1. Update dat_rmr_bind API (added lmr_handle input param)
2. Remove dapl_hash (there is no need for the hash)
3. Small changes in dapl_hca_alloc/dapl_hca_free functions
4. Integrate dapl_hca_link_ia/dapl_hca_unlink_ia into dapl_ia.c
   (no need for functios that just call LIST_ADD and LIST_DEL)
5. Integrate dapl_hca_alloc/dapl_hca_free to dapl_provider.c
   (no need for 2 files just for 2 simple function that kmalloc and kfree.
    There is not any special logic in this functions that need to separate
them
    into different files)

  Itamar

> -----Original Message-----
> From: Christoph Hellwig [mailto:hch at lst.de]
> Sent: Friday, June 17, 2005 7:41 PM
> To: Itamar & Shira Rabenstein
> Cc: openib-general at openib.org
> Subject: Re: [openib-general] Re: [PATCH][kdapl]update 
> dat_rmr_bind API
> & delete dapl_hash
> 
> 
> On Thu, Jun 16, 2005 at 09:20:27PM +0200, Itamar & Shira 
> Rabenstein wrote:
> > >Itamar,
> > >
> > >Can you seperate the RMR api change and hash table removal into a 
> > >seperate patch?
> > 
> > I dont see why, the new input param to dat_rmr_bind 
> (lmr_handle) come
> > in order that we will be able to remove the hash table so 
> this is one
> > patch.
> 
> In the Linux world we generally prefer faine-grained patches.  While
> the RMR api change is a prerequsite to remove the hash code it's not
> directly related.
> 
> > >Merging the HCA and IA code into the dapl_ia.[ch] files 
> doesn't feel 
> > >right to me. I think of the HCA as a DAT object just like 
> IAs, EPs, 
> > >LMRs, etc, and hence it should have its own file.
> > >
> > >james
> > 
> > 
> > There are 4 function in this files:
> > 
> > 2 were merged to provider code:
> >       dapl_hca_alloc(13 lines) :which i think is part of 
> the provider code .
> >       dapl_hca_free (1 line) == kfree , no need a function for this.
> > 2 were merged to ia code:
> >       dapl_hca_link_ia (3 lines )     : no need a function 
> to call list add
> >       dapl_hca_unlink_ia(3 lines)    : no need a function 
> to call list_del
> > 
> > I belive that we need to make the code more readable and to 
> have this functions
> > is not a good idea and to add 2 files for this is for sure 
> a bad idea ...
> 
> I agree your reasoning for the second set of functions at least.  But
> again this is not related to the API change at all, please separate it
> out to a patch of it's own, applying ontop of the API change and hash
> removal patches.
> 
> _______________________________________________
> openib-general mailing list
> openib-general at openib.org
> http://openib.org/mailman/listinfo/openib-general
> 
> To unsubscribe, please visit 
> http://openib.org/mailman/listinfo/openib-general
> 



More information about the general mailing list