[ofw] improve the time of handling events like port state change

[Leonid Keller]

 see inline

> -----Original Message-----
> From: Sean Hefty [mailto:sean.hefty at intel.com] 
> Sent: Wednesday, June 11, 2008 7:35 PM
> To: Leonid Keller; ofw at lists.openfabrics.org
> Subject: RE: [ofw] improve the time of handling events like 
> port state change
> 
> - for (i = 0; i < pkey_cache->table_len; ++i) {
> -  ret = ib_query_pkey(device, port, (u16)i, pkey_cache->table + i);
> + for (i = 0; i < pkey_cache->table_len; i+=32) {
> +  __be16 pkey_chunk[32];
> +  int size;
> +  ret = ib_query_pkey_chunk(device, port, (u16)i, pkey_chunk);
>    if (ret) {
> -   printk(KERN_WARNING "ib_query_pkey failed (%d) for %s 
> (index %d)\n",
> +   printk(KERN_WARNING "ib_query_pkey_chunk failed (%d) for 
> %s (index 
> +%d)\n",
>            ret, device->name, i);
>     goto err;
>    }
> +  size = min(32, pkey_cache->table_len - i);
> +  RtlCopyMemory(pkey_cache->table + i, pkey_chunk, size*sizeof(u16));
> 
> 
> Why not read directly into pkey_cache->table?

I will, thank you.

> 
> 
> 
> - for (i = 0; i < gid_cache->table_len; ++i) {
> -  ret = ib_query_gid(device, port, i, gid_cache->table + i);
> + for (i = 0; i < gid_cache->table_len; i+=8) {
> +  union ib_gid gid_chunk[8];
> +  int size;
> +  ret = ib_query_gid_chunk(device, port, i, gid_chunk);
>    if (ret) {
> -   printk(KERN_WARNING "ib_query_gid failed (%d) for %s 
> (index %d)\n",
> +   printk(KERN_WARNING "ib_query_gid_chunk failed (%d) for %s (index 
> +%d)\n",
>            ret, device->name, i);
>     goto err;
>    }
> +  size = min(8, gid_cache->table_len - i);
> +  RtlCopyMemory(gid_cache->table + i, gid_chunk, size*sizeof(union 
> +ib_gid));
> 
> 
> Same - why not read directly into gid_cache->table?
> 
> 
> 
>   write_lock_irq(&device->cache.lock);
> Index: hw/mlx4/kernel/bus/core/device.c
> ===================================================================
> --- hw/mlx4/kernel/bus/core/device.c (revision 1256)
> +++ hw/mlx4/kernel/bus/core/device.c (working copy)
> @@ -71,8 +71,8 @@
>   } mandatory_table[] = {
>    IB_MANDATORY_FUNC(query_device),
>    IB_MANDATORY_FUNC(query_port),
> -  IB_MANDATORY_FUNC(query_pkey),
> -  IB_MANDATORY_FUNC(query_gid),
> +  IB_MANDATORY_FUNC(query_pkey_chunk),
> +  IB_MANDATORY_FUNC(query_gid_chunk),
> 
> 
> Is this mandatory_table needed?  This is all internal to the 
> driver.  We should work on cleaning up things from the Linux port. 
> 
We'll do it once. For now we want to keep the similarity.
> 
> 
>  /**
> - * ib_query_gid - Get GID table entry
> + * ib_query_gid_chunk - Get a chunk of GID table entries
>   * @device:Device to query
>   * @port_num:Port number to query
>   * @index:GID table index to query
> - * @gid:Returned GID
> + * @gid:Returned GIDs chunk
>   *
> - * ib_query_gid() fetches the specified GID table entry.
> + * ib_query_gid_chunk() fetches the specified GID table enties chunk.
>   */
> -int ib_query_gid(struct ib_device *device,
> -   u8 port_num, int index, union ib_gid *gid)
> +int ib_query_gid_chunk(struct ib_device *device,
> +   u8 port_num, int index, union ib_gid gid[8])
>  {
> - return device->query_gid(device, port_num, index, gid);
> + return device->query_gid_chunk(device, port_num, index, gid);
>  }
> -EXPORT_SYMBOL(ib_query_gid);
> +EXPORT_SYMBOL(ib_query_gid_chunk);
>  
> 
> Likewise - just put the implementation here.  There's only 
> one device.  The use of function pointers throughout the mlx 
> and mthca drivers make it more difficult to understand and 
> modify, without any benefit.  If we're having to touch this 
> part of the code anyway, it'd be nice to simplify things at 
> least around those changes.
> 
> - Sean
> 
> 
> 


--
Leonid Keller
Mellanox Technologies LTD.
SW- Windows
Phone: +972 (4) 909 7200 (ext 372)
Mobile: +972 (54) 464 7702
E-mail: leonid at mellanox.co.il

----------------------------------------------------------------------
Emails belong on computers, trees belong in forests; if you must print
this, do it on recycled paper.
http://www.greenpeace.org/international/
----------------------------------------------------------------------


Disclaimer added by CodeTwo Exchange Rules
http://www.codetwo.com