[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