[openib-general] RE: [PATCH][kdapl] Integrate dapl_hca_alloc/dapl_hca_free to dapl _provider.c

Itamar Rabenstein itamar at mellanox.co.il
Thu Jun 23 14:02:29 PDT 2005


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:jlentini at netapp.com]
> 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 <itamar at mellanox.co.il>
> 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> 
> 



More information about the general mailing list