Hi!
I have some comments regarding version 0.9a1:
1. In api_probe(), there is a _get call, then a _set call. I think
that this opens up a window of opportunity for a race, given that the
affinity can be changed by another process with same EUID or
CAP_SYS_NICE. A typical PLPA usage would involve:
api_probe()
_get
<- _set from other PID
_set
_get
(just to be clear: these are syscalls and not plpa_... functions)
The second _get call would get the "wrong" result; this would be the
value that was obtained in the first _get and not the value that was
set from the other process. Given that there is no atomic
_get_and_set, I don't think that there is any way to prevent this if
the validation via _set is desired. A situation like this could
conceivably appear in a batch environment that knows about affinity
and sets it by itself for the launched processes.
I don't know what is the current kernel status regarding on-the-fly
adding/removing CPUs and maybe cpusets, but they would be represented
by the kernel variable "cpu_online_map"; this is checked when calling
_set and might return -EINVAL, which is the same error code returned
for wrong length - so if "cpu_online_map" changes as well between the
_get and _set, the return value might be wrongly interpreted to mean
"bad length".
2. The README file suggests that PLPA detects failures in the glibc
implementations - I think that this doesn't reflect the current
version which uses syscalls and doesn't do any glibc call. There is
also a paragraph in "How do I use PLPA?" section that starts with
"These functions perform the run-time test..." which I think is also
obsolete.
3. The README file and source comments contain some typos - patch
attached. Please apply the patch before making any modifications
suggested by the comment #2.
Thanks!
--
Bogdan Costescu
IWR - Interdisziplinaeres Zentrum fuer Wissenschaftliches Rechnen
Universitaet Heidelberg, INF 368, D-69120 Heidelberg, GERMANY
Telephone: +49 6221 54 8869, Telefax: +49 6221 54 8868
E-mail: Bogdan.Costescu_at_[hidden]
|
![[Flag]](../../../../../community/mirrors/flag-us.gif){kind=small}