[V4,1/9] bus: introduce hotplug failure handler

  When a hardware device is removed physically or the software disables
it, the hotplug occur. App need to call ether dev API to detach the device,
to unplug the device at the bus level and make access to the device
invalid. But the removal of the device from the software lists is not going
to be instantaneous, at this time if the data path still read/write the
device, it will cause MMIO error and result of the app crash out. So a
hotplug failure handle mechanism need to be used to guaranty app will not
crash out when hot unplug device.

To handle device hot plug failure is a bus-specific behavior, this patch
introduces a bus ops so that each kind of bus can implement its own logic.

Signed-off-by: Jeff Guo <jia.guo@intel.com>
---
v4->v3:
split patches to be small and clear.
---
 lib/librte_eal/common/include/rte_bus.h | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
  

29/06/2018 12:30, Jeff Guo:
>  /**
> + * Implementation a specific hot plug handler, which is responsible
> + * for handle the failure when hot remove the device, guaranty the system
> + * would not crash in the case.
> + * @param dev
> + *	Pointer of the device structure.
> + *
> + * @return
> + *	0 on success.
> + *	!0 on error.
> + */
> +typedef int (*rte_bus_hotplug_handler_t)(struct rte_device *dev);

[...]
> @@ -211,6 +224,8 @@ struct rte_bus {
>  	rte_bus_parse_t parse;       /**< Parse a device name */
>  	struct rte_bus_conf conf;    /**< Bus configuration */
>  	rte_bus_get_iommu_class_t get_iommu_class; /**< Get iommu class */
> +	rte_bus_hotplug_handler_t hotplug_handler;
> +						/**< handle hot plug on bus */

The name is misleading.
It is to handle unplugging but is called "hotplug".

In order to demonstrate how the handler is used, you should
introduce the code using this handler in the same patch.
  

On 7/4/2018 6:21 AM, Thomas Monjalon wrote:
> 29/06/2018 12:30, Jeff Guo:
>>   /**
>> + * Implementation a specific hot plug handler, which is responsible
>> + * for handle the failure when hot remove the device, guaranty the system
>> + * would not crash in the case.
>> + * @param dev
>> + *	Pointer of the device structure.
>> + *
>> + * @return
>> + *	0 on success.
>> + *	!0 on error.
>> + */
>> +typedef int (*rte_bus_hotplug_handler_t)(struct rte_device *dev);
> [...]
>> @@ -211,6 +224,8 @@ struct rte_bus {
>>   	rte_bus_parse_t parse;       /**< Parse a device name */
>>   	struct rte_bus_conf conf;    /**< Bus configuration */
>>   	rte_bus_get_iommu_class_t get_iommu_class; /**< Get iommu class */
>> +	rte_bus_hotplug_handler_t hotplug_handler;
>> +						/**< handle hot plug on bus */
> The name is misleading.
> It is to handle unplugging but is called "hotplug".

ok, so i prefer hotplug_failure_handler than hot_unplug_handler, since 
it is more explicit for failure handle, and more clearly.

> In order to demonstrate how the handler is used, you should
> introduce the code using this handler in the same patch.
>

sorry, i check the history of rte_bus.h, and the way is introduce ops at 
first, second implement in specific bus, then come across the usage.
I think that way clear and make sense. what do you think?
Anyway, i will check the commit log if is there any misleading.
  

04/07/2018 09:16, Guo, Jia:
> 
> On 7/4/2018 6:21 AM, Thomas Monjalon wrote:
> > 29/06/2018 12:30, Jeff Guo:
> >>   /**
> >> + * Implementation a specific hot plug handler, which is responsible
> >> + * for handle the failure when hot remove the device, guaranty the system
> >> + * would not crash in the case.
> >> + * @param dev
> >> + *	Pointer of the device structure.
> >> + *
> >> + * @return
> >> + *	0 on success.
> >> + *	!0 on error.
> >> + */
> >> +typedef int (*rte_bus_hotplug_handler_t)(struct rte_device *dev);
> > [...]
> >> @@ -211,6 +224,8 @@ struct rte_bus {
> >>   	rte_bus_parse_t parse;       /**< Parse a device name */
> >>   	struct rte_bus_conf conf;    /**< Bus configuration */
> >>   	rte_bus_get_iommu_class_t get_iommu_class; /**< Get iommu class */
> >> +	rte_bus_hotplug_handler_t hotplug_handler;
> >> +						/**< handle hot plug on bus */
> > The name is misleading.
> > It is to handle unplugging but is called "hotplug".
> 
> ok, so i prefer hotplug_failure_handler than hot_unplug_handler, since 
> it is more explicit for failure handle, and more clearly.
> 
> > In order to demonstrate how the handler is used, you should
> > introduce the code using this handler in the same patch.
> >
> 
> sorry, i check the history of rte_bus.h, and the way is introduce ops at 
> first, second implement in specific bus, then come across the usage.
> I think that way clear and make sense. what do you think?
> Anyway, i will check the commit log if is there any misleading.

I think it is better to call ops when they are introduced,
and implement the ops in second step.
  

On 7/4/2018 3:55 PM, Thomas Monjalon wrote:
> 04/07/2018 09:16, Guo, Jia:
>> On 7/4/2018 6:21 AM, Thomas Monjalon wrote:
>>> 29/06/2018 12:30, Jeff Guo:
>>>>    /**
>>>> + * Implementation a specific hot plug handler, which is responsible
>>>> + * for handle the failure when hot remove the device, guaranty the system
>>>> + * would not crash in the case.
>>>> + * @param dev
>>>> + *	Pointer of the device structure.
>>>> + *
>>>> + * @return
>>>> + *	0 on success.
>>>> + *	!0 on error.
>>>> + */
>>>> +typedef int (*rte_bus_hotplug_handler_t)(struct rte_device *dev);
>>> [...]
>>>> @@ -211,6 +224,8 @@ struct rte_bus {
>>>>    	rte_bus_parse_t parse;       /**< Parse a device name */
>>>>    	struct rte_bus_conf conf;    /**< Bus configuration */
>>>>    	rte_bus_get_iommu_class_t get_iommu_class; /**< Get iommu class */
>>>> +	rte_bus_hotplug_handler_t hotplug_handler;
>>>> +						/**< handle hot plug on bus */
>>> The name is misleading.
>>> It is to handle unplugging but is called "hotplug".
>> ok, so i prefer hotplug_failure_handler than hot_unplug_handler, since
>> it is more explicit for failure handle, and more clearly.
>>
>>> In order to demonstrate how the handler is used, you should
>>> introduce the code using this handler in the same patch.
>>>
>> sorry, i check the history of rte_bus.h, and the way is introduce ops at
>> first, second implement in specific bus, then come across the usage.
>> I think that way clear and make sense. what do you think?
>> Anyway, i will check the commit log if is there any misleading.
> I think it is better to call ops when they are introduced,
> and implement the ops in second step.
>

Hi, Thomas

sorry but i want to detail the relationship of the ops and api as bellow 
to try if we can get the better sequence.

Patch num:

1: introduce ops hotplug_failure_handler

2: implement ops hotplug_failure_handler

3:introduce ops sigbus_handler.

4:implement ops sigbus_handler

5: introduce helper rte_bus_sigbus_handler to call the ops sigbus_handler

6: introduce the mechanism to call helper rte_bus_sigbus_handler and 
call hotplug_failure_handler.

If per you said , could I modify the sequence like 6->5->3->4->1->2? I 
don't think it will make sense, and might be more confused.

And I think should be better that introduce each ops just say item, then 
when introduce the caller patch, the functional is ready to use by the 
patch.


if i did not got your point and you have other better sequence about 
that please explicit to let me know. Thanks.
  

05/07/2018 08:23, Guo, Jia:
> 
> On 7/4/2018 3:55 PM, Thomas Monjalon wrote:
> > 04/07/2018 09:16, Guo, Jia:
> >> On 7/4/2018 6:21 AM, Thomas Monjalon wrote:
> >>> 29/06/2018 12:30, Jeff Guo:
> >>>>    /**
> >>>> + * Implementation a specific hot plug handler, which is responsible
> >>>> + * for handle the failure when hot remove the device, guaranty the system
> >>>> + * would not crash in the case.
> >>>> + * @param dev
> >>>> + *	Pointer of the device structure.
> >>>> + *
> >>>> + * @return
> >>>> + *	0 on success.
> >>>> + *	!0 on error.
> >>>> + */
> >>>> +typedef int (*rte_bus_hotplug_handler_t)(struct rte_device *dev);
> >>> [...]
> >>>> @@ -211,6 +224,8 @@ struct rte_bus {
> >>>>    	rte_bus_parse_t parse;       /**< Parse a device name */
> >>>>    	struct rte_bus_conf conf;    /**< Bus configuration */
> >>>>    	rte_bus_get_iommu_class_t get_iommu_class; /**< Get iommu class */
> >>>> +	rte_bus_hotplug_handler_t hotplug_handler;
> >>>> +						/**< handle hot plug on bus */
> >>> The name is misleading.
> >>> It is to handle unplugging but is called "hotplug".
> >> ok, so i prefer hotplug_failure_handler than hot_unplug_handler, since
> >> it is more explicit for failure handle, and more clearly.
> >>
> >>> In order to demonstrate how the handler is used, you should
> >>> introduce the code using this handler in the same patch.
> >>>
> >> sorry, i check the history of rte_bus.h, and the way is introduce ops at
> >> first, second implement in specific bus, then come across the usage.
> >> I think that way clear and make sense. what do you think?
> >> Anyway, i will check the commit log if is there any misleading.
> > I think it is better to call ops when they are introduced,
> > and implement the ops in second step.
> >
> 
> Hi, Thomas
> 
> sorry but i want to detail the relationship of the ops and api as bellow 
> to try if we can get the better sequence.
> 
> Patch num:
> 
> 1: introduce ops hotplug_failure_handler
> 
> 2: implement ops hotplug_failure_handler
> 
> 3:introduce ops sigbus_handler.
> 
> 4:implement ops sigbus_handler
> 
> 5: introduce helper rte_bus_sigbus_handler to call the ops sigbus_handler
> 
> 6: introduce the mechanism to call helper rte_bus_sigbus_handler and 
> call hotplug_failure_handler.
> 
> If per you said , could I modify the sequence like 6->5->3->4->1->2? I 
> don't think it will make sense, and might be more confused.
> 
> And I think should be better that introduce each ops just say item, then 
> when introduce the caller patch, the functional is ready to use by the 
> patch.
> 
> 
> if i did not got your point and you have other better sequence about 
> that please explicit to let me know. Thanks.

The main concern is to be able to understand each patch separately.

When introducing a new op, we need to understand how it will be used.
But actually, no need to change patch organization,
you just need to provide a clear doxygen documentation,
and introduce the context in the commit log.