[ofa-general] Re: [PATCH 2/3] remove ib pkey gid and lmc cache

Michael S. Tsirkin mst at dev.mellanox.co.il
Wed May 2 10:18:29 PDT 2007


> Quoting Yosef Etigin <yosefe at voltaire.com>:
> Subject: [PATCH 2/3] remove ib pkey gid and lmc cache
> 
> Remove IB cache from core
> 
> * Remove pkey, gid, and lmc caches
> * Rewrite ib_find_gid and ib_find_pkey over blocking device queries 
> * Modify users of the cache to use these methods
> 

That's what we wanted to do, allright.  But there are some issues here.

> Signed-off-by: Yosef Etigin <yosefe at voltaire.com>
> ---
>  drivers/infiniband/core/cache.c         |  398 --------------------------------
>  include/rdma/ib_cache.h                 |  118 ---------
>  drivers/infiniband/core/Makefile        |    2 
>  drivers/infiniband/core/cm.c            |    8 
>  drivers/infiniband/core/cma.c           |    9 
>  drivers/infiniband/core/core_priv.h     |    3 
>  drivers/infiniband/core/device.c        |  143 ++++++++++-
>  drivers/infiniband/core/mad.c           |    5 
>  drivers/infiniband/core/multicast.c     |    3 
>  drivers/infiniband/core/sa_query.c      |    3 
>  drivers/infiniband/core/verbs.c         |    3 
>  drivers/infiniband/hw/mthca/mthca_av.c  |    3 
>  drivers/infiniband/hw/mthca/mthca_qp.c  |   10 
>  drivers/infiniband/ulp/ipoib/ipoib_cm.c |    3 
>  drivers/infiniband/ulp/ipoib/ipoib_ib.c |    2 
>  drivers/infiniband/ulp/srp/ib_srp.c     |    6 
>  include/rdma/ib_verbs.h                 |   37 ++
>  17 files changed, 196 insertions(+), 560 deletions(-)
> 
> Index: b/drivers/infiniband/core/device.c
> ===================================================================
> --- a/drivers/infiniband/core/device.c	2007-05-02 17:47:50.517342683 +0300
> +++ b/drivers/infiniband/core/device.c	2007-05-02 17:48:30.719181916 +0300
> @@ -149,6 +149,20 @@ static int alloc_name(char *name)
>  	return 0;
>  }
>  
> +
> +static inline int start_port(struct ib_device *device)
> +{
> +	return (device->node_type == RDMA_NODE_IB_SWITCH) ? 0 : 1;
> +}
> +
> +
> +static inline int end_port(struct ib_device *device)
> +{
> +	return (device->node_type == RDMA_NODE_IB_SWITCH) ?
> +		0 : device->phys_port_cnt;
> +}
> +
> +
>  /**
>   * ib_alloc_device - allocate an IB device struct
>   * @size:size of structure to allocate

No double-spacing please.
A single empty line is enough for separation.

> @@ -592,6 +606,128 @@ int ib_modify_port(struct ib_device *dev
>  }
>  EXPORT_SYMBOL(ib_modify_port);
>  
> +/**
> + * ib_find_gid - Returns the port number and index of a GID
> + * @device: Device to query.

Kill the "."

> + * @gid: GID to look for
> + * @port_num: Returned port number
> + * @index: Returned index
> + *
> + * ib_find_gid() returns the index of @pkey in the pkey table
> + * on port @port_num
> + */


The description is not really clear. For comparison, here's what
we had for ib_find_cached_gid:
> - * ib_find_cached_gid - Returns the port number and GID table index where
> - *   a specified GID value occurs.
> - * @device: The device to query.
> - * @gid: The GID value to search for.
> - * @port_num: The port number of the device where the GID value was found.
> - * @index: The index into the cached GID table where the GID was found.  This
> - *   parameter may be NULL.
> - *
> - * ib_find_cached_gid() searches for the specified GID value in
> - * the local software cache.

