Linux Archive

Linux Archive (http://www.linux-archive.org/)
-   Crash Utility (http://www.linux-archive.org/crash-utility/)
-   -   Remove the VOID_PTR facilitator macro (http://www.linux-archive.org/crash-utility/481814-remove-void_ptr-facilitator-macro.html)

Petr Tesarik 01-27-2011 12:27 PM

Remove the VOID_PTR facilitator macro
 
I've been wondering about the intended use of this facilitator macro
for some time and concluded that it is plainly wrong. It should
take a pointer value from a buffer, but what is the use of a pointer
that pointed to something on the _target_ machine? It cannot be
meaningfully dereferenced on the host.

Okay, pointer arithmetics is done in one place, but IMO it is better
to explicitly state the object size there, because that's how things
are done elsewhere in the code, so it is less surprising for anybody
reading the code.

The irony is underlined by the fact that all three uses of the macro
need a subsequent typecast to ulong, which proves the whole concept
flawed.

Signed-off-by: Petr Tesarik <ptesarik@suse.cz>

---
defs.h | 1 -
memory.c | 42 +++++++++++++++++++++---------------------
2 files changed, 21 insertions(+), 22 deletions(-)

--- a/defs.h
+++ b/defs.h
@@ -1760,7 +1760,6 @@ struct builtin_debug_table {
#define ULONG_PTR(ADDR) *((ulong **)((char *)(ADDR)))
#define USHORT(ADDR) *((ushort *)((char *)(ADDR)))
#define SHORT(ADDR) *((short *)((char *)(ADDR)))
-#define VOID_PTR(ADDR) *((void **)((char *)(ADDR)))

struct node_table {
int node_id;
--- a/memory.c
+++ b/memory.c
@@ -26,8 +26,8 @@ struct meminfo { /* general pu
ulong c_offset;
ulong c_num;
ulong s_mem;
- void *s_freep;
- ulong *s_index;
+ ulong s_freep;
+ ulong s_index;
ulong s_inuse;
ulong cpucached_cache;
ulong cpucached_slab;
@@ -139,7 +139,7 @@ static int next_kpage(ulong, ulong *);
static ulong last_vmalloc_address(void);
static ulong next_vmlist_vaddr(ulong);
static int next_identity_mapping(ulong, ulong *);
-static int vm_area_page_dump(ulong, ulong, ulong, ulong, void *,
+static int vm_area_page_dump(ulong, ulong, ulong, ulong, ulong,
struct reference *);
static int dump_swap_info(ulong, ulong *, ulong *);
static void swap_info_init(void);
@@ -3138,7 +3138,7 @@ vm_area_dump(ulong task, ulong flag, ulo
ulong vma;
ulong vm_start;
ulong vm_end;
- void *vm_next, *vm_mm;
+ ulong vm_next, vm_mm;
char *dentry_buf, *vma_buf, *file_buf;
ulong vm_flags;
ulong vm_file, inode;
@@ -3202,7 +3202,7 @@ vm_area_dump(ulong task, ulong flag, ulo
!DO_REF_SEARCH(ref))
fprintf(fp, vma_header);

- for (found = FALSE; vma; vma = (ulong)vm_next) {
+ for (found = FALSE; vma; vma = vm_next) {

if ((flag & PHYSADDR) && !DO_REF_SEARCH(ref))
fprintf(fp, "%s", vma_header);
@@ -3211,9 +3211,9 @@ vm_area_dump(ulong task, ulong flag, ulo
BZERO(buf1, BUFSIZE);
vma_buf = fill_vma_cache(vma);

- vm_mm = VOID_PTR(vma_buf + OFFSET(vm_area_struct_vm_mm));
+ vm_mm = ULONG(vma_buf + OFFSET(vm_area_struct_vm_mm));
vm_end = ULONG(vma_buf + OFFSET(vm_area_struct_vm_end));
- vm_next = VOID_PTR(vma_buf + OFFSET(vm_area_struct_vm_next));
+ vm_next = ULONG(vma_buf + OFFSET(vm_area_struct_vm_next));
vm_start = ULONG(vma_buf + OFFSET(vm_area_struct_vm_start));
vm_flags = SIZE(vm_area_struct_vm_flags) == sizeof(short) ?
USHORT(vma_buf+ OFFSET(vm_area_struct_vm_flags)) :
@@ -3335,7 +3335,7 @@ vm_area_page_dump(ulong vma,
ulong task,
ulong start,
ulong end,
- void *mm,
+ ulong mm,
struct reference *ref)
{
physaddr_t paddr;
@@ -3347,7 +3347,7 @@ vm_area_page_dump(ulong vma,
char buf3[BUFSIZE];
char buf4[BUFSIZE];

- if ((ulong)mm == symbol_value("init_mm"))
+ if (mm == symbol_value("init_mm"))
return FALSE;

if (!ref || DO_REF_DISPLAY(ref))
@@ -9720,9 +9720,9 @@ dump_slab(struct meminfo *si)
return;
}

- si->s_freep = VOID_PTR(si->slab_buf + OFFSET(kmem_slab_s_s_freep));
+ si->s_freep = ULONG(si->slab_buf + OFFSET(kmem_slab_s_s_freep));
si->s_inuse = ULONG(si->slab_buf + OFFSET(kmem_slab_s_s_inuse));
- si->s_index = ULONG_PTR(si->slab_buf + OFFSET(kmem_slab_s_s_index));
+ si->s_index = ULONG(si->slab_buf + OFFSET(kmem_slab_s_s_index));
s_offset = USHORT(si->slab_buf + OFFSET(kmem_slab_s_s_offset));

if (!(si->flags & ADDRESS_SPECIFIED)) {
@@ -9850,7 +9850,7 @@ dump_slab_percpu_v2(struct meminfo *si)
static void
gather_slab_free_list(struct meminfo *si)
{
- ulong *next, obj;
+ ulong next, obj;
ulong expected, cnt;

BNEG(si->addrlist, sizeof(ulong) * (si->c_num+1));
@@ -9885,16 +9885,16 @@ gather_slab_free_list(struct meminfo *si
*/

if (si->c_flags & SLAB_CFLGS_BUFCTL)
- obj = si->s_mem + ((next - si->s_index) * si->c_offset);
+ obj = si->s_mem + (next - si->s_index) * si->c_offset;
else
- obj = (ulong)next - si->c_offset;
+ obj = next - si->c_offset;

si->addrlist[cnt] = obj;

if (si->flags & ADDRESS_SPECIFIED) {
if (INSLAB(next, si) &&
- (si->spec_addr >= (ulong)next) &&
- (si->spec_addr < (ulong)(next + 1))) {
+ (si->spec_addr >= next) &&
+ (si->spec_addr < next + sizeof(ulong))) {
si->found = KMEM_BUFCTL_ADDR;
return;
}
@@ -9909,7 +9909,7 @@ gather_slab_free_list(struct meminfo *si
si->errors++;
}

- readmem((ulong)next, KVADDR, &next, sizeof(void *),
+ readmem(next, KVADDR, &next, sizeof(ulong),
"s_freep chain entry", FAULT_ON_ERROR);
} while (next);

@@ -10057,7 +10057,7 @@ static void
dump_slab_objects(struct meminfo *si)
{
int i, j;
- ulong *next;
+ ulong next;
int on_free_list;
ulong cnt, expected;
ulong bufctl, obj;
@@ -11561,7 +11561,7 @@ next_upage(struct task_context *tc, ulon
int found;
char *vma_buf;
ulong vm_start, vm_end;
- void *vm_next;
+ ulong vm_next;

if (!tc->mm_struct)
return FALSE;
@@ -11575,12 +11575,12 @@ next_upage(struct task_context *tc, ulon

vaddr = VIRTPAGEBASE(vaddr) + PAGESIZE(); /* first possible page */

- for (found = FALSE; vma; vma = (ulong)vm_next) {
+ for (found = FALSE; vma; vma = vm_next) {
vma_buf = fill_vma_cache(vma);

vm_start = ULONG(vma_buf + OFFSET(vm_area_struct_vm_start));
vm_end = ULONG(vma_buf + OFFSET(vm_area_struct_vm_end));
- vm_next = VOID_PTR(vma_buf + OFFSET(vm_area_struct_vm_next));
+ vm_next = ULONG(vma_buf + OFFSET(vm_area_struct_vm_next));

if (vaddr <= vm_start) {
*nextvaddr = vm_start;

--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility

Dave Anderson 01-27-2011 02:41 PM

Remove the VOID_PTR facilitator macro
 
----- Original Message -----
> I've been wondering about the intended use of this facilitator macro
> for some time and concluded that it is plainly wrong. It should
> take a pointer value from a buffer, but what is the use of a pointer
> that pointed to something on the _target_ machine? It cannot be
> meaningfully dereferenced on the host.
>
> Okay, pointer arithmetics is done in one place, but IMO it is better
> to explicitly state the object size there, because that's how things
> are done elsewhere in the code, so it is less surprising for anybody
> reading the code.
>
> The irony is underlined by the fact that all three uses of the macro
> need a subsequent typecast to ulong, which proves the whole concept
> flawed.

Probably so. It's been so long since I wrote that I don't recall
my original intent, other than maybe paranoia that some architecture
in the future would have pointers and longs of different sizes.

I'll leave the macro in defs.h though, since it's possible that
somebody's extension module uses it.

BTW, was there a reason that you changed this?:

if (si->c_flags & SLAB_CFLGS_BUFCTL)
- obj = si->s_mem + ((next - si->s_index) * si->c_offset);
+ obj = si->s_mem + (next - si->s_index) * si->c_offset;

Dave

> ---
> defs.h | 1 -
> memory.c | 42 +++++++++++++++++++++---------------------
> 2 files changed, 21 insertions(+), 22 deletions(-)
>
> --- a/defs.h
> +++ b/defs.h
> @@ -1760,7 +1760,6 @@ struct builtin_debug_table {
> #define ULONG_PTR(ADDR) *((ulong **)((char *)(ADDR)))
> #define USHORT(ADDR) *((ushort *)((char *)(ADDR)))
> #define SHORT(ADDR) *((short *)((char *)(ADDR)))
> -#define VOID_PTR(ADDR) *((void **)((char *)(ADDR)))
>
> struct node_table {
> int node_id;
> --- a/memory.c
> +++ b/memory.c
> @@ -26,8 +26,8 @@ struct meminfo { /* general pu
> ulong c_offset;
> ulong c_num;
> ulong s_mem;
> - void *s_freep;
> - ulong *s_index;
> + ulong s_freep;
> + ulong s_index;
> ulong s_inuse;
> ulong cpucached_cache;
> ulong cpucached_slab;
> @@ -139,7 +139,7 @@ static int next_kpage(ulong, ulong *);
> static ulong last_vmalloc_address(void);
> static ulong next_vmlist_vaddr(ulong);
> static int next_identity_mapping(ulong, ulong *);
> -static int vm_area_page_dump(ulong, ulong, ulong, ulong, void *,
> +static int vm_area_page_dump(ulong, ulong, ulong, ulong, ulong,
> struct reference *);
> static int dump_swap_info(ulong, ulong *, ulong *);
> static void swap_info_init(void);
> @@ -3138,7 +3138,7 @@ vm_area_dump(ulong task, ulong flag, ulo
> ulong vma;
> ulong vm_start;
> ulong vm_end;
> - void *vm_next, *vm_mm;
> + ulong vm_next, vm_mm;
> char *dentry_buf, *vma_buf, *file_buf;
> ulong vm_flags;
> ulong vm_file, inode;
> @@ -3202,7 +3202,7 @@ vm_area_dump(ulong task, ulong flag, ulo
> !DO_REF_SEARCH(ref))
> fprintf(fp, vma_header);
>
> - for (found = FALSE; vma; vma = (ulong)vm_next) {
> + for (found = FALSE; vma; vma = vm_next) {
>
> if ((flag & PHYSADDR) && !DO_REF_SEARCH(ref))
> fprintf(fp, "%s", vma_header);
> @@ -3211,9 +3211,9 @@ vm_area_dump(ulong task, ulong flag, ulo
> BZERO(buf1, BUFSIZE);
> vma_buf = fill_vma_cache(vma);
>
> - vm_mm = VOID_PTR(vma_buf + OFFSET(vm_area_struct_vm_mm));
> + vm_mm = ULONG(vma_buf + OFFSET(vm_area_struct_vm_mm));
> vm_end = ULONG(vma_buf + OFFSET(vm_area_struct_vm_end));
> - vm_next = VOID_PTR(vma_buf + OFFSET(vm_area_struct_vm_next));
> + vm_next = ULONG(vma_buf + OFFSET(vm_area_struct_vm_next));
> vm_start = ULONG(vma_buf + OFFSET(vm_area_struct_vm_start));
> vm_flags = SIZE(vm_area_struct_vm_flags) == sizeof(short) ?
> USHORT(vma_buf+ OFFSET(vm_area_struct_vm_flags)) :
> @@ -3335,7 +3335,7 @@ vm_area_page_dump(ulong vma,
> ulong task,
> ulong start,
> ulong end,
> - void *mm,
> + ulong mm,
> struct reference *ref)
> {
> physaddr_t paddr;
> @@ -3347,7 +3347,7 @@ vm_area_page_dump(ulong vma,
> char buf3[BUFSIZE];
> char buf4[BUFSIZE];
>
> - if ((ulong)mm == symbol_value("init_mm"))
> + if (mm == symbol_value("init_mm"))
> return FALSE;
>
> if (!ref || DO_REF_DISPLAY(ref))
> @@ -9720,9 +9720,9 @@ dump_slab(struct meminfo *si)
> return;
> }
>
> - si->s_freep = VOID_PTR(si->slab_buf + OFFSET(kmem_slab_s_s_freep));
> + si->s_freep = ULONG(si->slab_buf + OFFSET(kmem_slab_s_s_freep));
> si->s_inuse = ULONG(si->slab_buf + OFFSET(kmem_slab_s_s_inuse));
> - si->s_index = ULONG_PTR(si->slab_buf + OFFSET(kmem_slab_s_s_index));
> + si->s_index = ULONG(si->slab_buf + OFFSET(kmem_slab_s_s_index));
> s_offset = USHORT(si->slab_buf + OFFSET(kmem_slab_s_s_offset));
>
> if (!(si->flags & ADDRESS_SPECIFIED)) {
> @@ -9850,7 +9850,7 @@ dump_slab_percpu_v2(struct meminfo *si)
> static void
> gather_slab_free_list(struct meminfo *si)
> {
> - ulong *next, obj;
> + ulong next, obj;
> ulong expected, cnt;
>
> BNEG(si->addrlist, sizeof(ulong) * (si->c_num+1));
> @@ -9885,16 +9885,16 @@ gather_slab_free_list(struct meminfo *si
> */
>
> if (si->c_flags & SLAB_CFLGS_BUFCTL)
> - obj = si->s_mem + ((next - si->s_index) * si->c_offset);
> + obj = si->s_mem + (next - si->s_index) * si->c_offset;
> else
> - obj = (ulong)next - si->c_offset;
> + obj = next - si->c_offset;
>
> si->addrlist[cnt] = obj;
>
> if (si->flags & ADDRESS_SPECIFIED) {
> if (INSLAB(next, si) &&
> - (si->spec_addr >= (ulong)next) &&
> - (si->spec_addr < (ulong)(next + 1))) {
> + (si->spec_addr >= next) &&
> + (si->spec_addr < next + sizeof(ulong))) {
> si->found = KMEM_BUFCTL_ADDR;
> return;
> }
> @@ -9909,7 +9909,7 @@ gather_slab_free_list(struct meminfo *si
> si->errors++;
> }
>
> - readmem((ulong)next, KVADDR, &next, sizeof(void *),
> + readmem(next, KVADDR, &next, sizeof(ulong),
> "s_freep chain entry", FAULT_ON_ERROR);
> } while (next);
>
> @@ -10057,7 +10057,7 @@ static void
> dump_slab_objects(struct meminfo *si)
> {
> int i, j;
> - ulong *next;
> + ulong next;
> int on_free_list;
> ulong cnt, expected;
> ulong bufctl, obj;
> @@ -11561,7 +11561,7 @@ next_upage(struct task_context *tc, ulon
> int found;
> char *vma_buf;
> ulong vm_start, vm_end;
> - void *vm_next;
> + ulong vm_next;
>
> if (!tc->mm_struct)
> return FALSE;
> @@ -11575,12 +11575,12 @@ next_upage(struct task_context *tc, ulon
>
> vaddr = VIRTPAGEBASE(vaddr) + PAGESIZE(); /* first possible page */
>
> - for (found = FALSE; vma; vma = (ulong)vm_next) {
> + for (found = FALSE; vma; vma = vm_next) {
> vma_buf = fill_vma_cache(vma);
>
> vm_start = ULONG(vma_buf + OFFSET(vm_area_struct_vm_start));
> vm_end = ULONG(vma_buf + OFFSET(vm_area_struct_vm_end));
> - vm_next = VOID_PTR(vma_buf + OFFSET(vm_area_struct_vm_next));
> + vm_next = ULONG(vma_buf + OFFSET(vm_area_struct_vm_next));
>
> if (vaddr <= vm_start) {
> *nextvaddr = vm_start;
>
> --
> Crash-utility mailing list
> Crash-utility@redhat.com
> https://www.redhat.com/mailman/listinfo/crash-utility

--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility

"David Mair" 01-27-2011 05:26 PM

Remove the VOID_PTR facilitator macro
 
Hi Dave,

>>> On 1/27/2011 at 08:41 AM, in message
<748696339.157170.1296142883615.JavaMail.root@zmai l05.collab.prod.int.phx2.redha
.com>, Dave Anderson <anderson@redhat.com> wrote:

>
> ----- Original Message -----
>> I've been wondering about the intended use of this facilitator macro
>> for some time and concluded that it is plainly wrong. It should
>> take a pointer value from a buffer, but what is the use of a pointer
>> that pointed to something on the _target_ machine? It cannot be
>> meaningfully dereferenced on the host.
>>
>> Okay, pointer arithmetics is done in one place, but IMO it is better
>> to explicitly state the object size there, because that's how things
>> are done elsewhere in the code, so it is less surprising for anybody
>> reading the code.
>>
>> The irony is underlined by the fact that all three uses of the macro
>> need a subsequent typecast to ulong, which proves the whole concept
>> flawed.
>
> Probably so. It's been so long since I wrote that I don't recall
> my original intent, other than maybe paranoia that some architecture
> in the future would have pointers and longs of different sizes.
>
> I'll leave the macro in defs.h though, since it's possible that
> somebody's extension module uses it.
>
> BTW, was there a reason that you changed this?:
>
> if (si->c_flags & SLAB_CFLGS_BUFCTL)
> - obj = si->s_mem + ((next - si->s_index) * si->c_offset);
> + obj = si->s_mem + (next - si->s_index) * si->c_offset;

It wasn't my patch so I'm only offering my ACK for the original change and my reason is that multiply has higher operator precedence than add (and subtract) in normal arithmetic so the outer parentheses were never necessary anyway. They didn't create an error but they weren't needed either. The inner parentheses are required.

--
David Mair.

>
> Dave
>
>> ---
>> defs.h | 1 -
>> memory.c | 42 +++++++++++++++++++++---------------------
>> 2 files changed, 21 insertions(+), 22 deletions(-)
>>
>> --- a/defs.h
>> +++ b/defs.h
>> @@ -1760,7 +1760,6 @@ struct builtin_debug_table {
>> #define ULONG_PTR(ADDR) *((ulong **)((char *)(ADDR)))
>> #define USHORT(ADDR) *((ushort *)((char *)(ADDR)))
>> #define SHORT(ADDR) *((short *)((char *)(ADDR)))
>> -#define VOID_PTR(ADDR) *((void **)((char *)(ADDR)))
>>
>> struct node_table {
>> int node_id;
>> --- a/memory.c
>> +++ b/memory.c
>> @@ -26,8 +26,8 @@ struct meminfo { /* general pu
>> ulong c_offset;
>> ulong c_num;
>> ulong s_mem;
>> - void *s_freep;
>> - ulong *s_index;
>> + ulong s_freep;
>> + ulong s_index;
>> ulong s_inuse;
>> ulong cpucached_cache;
>> ulong cpucached_slab;
>> @@ -139,7 +139,7 @@ static int next_kpage(ulong, ulong *);
>> static ulong last_vmalloc_address(void);
>> static ulong next_vmlist_vaddr(ulong);
>> static int next_identity_mapping(ulong, ulong *);
>> -static int vm_area_page_dump(ulong, ulong, ulong, ulong, void *,
>> +static int vm_area_page_dump(ulong, ulong, ulong, ulong, ulong,
>> struct reference *);
>> static int dump_swap_info(ulong, ulong *, ulong *);
>> static void swap_info_init(void);
>> @@ -3138,7 +3138,7 @@ vm_area_dump(ulong task, ulong flag, ulo
>> ulong vma;
>> ulong vm_start;
>> ulong vm_end;
>> - void *vm_next, *vm_mm;
>> + ulong vm_next, vm_mm;
>> char *dentry_buf, *vma_buf, *file_buf;
>> ulong vm_flags;
>> ulong vm_file, inode;
>> @@ -3202,7 +3202,7 @@ vm_area_dump(ulong task, ulong flag, ulo
>> !DO_REF_SEARCH(ref))
>> fprintf(fp, vma_header);
>>
>> - for (found = FALSE; vma; vma = (ulong)vm_next) {
>> + for (found = FALSE; vma; vma = vm_next) {
>>
>> if ((flag & PHYSADDR) && !DO_REF_SEARCH(ref))
>> fprintf(fp, "%s", vma_header);
>> @@ -3211,9 +3211,9 @@ vm_area_dump(ulong task, ulong flag, ulo
>> BZERO(buf1, BUFSIZE);
>> vma_buf = fill_vma_cache(vma);
>>
>> - vm_mm = VOID_PTR(vma_buf + OFFSET(vm_area_struct_vm_mm));
>> + vm_mm = ULONG(vma_buf + OFFSET(vm_area_struct_vm_mm));
>> vm_end = ULONG(vma_buf + OFFSET(vm_area_struct_vm_end));
>> - vm_next = VOID_PTR(vma_buf + OFFSET(vm_area_struct_vm_next));
>> + vm_next = ULONG(vma_buf + OFFSET(vm_area_struct_vm_next));
>> vm_start = ULONG(vma_buf + OFFSET(vm_area_struct_vm_start));
>> vm_flags = SIZE(vm_area_struct_vm_flags) == sizeof(short) ?
>> USHORT(vma_buf+ OFFSET(vm_area_struct_vm_flags)) :
>> @@ -3335,7 +3335,7 @@ vm_area_page_dump(ulong vma,
>> ulong task,
>> ulong start,
>> ulong end,
>> - void *mm,
>> + ulong mm,
>> struct reference *ref)
>> {
>> physaddr_t paddr;
>> @@ -3347,7 +3347,7 @@ vm_area_page_dump(ulong vma,
>> char buf3[BUFSIZE];
>> char buf4[BUFSIZE];
>>
>> - if ((ulong)mm == symbol_value("init_mm"))
>> + if (mm == symbol_value("init_mm"))
>> return FALSE;
>>
>> if (!ref || DO_REF_DISPLAY(ref))
>> @@ -9720,9 +9720,9 @@ dump_slab(struct meminfo *si)
>> return;
>> }
>>
>> - si->s_freep = VOID_PTR(si->slab_buf + OFFSET(kmem_slab_s_s_freep));
>> + si->s_freep = ULONG(si->slab_buf + OFFSET(kmem_slab_s_s_freep));
>> si->s_inuse = ULONG(si->slab_buf + OFFSET(kmem_slab_s_s_inuse));
>> - si->s_index = ULONG_PTR(si->slab_buf + OFFSET(kmem_slab_s_s_index));
>> + si->s_index = ULONG(si->slab_buf + OFFSET(kmem_slab_s_s_index));
>> s_offset = USHORT(si->slab_buf + OFFSET(kmem_slab_s_s_offset));
>>
>> if (!(si->flags & ADDRESS_SPECIFIED)) {
>> @@ -9850,7 +9850,7 @@ dump_slab_percpu_v2(struct meminfo *si)
>> static void
>> gather_slab_free_list(struct meminfo *si)
>> {
>> - ulong *next, obj;
>> + ulong next, obj;
>> ulong expected, cnt;
>>
>> BNEG(si->addrlist, sizeof(ulong) * (si->c_num+1));
>> @@ -9885,16 +9885,16 @@ gather_slab_free_list(struct meminfo *si
>> */
>>
>> if (si->c_flags & SLAB_CFLGS_BUFCTL)
>> - obj = si->s_mem + ((next - si->s_index) * si->c_offset);
>> + obj = si->s_mem + (next - si->s_index) * si->c_offset;
>> else
>> - obj = (ulong)next - si->c_offset;
>> + obj = next - si->c_offset;
>>
>> si->addrlist[cnt] = obj;
>>
>> if (si->flags & ADDRESS_SPECIFIED) {
>> if (INSLAB(next, si) &&
>> - (si->spec_addr >= (ulong)next) &&
>> - (si->spec_addr < (ulong)(next + 1))) {
>> + (si->spec_addr >= next) &&
>> + (si->spec_addr < next + sizeof(ulong))) {
>> si->found = KMEM_BUFCTL_ADDR;
>> return;
>> }
>> @@ -9909,7 +9909,7 @@ gather_slab_free_list(struct meminfo *si
>> si->errors++;
>> }
>>
>> - readmem((ulong)next, KVADDR, &next, sizeof(void *),
>> + readmem(next, KVADDR, &next, sizeof(ulong),
>> "s_freep chain entry", FAULT_ON_ERROR);
>> } while (next);
>>
>> @@ -10057,7 +10057,7 @@ static void
>> dump_slab_objects(struct meminfo *si)
>> {
>> int i, j;
>> - ulong *next;
>> + ulong next;
>> int on_free_list;
>> ulong cnt, expected;
>> ulong bufctl, obj;
>> @@ -11561,7 +11561,7 @@ next_upage(struct task_context *tc, ulon
>> int found;
>> char *vma_buf;
>> ulong vm_start, vm_end;
>> - void *vm_next;
>> + ulong vm_next;
>>
>> if (!tc->mm_struct)
>> return FALSE;
>> @@ -11575,12 +11575,12 @@ next_upage(struct task_context *tc, ulon
>>
>> vaddr = VIRTPAGEBASE(vaddr) + PAGESIZE(); /* first possible page */
>>
>> - for (found = FALSE; vma; vma = (ulong)vm_next) {
>> + for (found = FALSE; vma; vma = vm_next) {
>> vma_buf = fill_vma_cache(vma);
>>
>> vm_start = ULONG(vma_buf + OFFSET(vm_area_struct_vm_start));
>> vm_end = ULONG(vma_buf + OFFSET(vm_area_struct_vm_end));
>> - vm_next = VOID_PTR(vma_buf + OFFSET(vm_area_struct_vm_next));
>> + vm_next = ULONG(vma_buf + OFFSET(vm_area_struct_vm_next));
>>
>> if (vaddr <= vm_start) {
>> *nextvaddr = vm_start;
>>
>> --
>> Crash-utility mailing list
>> Crash-utility@redhat.com
>> https://www.redhat.com/mailman/listinfo/crash-utility
>
> --
> Crash-utility mailing list
> Crash-utility@redhat.com
> https://www.redhat.com/mailman/listinfo/crash-utility



--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility

Dave Anderson 01-27-2011 05:47 PM

Remove the VOID_PTR facilitator macro
 
----- Original Message -----
> Hi Dave,
> >
> > BTW, was there a reason that you changed this?:
> >
> > if (si->c_flags & SLAB_CFLGS_BUFCTL)
> > - obj = si->s_mem + ((next - si->s_index) * si->c_offset);
> > + obj = si->s_mem + (next - si->s_index) * si->c_offset;
>
> It wasn't my patch so I'm only offering my ACK for the original change
> and my reason is that multiply has higher operator precedence than add
> (and subtract) in normal arithmetic so the outer parentheses were
> never necessary anyway. They didn't create an error but they weren't
> needed either. The inner parentheses are required.
>
> --
> David Mair.

Right I understand that, but the sources are full of such
"extra" parentheses, some to prevent compiler warnings, but
mostly just for ease of maintainability/understanding.

I just was wondering why that line was explicitly changed.

Dave

--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility

Petr Tesarik 01-27-2011 11:14 PM

Remove the VOID_PTR facilitator macro
 
Dne čtvrtek 27 Leden 2011 19:47:41 Dave Anderson napsal(a):
> ----- Original Message -----
>
> > Hi Dave,
> >
> > > BTW, was there a reason that you changed this?:
> > > if (si->c_flags & SLAB_CFLGS_BUFCTL)
> > >
> > > - obj = si->s_mem + ((next - si->s_index) * si->c_offset);
> > > + obj = si->s_mem + (next - si->s_index) * si->c_offset;
> >
> > It wasn't my patch so I'm only offering my ACK for the original change
> > and my reason is that multiply has higher operator precedence than add
> > (and subtract) in normal arithmetic so the outer parentheses were
> > never necessary anyway. They didn't create an error but they weren't
> > needed either. The inner parentheses are required.
> >
> > --
> > David Mair.
>
> Right I understand that, but the sources are full of such
> "extra" parentheses, some to prevent compiler warnings, but
> mostly just for ease of maintainability/understanding.
>
> I just was wondering why that line was explicitly changed.

No reason, except I removed them for some reason while developing the patch
and forgot to put them back afterwards. I can leave it as-is.

Oh, I've just realized that this actually changed the expression, because
originally both variables were pointers to ulong, so

(next - si->s_index)

should become

((next - si->s_index)/sizeof(ulong))

I wonder whether this was originally intended. Let me have one more look at
this.

Petr

--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility

Petr Tesarik 01-27-2011 11:32 PM

Remove the VOID_PTR facilitator macro
 
Dne pátek 28 Leden 2011 01:14:51 Petr Tesarik napsal(a):
> Dne čtvrtek 27 Leden 2011 19:47:41 Dave Anderson napsal(a):
> > ----- Original Message -----
> >
> > > Hi Dave,
> > >
> > > > BTW, was there a reason that you changed this?:
> > > > if (si->c_flags & SLAB_CFLGS_BUFCTL)
> > > >
> > > > - obj = si->s_mem + ((next - si->s_index) * si->c_offset);
> > > > + obj = si->s_mem + (next - si->s_index) * si->c_offset;
> > >
> > > It wasn't my patch so I'm only offering my ACK for the original change
> > > and my reason is that multiply has higher operator precedence than add
> > > (and subtract) in normal arithmetic so the outer parentheses were
> > > never necessary anyway. They didn't create an error but they weren't
> > > needed either. The inner parentheses are required.
> > >
> > > --
> > > David Mair.
> >
> > Right I understand that, but the sources are full of such
> > "extra" parentheses, some to prevent compiler warnings, but
> > mostly just for ease of maintainability/understanding.
> >
> > I just was wondering why that line was explicitly changed.
>
> No reason, except I removed them for some reason while developing the patch
> and forgot to put them back afterwards. I can leave it as-is.
>
> Oh, I've just realized that this actually changed the expression, because
> originally both variables were pointers to ulong, so
>
> (next - si->s_index)
>
> should become
>
> ((next - si->s_index)/sizeof(ulong))
>
> I wonder whether this was originally intended. Let me have one more look at
> this.

I had to dig back in 2.2.x sources, but the divide should indeed be there.
I'll send an updated patch tomorrow (too late for me in Europe now).

Petr

--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility

Petr Tesarik 01-28-2011 09:03 AM

Remove the VOID_PTR facilitator macro
 
I've been wondering about the intended use of this facilitator macro
for some time and concluded that it is plainly wrong. It should
take a pointer value from a buffer, but what is the use of a pointer
that pointed to something on the target machine?

Since it cannot be meaningfully dereferenced on the host, the only
(arguable) advantage is the ability to do pointer arithmetics on the
variables. This second patch does the correct thing wherever pointer
arithmetics was done, making the actual calculations more explicit.

Signed-off-by: Petr Tesarik <ptesarik@suse.cz>

---
memory.c | 47 +++++++++++++++++++++++++----------------------
1 file changed, 25 insertions(+), 22 deletions(-)

--- a/memory.c
+++ b/memory.c
@@ -26,8 +26,8 @@ struct meminfo { /* general pu
ulong c_offset;
ulong c_num;
ulong s_mem;
- void *s_freep;
- ulong *s_index;
+ ulong s_freep;
+ ulong s_index;
ulong s_inuse;
ulong cpucached_cache;
ulong cpucached_slab;
@@ -139,7 +139,7 @@ static int next_kpage(ulong, ulong *);
static ulong last_vmalloc_address(void);
static ulong next_vmlist_vaddr(ulong);
static int next_identity_mapping(ulong, ulong *);
-static int vm_area_page_dump(ulong, ulong, ulong, ulong, void *,
+static int vm_area_page_dump(ulong, ulong, ulong, ulong, ulong,
struct reference *);
static int dump_swap_info(ulong, ulong *, ulong *);
static void swap_info_init(void);
@@ -3138,7 +3138,7 @@ vm_area_dump(ulong task, ulong flag, ulo
ulong vma;
ulong vm_start;
ulong vm_end;
- void *vm_next, *vm_mm;
+ ulong vm_next, vm_mm;
char *dentry_buf, *vma_buf, *file_buf;
ulong vm_flags;
ulong vm_file, inode;
@@ -3202,7 +3202,7 @@ vm_area_dump(ulong task, ulong flag, ulo
!DO_REF_SEARCH(ref))
fprintf(fp, vma_header);

- for (found = FALSE; vma; vma = (ulong)vm_next) {
+ for (found = FALSE; vma; vma = vm_next) {

if ((flag & PHYSADDR) && !DO_REF_SEARCH(ref))
fprintf(fp, "%s", vma_header);
@@ -3211,9 +3211,9 @@ vm_area_dump(ulong task, ulong flag, ulo
BZERO(buf1, BUFSIZE);
vma_buf = fill_vma_cache(vma);

- vm_mm = VOID_PTR(vma_buf + OFFSET(vm_area_struct_vm_mm));
+ vm_mm = ULONG(vma_buf + OFFSET(vm_area_struct_vm_mm));
vm_end = ULONG(vma_buf + OFFSET(vm_area_struct_vm_end));
- vm_next = VOID_PTR(vma_buf + OFFSET(vm_area_struct_vm_next));
+ vm_next = ULONG(vma_buf + OFFSET(vm_area_struct_vm_next));
vm_start = ULONG(vma_buf + OFFSET(vm_area_struct_vm_start));
vm_flags = SIZE(vm_area_struct_vm_flags) == sizeof(short) ?
USHORT(vma_buf+ OFFSET(vm_area_struct_vm_flags)) :
@@ -3335,7 +3335,7 @@ vm_area_page_dump(ulong vma,
ulong task,
ulong start,
ulong end,
- void *mm,
+ ulong mm,
struct reference *ref)
{
physaddr_t paddr;
@@ -3347,7 +3347,7 @@ vm_area_page_dump(ulong vma,
char buf3[BUFSIZE];
char buf4[BUFSIZE];

- if ((ulong)mm == symbol_value("init_mm"))
+ if (mm == symbol_value("init_mm"))
return FALSE;

if (!ref || DO_REF_DISPLAY(ref))
@@ -9720,9 +9720,9 @@ dump_slab(struct meminfo *si)
return;
}

- si->s_freep = VOID_PTR(si->slab_buf + OFFSET(kmem_slab_s_s_freep));
+ si->s_freep = ULONG(si->slab_buf + OFFSET(kmem_slab_s_s_freep));
si->s_inuse = ULONG(si->slab_buf + OFFSET(kmem_slab_s_s_inuse));
- si->s_index = ULONG_PTR(si->slab_buf + OFFSET(kmem_slab_s_s_index));
+ si->s_index = ULONG(si->slab_buf + OFFSET(kmem_slab_s_s_index));
s_offset = USHORT(si->slab_buf + OFFSET(kmem_slab_s_s_offset));

if (!(si->flags & ADDRESS_SPECIFIED)) {
@@ -9850,7 +9850,7 @@ dump_slab_percpu_v2(struct meminfo *si)
static void
gather_slab_free_list(struct meminfo *si)
{
- ulong *next, obj;
+ ulong next, obj;
ulong expected, cnt;

BNEG(si->addrlist, sizeof(ulong) * (si->c_num+1));
@@ -9885,16 +9885,18 @@ gather_slab_free_list(struct meminfo *si
*/

if (si->c_flags & SLAB_CFLGS_BUFCTL)
- obj = si->s_mem + ((next - si->s_index) * si->c_offset);
+ obj = si->s_mem +
+ ((next - si->s_index) / sizeof(ulong)
+ * si->c_offset);
else
- obj = (ulong)next - si->c_offset;
+ obj = next - si->c_offset;

si->addrlist[cnt] = obj;

if (si->flags & ADDRESS_SPECIFIED) {
if (INSLAB(next, si) &&
- (si->spec_addr >= (ulong)next) &&
- (si->spec_addr < (ulong)(next + 1))) {
+ (si->spec_addr >= next) &&
+ (si->spec_addr < next + sizeof(ulong))) {
si->found = KMEM_BUFCTL_ADDR;
return;
}
@@ -9909,7 +9911,7 @@ gather_slab_free_list(struct meminfo *si
si->errors++;
}

- readmem((ulong)next, KVADDR, &next, sizeof(void *),
+ readmem(next, KVADDR, &next, sizeof(ulong),
"s_freep chain entry", FAULT_ON_ERROR);
} while (next);

@@ -10057,7 +10059,7 @@ static void
dump_slab_objects(struct meminfo *si)
{
int i, j;
- ulong *next;
+ ulong next;
int on_free_list;
ulong cnt, expected;
ulong bufctl, obj;
@@ -10088,7 +10090,8 @@ dump_slab_objects(struct meminfo *si)
if (si->c_flags & SLAB_CFLGS_BUFCTL) {
for (i = 0, next = si->s_index; i < si->c_num; i++, next++) {
obj = si->s_mem +
- ((next - si->s_index) * si->c_offset);
+ ((next - si->s_index) / sizeof(ulong)
+ * si->c_offset);
DUMP_SLAB_OBJECT();
}
} else {
@@ -11561,7 +11564,7 @@ next_upage(struct task_context *tc, ulon
int found;
char *vma_buf;
ulong vm_start, vm_end;
- void *vm_next;
+ ulong vm_next;

if (!tc->mm_struct)
return FALSE;
@@ -11575,12 +11578,12 @@ next_upage(struct task_context *tc, ulon

vaddr = VIRTPAGEBASE(vaddr) + PAGESIZE(); /* first possible page */

- for (found = FALSE; vma; vma = (ulong)vm_next) {
+ for (found = FALSE; vma; vma = vm_next) {
vma_buf = fill_vma_cache(vma);

vm_start = ULONG(vma_buf + OFFSET(vm_area_struct_vm_start));
vm_end = ULONG(vma_buf + OFFSET(vm_area_struct_vm_end));
- vm_next = VOID_PTR(vma_buf + OFFSET(vm_area_struct_vm_next));
+ vm_next = ULONG(vma_buf + OFFSET(vm_area_struct_vm_next));

if (vaddr <= vm_start) {
*nextvaddr = vm_start;

--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility

Dave Anderson 01-28-2011 01:08 PM

Remove the VOID_PTR facilitator macro
 
----- Original Message -----
> I've been wondering about the intended use of this facilitator macro
> for some time and concluded that it is plainly wrong. It should
> take a pointer value from a buffer, but what is the use of a pointer
> that pointed to something on the target machine?
>
> Since it cannot be meaningfully dereferenced on the host, the only
> (arguable) advantage is the ability to do pointer arithmetics on the
> variables. This second patch does the correct thing wherever pointer
> arithmetics was done, making the actual calculations more explicit.
>
> Signed-off-by: Petr Tesarik <ptesarik@suse.cz>

Everything *looks* correct -- but this patch doesn't fix anything, while
having the capacity to break what works. The slab-related changes still
concern me, at least until I test them.

BTW, in the future, when you post patches can you add them as attachments
instead of inlining them?

Thanks,
Dave


> ---
> memory.c | 47 +++++++++++++++++++++++++----------------------
> 1 file changed, 25 insertions(+), 22 deletions(-)
>
> --- a/memory.c
> +++ b/memory.c
> @@ -26,8 +26,8 @@ struct meminfo { /* general pu
> ulong c_offset;
> ulong c_num;
> ulong s_mem;
> - void *s_freep;
> - ulong *s_index;
> + ulong s_freep;
> + ulong s_index;
> ulong s_inuse;
> ulong cpucached_cache;
> ulong cpucached_slab;
> @@ -139,7 +139,7 @@ static int next_kpage(ulong, ulong *);
> static ulong last_vmalloc_address(void);
> static ulong next_vmlist_vaddr(ulong);
> static int next_identity_mapping(ulong, ulong *);
> -static int vm_area_page_dump(ulong, ulong, ulong, ulong, void *,
> +static int vm_area_page_dump(ulong, ulong, ulong, ulong, ulong,
> struct reference *);
> static int dump_swap_info(ulong, ulong *, ulong *);
> static void swap_info_init(void);
> @@ -3138,7 +3138,7 @@ vm_area_dump(ulong task, ulong flag, ulo
> ulong vma;
> ulong vm_start;
> ulong vm_end;
> - void *vm_next, *vm_mm;
> + ulong vm_next, vm_mm;
> char *dentry_buf, *vma_buf, *file_buf;
> ulong vm_flags;
> ulong vm_file, inode;
> @@ -3202,7 +3202,7 @@ vm_area_dump(ulong task, ulong flag, ulo
> !DO_REF_SEARCH(ref))
> fprintf(fp, vma_header);
>
> - for (found = FALSE; vma; vma = (ulong)vm_next) {
> + for (found = FALSE; vma; vma = vm_next) {
>
> if ((flag & PHYSADDR) && !DO_REF_SEARCH(ref))
> fprintf(fp, "%s", vma_header);
> @@ -3211,9 +3211,9 @@ vm_area_dump(ulong task, ulong flag, ulo
> BZERO(buf1, BUFSIZE);
> vma_buf = fill_vma_cache(vma);
>
> - vm_mm = VOID_PTR(vma_buf + OFFSET(vm_area_struct_vm_mm));
> + vm_mm = ULONG(vma_buf + OFFSET(vm_area_struct_vm_mm));
> vm_end = ULONG(vma_buf + OFFSET(vm_area_struct_vm_end));
> - vm_next = VOID_PTR(vma_buf + OFFSET(vm_area_struct_vm_next));
> + vm_next = ULONG(vma_buf + OFFSET(vm_area_struct_vm_next));
> vm_start = ULONG(vma_buf + OFFSET(vm_area_struct_vm_start));
> vm_flags = SIZE(vm_area_struct_vm_flags) == sizeof(short) ?
> USHORT(vma_buf+ OFFSET(vm_area_struct_vm_flags)) :
> @@ -3335,7 +3335,7 @@ vm_area_page_dump(ulong vma,
> ulong task,
> ulong start,
> ulong end,
> - void *mm,
> + ulong mm,
> struct reference *ref)
> {
> physaddr_t paddr;
> @@ -3347,7 +3347,7 @@ vm_area_page_dump(ulong vma,
> char buf3[BUFSIZE];
> char buf4[BUFSIZE];
>
> - if ((ulong)mm == symbol_value("init_mm"))
> + if (mm == symbol_value("init_mm"))
> return FALSE;
>
> if (!ref || DO_REF_DISPLAY(ref))
> @@ -9720,9 +9720,9 @@ dump_slab(struct meminfo *si)
> return;
> }
>
> - si->s_freep = VOID_PTR(si->slab_buf + OFFSET(kmem_slab_s_s_freep));
> + si->s_freep = ULONG(si->slab_buf + OFFSET(kmem_slab_s_s_freep));
> si->s_inuse = ULONG(si->slab_buf + OFFSET(kmem_slab_s_s_inuse));
> - si->s_index = ULONG_PTR(si->slab_buf + OFFSET(kmem_slab_s_s_index));
> + si->s_index = ULONG(si->slab_buf + OFFSET(kmem_slab_s_s_index));
> s_offset = USHORT(si->slab_buf + OFFSET(kmem_slab_s_s_offset));
>
> if (!(si->flags & ADDRESS_SPECIFIED)) {
> @@ -9850,7 +9850,7 @@ dump_slab_percpu_v2(struct meminfo *si)
> static void
> gather_slab_free_list(struct meminfo *si)
> {
> - ulong *next, obj;
> + ulong next, obj;
> ulong expected, cnt;
>
> BNEG(si->addrlist, sizeof(ulong) * (si->c_num+1));
> @@ -9885,16 +9885,18 @@ gather_slab_free_list(struct meminfo *si
> */
>
> if (si->c_flags & SLAB_CFLGS_BUFCTL)
> - obj = si->s_mem + ((next - si->s_index) * si->c_offset);
> + obj = si->s_mem +
> + ((next - si->s_index) / sizeof(ulong)
> + * si->c_offset);
> else
> - obj = (ulong)next - si->c_offset;
> + obj = next - si->c_offset;
>
> si->addrlist[cnt] = obj;
>
> if (si->flags & ADDRESS_SPECIFIED) {
> if (INSLAB(next, si) &&
> - (si->spec_addr >= (ulong)next) &&
> - (si->spec_addr < (ulong)(next + 1))) {
> + (si->spec_addr >= next) &&
> + (si->spec_addr < next + sizeof(ulong))) {
> si->found = KMEM_BUFCTL_ADDR;
> return;
> }
> @@ -9909,7 +9911,7 @@ gather_slab_free_list(struct meminfo *si
> si->errors++;
> }
>
> - readmem((ulong)next, KVADDR, &next, sizeof(void *),
> + readmem(next, KVADDR, &next, sizeof(ulong),
> "s_freep chain entry", FAULT_ON_ERROR);
> } while (next);
>
> @@ -10057,7 +10059,7 @@ static void
> dump_slab_objects(struct meminfo *si)
> {
> int i, j;
> - ulong *next;
> + ulong next;
> int on_free_list;
> ulong cnt, expected;
> ulong bufctl, obj;
> @@ -10088,7 +10090,8 @@ dump_slab_objects(struct meminfo *si)
> if (si->c_flags & SLAB_CFLGS_BUFCTL) {
> for (i = 0, next = si->s_index; i < si->c_num; i++, next++) {
> obj = si->s_mem +
> - ((next - si->s_index) * si->c_offset);
> + ((next - si->s_index) / sizeof(ulong)
> + * si->c_offset);
> DUMP_SLAB_OBJECT();
> }
> } else {
> @@ -11561,7 +11564,7 @@ next_upage(struct task_context *tc, ulon
> int found;
> char *vma_buf;
> ulong vm_start, vm_end;
> - void *vm_next;
> + ulong vm_next;
>
> if (!tc->mm_struct)
> return FALSE;
> @@ -11575,12 +11578,12 @@ next_upage(struct task_context *tc, ulon
>
> vaddr = VIRTPAGEBASE(vaddr) + PAGESIZE(); /* first possible page */
>
> - for (found = FALSE; vma; vma = (ulong)vm_next) {
> + for (found = FALSE; vma; vma = vm_next) {
> vma_buf = fill_vma_cache(vma);
>
> vm_start = ULONG(vma_buf + OFFSET(vm_area_struct_vm_start));
> vm_end = ULONG(vma_buf + OFFSET(vm_area_struct_vm_end));
> - vm_next = VOID_PTR(vma_buf + OFFSET(vm_area_struct_vm_next));
> + vm_next = ULONG(vma_buf + OFFSET(vm_area_struct_vm_next));
>
> if (vaddr <= vm_start) {
> *nextvaddr = vm_start;
>
> --
> Crash-utility mailing list
> Crash-utility@redhat.com
> https://www.redhat.com/mailman/listinfo/crash-utility

--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility

Dave Anderson 01-28-2011 08:10 PM

Remove the VOID_PTR facilitator macro
 
----- Original Message -----
> ----- Original Message -----
> > I've been wondering about the intended use of this facilitator macro
> > for some time and concluded that it is plainly wrong. It should
> > take a pointer value from a buffer, but what is the use of a pointer
> > that pointed to something on the target machine?
> >
> > Since it cannot be meaningfully dereferenced on the host, the only
> > (arguable) advantage is the ability to do pointer arithmetics on the
> > variables. This second patch does the correct thing wherever pointer
> > arithmetics was done, making the actual calculations more explicit.
> >
> > Signed-off-by: Petr Tesarik <ptesarik@suse.cz>
>
> Everything *looks* correct -- but this patch doesn't fix anything, while
> having the capacity to break what works. The slab-related changes still
> concern me, at least until I test them.

As it turns out, I can't even test the slab changes, because I don't have
any more sample dumpfiles that go that far back to "pre-percpu" days. That
being the case, I'm not going to change the parts that modify dump_slab(),
dump_slab_objects() and gather_slab_free_list(). Why bother? It's
essentially dead code, but if there are still any users of it out there,
I'm not going to risk breaking it on them.

So I'm afraid it's going to continue to offend your sensibilities -- sorry
about that...

On the other hand, the changes to vm_area_dump() and next_upage() certainly
look harmless enough.

Thanks,
Dave


> BTW, in the future, when you post patches can you add them as attachments
> instead of inlining them?
>
> Thanks,
> Dave

--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility

Dave Anderson 01-31-2011 02:51 PM

Remove the VOID_PTR facilitator macro
 
----- Original Message -----
> I've been wondering about the intended use of this facilitator macro
> for some time and concluded that it is plainly wrong. It should
> take a pointer value from a buffer, but what is the use of a pointer
> that pointed to something on the target machine?
>
> Since it cannot be meaningfully dereferenced on the host, the only
> (arguable) advantage is the ability to do pointer arithmetics on the
> variables. This second patch does the correct thing wherever pointer
> arithmetics was done, making the actual calculations more explicit.
>
> Signed-off-by: Petr Tesarik <ptesarik@suse.cz>

The changes to vm_area_dump(), vm_area_page_dump() and next_upage()
have been queued for the next release.

Thanks,
Dave

--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility


All times are GMT. The time now is 11:05 AM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.