Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1488255 > unrolled thread

[PATCH 00/14] GPU-DRM-OMAP: Fine-tuning for several function implementations

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 00/14] GPU-DRM-OMAP: Fine-tuning for several function  implementations SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-21 18:40 +0200
    [PATCH 02/14] GPU-DRM-OMAP: Replace another kmalloc() call by  kmalloc_array() in tiler_map_show() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-21 18:40 +0200
    [PATCH 01/14] GPU-DRM-OMAP: Use kmalloc_array() in tiler_map_show() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-21 18:40 +0200
    [PATCH 05/14] GPU-DRM-OMAP: Improve a size determination in  dmm_txn_append() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-21 18:50 +0200
    [PATCH 04/14] GPU-DRM-OMAP: Delete an unnecessary variable  initialisation in tiler_map_show() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-21 18:50 +0200
    [PATCH 04/14] GPU-DRM-OMAP: Delete an unnecessary variable  initialisation in tiler_map_show() Markus Elfring <Markus.Elfring@web.de> - 2016-09-21 18:50 +0200
    [PATCH 07/14] GPU-DRM-OMAP: Rename a jump label in omap_dmm_probe() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-21 18:50 +0200
    [PATCH 03/14] GPU-DRM-OMAP: Less function calls in tiler_map_show()  after error detection SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-21 18:50 +0200
    [PATCH 06/14] GPU-DRM-OMAP: Improve a size determination in  omap_dmm_probe() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-21 18:50 +0200
    [PATCH 08/14] GPU-DRM-OMAP: Rename a jump label in dmm_txn_commit() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-21 19:00 +0200
    [PATCH 13/14] GPU-DRM-OMAP: Rename a jump label in  omap_gem_new_dmabuf() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-21 19:00 +0200
    [PATCH 12/14] GPU-DRM-OMAP: Move a variable assignment in  omap_gem_attach_pages() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-21 19:00 +0200
    [PATCH 11/14] GPU-DRM-OMAP: Replace a kzalloc() call by kcalloc() in  omap_gem_attach_pages() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-21 19:00 +0200
    [PATCH 09/14] GPU-DRM-OMAP: Delete an unnecessary variable  initialisation in dmm_txn_commit() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-21 19:00 +0200
    [PATCH 14/14] GPU-DRM-OMAP: Rename a jump label in four functions SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-21 19:00 +0200
    [PATCH 10/14] GPU-DRM-OMAP: Use kmalloc_array() in  omap_gem_attach_pages() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-21 19:00 +0200
    Re: [PATCH 00/14] GPU-DRM-OMAP: Fine-tuning for several function  implementations Daniel Vetter <daniel@ffwll.ch> - 2016-09-22 08:50 +0200
      Re: [PATCH 00/14] GPU-DRM-OMAP: Fine-tuning for several function implementations Laurent Pinchart <laurent.pinchart@ideasonboard.com> - 2016-09-22 09:00 +0200
        Re: GPU-DRM-OMAP: Fine-tuning for several function implementations SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-22 11:20 +0200

#1488255 — [PATCH 00/14] GPU-DRM-OMAP: Fine-tuning for several function implementations

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 21 Sep 2016 18:28:38 +0200

Several update suggestions were taken into account
from static source code analysis.

Markus Elfring (14):
  Use kmalloc_array() in tiler_map_show()
  Replace another kmalloc() call by kmalloc_array() in tiler_map_show()
  Less function calls in tiler_map_show() after error detection
  Delete an unnecessary variable initialisation in tiler_map_show()
  Improve a size determination in dmm_txn_append()
  Improve a size determination in omap_dmm_probe()
  Rename a jump label in omap_dmm_probe()
  Rename a jump label in dmm_txn_commit()
  Delete an unnecessary variable initialisation in dmm_txn_commit()
  Use kmalloc_array() in omap_gem_attach_pages()
  Replace a kzalloc() call by kcalloc() in omap_gem_attach_pages()
  Move a variable assignment in omap_gem_attach_pages()
  Rename a jump label in omap_gem_new_dmabuf()
  Rename a jump label in four functions

 drivers/gpu/drm/omapdrm/omap_dmm_tiler.c | 58 ++++++++++++++++----------------
 drivers/gpu/drm/omapdrm/omap_gem.c       | 44 +++++++++++-------------
 2 files changed, 49 insertions(+), 53 deletions(-)

