Roland Dreier wrote:
>
> This moves the SDP in my tree to using the struct ib_client method for
> device enumeration.  There are still problems with adding and removing
> devices because the ip2pr module is still using static methods, but I
> think this fixes up SDP.
> 
> Libor, seem OK to commit?

Roland,

  Sorry that I missed this earlier. This looks good. Although, the change
in FMR initialization, once implemented, will result in more virtual 
address space utilization on systems with multiple HCAs...

-Libor


> Index: infiniband/ulp/sdp/sdp_conn.c
> ===================================================================
> --- infiniband/ulp/sdp/sdp_conn.c     (revision 836)
> +++ infiniband/ulp/sdp/sdp_conn.c     (working copy)
> @@ -28,6 +28,16 @@
>  static char _recv_pool_name[] = TS_SDP_SOCK_RECV_DATA_NAME;
>  
>  static struct sdev_root _dev_root_s;
> +
> +static void sdp_device_init_one(struct ib_device *device);
> +static void sdp_device_remove_one(struct ib_device *device);
> +
> +static struct ib_client sdp_client = {
> +     .name   = "sdp",
> +     .add    = sdp_device_init_one,
> +     .remove = sdp_device_remove_one
> +};
> +
>  /* --------------------------------------------------------------------- */
>  /*                                                                       */
>  /* module specific functions                                             */
> @@ -1016,27 +1026,17 @@
>       /*
>        * look up correct HCA and port
>        */
> -     for (hca = _dev_root_s.hca_list; NULL != hca; hca = hca->next) {
> +     hca = ib_get_client_data(device, &sdp_client);
> +     if (!hca)
> +             return -ERANGE;
>  
> -             if (device == hca->ca) {
> -
> -                     for (port = hca->port_list; NULL != port;
> -                          port = port->next) {
> -
> -                             if (hw_port == port->index) {
> -
> -                                     break;
> -                             }
> -                     }
> -
> +     for (port = hca->port_list; NULL != port; port = port->next)
> +             if (hw_port == port->index)
>                       break;
> -             }
> -     }
>  
> -     if (NULL == hca || NULL == port) {
> -
> +     if (!port)
>               return -ERANGE;
> -     }
> +
>       /*
>        * allocate creation parameters
>        */
> @@ -1815,12 +1815,6 @@
>                        off_t start_index,
>                        long *end_index)
>  {
> -     struct sdev_hca_port *port;
> -     struct sdev_hca *hca;
> -     u64 subnet_prefix;
> -     u64 guid;
> -     int hca_count;
> -     int port_count;
>       int offset = 0;
>  
>       TS_CHECK_NULL(buffer, -EINVAL);
> @@ -1857,40 +1851,8 @@
>               offset += sprintf((buffer + offset),
>                                 "max receive buffered:     <%d>\n",
>                                 _dev_root_s.recv_buff_max);
> -
> -             offset += sprintf((buffer + offset), "HCAs:\n");
>       }
> -     /*
> -      * HCA loop
> -      */
> -     for (hca = _dev_root_s.hca_list, hca_count = 0;
> -          NULL != hca; hca = hca->next, hca_count++) {
>  
> -             offset += sprintf((buffer + offset),
> -                               "  hca %02x: ca <%p> pd <%p> mem <%p> "
> -                               "l_key <%08x>\n",
> -                               hca_count, hca->ca, hca->pd, 
> -                               hca->mem_h, hca->l_key);
> -
> -             for (port = hca->port_list, port_count = 0;
> -                  NULL != port; port = port->next, port_count++) {
> -
> -                     subnet_prefix = cpu_to_be64(*(u64 *) (port->gid));
> -                     guid = cpu_to_be64(*(u64 *)(port->gid + sizeof(u64)));
> -
> -                     offset += sprintf((buffer + offset),
> -                                       "    port %02x: index <%d> gid "
> -                                       "<%08x%08x:%08x%08x>\n",
> -                                       port_count,
> -                                       port->index,
> -                                       (u32)((subnet_prefix >> 32) &
> -                                             0xffffffff),
> -                                       (u32)(subnet_prefix & 0xffffffff),
> -                                       (u32)((guid >> 32) & 0xffffffff),
> -                                       (u32)(guid & 0xffffffff));
> -             }
> -     }
> -
>       return offset;
>  } /* sdp_proc_dump_device */
>  
> @@ -1899,232 +1861,200 @@
>  /* initialization/cleanup functions                                      */
>  /*                                                                       */
>  /* --------------------------------------------------------------------- */
> +
>  /* ========================================================================= */
>  /*.._sdp_device_table_init -- create hca list */
> -static s32 _sdp_device_table_init(struct sdev_root *dev_root)
> +static void sdp_device_init_one(struct ib_device *device)
>  {
>  #ifdef _TS_SDP_AIO_SUPPORT
>       struct ib_fmr_pool_param fmr_param_s;
>  #endif
>       struct ib_phys_buf buffer_list;
>       struct ib_device_attr node_info;
> -     struct ib_device *hca_handle;
>       struct sdev_hca_port *port;
>       struct sdev_hca *hca;
> -     s32 result;
> -     s32 hca_count;
> -     s32 port_count;
> -     s32 fmr_size;
> +     int result;
> +     int port_count;
>  
> -     TS_CHECK_NULL(dev_root, -EINVAL);
> +     result = ib_query_device(device, &node_info);
> +     if (0 != result) {
>  
> -     TS_TRACE(MOD_LNX_SDP, T_VERY_VERBOSE, TRACE_FLOW_INOUT,
> -              "INIT: Probing HCA/Port list.");
> +             TS_TRACE(MOD_LNX_SDP, T_VERBOSE, TRACE_FLOW_FATAL,
> +                      "INIT: Error <%d> fetching HCA <%s> type.",
> +                      result, device->name);
> +             return;
> +     }
>       /*
> -      * first count number of HCA's
> +      * allocate per-HCA structure
>        */
> -     hca_count = 0;
> +     hca = kmalloc(sizeof(struct sdev_hca), GFP_KERNEL);
> +     if (NULL == hca) {
>  
> -     while (ib_device_get_by_index(hca_count)) {
> -
> -             hca_count++;
> +             TS_TRACE(MOD_LNX_SDP, T_VERBOSE, TRACE_FLOW_FATAL,
> +                      "INIT: Error allocating HCA <%s> memory.",
> +                      device->name);
> +             return;
>       }
> +     /*
> +      * init and insert into list.
> +      */
> +     memset(hca, 0, sizeof(struct sdev_hca));
>  
> -     fmr_size = TS_SDP_FMR_POOL_SIZE / hca_count;
> +     hca->fmr_pool = NULL;
> +     hca->mem_h    = NULL;
> +     hca->pd       = NULL;
> +     hca->ca       = device;
> +     /*
> +      * protection domain
> +      */
> +     hca->pd = ib_alloc_pd(hca->ca);
> +     if (IS_ERR(hca->pd)) {
>  
> -     for (hca_count = 0;
> -          (hca_handle = ib_device_get_by_index(hca_count)) != NULL;
> -          hca_count++) {
> -             if (!hca_handle || !try_module_get(hca_handle->owner))
> -                     continue;
> +             TS_TRACE(MOD_LNX_SDP, T_TERSE, TRACE_FLOW_FATAL,
> +                      "INIT: Error <%d> creating HCA <%s> protection domain.",
> +                      PTR_ERR(hca->pd), device->name);
> +             goto error;
> +     }
> +     /*
> +      * memory registration
> +      */
> +     buffer_list.addr = 0;
> +     buffer_list.size = (unsigned long)high_memory - PAGE_OFFSET;
>  
> -             result = ib_query_device(hca_handle, &node_info);
> -             if (0 != result) {
> +     hca->iova = 0;
>  
> -                     TS_TRACE(MOD_LNX_SDP, T_VERBOSE, TRACE_FLOW_FATAL,
> -                              "INIT: Error <%d> fetching HCA <%x:%d> type.",
> -                              result, hca_handle, hca_count);
> -                     goto error;
> -             }
> -             /*
> -              * allocate per-HCA structure
> -              */
> -             hca = kmalloc(sizeof(struct sdev_hca), GFP_KERNEL);
> -             if (NULL == hca) {
> +     hca->mem_h = ib_reg_phys_mr(hca->pd, 
> +                                 &buffer_list, 
> +                                 1,  /* list_len */
> +                                 IB_ACCESS_LOCAL_WRITE,
> +                                 &hca->iova);
> +     if (IS_ERR(hca->mem_h)) {
> +             result = PTR_ERR(hca->mem_h);
> +             TS_TRACE(MOD_LNX_SDP, T_TERSE, TRACE_FLOW_FATAL,
> +                      "INIT: Error <%d> registering HCA <%s> memory.",
> +                      result, device->name);
> +             goto error;
> +     }
>  
> -                     TS_TRACE(MOD_LNX_SDP, T_VERBOSE, TRACE_FLOW_FATAL,
> -                              "INIT: Error allocating HCA <%x:%d> memory.",
> -                              hca_handle, hca_count);
> -                     result = -ENOMEM;
> -                     goto error;
> -             }
> -             /*
> -              * init and insert into list.
> -              */
> -             memset(hca, 0, sizeof(struct sdev_hca));
> +     hca->l_key = hca->mem_h->lkey;
> +     hca->r_key = hca->mem_h->rkey;
>  
> -             hca->next = dev_root->hca_list;
> -             dev_root->hca_list = hca;
> +#ifdef _TS_SDP_AIO_SUPPORT
> +     /*
> +      * FMR allocation
> +      */
> +     fmr_param_s.pool_size = TS_SDP_FMR_POOL_SIZE;
> +     fmr_param_s.dirty_watermark = TS_SDP_FMR_DIRTY_SIZE;
> +     fmr_param_s.cache = 1;
> +     fmr_param_s.max_pages_per_fmr = TS_SDP_IOCB_PAGES_MAX;
> +     fmr_param_s.access = (IB_ACCESS_LOCAL_WRITE  |
> +                           IB_ACCESS_REMOTE_WRITE |
> +                           IB_ACCESS_REMOTE_READ);
>  
> -             hca->fmr_pool = NULL;
> -             hca->mem_h    = NULL;
> -             hca->pd       = NULL;
> -             hca->ca = hca_handle;
> -             /*
> -              * protection domain
> -              */
> -             hca->pd = ib_alloc_pd(hca->ca);
> -             if (IS_ERR(hca->pd)) {
> +     fmr_param_s.flush_function = NULL;
> +     /*
> +      * create SDP memory pool
> +      */
> +     result = ib_create_fmr_pool(hca->pd,
> +                                 &fmr_param_s, 
> +                                 &hca->fmr_pool);
> +     if (0 > result) {
>  
> -                     TS_TRACE(MOD_LNX_SDP, T_TERSE, TRACE_FLOW_FATAL,
> -                              "INIT: Error <%d> creating HCA <%x:%d> protection 
> domain.",
> -                              PTR_ERR(hca->pd), hca_handle, hca_count);
> -                     goto error;
> -             }
> -             /*
> -              * memory registration
> -              */
> -             buffer_list.addr = 0;
> -             buffer_list.size = (unsigned long)high_memory - PAGE_OFFSET;
> +             TS_TRACE(MOD_LNX_SDP, T_TERSE, TRACE_FLOW_FATAL,
> +                      "INIT: Error <%d> creating HCA <%s> fast memory pool.",
> +                      result, hca->ca);
> +             goto error;
> +     }
> +#endif /* _TS_SDP_AIO_SUPPORT */
> +     /*
> +      * port allocation
> +      */
> +     for (port_count = 0; port_count < node_info.phys_port_cnt; port_count++) {
>  
> -             hca->iova = 0;
> +             port = kmalloc(sizeof(struct sdev_hca_port), 
> +                            GFP_KERNEL);
> +             if (NULL == port) {
>  
> -             hca->mem_h = ib_reg_phys_mr(hca->pd, 
> -                                         &buffer_list, 
> -                                         1,  /* list_len */
> -                                         IB_ACCESS_LOCAL_WRITE,
> -                                         &hca->iova);
> -             if (IS_ERR(hca->mem_h)) {
> -                     result = PTR_ERR(hca->mem_h);
> -                     TS_TRACE(MOD_LNX_SDP, T_TERSE, TRACE_FLOW_FATAL,
> -                              "INIT: Error <%d> registering HCA <%x:%d> memory.",
> -                              result, hca_handle, hca_count);
> +                     TS_TRACE(MOD_LNX_SDP, T_VERBOSE,
> +                              TRACE_FLOW_FATAL,
> +                              "INIT: Error allocating HCA <%s> port <%d:%d> 
> memory.",
> +                              device->name, port_count,
> +                              node_info.phys_port_cnt);
> +
>                       goto error;
>               }
>  
> -             hca->l_key = hca->mem_h->lkey;
> -             hca->r_key = hca->mem_h->rkey;
> +             memset(port, 0, sizeof(struct sdev_hca_port));
>  
> -#ifdef _TS_SDP_AIO_SUPPORT
> -             /*
> -              * FMR allocation
> -              */
> -             fmr_param_s.pool_size = fmr_size;
> -             fmr_param_s.dirty_watermark = TS_SDP_FMR_DIRTY_SIZE;
> -             fmr_param_s.cache = 1;
> -             fmr_param_s.max_pages_per_fmr = TS_SDP_IOCB_PAGES_MAX;
> -             fmr_param_s.access = (IB_ACCESS_LOCAL_WRITE  |
> -                                   IB_ACCESS_REMOTE_WRITE |
> -                                   IB_ACCESS_REMOTE_READ);
> +             port->index    = port_count + 1;
> +             port->next     = hca->port_list;
> +             hca->port_list = port;
>  
> -             fmr_param_s.flush_function = NULL;
> -             /*
> -              * create SDP memory pool
> -              */
> -             result = ib_create_fmr_pool(hca->pd,
> -                                         &fmr_param_s, 
> -                                         &hca->fmr_pool);
> -             if (0 > result) {
> +             result = ib_query_gid(hca->ca, 
> +                                   port->index, 
> +                                   0,        /* index */
> +                                   (union ib_gid *) port->gid);
> +             if (0 != result) {
>  
> -                     TS_TRACE(MOD_LNX_SDP, T_TERSE, TRACE_FLOW_FATAL,
> -                              "INIT: Error <%d> creating HCA <%d:%d> fast memory 
> pool.",
> -                              result, hca->ca, hca_count);
> +                     TS_TRACE(MOD_LNX_SDP, T_VERBOSE,
> +                              TRACE_FLOW_FATAL,
> +                              "INIT: Error <%d> getting GID for HCA <%s> port 
> <%d:%d>",
> +                              result, hca->ca,
> +                              port->index, node_info.phys_port_cnt);
>                       goto error;
>               }
> -#endif /* _TS_SDP_AIO_SUPPORT */
> -             /*
> -              * port allocation
> -              */
> -             for (port_count = 0; port_count < node_info.phys_port_cnt;
> -                  port_count++) {
> +     }
>  
> -                     port = kmalloc(sizeof(struct sdev_hca_port), 
> -                                    GFP_KERNEL);
> -                     if (NULL == port) {
> +     ib_set_client_data(device, &sdp_client, hca);
>  
> -                             TS_TRACE(MOD_LNX_SDP, T_VERBOSE,
> -                                      TRACE_FLOW_FATAL,
> -                                      "INIT: Error allocating HCA <%d:%d> port 
> <%x:%d> memory.",
> -                                      hca_handle, hca_count, port_count,
> -                                      node_info.phys_port_cnt);
> +     return;
>  
> -                             result = -ENOMEM;
> -                             goto error;
> -                     }
> +error:
> +     for (port = hca->port_list; NULL != port; port = hca->port_list) {
> +             hca->port_list = port->next;
> +             port->next = NULL;
>  
> -                     memset(port, 0, sizeof(struct sdev_hca_port));
> +             kfree(port);
> +     }
>  
> -                     port->index    = port_count + 1;
> -                     port->next     = hca->port_list;
> -                     hca->port_list = port;
> +     if (hca->fmr_pool)
> +             (void)ib_destroy_fmr_pool(hca->fmr_pool);
>  
> -                     result = ib_query_gid(hca->ca, 
> -                                           port->index, 
> -                                           0,        /* index */
> -                                           (union ib_gid *) port->gid);
> -                     if (0 != result) {
> +     if (hca->mem_h)
> +             (void)ib_dereg_mr(hca->mem_h);
>  
> -                             TS_TRACE(MOD_LNX_SDP, T_VERBOSE,
> -                                      TRACE_FLOW_FATAL,
> -                                      "INIT: Error <%d> getting GID for HCA <%d:%d> 
> port <%d:%d>",
> -                                      result, hca->ca, hca_count,
> -                                      port->index, node_info.phys_port_cnt);
> -                             goto error;
> -                     }
> -             }
> -     }
> +     if (hca->pd)
> +             (void)ib_dealloc_pd(hca->pd);
>  
> -     return 0;
> -error:
> -     return result;
> +     kfree(hca);
>  } /* _sdp_device_table_init */
>  
>  /* ========================================================================= */
>  /*.._sdp_device_table_cleanup -- delete hca list */
> -static s32 _sdp_device_table_cleanup(struct sdev_root *dev_root)
> +static void sdp_device_remove_one(struct ib_device *device)
>  {
>       struct sdev_hca_port *port;
>       struct sdev_hca *hca;
>  
> -     TS_CHECK_NULL(dev_root, -EINVAL);
> -     /*
> -      * free all hca/ports
> -      */
> -     for (hca = dev_root->hca_list; NULL != hca; hca = dev_root->hca_list) {
> +     hca = ib_get_client_data(device, &sdp_client);
>  
> -             dev_root->hca_list = hca->next;
> -             hca->next = NULL;
> +     for (port = hca->port_list; NULL != port; port = hca->port_list) {
> +             hca->port_list = port->next;
> +             port->next = NULL;
>  
> -             for (port = hca->port_list; NULL != port; port = hca->port_list) {
> +             kfree(port);
> +     }
>  
> -                     hca->port_list = port->next;
> -                     port->next = NULL;
> +     if (hca->fmr_pool)
> +             (void)ib_destroy_fmr_pool(hca->fmr_pool);
>  
> -                     kfree(port);
> -             }
> +     if (hca->mem_h)
> +             (void)ib_dereg_mr(hca->mem_h);
>  
> -             if (NULL != hca->fmr_pool) {
> +     if (hca->pd)
> +             (void)ib_dealloc_pd(hca->pd);
>  
> -                     (void)ib_destroy_fmr_pool(hca->fmr_pool);
> -             }
> -
> -             if (hca->mem_h) {
> -
> -                     (void)ib_dereg_mr(hca->mem_h);
> -             }
> -
> -             if (hca->pd) {
> -
> -                     (void)ib_dealloc_pd(hca->pd);
> -             }
> -
> -             if (hca->ca)
> -                     module_put(hca->ca->owner);
> -
> -             kfree(hca);
> -     }
> -
> -     return 0;
> +     kfree(hca);
>  } /* _sdp_device_table_cleanup */
>  
>  /* ========================================================================= */
> @@ -2170,11 +2100,11 @@
>       /*
>        * Get HCA/port list
>        */
> -     result = _sdp_device_table_init(&_dev_root_s);
> +     result = ib_register_client(&sdp_client);
>       if (0 > result) {
>  
>               TS_TRACE(MOD_LNX_SDP, T_TERSE, TRACE_FLOW_FATAL,
> -                      "INIT: Error <%d> building HCA/port list.", result);
> +                      "INIT: Error <%d> registering SDP client.", result);
>               goto error_hca;
>       }
>       /*
> @@ -2281,8 +2211,8 @@
>       free_pages((unsigned long)_dev_root_s.sk_array, _dev_root_s.sk_ordr);
>  error_array:
>  error_size:
> +     ib_unregister_client(&sdp_client);
>  error_hca:
> -     (void)_sdp_device_table_cleanup(&_dev_root_s);
>       return result;
>  } /* sdp_conn_table_init */
>  
> @@ -2302,7 +2232,7 @@
>       /*
>        * delete list of HCAs/PORTs
>        */
> -     (void)_sdp_device_table_cleanup(&_dev_root_s);
> +     ib_unregister_client(&sdp_client);
>       /*
>        * drop socket table
>        */
> Index: infiniband/ulp/sdp/sdp_dev.h
> ===================================================================
> --- infiniband/ulp/sdp/sdp_dev.h      (revision 803)
> +++ infiniband/ulp/sdp/sdp_dev.h      (working copy)
> @@ -167,12 +167,8 @@
>       int send_buff_max;
>       int send_usig_max;
>       /*
> -      * devices. list of installed HCA's and some associated parameters
> -      */
> -     struct sdev_hca *hca_list;
> -     /*
>        * connections. The table is a simple linked list, since it does not
> -      * need to require fast lookup capabilities.
> +      * need fast lookup capabilities.
>        */
>       u32 sk_size;  /* socket array size */
>       u32 sk_ordr;  /* order size of region. */
_______________________________________________
openib-general mailing list
[EMAIL PROTECTED]
http://openib.org/mailman/listinfo/openib-general

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

Reply via email to