so how about just removing the last 2 lines (which don't apply now)
and reusing the description as is?
	
> + int ib_find_gid(struct ib_device *device,
> +		       union ib_gid	    *gid,
> +		       u8               *port_num,
> +		       u16              *index)
> +{

what's going on with alignment/whitespace here?

> +	struct ib_port_attr *tprops = NULL;
> +	union ib_gid tmp_gid;
> +	int ret;
> +	int port;
> +	int i;

Just one int will do:
	int i, port, ret;

> +	tprops = kmalloc(sizeof *tprops, GFP_ATOMIC);

Why ATOMIC?
What if allocation fails?

> +
> +	for (port = start_port(device); port <= end_port(device); ++port) {
> +		ret = ib_query_port(device, port, tprops);
> +		if (ret)
> +			continue;
> +
> +		for (i = 0; i < tprops->gid_tbl_len; ++i) {
> +			ret = ib_query_gid(device, port, i, &tmp_gid);
> +			if (ret)
> +				goto out;
> +			if (!memcmp(&tmp_gid, gid, sizeof *gid)) {
> +				*port_num = port;
> +				*index = i;
> +				ret = 0;
> +				goto out;
> +			}
> +		} /* for i */

Killthe comment pls.

> +	}
> +	ret = -ENOENT;
> +out:
> +	kfree(tprops);
> +	return ret;
> +}
> +EXPORT_SYMBOL(ib_find_gid);

Mostly same comments apply to other functions below.

> +/**
> + * ib_find_pkey - Returns the index of a PKey on a port
> + * @device: Device to query.
> + * @port_num: Port to query on
> + * @pkey: PKey to look for
> + * @index: Returned index
> + *
> + * ib_find_pkey() returns the index of @pkey in the pkey table
> + * on port @port_num
> + */
> +int ib_find_pkey(struct ib_device *device,
> +			u8                port_num,
> +			u16               pkey,
> +			u16              *index)
> +{
> +	struct ib_port_attr *tprops = NULL;
> +	int ret;
> +	int i = -1;

What does -1 do here?

> +	u16 tmp_pkey;
> +
> +	tprops = kmalloc(sizeof *tprops, GFP_ATOMIC);
> +
> +	ret = ib_query_port(device, port_num, tprops);
> +	if (ret) {
> +		printk(KERN_WARNING "ib_query_port failed , ret = %d\n", ret);
> +		goto out;
> +	}
> +
> +	for (i = 0; i < tprops->pkey_tbl_len; ++i) {
> +		ret = ib_query_pkey(device, port_num, i, &tmp_pkey);
> +		if (ret)
> +			goto out;
> +
> +		if (pkey == tmp_pkey) {
> +			*index = i;
> +			ret = 0;
> +			goto out;
> +		}
> +	}
> +	ret = -ENOENT;
> +
> +out:
> +	kfree(tprops);
> +	return ret;
> +}
> +EXPORT_SYMBOL(ib_find_pkey);
> +
> +/**
> + * ib_query_lmc - Returns the LMC of a port
> + * @device: Device to query.
> + * @port_num: Port to query on
> + * @lmc: Returned LMC
> + *
> + * ib_query_lmc() returns the LID mask control associated
> + * with port @port_num
> + */
> +int ib_query_lmc(struct ib_device *device,
> +		      u8                port_num,
> +		      u8                *lmc)
> +{
> +	struct ib_port_attr *tprops = NULL;
> +	int ret;
> +
> +	tprops = kmalloc(sizeof *tprops, GFP_ATOMIC);
> +	ret = ib_query_port(device, port_num, tprops);
> +	if (ret) goto err;

goto belongs on a line of its own.

> +
> +	*lmc = tprops->lmc;
> +err:
> +	kfree(tprops);
> +	return ret;
> +
> +}
> +EXPORT_SYMBOL(ib_query_lmc);
> +
>  static int __init ib_core_init(void)
>  {
>  	int ret;
> @@ -600,18 +736,11 @@ static int __init ib_core_init(void)
>  	if (ret)
>  		printk(KERN_WARNING "Couldn't create InfiniBand device class\n");
>  
> -	ret = ib_cache_setup();
> -	if (ret) {
> -		printk(KERN_WARNING "Couldn't set up InfiniBand P_Key/GID cache\n");
> -		ib_sysfs_cleanup();
> -	}
> -
>  	return ret;
>  }
>  
>  static void __exit ib_core_cleanup(void)
>  {
> -	ib_cache_cleanup();
>  	ib_sysfs_cleanup();
>  }
>  
> Index: b/drivers/infiniband/core/cache.c
> ===================================================================
> --- a/drivers/infiniband/core/cache.c	2007-05-02 17:47:49.878456482 +0300
> +++ /dev/null	1970-01-01 00:00:00.000000000 +0000
> @@ -1,398 +0,0 @@
> -/*
> - * Copyright (c) 2004 Topspin Communications.  All rights reserved.
> - * Copyright (c) 2005 Intel Corporation. All rights reserved.
> - * Copyright (c) 2005 Sun Microsystems, Inc. All rights reserved.
> - * Copyright (c) 2005 Voltaire, Inc. All rights reserved.
> - *
> - * This software is available to you under a choice of one of two
> - * licenses.  You may choose to be licensed under the terms of the GNU
> - * General Public License (GPL) Version 2, available from the file
> - * COPYING in the main directory of this source tree, or the
> - * OpenIB.org BSD license below:
> - *
> - *     Redistribution and use in source and binary forms, with or
> - *     without modification, are permitted provided that the following
> - *     conditions are met:
> - *
> - *      - Redistributions of source code must retain the above
> - *        copyright notice, this list of conditions and the following
> - *        disclaimer.
> - *
> - *      - Redistributions in binary form must reproduce the above
> - *        copyright notice, this list of conditions and the following
> - *        disclaimer in the documentation and/or other materials
> - *        provided with the distribution.
> - *
> - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> - * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> - * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> - * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> - * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> - * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> - * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> - * SOFTWARE.
> - *
> - * $Id: cache.c 1349 2004-12-16 21:09:43Z roland $
> - */
> -
> -#include <linux/module.h>
> -#include <linux/errno.h>
> -#include <linux/slab.h>
> -
> -#include <rdma/ib_cache.h>
> -
> -#include "core_priv.h"
> -
> -struct ib_pkey_cache {
> -	int             table_len;
> -	u16             table[0];
> -};
> -
> -struct ib_gid_cache {
> -	int             table_len;
> -	union ib_gid    table[0];
> -};
> -
> -struct ib_update_work {
> -	struct work_struct work;
> -	struct ib_device  *device;
> -	u8                 port_num;
> -};
> -
> -static inline int start_port(struct ib_device *device)
> -{
> -	return (device->node_type == RDMA_NODE_IB_SWITCH) ? 0 : 1;
> -}
> -
> -static inline int end_port(struct ib_device *device)
> -{
> -	return (device->node_type == RDMA_NODE_IB_SWITCH) ?
> -		0 : device->phys_port_cnt;
> -}
> -
> -int ib_get_cached_gid(struct ib_device *device,
> -		      u8                port_num,
> -		      int               index,
> -		      union ib_gid     *gid)
> -{
> -	struct ib_gid_cache *cache;
> -	unsigned long flags;
> -	int ret = 0;
> -
> -	if (port_num < start_port(device) || port_num > end_port(device))
> -		return -EINVAL;
> -
> -	read_lock_irqsave(&device->cache.lock, flags);
> -
> -	cache = device->cache.gid_cache[port_num - start_port(device)];
> -
> -	if (index < 0 || index >= cache->table_len)
> -		ret = -EINVAL;
> -	else
> -		*gid = cache->table[index];
> -
> -	read_unlock_irqrestore(&device->cache.lock, flags);
> -
> -	return ret;
> -}
> -EXPORT_SYMBOL(ib_get_cached_gid);
> -
> -int ib_find_cached_gid(struct ib_device *device,
> -		       union ib_gid	*gid,
> -		       u8               *port_num,
> -		       u16              *index)
> -{
> -	struct ib_gid_cache *cache;
> -	unsigned long flags;
> -	int p, i;
> -	int ret = -ENOENT;
> -
> -	*port_num = -1;
> -	if (index)
> -		*index = -1;
> -
> -	read_lock_irqsave(&device->cache.lock, flags);
> -
> -	for (p = 0; p <= end_port(device) - start_port(device); ++p) {
> -		cache = device->cache.gid_cache[p];
> -		for (i = 0; i < cache->table_len; ++i) {
> -			if (!memcmp(gid, &cache->table[i], sizeof *gid)) {
> -				*port_num = p + start_port(device);
> -				if (index)
> -					*index = i;
> -				ret = 0;
> -				goto found;
> -			}
> -		}
> -	}
> -found:
> -	read_unlock_irqrestore(&device->cache.lock, flags);
> -
> -	return ret;
> -}
> -EXPORT_SYMBOL(ib_find_cached_gid);
> -
> -int ib_get_cached_pkey(struct ib_device *device,
> -		       u8                port_num,
> -		       int               index,
> -		       u16              *pkey)
> -{
> -	struct ib_pkey_cache *cache;
> -	unsigned long flags;
> -	int ret = 0;
> -
> -	if (port_num < start_port(device) || port_num > end_port(device))
> -		return -EINVAL;
> -
> -	read_lock_irqsave(&device->cache.lock, flags);
> -
> -	cache = device->cache.pkey_cache[port_num - start_port(device)];
> -
> -	if (index < 0 || index >= cache->table_len)
> -		ret = -EINVAL;
> -	else
> -		*pkey = cache->table[index];
> -
> -	read_unlock_irqrestore(&device->cache.lock, flags);
> -
> -	return ret;
> -}
> -EXPORT_SYMBOL(ib_get_cached_pkey);
> -
> -int ib_find_cached_pkey(struct ib_device *device,
> -			u8                port_num,
> -			u16               pkey,
> -			u16              *index)
> -{
> -	struct ib_pkey_cache *cache;
> -	unsigned long flags;
> -	int i;
> -	int ret = -ENOENT;
> -
> -	if (port_num < start_port(device) || port_num > end_port(device))
> -		return -EINVAL;
> -
> -	read_lock_irqsave(&device->cache.lock, flags);
> -
> -	cache = device->cache.pkey_cache[port_num - start_port(device)];
> -
> -	*index = -1;
> -
> -	for (i = 0; i < cache->table_len; ++i)
> -		if ((cache->table[i] & 0x7fff) == (pkey & 0x7fff)) {
> -			*index = i;
> -			ret = 0;
> -			break;
> -		}
> -
> -	read_unlock_irqrestore(&device->cache.lock, flags);
> -
> -	return ret;
> -}
> -EXPORT_SYMBOL(ib_find_cached_pkey);
> -
> -int ib_get_cached_lmc(struct ib_device *device,
> -		      u8                port_num,
> -		      u8                *lmc)
> -{
> -	unsigned long flags;
> -	int ret = 0;
> -
> -	if (port_num < start_port(device) || port_num > end_port(device))
> -		return -EINVAL;
> -
> -	read_lock_irqsave(&device->cache.lock, flags);
> -	*lmc = device->cache.lmc_cache[port_num - start_port(device)];
> -	read_unlock_irqrestore(&device->cache.lock, flags);
> -
> -	return ret;
> -}
> -EXPORT_SYMBOL(ib_get_cached_lmc);
> -
> -static void ib_cache_update(struct ib_device *device,
> -			    u8                port)
> -{
> -	struct ib_port_attr       *tprops = NULL;
> -	struct ib_pkey_cache      *pkey_cache = NULL, *old_pkey_cache;
> -	struct ib_gid_cache       *gid_cache = NULL, *old_gid_cache;
> -	int                        i;
> -	int                        ret;
> -
> -	tprops = kmalloc(sizeof *tprops, GFP_KERNEL);
> -	if (!tprops)
> -		return;
> -
> -	ret = ib_query_port(device, port, tprops);
> -	if (ret) {
> -		printk(KERN_WARNING "ib_query_port failed (%d) for %s\n",
> -		       ret, device->name);
> -		goto err;
> -	}
> -
> -	pkey_cache = kmalloc(sizeof *pkey_cache + tprops->pkey_tbl_len *
> -			     sizeof *pkey_cache->table, GFP_KERNEL);
> -	if (!pkey_cache)
> -		goto err;
> -
> -	pkey_cache->table_len = tprops->pkey_tbl_len;
> -
> -	gid_cache = kmalloc(sizeof *gid_cache + tprops->gid_tbl_len *
> -			    sizeof *gid_cache->table, GFP_KERNEL);
> -	if (!gid_cache)
> -		goto err;
> -
> -	gid_cache->table_len = tprops->gid_tbl_len;
> -
> -	for (i = 0; i < pkey_cache->table_len; ++i) {
> -		ret = ib_query_pkey(device, port, i, pkey_cache->table + i);
> -		if (ret) {
> -			printk(KERN_WARNING "ib_query_pkey failed (%d) for %s (index %d)\n",
> -			       ret, device->name, i);
> -			goto err;
> -		}
> -	}
> -
> -	for (i = 0; i < gid_cache->table_len; ++i) {
> -		ret = ib_query_gid(device, port, i, gid_cache->table + i);
> -		if (ret) {
> -			printk(KERN_WARNING "ib_query_gid failed (%d) for %s (index %d)\n",
> -			       ret, device->name, i);
> -			goto err;
> -		}
> -	}
> -
> -	write_lock_irq(&device->cache.lock);
> -
> -	old_pkey_cache = device->cache.pkey_cache[port - start_port(device)];
> -	old_gid_cache  = device->cache.gid_cache [port - start_port(device)];
> -
> -	device->cache.pkey_cache[port - start_port(device)] = pkey_cache;
> -	device->cache.gid_cache [port - start_port(device)] = gid_cache;
> -
> -	device->cache.lmc_cache[port - start_port(device)] = tprops->lmc;
> -
> -	write_unlock_irq(&device->cache.lock);
> -
> -	kfree(old_pkey_cache);
> -	kfree(old_gid_cache);
> -	kfree(tprops);
> -	return;
> -
> -err:
> -	kfree(pkey_cache);
> -	kfree(gid_cache);
> -	kfree(tprops);
> -}
> -
> -static void ib_cache_task(struct work_struct *_work)
> -{
> -	struct ib_update_work *work =
> -		container_of(_work, struct ib_update_work, work);
> -
> -	ib_cache_update(work->device, work->port_num);
> -	kfree(work);
> -}
> -
> -static void ib_cache_event(struct ib_event_handler *handler,
> -			   struct ib_event *event)
> -{
> -	struct ib_update_work *work;
> -
> -	if (event->event == IB_EVENT_PORT_ERR    ||
> -	    event->event == IB_EVENT_PORT_ACTIVE ||
> -	    event->event == IB_EVENT_LID_CHANGE  ||
> -	    event->event == IB_EVENT_PKEY_CHANGE ||
> -	    event->event == IB_EVENT_SM_CHANGE   ||
> -	    event->event == IB_EVENT_CLIENT_REREGISTER) {
> -		work = kmalloc(sizeof *work, GFP_ATOMIC);
> -		if (work) {
> -			INIT_WORK(&work->work, ib_cache_task);
> -			work->device   = event->device;
> -			work->port_num = event->element.port_num;
> -			schedule_work(&work->work);
> -		}
> -	}
> -}
> -
> -static void ib_cache_setup_one(struct ib_device *device)
> -{
> -	int p;
> -
> -	rwlock_init(&device->cache.lock);
> -
> -	device->cache.pkey_cache =
> -		kmalloc(sizeof *device->cache.pkey_cache *
> -			(end_port(device) - start_port(device) + 1), GFP_KERNEL);
> -	device->cache.gid_cache =
> -		kmalloc(sizeof *device->cache.gid_cache *
> -			(end_port(device) - start_port(device) + 1), GFP_KERNEL);
> -
> -	device->cache.lmc_cache = kmalloc(sizeof *device->cache.lmc_cache *
> -					  (end_port(device) -
> -					   start_port(device) + 1),
> -					  GFP_KERNEL);
> -
> -	if (!device->cache.pkey_cache || !device->cache.gid_cache ||
> -	    !device->cache.lmc_cache) {
> -		printk(KERN_WARNING "Couldn't allocate cache "
> -		       "for %s\n", device->name);
> -		goto err;
> -	}
> -
> -	for (p = 0; p <= end_port(device) - start_port(device); ++p) {
> -		device->cache.pkey_cache[p] = NULL;
> -		device->cache.gid_cache [p] = NULL;
> -		ib_cache_update(device, p + start_port(device));
> -	}
> -
> -	INIT_IB_EVENT_HANDLER(&device->cache.event_handler,
> -			      device, ib_cache_event);
> -	if (ib_register_event_handler(&device->cache.event_handler))
> -		goto err_cache;
> -
> -	return;
> -
> -err_cache:
> -	for (p = 0; p <= end_port(device) - start_port(device); ++p) {
> -		kfree(device->cache.pkey_cache[p]);
> -		kfree(device->cache.gid_cache[p]);
> -	}
> -
> -err:
> -	kfree(device->cache.pkey_cache);
> -	kfree(device->cache.gid_cache);
> -	kfree(device->cache.lmc_cache);
> -}
> -
> -static void ib_cache_cleanup_one(struct ib_device *device)
> -{
> -	int p;
> -
> -	ib_unregister_event_handler(&device->cache.event_handler);
> -	flush_scheduled_work();
> -
> -	for (p = 0; p <= end_port(device) - start_port(device); ++p) {
> -		kfree(device->cache.pkey_cache[p]);
> -		kfree(device->cache.gid_cache[p]);
> -	}
> -
> -	kfree(device->cache.pkey_cache);
> -	kfree(device->cache.gid_cache);
> -	kfree(device->cache.lmc_cache);
> -}
> -
> -static struct ib_client cache_client = {
> -	.name   = "cache",
> -	.add    = ib_cache_setup_one,
> -	.remove = ib_cache_cleanup_one
> -};
> -
> -int __init ib_cache_setup(void)
> -{
> -	return ib_register_client(&cache_client);
> -}
> -
> -void __exit ib_cache_cleanup(void)
> -{
> -	ib_unregister_client(&cache_client);
> -}
> Index: b/include/rdma/ib_cache.h
> ===================================================================
> --- a/include/rdma/ib_cache.h	2007-05-02 17:47:13.398954200 +0300
> +++ /dev/null	1970-01-01 00:00:00.000000000 +0000
> @@ -1,118 +0,0 @@
> -/*
> - * Copyright (c) 2004 Topspin Communications.  All rights reserved.
> - * Copyright (c) 2005 Intel Corporation. All rights reserved.
> - * Copyright (c) 2005 Sun Microsystems, Inc. All rights reserved.
> - *
> - * This software is available to you under a choice of one of two
> - * licenses.  You may choose to be licensed under the terms of the GNU
> - * General Public License (GPL) Version 2, available from the file
> - * COPYING in the main directory of this source tree, or the
> - * OpenIB.org BSD license below:
> - *
> - *     Redistribution and use in source and binary forms, with or
> - *     without modification, are permitted provided that the following
> - *     conditions are met:
> - *
> - *      - Redistributions of source code must retain the above
> - *        copyright notice, this list of conditions and the following
> - *        disclaimer.
> - *
> - *      - Redistributions in binary form must reproduce the above
> - *        copyright notice, this list of conditions and the following
> - *        disclaimer in the documentation and/or other materials
> - *        provided with the distribution.
> - *
> - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> - * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> - * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> - * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> - * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> - * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> - * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> - * SOFTWARE.
> - *
> - * $Id: ib_cache.h 1349 2004-12-16 21:09:43Z roland $
> - */
> -
> -#ifndef _IB_CACHE_H
> -#define _IB_CACHE_H
> -
> -#include <rdma/ib_verbs.h>
> -
> -/**
> - * ib_get_cached_gid - Returns a cached GID table entry
> - * @device: The device to query.
> - * @port_num: The port number of the device to query.
> - * @index: The index into the cached GID table to query.
> - * @gid: The GID value found at the specified index.
> - *
> - * ib_get_cached_gid() fetches the specified GID table entry stored in
> - * the local software cache.
> - */
> -int ib_get_cached_gid(struct ib_device    *device,
> -		      u8                   port_num,
> -		      int                  index,
> -		      union ib_gid        *gid);
> -
> -/**
> - * ib_find_cached_gid - Returns the port number and GID table index where
> - *   a specified GID value occurs.
> - * @device: The device to query.
> - * @gid: The GID value to search for.
> - * @port_num: The port number of the device where the GID value was found.
> - * @index: The index into the cached GID table where the GID was found.  This
> - *   parameter may be NULL.
> - *
> - * ib_find_cached_gid() searches for the specified GID value in
> - * the local software cache.
> - */
> -int ib_find_cached_gid(struct ib_device *device,
> -		       union ib_gid	*gid,
> -		       u8               *port_num,
> -		       u16              *index);
> -
> -/**
> - * ib_get_cached_pkey - Returns a cached PKey table entry
> - * @device: The device to query.
> - * @port_num: The port number of the device to query.
> - * @index: The index into the cached PKey table to query.
> - * @pkey: The PKey value found at the specified index.
> - *
> - * ib_get_cached_pkey() fetches the specified PKey table entry stored in
> - * the local software cache.
> - */
> -int ib_get_cached_pkey(struct ib_device    *device_handle,
> -		       u8                   port_num,
> -		       int                  index,
> -		       u16                 *pkey);
> -
> -/**
> - * ib_find_cached_pkey - Returns the PKey table index where a specified
> - *   PKey value occurs.
> - * @device: The device to query.
> - * @port_num: The port number of the device to search for the PKey.
> - * @pkey: The PKey value to search for.
> - * @index: The index into the cached PKey table where the PKey was found.
> - *
> - * ib_find_cached_pkey() searches the specified PKey table in
> - * the local software cache.
> - */
> -int ib_find_cached_pkey(struct ib_device    *device,
> -			u8                   port_num,
> -			u16                  pkey,
> -			u16                 *index);
> -
> -/**
> - * ib_get_cached_lmc - Returns a cached lmc table entry
> - * @device: The device to query.
> - * @port_num: The port number of the device to query.
> - * @lmc: The lmc value for the specified port for that device.
> - *
> - * ib_get_cached_lmc() fetches the specified lmc table entry stored in
> - * the local software cache.
> - */
> -int ib_get_cached_lmc(struct ib_device *device,
> -		      u8                port_num,
> -		      u8                *lmc);
> -
> -#endif /* _IB_CACHE_H */
> Index: b/include/rdma/ib_verbs.h
> ===================================================================
> --- a/include/rdma/ib_verbs.h	2007-05-02 17:47:13.398954200 +0300
> +++ b/include/rdma/ib_verbs.h	2007-05-02 17:48:30.741177998 +0300
> @@ -1134,6 +1134,43 @@ int ib_modify_port(struct ib_device *dev
>  		   struct ib_port_modify *port_modify);
>  
>  /**
> + * ib_find_gid - Returns the port number and index of a GID
> + * @device: Device to query.
> + * @gid: GID to look for
> + * @port_num: Returned port number
> + * @index: Returned index
> + *
> + * ib_find_gid() returns the index of @pkey in the pkey table
> + * on port @port_num
> + */
> + int ib_find_gid(struct ib_device *device, union ib_gid *gid,
> +            u8 *port_num, u16 *index);
> +
> +/**
> + * ib_find_pkey - Returns the index of a PKey on a port
> + * @device: Device to query.
> + * @port_num: Port to query on
> + * @pkey: PKey to look for
> + * @index: Returned index
> + *
> + * ib_find_pkey() returns the index of @pkey in the pkey table
> + * on port @port_num
> + */
> +int ib_find_pkey(struct ib_device *device,	u8 port_num,
> +			  u16 pkey, u16 *index);
> +
> +/**
> + * ib_query_lmc - Returns the LMC of a port
> + * @device: Device to query.
> + * @port_num: Port to query on
> + * @lmc: Returned LMC
> + *
> + * ib_query_lmc() returns the LID mask control associated
> + * with port @port_num
> + */
> +int ib_query_lmc(struct ib_device *device, u8 port_num, u8 *lmc);
> +

I don't think we need this one in ib_verbs.h - it just does query_port once.
Let's keep the API simple. The only user is in mad.c - move
it there and make it static.

> +/**
>   * ib_alloc_pd - Allocates an unused protection domain.
>   * @device: The device on which to allocate the protection domain.
>   *
> Index: b/drivers/infiniband/core/Makefile
> ===================================================================
> --- a/drivers/infiniband/core/Makefile	2007-05-02 17:47:49.333553540 +0300
> +++ b/drivers/infiniband/core/Makefile	2007-05-02 17:48:30.741177998 +0300
> @@ -8,7 +8,7 @@ obj-$(CONFIG_INFINIBAND_USER_ACCESS) +=	
>  					$(user_access-y)
>  
>  ib_core-y :=			packer.o ud_header.o verbs.o sysfs.o \
> -				device.o fmr_pool.o cache.o
> +				device.o fmr_pool.o
>  
>  ib_mad-y :=			mad.o smi.o agent.o mad_rmpp.o
>  
> Index: b/drivers/infiniband/core/cm.c
> ===================================================================
> --- a/drivers/infiniband/core/cm.c	2007-05-02 17:47:49.762477140 +0300
> +++ b/drivers/infiniband/core/cm.c	2007-05-02 17:48:30.744177464 +0300
> @@ -46,8 +46,8 @@
>  #include <linux/spinlock.h>
>  #include <linux/workqueue.h>
>  
> -#include <rdma/ib_cache.h>
>  #include <rdma/ib_cm.h>
> +#include <rdma/ib_verbs.h>
>  #include "cm_msgs.h"
>  
>  MODULE_AUTHOR("Sean Hefty");
> @@ -275,7 +275,7 @@ static int cm_init_av_by_path(struct ib_
>  
>  	read_lock_irqsave(&cm.device_lock, flags);
>  	list_for_each_entry(cm_dev, &cm.device_list, list) {
> -		if (!ib_find_cached_gid(cm_dev->device, &path->sgid,
> +		if (!ib_find_gid(cm_dev->device, &path->sgid,
>  					&p, NULL)) {
>  			port = &cm_dev->port[p-1];
>  			break;
> @@ -286,7 +286,7 @@ static int cm_init_av_by_path(struct ib_
>  	if (!port)
>  		return -EINVAL;
>  
> -	ret = ib_find_cached_pkey(cm_dev->device, port->port_num,
> +	ret = ib_find_pkey(cm_dev->device, port->port_num,
>  				  be16_to_cpu(path->pkey), &av->pkey_index);
>  	if (ret)
>  		return ret;
> @@ -1382,7 +1382,7 @@ static int cm_req_handler(struct cm_work
>  	cm_format_paths_from_req(req_msg, &work->path[0], &work->path[1]);
>  	ret = cm_init_av_by_path(&work->path[0], &cm_id_priv->av);
>  	if (ret) {
> -		ib_get_cached_gid(work->port->cm_dev->device,
> +		ib_query_gid(work->port->cm_dev->device,
>  				  work->port->port_num, 0, &work->path[0].sgid);
>  		ib_send_cm_rej(cm_id, IB_CM_REJ_INVALID_GID,
>  			       &work->path[0].sgid, sizeof work->path[0].sgid,
> Index: b/drivers/infiniband/core/cma.c
> ===================================================================
> --- a/drivers/infiniband/core/cma.c	2007-05-02 17:47:50.749301367 +0300
> +++ b/drivers/infiniband/core/cma.c	2007-05-02 17:48:30.746177108 +0300
> @@ -41,7 +41,6 @@
>  
>  #include <rdma/rdma_cm.h>
>  #include <rdma/rdma_cm_ib.h>
> -#include <rdma/ib_cache.h>
>  #include <rdma/ib_cm.h>
>  #include <rdma/ib_sa.h>
>  #include <rdma/iw_cm.h>
> @@ -325,7 +324,7 @@ static int cma_acquire_dev(struct rdma_i
>  	}
>  
>  	list_for_each_entry(cma_dev, &dev_list, list) {
> -		ret = ib_find_cached_gid(cma_dev->device, &gid,
> +		ret = ib_find_gid(cma_dev->device, &gid,
>  					 &id_priv->id.port_num, NULL);
>  		if (!ret) {
>  			ret = cma_set_qkey(cma_dev->device,
> @@ -514,7 +513,7 @@ static int cma_ib_init_qp_attr(struct rd
>  	struct rdma_dev_addr *dev_addr = &id_priv->id.route.addr.dev_addr;
>  	int ret;
>  
> -	ret = ib_find_cached_pkey(id_priv->id.device, id_priv->id.port_num,
> +	ret = ib_find_pkey(id_priv->id.device, id_priv->id.port_num,
>  				  ib_addr_get_pkey(dev_addr),
>  				  &qp_attr->pkey_index);
>  	if (ret)
> @@ -1658,11 +1657,11 @@ static int cma_bind_loopback(struct rdma
>  	cma_dev = list_entry(dev_list.next, struct cma_device, list);
>  
>  port_found:
> -	ret = ib_get_cached_gid(cma_dev->device, p, 0, &gid);
> +	ret = ib_query_gid(cma_dev->device, p, 0, &gid);
>  	if (ret)
>  		goto out;
>  
> -	ret = ib_get_cached_pkey(cma_dev->device, p, 0, &pkey);
> +	ret = ib_query_pkey(cma_dev->device, p, 0, &pkey);
>  	if (ret)
>  		goto out;
>  
> Index: b/drivers/infiniband/core/mad.c
> ===================================================================
> --- a/drivers/infiniband/core/mad.c	2007-05-02 17:47:50.423359423 +0300
> +++ b/drivers/infiniband/core/mad.c	2007-05-02 17:48:30.748176751 +0300
> @@ -34,7 +34,6 @@
>   * $Id: mad.c 5596 2006-03-03 01:00:07Z sean.hefty $
>   */
>  #include <linux/dma-mapping.h>
> -#include <rdma/ib_cache.h>
>  
>  #include "mad_priv.h"
>  #include "mad_rmpp.h"
> @@ -1707,13 +1706,13 @@ static inline int rcv_has_same_gid(struc
>  	if (!send_resp && rcv_resp) {
>  		/* is request/response. */
>  		if (!(attr.ah_flags & IB_AH_GRH)) {
> -			if (ib_get_cached_lmc(device, port_num, &lmc))
> +			if (ib_query_lmc(device, port_num, &lmc))

Just do query_port here.

>  				return 0;
>  			return (!lmc || !((attr.src_path_bits ^
>  					   rwc->wc->dlid_path_bits) &
>  					  ((1 << lmc) - 1)));
>  		} else {
> -			if (ib_get_cached_gid(device, port_num,
> +			if (ib_query_gid(device, port_num,
>  					      attr.grh.sgid_index, &sgid))
>  				return 0;
>  			return !memcmp(sgid.raw, rwc->recv_buf.grh->dgid.raw,
> Index: b/drivers/infiniband/core/multicast.c
> ===================================================================
> --- a/drivers/infiniband/core/multicast.c	2007-05-02 17:47:51.014254173 +0300
> +++ b/drivers/infiniband/core/multicast.c	2007-05-02 17:48:30.749176573 +0300
> @@ -38,7 +38,6 @@
>  #include <linux/bitops.h>
>  #include <linux/random.h>
>  
> -#include <rdma/ib_cache.h>
>  #include "sa.h"
>  
>  static void mcast_add_one(struct ib_device *device);
> @@ -686,7 +685,7 @@ int ib_init_ah_from_mcmember(struct ib_d
>  	u16 gid_index;
>  	u8 p;
>  
> -	ret = ib_find_cached_gid(device, &rec->port_gid, &p, &gid_index);
> +	ret = ib_find_gid(device, &rec->port_gid, &p, &gid_index);
>  	if (ret)
>  		return ret;
>  
> Index: b/drivers/infiniband/core/sa_query.c
> ===================================================================
> --- a/drivers/infiniband/core/sa_query.c	2007-05-02 17:47:49.689490140 +0300
> +++ b/drivers/infiniband/core/sa_query.c	2007-05-02 17:48:30.749176573 +0300
> @@ -47,7 +47,6 @@
>  #include <linux/workqueue.h>
>  
>  #include <rdma/ib_pack.h>
> -#include <rdma/ib_cache.h>
>  #include "sa.h"
>  
>  MODULE_AUTHOR("Roland Dreier");
> @@ -477,7 +476,7 @@ int ib_init_ah_from_path(struct ib_devic
>  		ah_attr->ah_flags = IB_AH_GRH;
>  		ah_attr->grh.dgid = rec->dgid;
>  
> -		ret = ib_find_cached_gid(device, &rec->sgid, &port_num,
> +		ret = ib_find_gid(device, &rec->sgid, &port_num,
>  					 &gid_index);
>  		if (ret)
>  			return ret;
> Index: b/drivers/infiniband/core/verbs.c
> ===================================================================
> --- a/drivers/infiniband/core/verbs.c	2007-05-02 17:47:49.091596637 +0300
> +++ b/drivers/infiniband/core/verbs.c	2007-05-02 17:48:30.750176395 +0300
> @@ -43,7 +43,6 @@
>  #include <linux/string.h>
>  
>  #include <rdma/ib_verbs.h>
> -#include <rdma/ib_cache.h>
>  
>  int ib_rate_to_mult(enum ib_rate rate)
>  {
> @@ -159,7 +158,7 @@ int ib_init_ah_from_wc(struct ib_device 
>  		ah_attr->ah_flags = IB_AH_GRH;
>  		ah_attr->grh.dgid = grh->sgid;
>  
> -		ret = ib_find_cached_gid(device, &grh->dgid, &port_num,
> +		ret = ib_find_gid(device, &grh->dgid, &port_num,
>  					 &gid_index);
>  		if (ret)
>  			return ret;
> Index: b/drivers/infiniband/hw/mthca/mthca_av.c
> ===================================================================
> --- a/drivers/infiniband/hw/mthca/mthca_av.c	2007-05-02 17:47:53.157872352 +0300
> +++ b/drivers/infiniband/hw/mthca/mthca_av.c	2007-05-02 17:48:30.751176217 +0300
> @@ -37,7 +37,6 @@
>  #include <linux/slab.h>
>  
>  #include <rdma/ib_verbs.h>
> -#include <rdma/ib_cache.h>
>  
>  #include "mthca_dev.h"
>  
> @@ -279,7 +278,7 @@ int mthca_read_ah(struct mthca_dev *dev,
>  			(be32_to_cpu(ah->av->sl_tclass_flowlabel) >> 20) & 0xff;
>  		header->grh.flow_label    =
>  			ah->av->sl_tclass_flowlabel & cpu_to_be32(0xfffff);
> -		ib_get_cached_gid(&dev->ib_dev,
> +		ib_query_gid(&dev->ib_dev,
>  				  be32_to_cpu(ah->av->port_pd) >> 24,
>  				  ah->av->gid_index % dev->limits.gid_table_len,
>  				  &header->grh.source_gid);
> Index: b/drivers/infiniband/hw/mthca/mthca_qp.c
> ===================================================================
> --- a/drivers/infiniband/hw/mthca/mthca_qp.c	2007-05-02 17:47:53.153873064 +0300
> +++ b/drivers/infiniband/hw/mthca/mthca_qp.c	2007-05-02 18:04:14.123981858 +0300
> @@ -40,9 +40,8 @@
>  
>  #include <asm/io.h>
>  
> -#include <rdma/ib_verbs.h>
> -#include <rdma/ib_cache.h>
>  #include <rdma/ib_pack.h>
> +#include <rdma/ib_verbs.h>
>  
>  #include "mthca_dev.h"
>  #include "mthca_cmd.h"
> @@ -1485,11 +1484,10 @@ static int build_mlx_header(struct mthca
>  		sqp->ud_header.lrh.source_lid = IB_LID_PERMISSIVE;
>  	sqp->ud_header.bth.solicited_event = !!(wr->send_flags & IB_SEND_SOLICITED);
>  	if (!sqp->qp.ibqp.qp_num)
> -		ib_get_cached_pkey(&dev->ib_dev, sqp->qp.port,
> -				   sqp->pkey_index, &pkey);
> +		ib_query_pkey(&dev->ib_dev, sqp->qp.port, sqp->pkey_index, &pkey);
>  	else
> -		ib_get_cached_pkey(&dev->ib_dev, sqp->qp.port,
> -				   wr->wr.ud.pkey_index, &pkey);
> +		ib_query_pkey(&dev->ib_dev, sqp->qp.port, wr->wr.ud.pkey_index, &pkey);
> +
>  	sqp->ud_header.bth.pkey = cpu_to_be16(pkey);
>  	sqp->ud_header.bth.destination_qpn = cpu_to_be32(wr->wr.ud.remote_qpn);
>  	sqp->ud_header.bth.psn = cpu_to_be32((sqp->send_psn++) & ((1 << 24) - 1));
> Index: b/drivers/infiniband/ulp/ipoib/ipoib_cm.c
> ===================================================================
> --- a/drivers/infiniband/ulp/ipoib/ipoib_cm.c	2007-05-02 17:47:52.042071098 +0300
> +++ b/drivers/infiniband/ulp/ipoib/ipoib_cm.c	2007-05-02 17:48:30.753175861 +0300
> @@ -33,7 +33,6 @@
>   */
>  
>  #include <rdma/ib_cm.h>
> -#include <rdma/ib_cache.h>
>  #include <net/dst.h>
>  #include <net/icmp.h>
>  #include <linux/icmpv6.h>
> @@ -759,7 +758,7 @@ static int ipoib_cm_modify_tx_init(struc
>  	struct ipoib_dev_priv *priv = netdev_priv(dev);
>  	struct ib_qp_attr qp_attr;
>  	int qp_attr_mask, ret;
> -	ret = ib_find_cached_pkey(priv->ca, priv->port, priv->pkey, &qp_attr.pkey_index);
> +	ret = ib_find_pkey(priv->ca, priv->port, priv->pkey, &qp_attr.pkey_index);
>  	if (ret) {
>  		ipoib_warn(priv, "pkey 0x%x not in cache: %d\n", priv->pkey, ret);
>  		return ret;
> Index: b/drivers/infiniband/ulp/ipoib/ipoib_ib.c
> ===================================================================
> --- a/drivers/infiniband/ulp/ipoib/ipoib_ib.c	2007-05-02 17:48:30.150283249 +0300
> +++ b/drivers/infiniband/ulp/ipoib/ipoib_ib.c	2007-05-02 17:48:30.754175683 +0300
> @@ -38,7 +38,7 @@
>  #include <linux/delay.h>
>  #include <linux/dma-mapping.h>
>  
> -#include <rdma/ib_cache.h>
> +#include <rdma/ib_verbs.h>
>  
>  #include "ipoib.h"
>  
> Index: b/drivers/infiniband/ulp/srp/ib_srp.c
> ===================================================================
> --- a/drivers/infiniband/ulp/srp/ib_srp.c	2007-05-02 17:47:52.336018740 +0300
> +++ b/drivers/infiniband/ulp/srp/ib_srp.c	2007-05-02 17:48:30.755175505 +0300
> @@ -48,8 +48,6 @@
>  #include <scsi/scsi_dbg.h>
>  #include <scsi/srp.h>
>  
> -#include <rdma/ib_cache.h>
> -
>  #include "ib_srp.h"
>  
>  #define DRV_NAME	"ib_srp"
> @@ -164,7 +162,7 @@ static int srp_init_qp(struct srp_target
>  	if (!attr)
>  		return -ENOMEM;
>  
> -	ret = ib_find_cached_pkey(target->srp_host->dev->dev,
> +	ret = ib_find_pkey(target->srp_host->dev->dev,
>  				  target->srp_host->port,
>  				  be16_to_cpu(target->path.pkey),
>  				  &attr->pkey_index);
> @@ -1780,7 +1778,7 @@ static ssize_t srp_create_target(struct 
>  	if (ret)
>  		goto err;
>  
> -	ib_get_cached_gid(host->dev->dev, host->port, 0, &target->path.sgid);
> +	ib_query_gid(host->dev->dev, host->port, 0, &target->path.sgid);
>  
>  	printk(KERN_DEBUG PFX "new target: id_ext %016llx ioc_guid %016llx pkey %04x "
>  	       "service_id %016llx dgid %04x:%04x:%04x:%04x:%04x:%04x:%04x:%04x\n",
> Index: b/drivers/infiniband/core/core_priv.h
> ===================================================================
> --- a/drivers/infiniband/core/core_priv.h	2007-05-02 17:47:50.519342327 +0300
> +++ b/drivers/infiniband/core/core_priv.h	2007-05-02 17:48:30.755175505 +0300
> @@ -46,7 +46,4 @@ void ib_device_unregister_sysfs(struct i
>  int  ib_sysfs_setup(void);
>  void ib_sysfs_cleanup(void);
>  
> -int  ib_cache_setup(void);
> -void ib_cache_cleanup(void);
> -
>  #endif /* _CORE_PRIV_H */
> _______________________________________________
> general mailing list
> general at lists.openfabrics.org
> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
> 
> To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

-- 
MST



More information about the general mailing list