-- 
2.10.0

[toc] | [next] | [standalone]

#1488259 — [PATCH 02/14] GPU-DRM-OMAP: Replace another kmalloc() call by kmalloc_array() in tiler_map_show()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 21 Sep 2016 12:54:07 +0200

A multiplication for the size determination of a memory allocation
indicated that an array data structure should be processed.
Thus use the corresponding function "kmalloc_array" at another place.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/omapdrm/omap_dmm_tiler.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
index 7b32dd3..3a4f91b 100644
--- a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
+++ b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
@@ -918,8 +918,7 @@ int tiler_map_show(struct seq_file *s, void *arg)
 	h_adj = omap_dmm->container_height / ydiv;
 	w_adj = omap_dmm->container_width / xdiv;
 	map = kmalloc_array(h_adj, sizeof(*map), GFP_KERNEL);
-	global_map = kmalloc((w_adj + 1) * h_adj, GFP_KERNEL);
-
+	global_map = kmalloc_array(h_adj, w_adj + 1, GFP_KERNEL);
 	if (!map || !global_map)
 		goto error;
 
-- 
2.10.0

[toc] | [prev] | [next] | [standalone]

#1488261 — [PATCH 01/14] GPU-DRM-OMAP: Use kmalloc_array() in tiler_map_show()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 21 Sep 2016 12:23:46 +0200

A multiplication for the size determination of a memory allocation
indicated that an array data structure should be processed.
Thus use the corresponding function "kmalloc_array".

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/omapdrm/omap_dmm_tiler.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
index 4ceed7a9..7b32dd3 100644
--- a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
+++ b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
@@ -917,8 +917,7 @@ int tiler_map_show(struct seq_file *s, void *arg)
 
 	h_adj = omap_dmm->container_height / ydiv;
 	w_adj = omap_dmm->container_width / xdiv;
-
-	map = kmalloc(h_adj * sizeof(*map), GFP_KERNEL);
+	map = kmalloc_array(h_adj, sizeof(*map), GFP_KERNEL);
 	global_map = kmalloc((w_adj + 1) * h_adj, GFP_KERNEL);
 
 	if (!map || !global_map)
-- 
2.10.0

[toc] | [prev] | [next] | [standalone]

#1488264 — [PATCH 05/14] GPU-DRM-OMAP: Improve a size determination in dmm_txn_append()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 21 Sep 2016 13:53:11 +0200

Replace the specification of a data structure by a pointer dereference
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/omapdrm/omap_dmm_tiler.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
index c262ef5..f110965 100644
--- a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
+++ b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
@@ -224,7 +224,7 @@ static void dmm_txn_append(struct dmm_txn *txn, struct pat_area *area,
 	int rows = (1 + area->y1 - area->y0);
 	int i = columns*rows;
 
-	pat = alloc_dma(txn, sizeof(struct pat), &pat_pa);
+	pat = alloc_dma(txn, sizeof(*pat), &pat_pa);
 
 	if (txn->last_pat)
 		txn->last_pat->next_pa = (uint32_t)pat_pa;
-- 
2.10.0

[toc] | [prev] | [next] | [standalone]

#1488266 — [PATCH 04/14] GPU-DRM-OMAP: Delete an unnecessary variable initialisation in tiler_map_show()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 21 Sep 2016 13:31:45 +0200

The local variable "map" will be set to an appropriate pointer a bit later.
Thus omit the explicit initialisation at the beginning.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/omapdrm/omap_dmm_tiler.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
index 60beeb9..c262ef5 100644
--- a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
+++ b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
@@ -896,7 +896,7 @@ static void map_2d_info(char **map, int xdiv, int ydiv, char *nice,
 int tiler_map_show(struct seq_file *s, void *arg)
 {
 	int xdiv = 2, ydiv = 1;
-	char **map = NULL, *global_map;
+	char **map, *global_map;
 	struct tiler_block *block;
 	struct tcm_area a, p;
 	int i;
-- 
2.10.0

[toc] | [prev] | [next] | [standalone]

#1488268 — [PATCH 04/14] GPU-DRM-OMAP: Delete an unnecessary variable initialisation in tiler_map_show()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 21 Sep 2016 13:31:45 +0200

The local variable "map" will be set to an appropriate pointer a bit later.
Thus omit the explicit initialisation at the beginning.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/omapdrm/omap_dmm_tiler.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
index 60beeb9..c262ef5 100644
--- a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
+++ b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
@@ -896,7 +896,7 @@ static void map_2d_info(char **map, int xdiv, int ydiv, char *nice,
 int tiler_map_show(struct seq_file *s, void *arg)
 {
 	int xdiv = 2, ydiv = 1;
-	char **map = NULL, *global_map;
+	char **map, *global_map;
 	struct tiler_block *block;
 	struct tcm_area a, p;
 	int i;
-- 
2.10.0

[toc] | [prev] | [next] | [standalone]

#1488269 — [PATCH 07/14] GPU-DRM-OMAP: Rename a jump label in omap_dmm_probe()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 21 Sep 2016 17:30:25 +0200

Adjust jump labels according to the current Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/omapdrm/omap_dmm_tiler.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
index c6a7197..5f6f21b 100644
--- a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
+++ b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
@@ -624,7 +624,7 @@ static int omap_dmm_probe(struct platform_device *dev)
 
 	omap_dmm = kzalloc(sizeof(*omap_dmm), GFP_KERNEL);
 	if (!omap_dmm)
-		goto fail;
+		goto check_dmm_removal;
 
 	/* initialize lists */
 	INIT_LIST_HEAD(&omap_dmm->alloc_head);
@@ -648,20 +648,20 @@ static int omap_dmm_probe(struct platform_device *dev)
 	mem = platform_get_resource(dev, IORESOURCE_MEM, 0);
 	if (!mem) {
 		dev_err(&dev->dev, "failed to get base address resource\n");
-		goto fail;
+		goto check_dmm_removal;
 	}
 
 	omap_dmm->base = ioremap(mem->start, SZ_2K);
 
 	if (!omap_dmm->base) {
 		dev_err(&dev->dev, "failed to get dmm base address\n");
-		goto fail;
+		goto check_dmm_removal;
 	}
 
 	omap_dmm->irq = platform_get_irq(dev, 0);
 	if (omap_dmm->irq < 0) {
 		dev_err(&dev->dev, "failed to get IRQ resource\n");
-		goto fail;
+		goto check_dmm_removal;
 	}
 
 	omap_dmm->dev = &dev->dev;
@@ -699,7 +699,7 @@ static int omap_dmm_probe(struct platform_device *dev)
 		dev_err(&dev->dev, "couldn't register IRQ %d, error %d\n",
 			omap_dmm->irq, ret);
 		omap_dmm->irq = -1;
-		goto fail;
+		goto check_dmm_removal;
 	}
 
 	/* Enable all interrupts for each refill engine except
@@ -714,13 +714,13 @@ static int omap_dmm_probe(struct platform_device *dev)
 	if (!omap_dmm->dummy_page) {
 		dev_err(&dev->dev, "could not allocate dummy page\n");
 		ret = -ENOMEM;
-		goto fail;
+		goto check_dmm_removal;
 	}
 
 	/* set dma mask for device */
 	ret = dma_set_coherent_mask(&dev->dev, DMA_BIT_MASK(32));
 	if (ret)
-		goto fail;
+		goto check_dmm_removal;
 
 	omap_dmm->dummy_pa = page_to_phys(omap_dmm->dummy_page);
 
@@ -730,7 +730,7 @@ static int omap_dmm_probe(struct platform_device *dev)
 					   &omap_dmm->refill_pa, GFP_KERNEL);
 	if (!omap_dmm->refill_va) {
 		dev_err(&dev->dev, "could not allocate refill memory\n");
-		goto fail;
+		goto check_dmm_removal;
 	}
 
 	/* alloc engines */
@@ -739,7 +739,7 @@ static int omap_dmm_probe(struct platform_device *dev)
 				    GFP_KERNEL);
 	if (!omap_dmm->engines) {
 		ret = -ENOMEM;
-		goto fail;
+		goto check_dmm_removal;
 	}
 
 	for (i = 0; i < omap_dmm->num_engines; i++) {
@@ -758,7 +758,7 @@ static int omap_dmm_probe(struct platform_device *dev)
 				GFP_KERNEL);
 	if (!omap_dmm->tcm) {
 		ret = -ENOMEM;
-		goto fail;
+		goto check_dmm_removal;
 	}
 
 	/* init containers */
@@ -772,7 +772,7 @@ static int omap_dmm_probe(struct platform_device *dev)
 		if (!omap_dmm->tcm[i]) {
 			dev_err(&dev->dev, "failed to allocate container\n");
 			ret = -ENOMEM;
-			goto fail;
+			goto check_dmm_removal;
 		}
 
 		omap_dmm->tcm[i]->lut_id = i;
@@ -812,8 +812,7 @@ static int omap_dmm_probe(struct platform_device *dev)
 	dev_info(omap_dmm->dev, "initialized all PAT entries\n");
 
 	return 0;
-
-fail:
+ check_dmm_removal:
 	if (omap_dmm_remove(dev))
 		dev_err(&dev->dev, "cleanup failed\n");
 	return ret;
-- 
2.10.0

[toc] | [prev] | [next] | [standalone]

#1488271 — [PATCH 03/14] GPU-DRM-OMAP: Less function calls in tiler_map_show() after error detection

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 21 Sep 2016 13:16:20 +0200

The kfree() function was called in up to two cases
by the tiler_map_show() function during error handling even if
the passed variable contained a null pointer.

* Adjust jump targets according to the Linux coding style convention.

* Split a condition check for memory allocation failures so that
  each pointer from these function calls will be checked immediately.

  See also background information:
  Topic "CWE-754: Improper check for unusual or exceptional conditions"
  Link: https://cwe.mitre.org/data/definitions/754.html

* Return directly after a call of the function "kmalloc_array" failed
  at the beginning.

* Move an assignment for the local variable "w_adj" behind the first
  memory allocation.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/omapdrm/omap_dmm_tiler.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
index 3a4f91b..60beeb9 100644
--- a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
+++ b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
@@ -916,11 +916,14 @@ int tiler_map_show(struct seq_file *s, void *arg)
 	}
 
 	h_adj = omap_dmm->container_height / ydiv;
-	w_adj = omap_dmm->container_width / xdiv;
 	map = kmalloc_array(h_adj, sizeof(*map), GFP_KERNEL);
+	if (!map)
+		return 0;
+
+	w_adj = omap_dmm->container_width / xdiv;
 	global_map = kmalloc_array(h_adj, w_adj + 1, GFP_KERNEL);
-	if (!map || !global_map)
-		goto error;
+	if (!global_map)
+		goto free_map;
 
 	for (lut_idx = 0; lut_idx < omap_dmm->num_lut; lut_idx++) {
 		memset(map, 0, h_adj * sizeof(*map));
@@ -982,10 +985,9 @@ int tiler_map_show(struct seq_file *s, void *arg)
 		}
 	}
 
