<div dir="auto"><div>Dear Dan,</div><div dir="auto"><br></div><div dir="auto">Your mail flashes back to my memory 5 years ago.</div><div dir="auto">Back then, I had cleaned up the exynos driver.</div><div dir="auto"><br></div><div dir="auto">And the replacement IS_ERR was one of items.</div><div dir="auto"><br></div><div dir="auto">IMHO it is still better to modify those two functions, drm_gem_cma_prime_get_sg_table and <span style="font-family:sans-serif">xen_drm_front_gem_get_sg_table.</span></div><div dir="auto"><font face="sans-serif"><br></font></div><div dir="auto"><font face="sans-serif">Thank you.</font></div><div dir="auto"><font face="sans-serif">Best regards YJ</font></div><div dir="auto"><font face="sans-serif"><br></font><br><div class="gmail_quote" dir="auto"><div dir="ltr">On Thu, 14 Jun 2018, 23:42 Dan Carpenter, <<a href="mailto:dan.carpenter@oracle.com">dan.carpenter@oracle.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hello YoungJun Cho,<br>
<br>
The patch 7e3d88f9cce3: "drm/prime: replace NULL with error value in<br>
drm_prime_pages_to_sg" from Jun 24, 2013, leads to the following<br>
static checker warning:<br>
<br>
drivers/gpu/drm/drm_prime.c:317 drm_gem_map_dma_buf()<br>
warn: 'sgt' can also be NULL<br>
<br>
drivers/gpu/drm/drm_prime.c<br>
307 /*<br>
308 * two mappings with different directions for the same attachment are<br>
309 * not allowed<br>
310 */<br>
311 if (WARN_ON(prime_attach->dir != DMA_NONE))<br>
312 return ERR_PTR(-EBUSY);<br>
313 <br>
314 sgt = obj->dev->driver->gem_prime_get_sg_table(obj);<br>
<br>
The problematic functions here are drm_gem_cma_prime_get_sg_table() and<br>
xen_drm_front_gem_get_sg_table(). My preference would be to update<br>
those functions to return error pointers, but I'm not familiar with the<br>
code or able to test it.<br><br>
But this static checker test seems pretty good so I'm likely to publish<br>
it soon and then this sort of bug will normally be caught quickly.<br>
<br>
315 <br>
316 if (!IS_ERR(sgt)) {<br>
317 if (!dma_map_sg_attrs(attach->dev, sgt->sgl, sgt->nents, dir,<br>
318 DMA_ATTR_SKIP_CPU_SYNC)) {<br>
319 sg_free_table(sgt);<br>
320 kfree(sgt);<br>
321 sgt = ERR_PTR(-ENOMEM);<br>
322 } else {<br>
323 prime_attach->sgt = sgt;<br>
324 prime_attach->dir = dir;<br>
325 }<br>
326 }<br>
327 <br>
328 return sgt;<br>
<br>
regards,<br>
dan carpenter<br>
_______________________________________________<br>
dri-devel mailing list<br>
<a href="mailto:dri-devel@lists.freedesktop.org" target="_blank" rel="noreferrer">dri-devel@lists.freedesktop.org</a><br>
<a href="https://lists.freedesktop.org/mailman/listinfo/dri-devel" rel="noreferrer noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/dri-devel</a><br>
</blockquote></div></div></div>