hi James,

I think that you are worng. 
There is no functionalty to this struct. 
it is only a struct that hold some data together.
if we look into the files dapl_hca_util.[h.c]
we will find 4 functions:
1) dapl_hca_alloc  ->  which is kmalloc and set to 4 data members of this
struct
2) dapl_hca_free   ->  only 1 line  == kfree   
3) dapl_hca_link_ia    -> which is only a call to list_add
4) dapl_hca_unlink_ia  -> which is only a call to list_del

you have found out that functions 3 & 4 are not needed so we need to delete
them.

so i dont see why we should have 2 files for kmalloc and kfree

HCA is not like ia,evd,ep, ...
the others are dapl spec structs so they should be in separete files ,
hca is just a name that come from IB it is part of the provider code.
I believe that in other protocols implementaions they will not use "HCA".

 Itamar 

> -----Original Message-----
> From: James Lentini [mailto:[EMAIL PROTECTED]
> Sent: Thursday, June 23, 2005 11:27 PM
> To: Itamar Rabenstein
> Cc: openib-general
> Subject: Re: [PATCH][kdapl] Integrate dapl_hca_alloc/dapl_hca_free to
> dapl_provider.c
> 
> 
> 
> For consistency, I think we should keep the HCA object in its own 
> file. 
> 
> However, I'm not sure we need an HCA object. Is there a better way to 
> organize the data being stored in the dapl_hca structure? I have this 
> feeling that we should merge all the structures that we create on a 
> per-hca basis (the dapl_provider_entry, dat_provider table, and 
> dapl_hca).
> 
> james
> 
> On Mon, 20 Jun 2005, Itamar Rabenstein wrote:
> 
> itamar> ntegrate dapl_hca_alloc/dapl_hca_free to dapl_provider.c
> itamar> (no need for 2 files just for 2 simple function that 
> kmalloc and kfree.
> itamar>  There is not any special logic in this functions 
> that need to separate
> itamar>  them into different files)
> itamar> 
> itamar> Signed-off-by: Itamar Rabenstein <[EMAIL PROTECTED]>
> itamar> 
> itamar> diff -Nurp -X dontdiff dat-provider_link_ia/Makefile 
> dat-provider/Makefile
> itamar> --- dat-provider_link_ia/Makefile     Sun Jun 19 18:43:16 2005
> itamar> +++ dat-provider/Makefile     Sun Jun 19 19:45:15 2005
> itamar> @@ -20,7 +20,6 @@ PROVIDER_MODULES := \
> itamar>          dapl_cr                      \
> itamar>          dapl_ep                         \
> itamar>          dapl_evd                        \
> itamar> -        dapl_hca_util                        \
> itamar>          dapl_ia                      \
> itamar>          dapl_lmr                     \
> itamar>          dapl_provider                        \
> itamar> diff -Nurp -X dontdiff 
> dat-provider_link_ia/dapl_hca_util.c dat-provider/dapl_hca_util.c
> itamar> --- dat-provider_link_ia/dapl_hca_util.c      Sun Jun 
> 19 18:43:16 2005
> itamar> +++ dat-provider/dapl_hca_util.c      Thu Jan  1 02:00:00 1970
> itamar> @@ -1,90 +0,0 @@
> itamar> -/*
> itamar> - * Copyright (c) 2002-2005, Network Appliance, Inc. 
> All rights reserved.
> itamar> - * Copyright (c) 2005 Sun Microsystems, Inc. All 
> rights reserved.
> itamar> - *
> itamar> - * This Software is licensed under one of the 
> following licenses:
> itamar> - *
> itamar> - * 1) under the terms of the "Common Public License 
> 1.0" a copy of which is
> itamar> - *    available from the Open Source Initiative, see
> itamar> - *    http://www.opensource.org/licenses/cpl.php.
> itamar> - *
> itamar> - * 2) under the terms of the "The BSD License" a 
> copy of which is
> itamar> - *    available from the Open Source Initiative, see
> itamar> - *    http://www.opensource.org/licenses/bsd-license.php.
> itamar> - *
> itamar> - * 3) under the terms of the "GNU General Public 
> License (GPL) Version 2" a
> itamar> - *    copy of which is available from the Open 
> Source Initiative, see
> itamar> - *    http://www.opensource.org/licenses/gpl-license.php.
> itamar> - *
> itamar> - * Licensee has the right to choose one of the above 
> licenses.
> itamar> - *
> itamar> - * Redistributions of source code must retain the 
> above copyright
> itamar> - * notice and one of the license notices.
> itamar> - *
> itamar> - * Redistributions in binary form must reproduce 
> both the above copyright
> itamar> - * notice, one of the license notices in the documentation
> itamar> - * and/or other materials provided with the distribution.
> itamar> - */
> itamar> -
> itamar> -/*
> itamar> - * $Id: dapl_hca_util.c 2640 2005-06-16 16:22:46Z jlentini $
> itamar> - */
> itamar> -
> itamar> -#include "dapl.h"
> itamar> -#include "dapl_openib_util.h"
> itamar> -#include "dapl_provider.h"
> itamar> -#include "dapl_hca_util.h"
> itamar> -
> itamar> -/*
> itamar> - * dapl_hca_alloc
> itamar> - *
> itamar> - * alloc and initialize an HCA struct
> itamar> - *
> itamar> - * Input:
> itamar> - *   name
> itamar> - *      port
> itamar> - *
> itamar> - * Output:
> itamar> - *   hca
> itamar> - *
> itamar> - * Returns:
> itamar> - *   none
> itamar> - *
> itamar> - */
> itamar> -struct dapl_hca *dapl_hca_alloc(char *name, struct 
> ib_device *device, u8 port)
> itamar> -{
> itamar> -     struct dapl_hca *hca;
> itamar> -     int malloc_size = sizeof *hca + strlen(name) + 1;
> itamar> -              
> itamar> -     hca = kmalloc(malloc_size, GFP_ATOMIC);
> itamar> -     if (hca) {
> itamar> -             memset(hca, 0, malloc_size);
> itamar> -             spin_lock_init(&hca->lock);
> itamar> -             INIT_LIST_HEAD(&hca->ia_list);
> itamar> -             hca->name = (char *)hca + sizeof *hca;
> itamar> -             strcpy(hca->name, name);
> itamar> -             hca->ib_hca_handle = device;
> itamar> -             hca->port_num = port;
> itamar> -     }
> itamar> -     return hca;
> itamar> -}
> itamar> -
> itamar> -/*
> itamar> - * dapl_hca_free
> itamar> - *
> itamar> - * free an IA INFO struct
> itamar> - *
> itamar> - * Input:
> itamar> - *   hca
> itamar> - *
> itamar> - * Output:
> itamar> - *   none
> itamar> - *
> itamar> - * Returns:
> itamar> - *   none
> itamar> - *
> itamar> - */
> itamar> -void dapl_hca_free(struct dapl_hca *hca)
> itamar> -{
> itamar> -     kfree(hca);
> itamar> -}
> itamar> diff -Nurp -X dontdiff 
> dat-provider_link_ia/dapl_hca_util.h dat-provider/dapl_hca_util.h
> itamar> --- dat-provider_link_ia/dapl_hca_util.h      Sun Jun 
> 19 18:43:16 2005
> itamar> +++ dat-provider/dapl_hca_util.h      Thu Jan  1 02:00:00 1970
> itamar> @@ -1,42 +0,0 @@
> itamar> -/*
> itamar> - * Copyright (c) 2002-2005, Network Appliance, Inc. 
> All rights reserved.
> itamar> - * Copyright (c) 2005 Sun Microsystems, Inc. All 
> rights reserved.
> itamar> - *
> itamar> - * This Software is licensed under one of the 
> following licenses:
> itamar> - *
> itamar> - * 1) under the terms of the "Common Public License 
> 1.0" a copy of which is
> itamar> - *    available from the Open Source Initiative, see
> itamar> - *    http://www.opensource.org/licenses/cpl.php.
> itamar> - *
> itamar> - * 2) under the terms of the "The BSD License" a 
> copy of which is
> itamar> - *    available from the Open Source Initiative, see
> itamar> - *    http://www.opensource.org/licenses/bsd-license.php.
> itamar> - *
> itamar> - * 3) under the terms of the "GNU General Public 
> License (GPL) Version 2" a
> itamar> - *    copy of which is available from the Open 
> Source Initiative, see
> itamar> - *    http://www.opensource.org/licenses/gpl-license.php.
> itamar> - *
> itamar> - * Licensee has the right to choose one of the above 
> licenses.
> itamar> - *
> itamar> - * Redistributions of source code must retain the 
> above copyright
> itamar> - * notice and one of the license notices.
> itamar> - *
> itamar> - * Redistributions in binary form must reproduce 
> both the above copyright
> itamar> - * notice, one of the license notices in the documentation
> itamar> - * and/or other materials provided with the distribution.
> itamar> - */
> itamar> -
> itamar> -/*
> itamar> - * $Id: dapl_hca_util.h 2640 2005-06-16 16:22:46Z jlentini $
> itamar> - */
> itamar> -
> itamar> -#ifndef DAPL_HCA_UTIL_H
> itamar> -#define DAPL_HCA_UTIL_H
> itamar> -
> itamar> -#include "dapl.h"
> itamar> -
> itamar> -struct dapl_hca *dapl_hca_alloc(char *name, struct 
> ib_device *device, u8 port);
> itamar> -
> itamar> -void dapl_hca_free(struct dapl_hca *hca);
> itamar> -
> itamar> -#endif
> itamar> diff -Nurp -X dontdiff 
> dat-provider_link_ia/dapl_provider.c dat-provider/dapl_provider.c
> itamar> --- dat-provider_link_ia/dapl_provider.c      Sun Jun 
> 19 18:43:16 2005
> itamar> +++ dat-provider/dapl_provider.c      Sun Jun 19 19:47:16 2005
> itamar> @@ -35,7 +35,6 @@
> itamar>  #include <dat.h>
> itamar>  
> itamar>  #include "dapl.h"
> itamar> -#include "dapl_hca_util.h"
> itamar>  #include "dapl_provider.h"
> itamar>  #include "dapl_util.h"
> itamar>  #include "dapl_openib_util.h"
> itamar> @@ -246,6 +245,24 @@ static void dapl_provider_info_init(stru
> itamar>       provider_info->ia_name[i+1] = '\0';
> itamar>  }
> itamar>  
> itamar> +static struct dapl_hca *dapl_hca_alloc(char *name, 
> struct ib_device *device, u8 port)
> itamar> +{
> itamar> +     struct dapl_hca *hca;
> itamar> +     int malloc_size = sizeof *hca + strlen(name) + 1;
> itamar> +              
> itamar> +     hca = kmalloc(malloc_size, GFP_ATOMIC);
> itamar> +     if (hca) {
> itamar> +             memset(hca, 0, malloc_size);
> itamar> +             spin_lock_init(&hca->lock);
> itamar> +             INIT_LIST_HEAD(&hca->ia_list);
> itamar> +             hca->name = (char *)hca + sizeof *hca;
> itamar> +             strcpy(hca->name, name);
> itamar> +             hca->ib_hca_handle = device;
> itamar> +             hca->port_num = port;
> itamar> +     }
> itamar> +     return hca;
> itamar> +}
> itamar> +
> itamar>  static void dapl_add_port(struct ib_device *device, u8 port)
> itamar>  {
> itamar>       struct dat_provider_info provider_info;
> itamar> @@ -305,7 +322,7 @@ error:
> itamar>                       
> (void)dapl_provider_list_remove(provider_info.ia_name);
> itamar>  
> itamar>               if (NULL != hca)
> itamar> -                     dapl_hca_free(hca);
> itamar> +                     kfree(hca);
> itamar>       }
> itamar>  }
> itamar>  
> itamar> @@ -338,7 +355,7 @@ static void dapl_remove_port(struct ib_d
> itamar>                            provider_info.ia_name);
> itamar>       }
> itamar>  
> itamar> -     dapl_hca_free(provider->extension);
> itamar> +     kfree(provider->extension);
> itamar>  
> itamar>       dapl_provider_list_remove(provider_info.ia_name);
> itamar>  }
> itamar> -- 
> itamar> Itamar
> itamar> 
> 
_______________________________________________
openib-general mailing list
[email protected]
http://openib.org/mailman/listinfo/openib-general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Reply via email to