* [bug report] drm: remove usage of drm_pci_alloc/free
@ 2021-05-14 13:54 Dan Carpenter
2021-05-14 15:13 ` Joseph Kogut
2021-05-14 20:48 ` [PATCH 1/1] drm: drm_legacy_addbufs_pci(): fix return without cleanup Joseph Kogut
0 siblings, 2 replies; 5+ messages in thread
From: Dan Carpenter @ 2021-05-14 13:54 UTC (permalink / raw)
To: joseph.kogut; +Cc: dri-devel
Hello Joseph Kogut,
The patch 70556e24e18e: "drm: remove usage of drm_pci_alloc/free"
from Apr 22, 2021, leads to the following static checker warning:
drivers/gpu/drm/drm_bufs.c:1090 drm_legacy_addbufs_pci()
warn: inconsistent returns '&dev->struct_mutex'.
Locked on : 988
Unlocked on: 938,944,951,959,973,1005,1042,1060,1090
drivers/gpu/drm/drm_bufs.c
965 temp_pagelist = kmalloc_array(dma->page_count + (count << page_order),
966 sizeof(*dma->pagelist),
967 GFP_KERNEL);
968 if (!temp_pagelist) {
969 kfree(entry->buflist);
970 kfree(entry->seglist);
971 mutex_unlock(&dev->struct_mutex);
972 atomic_dec(&dev->buf_alloc);
973 return -ENOMEM;
There is a bunch of clean up happenning on the existing returns.
974 }
975 memcpy(temp_pagelist,
976 dma->pagelist, dma->page_count * sizeof(*dma->pagelist));
977 DRM_DEBUG("pagelist: %d entries\n",
978 dma->page_count + (count << page_order));
979
980 entry->buf_size = size;
981 entry->page_order = page_order;
982 byte_count = 0;
983 page_count = 0;
984
985 while (entry->buf_count < count) {
986 dmah = kmalloc(sizeof(drm_dma_handle_t), GFP_KERNEL);
987 if (!dmah)
988 return -ENOMEM;
This new return has no clean up. We a mutex_unlock(&dev->struct_mutex).
989
990 dmah->size = total;
991 dmah->vaddr = dma_alloc_coherent(dev->dev,
992 dmah->size,
993 &dmah->busaddr,
994 GFP_KERNEL);
995 if (!dmah->vaddr) {
996 kfree(dmah);
997
998 /* Set count correctly so we free the proper amount. */
999 entry->buf_count = count;
1000 entry->seg_count = count;
1001 drm_cleanup_buf_error(dev, entry);
1002 kfree(temp_pagelist);
1003 mutex_unlock(&dev->struct_mutex);
1004 atomic_dec(&dev->buf_alloc);
1005 return -ENOMEM;
1006 }
1007 entry->seglist[entry->seg_count++] = dmah;
1008 for (i = 0; i < (1 << page_order); i++) {
1009 DRM_DEBUG("page %d @ 0x%08lx\n",
regards,
dan carpenter
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [bug report] drm: remove usage of drm_pci_alloc/free
2021-05-14 13:54 [bug report] drm: remove usage of drm_pci_alloc/free Dan Carpenter
@ 2021-05-14 15:13 ` Joseph Kogut
2021-05-14 16:34 ` Joseph Kogut
2021-05-14 20:48 ` [PATCH 1/1] drm: drm_legacy_addbufs_pci(): fix return without cleanup Joseph Kogut
1 sibling, 1 reply; 5+ messages in thread
From: Joseph Kogut @ 2021-05-14 15:13 UTC (permalink / raw)
To: Dan Carpenter; +Cc: dri-devel
Hi Dan,
On Fri, May 14, 2021 at 6:54 AM Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> Hello Joseph Kogut,
>
> The patch 70556e24e18e: "drm: remove usage of drm_pci_alloc/free"
> from Apr 22, 2021, leads to the following static checker warning:
>
> drivers/gpu/drm/drm_bufs.c:1090 drm_legacy_addbufs_pci()
> warn: inconsistent returns '&dev->struct_mutex'.
> Locked on : 988
> Unlocked on: 938,944,951,959,973,1005,1042,1060,1090
>
> drivers/gpu/drm/drm_bufs.c
> 965 temp_pagelist = kmalloc_array(dma->page_count + (count << page_order),
> 966 sizeof(*dma->pagelist),
> 967 GFP_KERNEL);
> 968 if (!temp_pagelist) {
> 969 kfree(entry->buflist);
> 970 kfree(entry->seglist);
> 971 mutex_unlock(&dev->struct_mutex);
> 972 atomic_dec(&dev->buf_alloc);
> 973 return -ENOMEM;
>
> There is a bunch of clean up happenning on the existing returns.
>
> 974 }
> 975 memcpy(temp_pagelist,
> 976 dma->pagelist, dma->page_count * sizeof(*dma->pagelist));
> 977 DRM_DEBUG("pagelist: %d entries\n",
> 978 dma->page_count + (count << page_order));
> 979
> 980 entry->buf_size = size;
> 981 entry->page_order = page_order;
> 982 byte_count = 0;
> 983 page_count = 0;
> 984
> 985 while (entry->buf_count < count) {
> 986 dmah = kmalloc(sizeof(drm_dma_handle_t), GFP_KERNEL);
> 987 if (!dmah)
> 988 return -ENOMEM;
>
> This new return has no clean up. We a mutex_unlock(&dev->struct_mutex).
>
> 989
> 990 dmah->size = total;
> 991 dmah->vaddr = dma_alloc_coherent(dev->dev,
> 992 dmah->size,
> 993 &dmah->busaddr,
> 994 GFP_KERNEL);
> 995 if (!dmah->vaddr) {
> 996 kfree(dmah);
> 997
> 998 /* Set count correctly so we free the proper amount. */
> 999 entry->buf_count = count;
> 1000 entry->seg_count = count;
> 1001 drm_cleanup_buf_error(dev, entry);
> 1002 kfree(temp_pagelist);
> 1003 mutex_unlock(&dev->struct_mutex);
> 1004 atomic_dec(&dev->buf_alloc);
> 1005 return -ENOMEM;
> 1006 }
> 1007 entry->seglist[entry->seg_count++] = dmah;
> 1008 for (i = 0; i < (1 << page_order); i++) {
> 1009 DRM_DEBUG("page %d @ 0x%08lx\n",
>
> regards,
> dan carpenter
Thanks for the report, I'll follow up with a patch to fix this.
Best,
Joseph
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [bug report] drm: remove usage of drm_pci_alloc/free
2021-05-14 15:13 ` Joseph Kogut
@ 2021-05-14 16:34 ` Joseph Kogut
2021-05-15 6:57 ` Dan Carpenter
0 siblings, 1 reply; 5+ messages in thread
From: Joseph Kogut @ 2021-05-14 16:34 UTC (permalink / raw)
To: Dan Carpenter; +Cc: dri-devel
On Fri, May 14, 2021 at 8:13 AM Joseph Kogut <joseph.kogut@gmail.com> wrote:
>
> Hi Dan,
>
> On Fri, May 14, 2021 at 6:54 AM Dan Carpenter <dan.carpenter@oracle.com> wrote:
> >
> > Hello Joseph Kogut,
> >
> > The patch 70556e24e18e: "drm: remove usage of drm_pci_alloc/free"
> > from Apr 22, 2021, leads to the following static checker warning:
> >
> > drivers/gpu/drm/drm_bufs.c:1090 drm_legacy_addbufs_pci()
> > warn: inconsistent returns '&dev->struct_mutex'.
> > Locked on : 988
> > Unlocked on: 938,944,951,959,973,1005,1042,1060,1090
> >
Do you mind if I copy this portion of the bug report in the commit message?
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/1] drm: drm_legacy_addbufs_pci(): fix return without cleanup
2021-05-14 13:54 [bug report] drm: remove usage of drm_pci_alloc/free Dan Carpenter
2021-05-14 15:13 ` Joseph Kogut
@ 2021-05-14 20:48 ` Joseph Kogut
1 sibling, 0 replies; 5+ messages in thread
From: Joseph Kogut @ 2021-05-14 20:48 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Joseph Kogut, dri-devel, Dan Carpenter
The patch 70556e24e18e: "drm: remove usage of drm_pci_alloc/free" leads
to the following static checker warning:
drivers/gpu/drm/drm_bufs.c:1090 drm_legacy_addbufs_pci()
warn: inconsistent returns '&dev->struct_mutex'.
Locked on : 988
Unlocked on: 938,944,951,959,973,1005,1042,1060,1090
Fix the return without cleanup by removing the early return and taking
the original return path on allocation failure, including the intended
unlocks.
Signed-off-by: Joseph Kogut <joseph.kogut@gmail.com>
---
drivers/gpu/drm/drm_bufs.c | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c
index 94bc1f6049c9..ea3ca81be9dd 100644
--- a/drivers/gpu/drm/drm_bufs.c
+++ b/drivers/gpu/drm/drm_bufs.c
@@ -984,17 +984,18 @@ int drm_legacy_addbufs_pci(struct drm_device *dev,
while (entry->buf_count < count) {
dmah = kmalloc(sizeof(drm_dma_handle_t), GFP_KERNEL);
- if (!dmah)
- return -ENOMEM;
-
- dmah->size = total;
- dmah->vaddr = dma_alloc_coherent(dev->dev,
- dmah->size,
- &dmah->busaddr,
- GFP_KERNEL);
- if (!dmah->vaddr) {
- kfree(dmah);
-
+ if (dmah) {
+ dmah->size = total;
+ dmah->vaddr = dma_alloc_coherent(dev->dev,
+ dmah->size,
+ &dmah->busaddr,
+ GFP_KERNEL);
+ if (!dmah->vaddr) {
+ kfree(dmah);
+ dmah = NULL;
+ }
+ }
+ if (!dmah) {
/* Set count correctly so we free the proper amount. */
entry->buf_count = count;
entry->seg_count = count;
--
2.31.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [bug report] drm: remove usage of drm_pci_alloc/free
2021-05-14 16:34 ` Joseph Kogut
@ 2021-05-15 6:57 ` Dan Carpenter
0 siblings, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2021-05-15 6:57 UTC (permalink / raw)
To: Joseph Kogut; +Cc: dri-devel
On Fri, May 14, 2021 at 09:34:27AM -0700, Joseph Kogut wrote:
> On Fri, May 14, 2021 at 8:13 AM Joseph Kogut <joseph.kogut@gmail.com> wrote:
> >
> > Hi Dan,
> >
> > On Fri, May 14, 2021 at 6:54 AM Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > >
> > > Hello Joseph Kogut,
> > >
> > > The patch 70556e24e18e: "drm: remove usage of drm_pci_alloc/free"
> > > from Apr 22, 2021, leads to the following static checker warning:
> > >
> > > drivers/gpu/drm/drm_bufs.c:1090 drm_legacy_addbufs_pci()
> > > warn: inconsistent returns '&dev->struct_mutex'.
> > > Locked on : 988
> > > Unlocked on: 938,944,951,959,973,1005,1042,1060,1090
> > >
>
> Do you mind if I copy this portion of the bug report in the commit message?
Yeah, that's normal. No need to ask.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-05-15 7:18 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-14 13:54 [bug report] drm: remove usage of drm_pci_alloc/free Dan Carpenter
2021-05-14 15:13 ` Joseph Kogut
2021-05-14 16:34 ` Joseph Kogut
2021-05-15 6:57 ` Dan Carpenter
2021-05-14 20:48 ` [PATCH 1/1] drm: drm_legacy_addbufs_pci(): fix return without cleanup Joseph Kogut
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.