Re: [PATCH rdma-core 03/10] verbs: Split init_device into a match and alloc/bind step

Yishai Hadas
2017-09-27T13:49:19Z

On 9/27/2017 10:55 AM, Leon Romanovsky wrote:
> On Tue, Sep 26, 2017 at 07:00:16PM +0300, Yishai Hadas wrote:
>> On 9/20/2017 12:18 AM, Jason Gunthorpe wrote:
>>> The match step determines if the provider should bind to the sysfs device,
>>> and the allocation step creates the verbs_device and cleanly matches the
>>> uninit_device step which frees it.
>>>
>>> This split makes it simpler to factor out all the duplicated code in the
>>> match step.
>>>
>>> Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
>>> ---
>>> libibverbs/driver.h | 21 ++++++++++++++-
>>> libibverbs/init.c | 74 ++++++++++++++++++++++++++++++++++-------------------
>>> 2 files changed, 68 insertions(+), 27 deletions(-)
>>>
>>> diff --git a/libibverbs/driver.h b/libibverbs/driver.h
>>> index f9f5cfa2308c2f..397441f49a0e5a 100644
>>> --- a/libibverbs/driver.h
>>> +++ b/libibverbs/driver.h
>>> @@ -40,6 +40,7 @@
>>> #include <infiniband/kern-abi.h>
>>> #include <ccan/list.h>
>>> #include <config.h>
>>> +#include <stdbool.h>
>>> #ifdef __cplusplus
>>> # define BEGIN_C_DECLS extern "C" {
>>> @@ -95,10 +96,27 @@ struct verbs_qp {
>>> struct verbs_xrcd *xrcd;
>>> };
>>> +/* A rdma device detected in sysfs */
>>> +struct verbs_sysfs_dev {
>>> + struct list_node entry;
>>> + void *provider_data;
>>
>> It looks as this field isn't used in the series, correct ? if this is the
>> case better add when it'll become applicable.
>
> Yishai,
>
> Jason is a little bit overwhelmed now and because this series was posted
> long time here and got a lot of Acked-by tags, I would be happy to take it
> and move forward. Do you see any issues which he can't fix after this
> series merged?
>

The above is some cleanup note that can be done later if we want to
merge the series soon.

Other verbs changes in the series looks fine to me.
Reviewed-by: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>