-error:
-	kfree(map);
 	kfree(global_map);
-
+ free_map:
+	kfree(map);
 	return 0;
 }
 #endif
-- 
2.10.0

[toc] | [prev] | [next] | [standalone]

#1488272 — [PATCH 06/14] GPU-DRM-OMAP: Improve a size determination in omap_dmm_probe()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 21 Sep 2016 17:21:57 +0200

Replace the specification of a data structure by a pointer dereference
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/omapdrm/omap_dmm_tiler.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
index f110965..c6a7197 100644
--- a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
+++ b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
@@ -735,7 +735,8 @@ static int omap_dmm_probe(struct platform_device *dev)
 
 	/* alloc engines */
 	omap_dmm->engines = kcalloc(omap_dmm->num_engines,
-				    sizeof(struct refill_engine), GFP_KERNEL);
+				    sizeof(*omap_dmm->engines),
+				    GFP_KERNEL);
 	if (!omap_dmm->engines) {
 		ret = -ENOMEM;
 		goto fail;
-- 
2.10.0

[toc] | [prev] | [next] | [standalone]

#1488273 — [PATCH 08/14] GPU-DRM-OMAP: Rename a jump label in dmm_txn_commit()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 21 Sep 2016 17:32:42 +0200

Adjust a jump target so that redundant checks can be avoided at the end.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/omapdrm/omap_dmm_tiler.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
index 5f6f21b..c8ced158 100644
--- a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
+++ b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
@@ -269,7 +269,7 @@ static int dmm_txn_commit(struct dmm_txn *txn, bool wait)
 	if (!txn->last_pat) {
 		dev_err(engine->dmm->dev, "need at least one txn\n");
 		ret = -EINVAL;
-		goto cleanup;
+		goto release_engine;
 	}
 
 	txn->last_pat->next_pa = 0;
@@ -281,7 +281,7 @@ static int dmm_txn_commit(struct dmm_txn *txn, bool wait)
 	ret = wait_status(engine, DMM_PATSTATUS_READY);
 	if (ret) {
 		ret = -EFAULT;
-		goto cleanup;
+		goto release_engine;
 	}
 
 	/* mark whether it is async to denote list management in IRQ handler */
@@ -301,9 +301,9 @@ static int dmm_txn_commit(struct dmm_txn *txn, bool wait)
 		}
 	}
 
-cleanup:
 	/* only place engine back on list if we are done with it */
 	if (ret || wait)
+ release_engine:
 		release_engine(engine);
 
 	return ret;
-- 
2.10.0

[toc] | [prev] | [next] | [standalone]

#1488275 — [PATCH 13/14] GPU-DRM-OMAP: Rename a jump label in omap_gem_new_dmabuf()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 21 Sep 2016 17:45:04 +0200

Adjust jump labels according to the current Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/omapdrm/omap_gem.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c
index 3c49ad9..92510de 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem.c
@@ -1442,7 +1442,7 @@ struct drm_gem_object *omap_gem_new_dmabuf(struct drm_device *dev, size_t size,
 	obj = omap_gem_new(dev, gsize, OMAP_BO_MEM_DMABUF | OMAP_BO_WC);
 	if (!obj) {
 		obj = ERR_PTR(-ENOMEM);
-		goto done;
+		goto unlock;
 	}
 
 	omap_obj = to_omap_bo(obj);
@@ -1462,7 +1462,7 @@ struct drm_gem_object *omap_gem_new_dmabuf(struct drm_device *dev, size_t size,
 		if (!pages) {
 			omap_gem_free_object(obj);
 			obj = ERR_PTR(-ENOMEM);
-			goto done;
+			goto unlock;
 		}
 
 		omap_obj->pages = pages;
@@ -1476,11 +1476,10 @@ struct drm_gem_object *omap_gem_new_dmabuf(struct drm_device *dev, size_t size,
 		if (WARN_ON(i != npages)) {
 			omap_gem_free_object(obj);
 			obj = ERR_PTR(-ENOMEM);
-			goto done;
+			goto unlock;
 		}
 	}
-
-done:
+ unlock:
 	mutex_unlock(&dev->struct_mutex);
 	return obj;
 }
-- 
2.10.0

[toc] | [prev] | [next] | [standalone]

#1488277 — [PATCH 12/14] GPU-DRM-OMAP: Move a variable assignment in omap_gem_attach_pages()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 21 Sep 2016 17:42:28 +0200

Move one assignment for the local variable "npages" so that its setting
will only be performed after a call of the function "drm_gem_get_pages"
succeeded by this function.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/omapdrm/omap_gem.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c
index 26f1212..3c49ad9 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem.c
@@ -243,7 +243,7 @@ static int omap_gem_attach_pages(struct drm_gem_object *obj)
 	struct drm_device *dev = obj->dev;
 	struct omap_gem_object *omap_obj = to_omap_bo(obj);
 	struct page **pages;
-	int npages = obj->size >> PAGE_SHIFT;
+	int npages;
 	int i, ret;
 	dma_addr_t *addrs;
 
@@ -255,6 +255,8 @@ static int omap_gem_attach_pages(struct drm_gem_object *obj)
 		return PTR_ERR(pages);
 	}
 
