search

pytorch / cpuinfo

CPU INFOrmation library (x86/x86-64/ARM/ARM64, Linux/Windows/Android/macOS/iOS)
BSD 2-Clause "Simplified" License
1.01k stars 319 forks source link

Fix a build error some users are experiencing wherein __NR_getcpu is not defined #37

Closed AshkanAliabadi closed 4 years ago

AshkanAliabadi commented 4 years ago

https://github.com/pytorch/cpuinfo/issues/36

rgommers commented 4 years ago

In addition to what @pearu said, I would change line 314 from #ifdef __linux__ to:

if defined(__linux__) && defined(__NR_getcpu)

That will fix things on older kernels that don't have __NR_getcpu at all as well.

Maratyszcza commented 4 years ago

I second @pearu's suggestion to include asm-generic/unistd.h only if the unistd.h doesn't have the right symbol. However, the suggestion in @rgommers's comment is very dangerous: it introduce the risk of silently regressing performance (because cpuinfo_get_current_* functions return default result) without any indication of failure. getcpu syscall was introduced in Linux 2.6.19 released on Nov 29, 2006 so we can just assume that it is always supported.

Maratyszcza commented 4 years ago

LGTM

rgommers commented 4 years ago

However, the suggestion in @rgommers's comment is very dangerous: it introduce the risk of silently regressing performance

I don't think that's true, since it can only fail if the symbol is missing completely, i.e. for old kernels. Also, pretty much every use of __NR_getcpu in public code (including in Tensorflow here) is protected like that. Within 2 weeks of merging the unprotected version, multiple people ran into build issues.

That said, there was no response yet from @bryan-lunt on gh-36, and this does fix things for @pearu and me, so merging this PR as-is is an improvement. It's still likely to come back as runtime errors once people on older clusters install the next release though.

rgommers commented 4 years ago

A ping on this, can someone merge this PR please?

rgommers commented 4 years ago

Thanks! PR to update submodule in PyTorch at https://github.com/pytorch/pytorch/pull/37083