Skip to contentSkip to conversation

ppc64 kexec tools rm platform fix

PATCH Michael Ellerman 2 participants 2 messages Apr 10, 2006 – Apr 11, 2006 1 message not in the archive

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Michael Ellerman

author
#1 lore raw

Hi David,

Thanks for working on this one. Few comments below ...

Quoted 8 lines · David Wilder

This patch was discussed earlier on this list. (see posting by Haren Myneni <[email protected]>).

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)

Quoted 63 lines

---
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

(quoted diff) +0 -25
@@ -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);
(quoted diff) +0 -1
@@ -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 */

(quoted diff) +0 -4
-                       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) {

Review

Quoted 10 lines
(quoted diff) +6 -1
-                               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?

Quoted 23 lines
                        }
(quoted diff) +3 -8
@@ -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");

Review static int get_devtree_details(unsigned · line 390

Quoted 10 lines
(quoted diff) +6 -1
                        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

Signature · 8 lines
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
#2 lore raw
Quoted 22 lines · David Wilder

I lost Michael's reply on the list.
Here it is with my response..

Quoted 1 line · Michael

/ - 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;
Quoted 6 lines

/
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

Signature · 8 lines
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