> Quoting Yosef Etigin <[EMAIL PROTECTED]>:
> Subject: Re: [PATCHv3 1/2] core: uncached "find gid" and "find pkey" queries
> 
> Michael S. Tsirkin wrote:
> >>Quoting Yosef Etigin <[EMAIL PROTECTED]>:
> >>Subject: Re: [PATCHv3 1/2] core: uncached "find gid" and "find pkey" queries
> >>
> >>Michael S. Tsirkin wrote:
> >>
> >>><plug>
> >>>Have you read the boring list of rules?
> >>>http://git.openfabrics.org/~mst/boring.txt
> >>></plug>
> >>>
> >>Thanks for the pointer.
> > 
> > 
> > This still violates rule 4c in the above (chapter 2 in CodingStyle).
> > 
> Isn't chapter 2 about placing braces?

Yes, I see you've fixed this. Some last pedantic nits:

> core: uncached "find gid" and "find pkey" queries
> 
> * Add ib_find_gid and ib_find_pkey over uncached device queries.
>   The calls might block but the returns are always up-to-date. 
> * Cache pky,gid table lengths in core to avoid port info queries.
> 
> 
> Signed-off-by: Yosef Etigin <[EMAIL PROTECTED]>
> ---
>  drivers/infiniband/core/device.c |  138 
> +++++++++++++++++++++++++++++++++++++++
>  include/rdma/ib_verbs.h          |   25 +++++++
>  2 files changed, 163 insertions(+)
> 
> Index: b/drivers/infiniband/core/device.c
> ===================================================================
> --- a/drivers/infiniband/core/device.c        2007-05-08 15:46:36.000000000 
> +0300
> +++ b/drivers/infiniband/core/device.c        2007-05-09 11:08:29.913598989 
> +0300
> @@ -149,6 +149,18 @@ 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
> @@ -208,6 +220,55 @@ static int add_client_context(struct ib_
>       return 0;
>  }
>  
> +/* read the lengths of pkey,gid tables on each port */
> +static inline int read_port_table_lengths(struct ib_device *device)

This function is too big to be inline.

> +{
> +     struct ib_port_attr *tprops = NULL;
> +     int num_ports, ret = -ENOMEM;
> +     u8 port_index;
> +
> +     tprops = kmalloc(sizeof *tprops, GFP_KERNEL);
> +     if (!tprops)
> +             goto out;
> +
> +     num_ports = end_port(device) - start_port(device) + 1;
> +
> +     device->pkey_tbl_len = kmalloc(sizeof *device->pkey_tbl_len *
> +                                             num_ports, GFP_KERNEL);
> +     if (!device->pkey_tbl_len)
> +             goto out;
> +
> +     device->gid_tbl_len = kmalloc(sizeof *device->gid_tbl_len *
> +                                             num_ports, GFP_KERNEL);
> +     if (!device->gid_tbl_len)
> +             goto err1;
> +
> +     for (port_index = 0; port_index < num_ports; ++port_index) {
> +             ret = ib_query_port(device, port_index + start_port(device),
> +                                     tprops);
> +             if (ret)
> +                     goto err2;
> +             device->pkey_tbl_len[port_index] = tprops->pkey_tbl_len;
> +             device->gid_tbl_len[port_index] = tprops->gid_tbl_len;
> +     }
> +
> +     ret = 0;
> +     goto out;
> +err2:
> +     kfree(device->gid_tbl_len);
> +err1:
> +     kfree(device->pkey_tbl_len);
> +out:
> +     kfree(tprops);
> +     return ret;
> +}
> +
> +static inline void free_port_table_lengths(struct ib_device *device)
> +{
> +     kfree(device->gid_tbl_len);
> +     kfree(device->pkey_tbl_len);
> +}
> +
>  /**
>   * ib_register_device - Register an IB device with IB core
>   * @device:Device to register
> @@ -239,6 +300,13 @@ int ib_register_device(struct ib_device 
>       spin_lock_init(&device->event_handler_lock);
>       spin_lock_init(&device->client_data_lock);
>  
> +     ret = read_port_table_lengths(device);
> +     if (ret) {
> +             printk(KERN_WARNING "Couldn't create table lengths cache for 
> device %s\n",
> +                    device->name);
> +             goto out;
> +     }
> +
>       ret = ib_device_register_sysfs(device);
>       if (ret) {
>               printk(KERN_WARNING "Couldn't register device %s with driver 
> model\n",
> @@ -284,6 +352,8 @@ void ib_unregister_device(struct ib_devi
>  
>       list_del(&device->core_list);
>  
> +     free_port_table_lengths(device);
> +
>       mutex_unlock(&device_mutex);
>  
>       spin_lock_irqsave(&device->client_data_lock, flags);
> @@ -592,6 +662,74 @@ int ib_modify_port(struct ib_device *dev
>  }
>  EXPORT_SYMBOL(ib_modify_port);
>  
> +/**
> + * ib_find_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 GID table where the GID was found.  This
> + *   parameter may be NULL.
> + */
> +int ib_find_gid(struct ib_device *device, union ib_gid *gid,
> +                    u8 *port_num, u16 *index)

Either indent with tabs only here, or use spaces to align continuation at (.

> +{
> +     union ib_gid tmp_gid;
> +     int ret, port, i, tbl_len;
> +
> +     for (port = start_port(device); port <= end_port(device); ++port) {
> +             tbl_len = device->gid_tbl_len[port - start_port(device)];
> +             for (i = 0; i < 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;
> +                     }
> +             }
> +     }
> +     ret = -ENOENT;
> +out:
> +     return ret;
> +}
> +EXPORT_SYMBOL(ib_find_gid);
> +
> +/**
> + * ib_find_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 PKey table where the PKey was found.
> + */
> +int ib_find_pkey(struct ib_device *device,
> +                     u8 port_num, u16 pkey, u16 *index)
> +{
> +     int ret, i, tbl_len;
> +     u16 tmp_pkey;
> +
> +     tbl_len = device->pkey_tbl_len[port_num - start_port(device)];
> +     for (i = 0; i < 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:
> +     return ret;
> +}
> +EXPORT_SYMBOL(ib_find_pkey);
> +
>  static int __init ib_core_init(void)
>  {
>       int ret;
> Index: b/include/rdma/ib_verbs.h
> ===================================================================
> --- a/include/rdma/ib_verbs.h 2007-05-08 15:45:45.000000000 +0300
> +++ b/include/rdma/ib_verbs.h 2007-05-08 18:48:23.000000000 +0300
> @@ -1058,6 +1058,8 @@ struct ib_device {
>       __be64                       node_guid;
>       u8                           node_type;
>       u8                           phys_port_cnt;
> +     int                          *pkey_tbl_len;
> +     int                          *gid_tbl_len;
>  };
>  
>  struct ib_client {
> @@ -1134,6 +1136,29 @@ int ib_modify_port(struct ib_device *dev
>                  struct ib_port_modify *port_modify);
>  
>  /**
> + * ib_find_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 GID table where the GID was found.  This
> + *   parameter may be NULL.
> + */
> +int ib_find_gid(struct ib_device *device, union ib_gid *gid,
> +                    u8 *port_num, u16 *index);

And here, too.

> +
> +/**
> + * ib_find_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 PKey table where the PKey was found.
> + */
> +int ib_find_pkey(struct ib_device *device,
> +                     u8 port_num, u16 pkey, u16 *index);
> +
> +/**
>   * ib_alloc_pd - Allocates an unused protection domain.
>   * @device: The device on which to allocate the protection domain.
>   *

-- 
MST
_______________________________________________
general mailing list
[email protected]
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general

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

Reply via email to