[PATCH 0/2] staging: bcm2835-camera: remove get_msg_context() and release_msg_context()

[PATCH 0/2] staging: bcm2835-camera: remove get_msg_context() and release_msg_context()

[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

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

-- 
2.7.4

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

[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 <aishpant@gmail.com>
---
 .../staging/vc04_services/bcm2835-camera/mmal-vchiq.c    | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
index ca6e9eb..1650bbf 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
@@ -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;
-- 
2.7.4

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

[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(-)

diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
index 1650bbf..4aea9da 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
@@ -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

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

[Julia Lawall]

On Wed, 8 Mar 2017, Aishwarya Pant wrote:

> 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

> ---
>  drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c | 13 +++----------
>  1 file changed, 3 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
> index 1650bbf..4aea9da 100644
> --- a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
> +++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
> @@ -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 outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> 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.
>

Re: [Outreachy kernel] [PATCH 0/2] staging: bcm2835-camera: remove get_msg_context() and release_msg_context()

[Julia Lawall]

On Wed, 8 Mar 2017, Aishwarya Pant wrote:

> 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

>
>  .../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 outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> 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.
>

Re: [Outreachy kernel] [PATCH 0/2] staging: bcm2835-camera: remove get_msg_context() and release_msg_context()

[Aishwarya Pant]

On Wed, Mar 08, 2017 at 08:41:38AM +0100, Julia Lawall wrote:
> 
> 
> On Wed, 8 Mar 2017, Aishwarya Pant wrote:
> 
> > 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?

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.


> 
> julia
> 
> >
> >  .../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 outreachy-kernel+unsubscribe@googlegroups.com.
> > To post to this group, send email to outreachy-kernel@googlegroups.com.
> > 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.
> >

Re: [Outreachy kernel] [PATCH 0/2] staging: bcm2835-camera: remove get_msg_context() and release_msg_context()

[Julia Lawall]

On Wed, 8 Mar 2017, Aishwarya Pant wrote:

> On Wed, Mar 08, 2017 at 08:41:38AM +0100, Julia Lawall wrote:
> >
> >
> > On Wed, 8 Mar 2017, Aishwarya Pant wrote:
> >
> > > 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?
>
> 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

>
>
> >
> > julia
> >
> > >
> > >  .../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 outreachy-kernel+unsubscribe@googlegroups.com.
> > > To post to this group, send email to outreachy-kernel@googlegroups.com.
> > > 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.
> > >
>

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

[Greg KH]

On Wed, Mar 08, 2017 at 10:14:28AM +0530, Aishwarya Pant wrote:
> 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 <aishpant@gmail.com>
> ---
>  .../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

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

[Aishwarya Pant]

On Thu, Mar 09, 2017 at 02:36:51PM +0100, Greg KH wrote:
> On Wed, Mar 08, 2017 at 10:14:28AM +0530, Aishwarya Pant wrote:
> > 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 <aishpant@gmail.com>
> > ---
> >  .../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.

> 
> thanks,
> 
> greg k-h