<html xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0cm;
font-size:10.0pt;
font-family:"Calibri",sans-serif;}
span.EmailStyle19
{mso-style-type:personal-reply;
font-family:"Calibri",sans-serif;
color:windowtext;}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;}
@page WordSection1
{size:612.0pt 792.0pt;
margin:72.0pt 72.0pt 72.0pt 72.0pt;}
div.WordSection1
{page:WordSection1;}
--></style>
</head>
<body lang="en-IT" link="#0563C1" vlink="#954F72" style="word-wrap:break-word">
<div class="WordSection1">
<div>
<p class="MsoNormal"><span style="font-size:11.0pt">> From: Thomas Monjalon <thomas@monjalon.net><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> Date: Monday, 22 November 2021 at 12:23<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> To: Elena Agostini <eagostini@nvidia.com><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> Cc: dev@dpdk.org <dev@dpdk.org><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> Subject: Re: [PATCH v3] gpudev: manage NULL pointer<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> External email: Use caution opening links or attachments>
<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> <o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> 22/11/2021 19:24, eagostini@nvidia.com:<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> > From: Elena Agostini <eagostini@nvidia.com><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> ><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> > gpudev free and unregister functions return gracefully if input pointer is NULL>
<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> We could add that the API doc was indicating NULL as a no-op accepted value.>
<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> Another explanation to add: cuda driver checks are removed because redundant<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> with the checks added in gpudev library.>
<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> > Fixes: 818a067baf90 ("gpudev: manage NULL pointer")>
<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> It should be:<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> Fixes: e818c4e2bf50 ("gpudev: add memory API")>
<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> ><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> > Signed-off-by: Elena Agostini <eagostini@nvidia.com><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> > ---<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> > drivers/gpu/cuda/cuda.c | 6 ------<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> > lib/gpudev/gpudev.c | 6 ++++++<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> > 2 files changed, 6 insertions(+), 6 deletions(-)<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> [...]<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> > --- a/lib/gpudev/gpudev.c<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> > +++ b/lib/gpudev/gpudev.c<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> > @@ -569,6 +569,9 @@ rte_gpu_mem_free(int16_t dev_id, void *ptr)<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> > {<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> > struct rte_gpu *dev;<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> ><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> > + if (ptr == NULL)<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> > + return 0;<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> > +<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> > dev = gpu_get_by_id(dev_id);<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> > if (dev == NULL) {<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> > GPU_LOG(ERR, "free mem for invalid device ID %d", dev_id);>
<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> I think we should keep this check first.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">Why should gpudev waste more latency in looking for the device if the ptr is NULL?<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> > @@ -612,6 +615,9 @@ rte_gpu_mem_unregister(int16_t dev_id, void *ptr)<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> > {<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> > struct rte_gpu *dev;<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> ><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> > + if (ptr == NULL)<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> > + return 0;<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> > +<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> > dev = gpu_get_by_id(dev_id);<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> > if (dev == NULL) {<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> > GPU_LOG(ERR, "unregister mem for invalid device ID %d", dev_id);>
<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> Same here.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> There is third function where NULL should be accepted: rte_gpu_mem_register>
<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">Thanks, let me add it<o:p></o:p></span></p>
</div>
</div>
</body>
</html>