[ovs-dev] [PATCH V2 1/5] ovs-numa: Add ovs-numa.{c, h} for extracting and storing cpu socket and cpu core info.

Ben Pfaff blp at nicira.com
Thu Jun 26 22:19:28 UTC 2014


On Tue, Jun 24, 2014 at 06:40:02PM -0700, Alex Wang wrote:
> Signed-off-by: Alex Wang <alexw at nicira.com>
> 
> ---
> PATCH -> V2:
> - Use readdir_r() instead of readdir() for reentrency.
> - Address review comments from Thomas Graf.
> - Add dummy interface for WIN32 case.

I think that this should be dummied not just on Windows but on any
non-Linux kernel, because I doubt that *BSD has the same NUMA interface
as Linux.

Usually, I prefer to write assertions based on predicate functions,
rather than write specialized assertions for predicates.  So my natural
inclination would be something like:
        ovs_assert(cpu_socket_id_is_valid(sid)); 
instead of
        CPU_SOCKET_ID_ASSERT(sid);

contain_all_digits() could be simplified to:
        return str[strspn(str, "0123456789")] == '\0';
although your implementation is fine too.

discover_sockets_and_cores() uses a fixed-size buffer and then checks
that the file name fits.  I guess it would easier to use xasprintf().

SYS_SOCKET_DIR is only used in one place.  It might be easier to
understand if the file name were just inlined.

If /sys is not mounted or does not contain the expected files, then the
number of cpus and cores will be zero, I think.  But shouldn't the
runtime inability to discover cpus and cores behave the same as a
platform that can't discover cpus and cores at all?  On those platforms
the various functions return the "unspec" values.

I haven't read the later patches.  Do they treat "unspec" differently
from a single core and CPU?

I would use readdir() instead of readdir_r().  See
http://elliotth.blogspot.com/2012/10/how-not-to-use-readdirr3.html for
more information.

Acked-by: Ben Pfaff <blp at nicira.com>



More information about the dev mailing list