Re: [PATCH v17 3/3] vfio/nvgrace-gpu: Add vfio pci variant module for grace hopper

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



>> >
>> > IMO, this seems like adding too much code to reduce the call length for a
>> > very specific case. If there aren't any strong opinion on this, I'm planning to
>> > leave this code as it is.
>>
>> a slight difference. if mem_count==0 the result should always succeed
>> no matter nvgrace_gpu_map_device_mem() succeeds or not. Of course
>> if it fails it's already a big problem probably nobody cares about the subtle
>> difference when reading non-exist range.
>>
>> but regarding to readability it's still clearer:
>>
>> if (mem_count)
>>       nvgrace_gpu_map_and_read();
>>
>
> The below has better flow imo vs conditionalizing the call to
> map_and_read/write and subsequent error handling, but I don't think
> either adds too much code.  Thanks,
>
> Alex
>
> --- a/drivers/vfio/pci/nvgrace-gpu/main.c
> +++ b/drivers/vfio/pci/nvgrace-gpu/main.c
> @@ -429,6 +429,9 @@ nvgrace_gpu_map_and_read(struct nvgrace_gpu_vfio_pci_core_device *nvdev,
>         u64 offset = *ppos & VFIO_PCI_OFFSET_MASK;
>         int ret;
>
> +       if (!mem_count)
> +               return 0;
> +
>         /*
>         * Handle read on the BAR regions. Map to the target device memory
>          * physical address and copy to the request read buffer.
> @@ -547,6 +550,9 @@ nvgrace_gpu_map_and_write(struct nvgrace_gpu_vfio_pci_core_device *nvdev,
>         loff_t pos = *ppos & VFIO_PCI_OFFSET_MASK;
>         int ret;
>
> +       if (!mem_count)
> +               return 0;
> +
>         ret = nvgrace_gpu_map_device_mem(index, nvdev);
>         if (ret)
>                return ret;

Sure, will update it as mentioned above.




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux