staging: bcm2835-camera: remove get_msg_context() and release_msg_context()

PATCH In review Aishwarya Pant 3 participants 9 messages Mar 8, 2017 – Mar 9, 2017

Subsystems (3)

Staging Broadcom BCM2711/BCM2835 Arm architecture Broadcom BCM281XX/BCM11XXX/BCM216XX Arm architecture

lore

Share & tools

lore

Aishwarya Pant

author

2017-03-08 04:52 UTC #0 lore raw

cover staging: bcm2835-camera: remove get_msg_context() and release_msg_context()

This patch replaces in mmal-vchiq.c:
- get_msg_context(..) with a kzalloc at the caller
- release_msg_context(..) with a kfree at the caller

Aishwarya Pant (2):
staging: bcm2835-camera: replace get_msg_context() with kzalloc
staging: bcm2835-camera: replace release_msg_context() with kfree

 .../vc04_services/bcm2835-camera/mmal-vchiq.c      | 29 ++++------------------
 1 file changed, 5 insertions(+), 24 deletions(-)

Signature · 1 line

2.7.4

Aishwarya Pant

author

2017-03-08 04:52 UTC #1 lore raw

PATCH 1/2 staging: bcm2835-camera: replace get_msg_context() with kzalloc

This patch replaces function get_msg_context(..) with a kzalloc at the caller. This is safe as the only usage found in mmal-vchiq.c has been refactored.

Signed-off-by Aishwarya Pant <[email protected]>

---
 .../staging/vc04_services/bcm2835-camera/mmal-vchiq.c    | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c +2 ~~-14~~

index ca6e9eb..1650bbf 100644
@@ -163,18 +163,6 @@ struct vchiq_mmal_instance {
 	struct vchiq_mmal_component component[VCHIQ_MMAL_MAX_COMPONENTS];
 };
 
-static struct mmal_msg_context *get_msg_context(struct vchiq_mmal_instance
-						*instance)
-{
-	struct mmal_msg_context *msg_context;
-
-	/* todo: should this be allocated from a pool to avoid kmalloc */
-	msg_context = kmalloc(sizeof(*msg_context), GFP_KERNEL);
-	memset(msg_context, 0, sizeof(*msg_context));
-
-	return msg_context;
-}
-
 static void release_msg_context(struct mmal_msg_context *msg_context)
 {
 	kfree(msg_context);

@@ -395,8 +383,8 @@ buffer_from_host(struct vchiq_mmal_instance *instance,
 	if (mutex_lock_interruptible(&instance->bulk_mutex))
 		return -EINTR;
 
-	/* get context */
-	msg_context = get_msg_context(instance);
+	/* todo: should this be allocated from a pool to avoid kzalloc */
+	msg_context = kzalloc(sizeof(*msg_context), GFP_KERNEL);
 	if (!msg_context) {
 		ret = -ENOMEM;
 		goto unlock;

Signature · 1 line

2.7.4

Aishwarya Pant

author

2017-03-08 04:53 UTC #4 lore raw

PATCH 2/2 staging: bcm2835-camera: replace release_msg_context() with kfree

In reply to Aishwarya Pant

This patch replaces function release_msg_context with a kfree(msg_context).

Removed the comment /* todo: is this correct error value? */ since ret = vchi_queue_kernel_message(...) returns either VCHIQ_ERROR (-1) or VCHIQ_SUCCESS (0), so the check if (ret != 0) before kfree is correct.

---
 drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c +3 ~~-10~~

index 1650bbf..4aea9da 100644
@@ -163,11 +163,6 @@ struct vchiq_mmal_instance {
 	struct vchiq_mmal_component component[VCHIQ_MMAL_MAX_COMPONENTS];
 };
 
-static void release_msg_context(struct mmal_msg_context *msg_context)
-{
-	kfree(msg_context);
-}
-
 /* deals with receipt of event to host message */
 static void event_to_host_cb(struct vchiq_mmal_instance *instance,
 			     struct mmal_msg *msg, u32 msg_len)

@@ -199,7 +194,7 @@ static void buffer_work_cb(struct work_struct *work)
 					    msg_context->u.bulk.pts);
 
 	/* release message context */
-	release_msg_context(msg_context);
+	kfree(msg_context);
 }
 
 /* enqueue a bulk receive for a given message context */

@@ -437,10 +432,8 @@ buffer_from_host(struct vchiq_mmal_instance *instance,
 					sizeof(struct mmal_msg_header) +
 					sizeof(m.u.buffer_from_host));
 
-	if (ret != 0) {
-		release_msg_context(msg_context);
-		/* todo: is this correct error value? */
-	}
+	if (ret != 0)
+		kfree(msg_context);
 
 	vchi_service_release(instance->handle);

Signature · 1 line

2.7.4

Julia Lawall

2017-03-08 07:00 UTC #5 lore raw

Re: [Outreachy kernel] [PATCH 2/2] staging: bcm2835-camera: replace release_msg_context() with kfree

Quoted 6 lines · Aishwarya Pant

This patch replaces function release_msg_context with a kfree(msg_context).
Removed the comment /* todo: is this correct error value? */ since ret = vchi_queue_kernel_message(...) returns either VCHIQ_ERROR (-1) or VCHIQ_SUCCESS (0), so the check if (ret != 0) before kfree is correct.

It could be nice in the future to use standard kernel error values.

julia

Quoted 52 lines

---
 drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c +3 ~~-11~~

index 1650bbf..4aea9da 100644
@@ -163,11 +163,6 @@ struct vchiq_mmal_instance {
 	struct vchiq_mmal_component component[VCHIQ_MMAL_MAX_COMPONENTS];
 };
 
-static void release_msg_context(struct mmal_msg_context *msg_context)
-{
-	kfree(msg_context);
-}
-
 /* deals with receipt of event to host message */
 static void event_to_host_cb(struct vchiq_mmal_instance *instance,
 			     struct mmal_msg *msg, u32 msg_len)

@@ -199,7 +194,7 @@ static void buffer_work_cb(struct work_struct *work)
 					    msg_context->u.bulk.pts);
 
 	/* release message context */
-	release_msg_context(msg_context);
+	kfree(msg_context);
 }
 
 /* enqueue a bulk receive for a given message context */

@@ -437,10 +432,8 @@ buffer_from_host(struct vchiq_mmal_instance *instance,
 					sizeof(struct mmal_msg_header) +
 					sizeof(m.u.buffer_from_host));
 
-	if (ret != 0) {
-		release_msg_context(msg_context);
-		/* todo: is this correct error value? */
-	}
+	if (ret != 0)
+		kfree(msg_context);
 
 	vchi_service_release(instance->handle);
 
--

2.7.4

--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/9013e0c8c0f960b553a4ebd53ded11b22dfda259.1488948214.git.aishpant%40gmail.com.
For more options, visit https://groups.google.com/d/optout.

Julia Lawall

2017-03-08 07:41 UTC #6 lore raw

In reply to Aishwarya Pant

Quoted 7 lines · Aishwarya Pant

This patch replaces in mmal-vchiq.c:
- get_msg_context(..) with a kzalloc at the caller
- release_msg_context(..) with a kfree at the caller
Aishwarya Pant (2):
staging: bcm2835-camera: replace get_msg_context() with kzalloc
staging: bcm2835-camera: replace release_msg_context() with kfree

Haven't you sent these before?

julia

Quoted 14 lines

 .../vc04_services/bcm2835-camera/mmal-vchiq.c      | 29 ++++------------------
 1 file changed, 5 insertions(+), 24 deletions(-)
--
2.7.4
--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/cover.1488948214.git.aishpant%40gmail.com.
For more options, visit https://groups.google.com/d/optout.

Aishwarya Pant

author

2017-03-08 11:47 UTC #7 lore raw

Quoted 8 lines · Julia Lawall

Quoted 4 lines · Aishwarya Pant
This patch replaces in mmal-vchiq.c:
- get_msg_context(..) with a kzalloc at the caller
- release_msg_context(..) with a kfree at the caller

Quoted 6 lines

Quoted 3 lines
Aishwarya Pant (2):
staging: bcm2835-camera: replace get_msg_context() with kzalloc
staging: bcm2835-camera: replace release_msg_context() with kfree
Haven't you sent these before?

I had sent patch 1/2 before as a two part patchset. Greg asked me to re-send the patchset as a single patch. Later I noticed the function release_msg_context() could also be removed and added it here.

Quoted 17 lines

julia
Quoted 14 lines
 .../vc04_services/bcm2835-camera/mmal-vchiq.c      | 29 ++++------------------
 1 file changed, 5 insertions(+), 24 deletions(-)
--
2.7.4
--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/cover.1488948214.git.aishpant%40gmail.com.
For more options, visit https://groups.google.com/d/optout.

Julia Lawall

2017-03-08 12:27 UTC #8 lore raw

Quoted 14 lines · Aishwarya Pant

Quoted 13 lines · Julia Lawall
Quoted 7 lines · Aishwarya Pant
This patch replaces in mmal-vchiq.c:
- get_msg_context(..) with a kzalloc at the caller
- release_msg_context(..) with a kfree at the caller
Aishwarya Pant (2):
staging: bcm2835-camera: replace get_msg_context() with kzalloc
staging: bcm2835-camera: replace release_msg_context() with kfree

Quoted 5 lines

Quoted 1 line
Haven't you sent these before?
I had sent patch 1/2 before as a two part patchset. Greg asked me to re-send the patchset as a single patch. Later I noticed the function release_msg_context() could also be removed and added it here.

OK, I guess if it is all reorganized then v2 is not appropriate.

julia

Quoted 20 lines

Quoted 19 lines
julia
Quoted 14 lines
 .../vc04_services/bcm2835-camera/mmal-vchiq.c      | 29 ++++------------------
 1 file changed, 5 insertions(+), 24 deletions(-)
--
2.7.4
--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/cover.1488948214.git.aishpant%40gmail.com.
For more options, visit https://groups.google.com/d/optout.

Greg KH

2017-03-09 13:36 UTC #2 lore raw

Re: [Outreachy kernel] [PATCH 1/2] staging: bcm2835-camera: replace get_msg_context() with kzalloc

In reply to Aishwarya Pant

Quoted 8 lines · Aishwarya Pant

This patch replaces function get_msg_context(..) with a kzalloc at the caller. This is safe as the only usage found in mmal-vchiq.c has been refactored.
Signed-off-by Aishwarya Pant <[email protected]>
---
 .../staging/vc04_services/bcm2835-camera/mmal-vchiq.c    | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

This, and the 2/2 patch do not apply at all to my tree :(

Please fix up and resend.

thanks,

greg k-h

Aishwarya Pant

author

2017-03-09 14:21 UTC #3 lore raw

Re: [Outreachy kernel] [PATCH 1/2] staging: bcm2835-camera: replace get_msg_context() with kzalloc

Quoted 5 lines · Greg KH

Quoted 4 lines · Aishwarya Pant
This patch replaces function get_msg_context(..) with a kzalloc at the caller. This is safe as the only usage found in mmal-vchiq.c has been refactored.

Quoted 8 lines

Quoted 4 lines
Signed-off-by Aishwarya Pant <[email protected]>
---
 .../staging/vc04_services/bcm2835-camera/mmal-vchiq.c    | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)
This, and the 2/2 patch do not apply at all to my tree :(
Please fix up and resend.

Hey

I checked this file. The definitions of get_msg_context(..) and release_msg_context(..) have changed completely, they do more work than a simple alloc and de-alloc. So these patches don't make sense anymore.

There is still scope to replace kmalloc + memset with a kzalloc, so I'll do that and send a separate patch.

Quoted 4 lines

thanks,
greg k-h