* Re: [Fastboot] [PATCH]ppc64 kexec tools rm platform fix
[not found] <443700CB.3090909@us.ibm.com>
@ 2006-04-10 8:53 ` Michael Ellerman
2006-04-11 9:36 ` Michael Ellerman
0 siblings, 1 reply; 2+ messages in thread
From: Michael Ellerman @ 2006-04-10 8:53 UTC (permalink / raw)
To: David Wilder; +Cc: Mohan Kumar, fastboot, linuxppc-dev list
[-- Attachment #1: Type: text/plain, Size: 6035 bytes --]
Hi David,
Thanks for working on this one. Few comments below ...
On Fri, 2006-04-07 at 17:16 -0700, David Wilder wrote:
> This patch was discussed earlier on this list. (see posting by Haren
> Myneni <hbabu@us.ibm.com>).
>
> In recent kernels, the platform property is removed from the
> /proc/device-tree.This property is used to determine whether the
> platform is LPAR or non-lpar, and reads htab-* and tce-* properties
> based on the platform. Fixed this issue such that read these properties
> if exists, otherwise continue instead of exiting with an error message.
(Copied from attachment, won't apply, original here
http://lists.osdl.org/pipermail/fastboot/2006-April/002765.html)
> ---
> kexec-tools-1.101/kexec/arch/ppc64/kexec-ppc64.c.orig 2006-04-08
> 16:09:20.000000000 -0700
> +++ kexec-tools-1.101/kexec/arch/ppc64/kexec-ppc64.c 2006-04-08
> 16:23:26.000000000 -0700
> @@ -34,13 +34,8 @@
> #include "crashdump-ppc64.h"
> #include <arch/options.h>
>
> -/* Platforms supported by kexec on PPC64 */
> -#define PLATFORM_PSERIES 0x0100
> -#define PLATFORM_PSERIES_LPAR 0x0101
> -
> static struct exclude_range exclude_range[MAX_MEMORY_RANGES];
> static unsigned long long rmo_top;
> -static unsigned int platform;
> static struct memory_range memory_range[MAX_MEMORY_RANGES];
> static struct memory_range base_memory_range[MAX_MEMORY_RANGES];
> unsigned long long memory_max = 0;
> @@ -201,26 +196,6 @@ static int get_devtree_details(unsigned
> }
>
> if (strncmp(dentry->d_name, "chosen", 6) == 0) {
> - /* get platform details from /chosen node */
> - strcat(fname, "/linux,platform");
> - if ((file = fopen(fname, "r")) == NULL) {
> - perror(fname);
> - closedir(cdir);
> - closedir(dir);
> - return -1;
> - }
> - if (fread(&platform, sizeof(int), 1, file) !=
> 1) {
> - perror(fname);
> - fclose(file);
> - closedir(cdir);
> - closedir(dir);
> - return -1;
> - }
> - fclose(file);
> -
> - memset(fname, 0, sizeof(fname));
> - strcpy(fname, device_tree);
> - strcat(fname, dentry->d_name);
> strcat(fname, "/linux,kernel-end");
> if ((file = fopen(fname, "r")) == NULL) {
> perror(fname);
> @@ -291,18 +266,18 @@ static int get_devtree_details(unsigned
> reserve(KDUMP_BACKUP_LIMIT,
> crash_base-KDUMP_BACKUP_LIMIT);
> }
>
> - /* if LPAR, no need to read any more
> from /chosen */
> - if (platform != PLATFORM_PSERIES) {
> - closedir(cdir);
> - continue;
> - }
> memset(fname, 0, sizeof(fname));
> strcpy(fname, device_tree);
> strcat(fname, dentry->d_name);
> strcat(fname, "/linux,htab-base");
> if ((file = fopen(fname, "r")) == NULL) {
> - perror(fname);
> closedir(cdir);
> + if (errno == ENOENT) {
> + /* Non LPAR */
> + errno = 0;
> + continue;
> + }
> + perror(fname);
> closedir(dir);
> return -1;
I don't think you want to do the closedir() before the if. You certainly
don't need to do it twice?
> }
> @@ -394,23 +369,23 @@ static int get_devtree_details(unsigned
> }
> rmo_base = ((unsigned long long *)buf)[0];
> rmo_top = rmo_base + ((unsigned long long
> *)buf)[1];
> - if (platform == PLATFORM_PSERIES) {
> - if (rmo_top > 0x30000000UL)
> - rmo_top = 0x30000000UL;
> - }
> + if (rmo_top > 0x30000000UL)
> + rmo_top = 0x30000000UL;
> +
> fclose(file);
> closedir(cdir);
> } /* memory */
>
> if (strncmp(dentry->d_name, "pci@", 4) == 0) {
> - if (platform != PLATFORM_PSERIES) {
> - closedir(cdir);
> - continue;
> - }
> strcat(fname, "/linux,tce-base");
> if ((file = fopen(fname, "r")) == NULL) {
> - perror(fname);
> closedir(cdir);
> + if (errno == ENOENT) {
> + /* Non LPAR */
> + errno = 0;
> + continue;
> + }
> + perror(fname);
> closedir(dir);
Same comment here.
cheers
--
Michael Ellerman
IBM OzLabs
wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)
We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 191 bytes --]
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [Fastboot] [PATCH]ppc64 kexec tools rm platform fix
2006-04-10 8:53 ` [Fastboot] [PATCH]ppc64 kexec tools rm platform fix Michael Ellerman
@ 2006-04-11 9:36 ` Michael Ellerman
0 siblings, 0 replies; 2+ messages in thread
From: Michael Ellerman @ 2006-04-11 9:36 UTC (permalink / raw)
To: David Wilder; +Cc: Mohan Kumar, linuxppc-dev list, fastboot
David Wilder wrote:
> I lost Michael's reply on the list.
> Here it is with my response..
>
> Michael wrote:
>
> >/ - continue;
> />/ - }
> />/ memset(fname, 0, sizeof(fname));
> />/ strcpy(fname, device_tree);
> />/ strcat(fname, dentry->d_name);
> />/ strcat(fname, "/linux,htab-base");
> />/ if ((file = fopen(fname, "r")) == NULL) {
> />/ - perror(fname);
> />/ closedir(cdir);
> />/ + if (errno == ENOENT) {
> />/ + /* Non LPAR */
> />/ + errno = 0;
> />/ + continue;
> />/ + }
> />/ + perror(fname);
> />/ closedir(dir);
> />/ return -1;
> /
> I don't think you want to do the closedir() before the if. You
> certainly
> don't need to do it twice?
>
> The two closedir() calls are not closing the same thing.
Right. We don't seem to close cdir if it all works though. That code
needs some serious restructuring, 350+ line functions are not cool.
cheers
--
Michael Ellerman
IBM OzLabs
wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)
We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2006-04-11 9:36 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <443700CB.3090909@us.ibm.com>
2006-04-10 8:53 ` [Fastboot] [PATCH]ppc64 kexec tools rm platform fix Michael Ellerman
2006-04-11 9:36 ` Michael Ellerman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).