Open MPI logo

Hardware Locality Development Mailing List Archives

  |   Home   |   Support   |   FAQ   |   all Hardware Locality Development mailing list

Subject: Re: [hwloc-devel] memory size attributes
From: Brice Goglin (Brice.Goglin_at_[hidden])
Date: 2010-01-19 15:15:19


Jeff Squyres wrote:
> On Jan 19, 2010, at 3:20 AM, Brice Goglin wrote:
>
>> I propose the following changes to the object attributes. struct
>> hwloc_obj now contains a struct hwloc_memory pointer:
>>
>
> Is there any reason not to make this a sub-struct included in hwloc_obj (vs. a pointer that requires an additional malloc)? I'm not saying that malloc performance and/or the cost of dereferencing the pointer is a big deal -- it just feels "un-tidy" to have another malloc for something that will be used in many situations.
>

IIRC, it was changed into a pointer before the first release to reduce
ABI changes inside the the main hwloc_obj struct when attributes are
changed. I don't think hwloc_obj and attributes will be stable before
hwloc 2.0 so I don't really care whether it's a pointer or not :)

>> struct hwloc_memory {
>> unsigned long total_kB; /* Total memory in this object and its children */
>> unsigned long local_kB; /* Local memory in this object */
>>
>
> FWIW, I'm not wild about the capitol "B" at the end, but that's mainly because I'm lazy. ;-) How about total_mem and local_mem, and we specify that they are in units of bytes?
>
> I say "bytes" instead of "kilobytes" because it might be easier to avoid getting into the "what does kilo mean -- 1000 or 1024?" arguments.
>

kilo mean 1000, otherwise it's called kibi :)

But I am ok with changing all this into bytes or whatever.

> Also, do we want to make them uint64_t's? A system's total memory could get pretty large -- more than 4B bytes on 32 bit systems...? Hmm. I'm indecisive on this one -- I can think of reasons for and against... I guess I just prefer future-proofing as much as possible.
>

I thought about that, but unsigned long was ok for 32bits system when
storing values as kilobytes. If switching to bytes, we should use uint64_t.

>> struct hwloc_memory_pages {
>> unsigned long size;
>> unsigned long count;
>> } * local_pages; /* 0-terminated array of pages or hugepages sizes and counts */
>> }
>>
>
> In the worst case, we've added 2*sizeof(long)+sizeof(struct*) to the size of the hwloc_obj struct for objects that don't have memory. But I don't think that's a big deal, right?
>

It's probably negligible against the size of cpusets since those are
statically allocated so far. By the way, (I may have some students
working on dynamic cpusets in the near future.

> Or maybe just total and local memory set to 0?
>

Don't know. I thought having memory point to NULL when the object can
obviously contain no memory could help, but it's not a big deal.

> I'm not sure I understand what you mean here -- are you thinking of moving the DMI fields to the general key=value arrays? What is "DMI"?
>

Yes.
Try running dmidecode on a machine (as root) to see what DMI tells you,
and see http://en.wikipedia.org/wiki/Desktop_Management_Interface for a
bit of doc. It's basically a way to recognize the exact machine model
and apply some hardware quirks. For instance hwloc could use that to
find out the hypertransport topology since the BIOS doesn't tell you how
opterons are connected (we did that in other software in the past).

>> Actually, maybe obj->name should go there as well before it mixes to
>> many meanings. It's mostly unused in trunk, except to store "Kerrighed"
>> since very recently. We talked about using it to store the hostname of
>> Machine objects. And the libpci branch uses it to store the PCI
>> vendor/model string and OS device names. Maybe all these shouldn't be in
>> obj->name and rather in the array of strings as OSName=Kerrighed,
>> Hostname=foobar, PCIVendor=foo, PCIModel=bar, OSDevice=eth2, ...
>>
>
> How about both? It can be useful to have a simple/common field to print a human-readable text field (even if it's only a summary and the real details are in key=value pairs).
>

Sure.

Brice