[PATCH] Kexec: Common alloc

[PATCH] Kexec: Common alloc

[Magnus Damm]

Kexec: Common alloc

This patch reduces code redundancy by introducing a new function called
kimage_common_alloc() which is used to set up image->control_code_page.

Signed-off-by: Magnus Damm <magnus@valinux.co.jp>
---

Applies on top of linux-2.6.17-rc1-git5 + "Kexec: Remove duplicate rimage"

 kexec.c |   51 ++++++++++++++++++++-------------------------------
 1 files changed, 20 insertions(+), 31 deletions(-)

--- 0004/kernel/kexec.c
+++ work/kernel/kexec.c	2006-04-12 16:30:34.000000000 +0900
@@ -205,34 +205,36 @@ out:
 
 }
 
-static int kimage_normal_alloc(struct kimage **rimage, unsigned long entry,
-				unsigned long nr_segments,
-				struct kexec_segment __user *segments)
+static int kimage_common_alloc(struct kimage *image)
 {
-	int result;
-	struct kimage *image;
-
-	/* Allocate and initialize a controlling structure */
-	image = NULL;
-	result = do_kimage_alloc(&image, entry, nr_segments, segments);
-	if (result)
-		goto out;
-
 	/*
-	 * Find a location for the control code buffer, and add it
+	 * Find a location for the control code buffer, and add
 	 * the vector of segments so that it's pages will also be
 	 * counted as destination pages.
 	 */
-	result = -ENOMEM;
 	image->control_code_page = kimage_alloc_control_pages(image,
 					   get_order(KEXEC_CONTROL_CODE_SIZE));
 	if (!image->control_code_page) {
 		printk(KERN_ERR "Could not allocate control_code_buffer\n");
-		goto out;
+		return -ENOMEM;
 	}
 
-	result = 0;
- out:
+	return 0;
+}
+
+static int kimage_normal_alloc(struct kimage **rimage, unsigned long entry,
+				unsigned long nr_segments,
+				struct kexec_segment __user *segments)
+{
+	int result;
+	struct kimage *image;
+
+	/* Allocate and initialize a controlling structure */
+	image = NULL;
+	result = do_kimage_alloc(&image, entry, nr_segments, segments);
+	if (!result)
+		result = kimage_common_alloc(image);
+
 	if (result == 0)
 		*rimage = image;
 	else
@@ -287,20 +289,7 @@ static int kimage_crash_alloc(struct kim
 			goto out;
 	}
 
-	/*
-	 * Find a location for the control code buffer, and add
-	 * the vector of segments so that it's pages will also be
-	 * counted as destination pages.
-	 */
-	result = -ENOMEM;
-	image->control_code_page = kimage_alloc_control_pages(image,
-					   get_order(KEXEC_CONTROL_CODE_SIZE));
-	if (!image->control_code_page) {
-		printk(KERN_ERR "Could not allocate control_code_buffer\n");
-		goto out;
-	}
-
-	result = 0;
+	result = kimage_common_alloc(image);
 out:
 	if (result == 0)
 		*rimage = image;

Re: [Fastboot] [PATCH] Kexec: Common alloc

[Vivek Goyal]

On Wed, Apr 12, 2006 at 05:33:02PM +0900, Magnus Damm wrote:
> Kexec: Common alloc
> 
> This patch reduces code redundancy by introducing a new function called
> kimage_common_alloc() which is used to set up image->control_code_page.
> 
> Signed-off-by: Magnus Damm <magnus@valinux.co.jp>
> ---
> 
> Applies on top of linux-2.6.17-rc1-git5 + "Kexec: Remove duplicate rimage"
> 
>  kexec.c |   51 ++++++++++++++++++++-------------------------------
>  1 files changed, 20 insertions(+), 31 deletions(-)
> 
> --- 0004/kernel/kexec.c
> +++ work/kernel/kexec.c	2006-04-12 16:30:34.000000000 +0900
> @@ -205,34 +205,36 @@ out:
>  
>  }
>  
> -static int kimage_normal_alloc(struct kimage **rimage, unsigned long entry,
> -				unsigned long nr_segments,
> -				struct kexec_segment __user *segments)
> +static int kimage_common_alloc(struct kimage *image)
>  {
> -	int result;
> -	struct kimage *image;
> -
> -	/* Allocate and initialize a controlling structure */
> -	image = NULL;
> -	result = do_kimage_alloc(&image, entry, nr_segments, segments);
> -	if (result)
> -		goto out;
> -
>  	/*
> -	 * Find a location for the control code buffer, and add it
> +	 * Find a location for the control code buffer, and add
>  	 * the vector of segments so that it's pages will also be
>  	 * counted as destination pages.
>  	 */
> -	result = -ENOMEM;
>  	image->control_code_page = kimage_alloc_control_pages(image,
>  					   get_order(KEXEC_CONTROL_CODE_SIZE));
>  	if (!image->control_code_page) {
>  		printk(KERN_ERR "Could not allocate control_code_buffer\n");
> -		goto out;
> +		return -ENOMEM;

Ok. So effectively call to the the function kimage_alloc_control_pages() and
its return code handling is being wrapped in another function. This function
is called at only two places. Not quite convinced that this duplication is
significant enough that we introduce another function to wrap a function
call.

Thanks
Vivek

Re: [Fastboot] [PATCH] Kexec: Common alloc

[Eric W. Biederman]

Vivek Goyal <vgoyal@in.ibm.com> writes:

> On Wed, Apr 12, 2006 at 05:33:02PM +0900, Magnus Damm wrote:
>> Kexec: Common alloc
>> 
>> This patch reduces code redundancy by introducing a new function called
>> kimage_common_alloc() which is used to set up image->control_code_page.
>> 
>> Signed-off-by: Magnus Damm <magnus@valinux.co.jp>
>> ---
>> 
>> Applies on top of linux-2.6.17-rc1-git5 + "Kexec: Remove duplicate rimage"
>> 
>>  kexec.c |   51 ++++++++++++++++++++-------------------------------
>>  1 files changed, 20 insertions(+), 31 deletions(-)
>> 
>> --- 0004/kernel/kexec.c
>> +++ work/kernel/kexec.c	2006-04-12 16:30:34.000000000 +0900
>> @@ -205,34 +205,36 @@ out:
>>  
>>  }
>>  
>> -static int kimage_normal_alloc(struct kimage **rimage, unsigned long entry,
>> -				unsigned long nr_segments,
>> -				struct kexec_segment __user *segments)
>> +static int kimage_common_alloc(struct kimage *image)
>>  {
>> -	int result;
>> -	struct kimage *image;
>> -
>> -	/* Allocate and initialize a controlling structure */
>> -	image = NULL;
>> -	result = do_kimage_alloc(&image, entry, nr_segments, segments);
>> -	if (result)
>> -		goto out;
>> -
>>  	/*
>> -	 * Find a location for the control code buffer, and add it
>> +	 * Find a location for the control code buffer, and add
>>  	 * the vector of segments so that it's pages will also be
>>  	 * counted as destination pages.
>>  	 */
>> -	result = -ENOMEM;
>>  	image->control_code_page = kimage_alloc_control_pages(image,
>>  					   get_order(KEXEC_CONTROL_CODE_SIZE));
>>  	if (!image->control_code_page) {
>>  		printk(KERN_ERR "Could not allocate control_code_buffer\n");
>> -		goto out;
>> +		return -ENOMEM;
>
> Ok. So effectively call to the the function kimage_alloc_control_pages() and
> its return code handling is being wrapped in another function. This function
> is called at only two places. Not quite convinced that this duplication is
> significant enough that we introduce another function to wrap a function
> call.

Agreed.

Eric