+	npages = obj->size >> PAGE_SHIFT;
+
 	/* for non-cached buffers, ensure the new pages are clean because
 	 * DSS, GPU, etc. are not cache coherent:
 	 */
-- 
2.10.0

[toc] | [prev] | [next] | [standalone]

#1488279 — [PATCH 11/14] GPU-DRM-OMAP: Replace a kzalloc() call by kcalloc() in omap_gem_attach_pages()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 21 Sep 2016 17:40:20 +0200

The script "checkpatch.pl" can point information out like the following.

WARNING: Prefer kcalloc over kzalloc with multiply

Thus fix the affected source code place.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/omapdrm/omap_gem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c
index e4f1924..26f1212 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem.c
@@ -283,7 +283,7 @@ static int omap_gem_attach_pages(struct drm_gem_object *obj)
 			}
 		}
 	} else {
-		addrs = kzalloc(npages * sizeof(*addrs), GFP_KERNEL);
+		addrs = kcalloc(npages, sizeof(*addrs), GFP_KERNEL);
 		if (!addrs) {
 			ret = -ENOMEM;
 			goto free_pages;
-- 
2.10.0

[toc] | [prev] | [next] | [standalone]

#1488281 — [PATCH 09/14] GPU-DRM-OMAP: Delete an unnecessary variable initialisation in dmm_txn_commit()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 21 Sep 2016 17:34:40 +0200

The local variable "ret" will be set to an appropriate value a bit later.
Thus omit the explicit initialisation at the beginning.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/omapdrm/omap_dmm_tiler.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
index c8ced158..c5c3793 100644
--- a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
+++ b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
@@ -262,7 +262,7 @@ static void dmm_txn_append(struct dmm_txn *txn, struct pat_area *area,
  */
 static int dmm_txn_commit(struct dmm_txn *txn, bool wait)
 {
-	int ret = 0;
+	int ret;
 	struct refill_engine *engine = txn->engine_handle;
 	struct dmm *dmm = engine->dmm;
 
-- 
2.10.0

[toc] | [prev] | [next] | [standalone]

#1488283 — [PATCH 14/14] GPU-DRM-OMAP: Rename a jump label in four functions

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 21 Sep 2016 18:00:23 +0200

Adjust jump labels according to the current Linux coding style convention.
Thus replace the identifier "fail" by "unlock" for this refactoring.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/omapdrm/omap_gem.c | 27 +++++++++++----------------
 1 file changed, 11 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c
index 92510de..ea7ad1c 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem.c
@@ -549,7 +549,7 @@ int omap_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 	/* if a shmem backed object, make sure we have pages attached now */
 	ret = get_pages(obj, &pages);
 	if (ret)
-		goto fail;
+		goto unlock;
 
 	/* where should we do corresponding put_pages().. we are mapping
 	 * the original page, rather than thru a GART, so we can't rely
@@ -561,9 +561,7 @@ int omap_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 		ret = fault_2d(obj, vma, vmf);
 	else
 		ret = fault_1d(obj, vma, vmf);
-
-
-fail:
+ unlock:
 	mutex_unlock(&dev->struct_mutex);
 	switch (ret) {
 	case 0:
@@ -682,14 +680,13 @@ int omap_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev,
 	obj = drm_gem_object_lookup(file, handle);
 	if (obj == NULL) {
 		ret = -ENOENT;
-		goto fail;
+		goto unlock;
 	}
 
 	*offset = omap_gem_mmap_offset(obj);
 
 	drm_gem_object_unreference_unlocked(obj);
-
-fail:
+ unlock:
 	return ret;
 }
 
@@ -719,13 +716,12 @@ int omap_gem_roll(struct drm_gem_object *obj, uint32_t roll)
 		struct page **pages;
 		ret = get_pages(obj, &pages);
 		if (ret)
-			goto fail;
+			goto unlock;
 		ret = tiler_pin(omap_obj->block, pages, npages, roll, true);
 		if (ret)
 			dev_err(obj->dev->dev, "could not repin: %d\n", ret);
 	}
-
-fail:
+ unlock:
 	mutex_unlock(&obj->dev->struct_mutex);
 
 	return ret;
@@ -825,7 +821,7 @@ int omap_gem_get_paddr(struct drm_gem_object *obj,
 
 			ret = get_pages(obj, &pages);
 			if (ret)
-				goto fail;
+				goto unlock;
 
 			if (omap_obj->flags & OMAP_BO_TILED) {
 				block = tiler_reserve_2d(fmt,
@@ -839,7 +835,7 @@ int omap_gem_get_paddr(struct drm_gem_object *obj,
 				ret = PTR_ERR(block);
 				dev_err(obj->dev->dev,
 					"could not remap: %d (%d)\n", ret, fmt);
-				goto fail;
+				goto unlock;
 			}
 
 			/* TODO: enable async refill.. */
@@ -849,7 +845,7 @@ int omap_gem_get_paddr(struct drm_gem_object *obj,
 				tiler_release(block);
 				dev_err(obj->dev->dev,
 						"could not pin: %d\n", ret);
-				goto fail;
+				goto unlock;
 			}
 
 			omap_obj->paddr = tiler_ssptr(block);
@@ -865,10 +861,9 @@ int omap_gem_get_paddr(struct drm_gem_object *obj,
 		*paddr = omap_obj->paddr;
 	} else {
 		ret = -EINVAL;
-		goto fail;
+		goto unlock;
 	}
-
-fail:
+ unlock:
 	mutex_unlock(&obj->dev->struct_mutex);
 
 	return ret;
-- 
2.10.0

[toc] | [prev] | [next] | [standalone]

#1488284 — [PATCH 10/14] GPU-DRM-OMAP: Use kmalloc_array() in omap_gem_attach_pages()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 21 Sep 2016 17:37:04 +0200

A multiplication for the size determination of a memory allocation
indicated that an array data structure should be processed.
Thus use the corresponding function "kmalloc_array".

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/omapdrm/omap_gem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c
index 505dee0..e4f1924 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem.c
@@ -259,7 +259,7 @@ static int omap_gem_attach_pages(struct drm_gem_object *obj)
 	 * DSS, GPU, etc. are not cache coherent:
 	 */
 	if (omap_obj->flags & (OMAP_BO_WC|OMAP_BO_UNCACHED)) {
-		addrs = kmalloc(npages * sizeof(*addrs), GFP_KERNEL);
+		addrs = kmalloc_array(npages, sizeof(*addrs), GFP_KERNEL);
 		if (!addrs) {
 			ret = -ENOMEM;
 			goto free_pages;
-- 
2.10.0

[toc] | [prev] | [next] | [standalone]

#1488576

On Wed, Sep 21, 2016 at 06:35:59PM +0200, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 21 Sep 2016 18:28:38 +0200
> 
> Several update suggestions were taken into account
> from static source code analysis.

For the next pile of driver patches _please_ talk with driver maintainers
before starting to create&submit patches. Like I said I won't take them,
and many of your changes are not clear-cut at all, so I expect many driver
maintaines also won't take them. Again, your contributions are welcome,
but blindly following suggestions from code checkers in drivers you cant
test isn't really all that useful. At the scale you're doing it, I think
it's mostly wasting everyone's time :( I'd like to avoid that.

Thanks, Daniel
> 
> Markus Elfring (14):
>   Use kmalloc_array() in tiler_map_show()
>   Replace another kmalloc() call by kmalloc_array() in tiler_map_show()
>   Less function calls in tiler_map_show() after error detection
>   Delete an unnecessary variable initialisation in tiler_map_show()
>   Improve a size determination in dmm_txn_append()
>   Improve a size determination in omap_dmm_probe()
>   Rename a jump label in omap_dmm_probe()
>   Rename a jump label in dmm_txn_commit()
>   Delete an unnecessary variable initialisation in dmm_txn_commit()
>   Use kmalloc_array() in omap_gem_attach_pages()
>   Replace a kzalloc() call by kcalloc() in omap_gem_attach_pages()
>   Move a variable assignment in omap_gem_attach_pages()
>   Rename a jump label in omap_gem_new_dmabuf()
>   Rename a jump label in four functions
> 
>  drivers/gpu/drm/omapdrm/omap_dmm_tiler.c | 58 ++++++++++++++++----------------
>  drivers/gpu/drm/omapdrm/omap_gem.c       | 44 +++++++++++-------------
>  2 files changed, 49 insertions(+), 53 deletions(-)
> 
> -- 
> 2.10.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[toc] | [prev] | [next] | [standalone]

#1488580 — Re: [PATCH 00/14] GPU-DRM-OMAP: Fine-tuning for several function implementations

On Thursday 22 Sep 2016 08:45:01 Daniel Vetter wrote:
> On Wed, Sep 21, 2016 at 06:35:59PM +0200, SF Markus Elfring wrote:
> > From: Markus Elfring <elfring@users.sourceforge.net>
> > Date: Wed, 21 Sep 2016 18:28:38 +0200
> > 
> > Several update suggestions were taken into account
> > from static source code analysis.
> 
> For the next pile of driver patches _please_ talk with driver maintainers
> before starting to create&submit patches. Like I said I won't take them,
> and many of your changes are not clear-cut at all, so I expect many driver
> maintaines also won't take them. Again, your contributions are welcome,
> but blindly following suggestions from code checkers in drivers you cant
> test isn't really all that useful. At the scale you're doing it, I think
> it's mostly wasting everyone's time :( I'd like to avoid that.

I second that. After a very quick review, I see that the series splits related 
changes in multiple patches. I've already commented in reply to another series 
submitted by Markus that patches should then be combined. I will thus ignore 
this series completely for the time being.

> > Markus Elfring (14):
> >   Use kmalloc_array() in tiler_map_show()
> >   Replace another kmalloc() call by kmalloc_array() in tiler_map_show()
> >   Less function calls in tiler_map_show() after error detection
> >   Delete an unnecessary variable initialisation in tiler_map_show()
> >   Improve a size determination in dmm_txn_append()
> >   Improve a size determination in omap_dmm_probe()
> >   Rename a jump label in omap_dmm_probe()
> >   Rename a jump label in dmm_txn_commit()
> >   Delete an unnecessary variable initialisation in dmm_txn_commit()
> >   Use kmalloc_array() in omap_gem_attach_pages()
> >   Replace a kzalloc() call by kcalloc() in omap_gem_attach_pages()
> >   Move a variable assignment in omap_gem_attach_pages()
> >   Rename a jump label in omap_gem_new_dmabuf()
> >   Rename a jump label in four functions
> >  
> >  drivers/gpu/drm/omapdrm/omap_dmm_tiler.c | 58 +++++++++++++--------------
> >  drivers/gpu/drm/omapdrm/omap_gem.c       | 44 +++++++++++-------------
> >  2 files changed, 49 insertions(+), 53 deletions(-)

-- 
Regards,

Laurent Pinchart

[toc] | [prev] | [next] | [standalone]

#1488680 — Re: GPU-DRM-OMAP: Fine-tuning for several function implementations

>> For the next pile of driver patches _please_ talk with driver maintainers
>> before starting to create&submit patches.

Did the software development discussion start a bit here?

Would you like to support an other "talking style" on a conference
like in Berlin next month?


>> Like I said I won't take them,

It's a pity.


>> and many of your changes are not clear-cut at all,

I know that specific update suggestions could be interpreted as controversial.


>> so I expect many driver maintaines also won't take them.

I am curious on useful responses.


>> Again, your contributions are welcome,

Thanks for another bit of constructive feedback.


>> but blindly following suggestions from code checkers in drivers

I propose to dare another look at corresponding information sources.


>> you cant test isn't really all that useful.

I have got an other impression.

How many improvements can still be achieved by usual (advanced) collaboration
techniques for free software development?


>> At the scale you're doing it, I think it's mostly wasting everyone's time

I hope not.


> :( I'd like to avoid that.

I am going to point more update opportunities out also for various Linux software.


> I second that.

Thanks for your opinion on this issue.


> After a very quick review, I see that the series splits related changes
> in multiple patches.

I chose a specific patch granularity for this proposal.


> I've already commented in reply to another series submitted by Markus
> that patches should then be combined.

Will such a combination depend on any more agreements between the involved contributors?


> I will thus ignore this series completely for the time being.

I hope that you can give similar ideas a second chance somehow.

Regards,
Markus

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web