[dpdk-dev] [PATCH v12 4/7] bus/pci: implement sigbus handler ops
Jeff Guo
jia.guo at intel.com
Thu Oct 4 05:58:15 CEST 2018
On 10/2/2018 10:39 PM, Burakov, Anatoly wrote:
> On 02-Oct-18 1:35 PM, Jeff Guo wrote:
>> This patch implements the ops for the PCI bus sigbus handler. It
>> finds the
>> PCI device that is being hot-unplugged and calls the relevant ops of the
>> hot-unplug handler to handle the hot-unplug failure of the device.
>>
>> Signed-off-by: Jeff Guo <jia.guo at intel.com>
>> Acked-by: Shaopeng He <shaopeng.he at intel.com>
>> ---
>> v12->v11:
>> no change.
>> ---
>> drivers/bus/pci/pci_common.c | 53
>> ++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 53 insertions(+)
>>
>> diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c
>> index d286234..f313fe9 100644
>> --- a/drivers/bus/pci/pci_common.c
>> +++ b/drivers/bus/pci/pci_common.c
>> @@ -405,6 +405,36 @@ pci_find_device(const struct rte_device *start,
>> rte_dev_cmp_t cmp,
>> return NULL;
>> }
>> +/**
>> + * find the device which encounter the failure, by iterate over all
>> device on
>> + * PCI bus to check if the memory failure address is located in the
>> range
>> + * of the BARs of the device.
>> + */
>> +static struct rte_pci_device *
>> +pci_find_device_by_addr(const void *failure_addr)
>> +{
>> + struct rte_pci_device *pdev = NULL;
>> + int i;
>> +
>> + FOREACH_DEVICE_ON_PCIBUS(pdev) {
>> + for (i = 0; i != RTE_DIM(pdev->mem_resource); i++) {
>> + if ((uint64_t)(uintptr_t)failure_addr >=
>> + (uint64_t)(uintptr_t)pdev->mem_resource[i].addr &&
>> + (uint64_t)(uintptr_t)failure_addr <
>> + (uint64_t)(uintptr_t)pdev->mem_resource[i].addr +
>> + pdev->mem_resource[i].len) {
>
> You must *really* dislike local variables :) Suggested rewriting:
>
> const void *start, *end;
> size_t len;
>
> start = pdev->mem_resource[i].addr;
> len = pdev->mem_resource[i].len;
> end = RTE_PTR_ADD(start, len);
>
> if (failure_addr >= start && failure_addr < end) {
> ...
> }
>
> I think this is way clearer.
>
good point, local variable might be good and helpful. Thanks.
>> + RTE_LOG(INFO, EAL, "Failure address "
>> + "%16.16"PRIx64" belongs to "
>> + "device %s!\n",
>> + (uint64_t)(uintptr_t)failure_addr,
>> + pdev->device.name);
>
> I feel like this should have DEBUG level, rather than INFO.
> Alternatively, if you really feel like this should be at level INFO,
> the message should be reworded because the word "failure" might give
> the wrong impression :)
>
> (but really, i think this is info useful for debugging purposes but
> not interesting generally, so it should be under DEBUG IMO)
>
ok, i accept it.
More information about the dev
mailing list