On 10/02/2025 09:59, Ritesh Raj Sarraf wrote:
On Mon, 2025-02-10 at 04:27 +0100, Louis Sautier wrote:
It has now been ~2 weeks since I opened the issue on GitHub and
almost a
month since I emailed the author.
Could you or another maintainer check my merge request at
https://salsa.debian.org/linux-blocks-team/smp-utils/-/merge_requests/4
please? Having the option parsing code restored would be of great
help.
Hello Louis,
Hi again,
I've copied some of the other DDs who may have stake in smp-utils. I
personally no more actively maintain these packages but hope to
help/chime in, if I have time.
I've copied Douglas on this email, for what record I have.
I have no idea why you are facing this problem. The changes look fine
to me. They are just standardizing on using a more standard interface
Let me try to explain. The previous version determined the index of the
last phy to display with this very simple snippet:
num = op->do_num ? (op->phy_id + op->do_num) : MAX_PHY_ID;
if -n is passed (do_num), the index of the last phy to display was the
value passed to -p (phy_id) + the value passed to -n.
If -n was not used, the index of the last device was a constant set to
254 (I assume something caused the loop to stop before 254 because it
worked fine).
The following loop was used (still is in the newer version) to display phys:
for (k = op->phy_id; k < num; ++k) {
So, if we passed -n 5 -p 10, num would get the value 10 + 5 = 15. This
means smp_discover displayed devices starting at 10 and stopping at 15,
which was fine.
Now, the newer version initializes num at the number of phys, this is
fine when -n and -p are not used.
num = get_num_phys(top, op, &has_t2t);
The problem is this line:
num -= op->phy_id
It causes the index of the last phy to depend on the value of -p, the
index of the first phy. This seems wrong.
If we only pass -p 10, assuming a total of 30 phys, num takes the value
30 - 10 = 20, which is what I am observing in the initial bug report.
Only the first 20 phys are displayed although I only changed the value
of -p, not -n.
Then, this line breaks it even further if -n is used:
num = (num > op->do_num) ? op->do_num : num;
If we pass -p 10 -n 5, num was 20 (> 5), so it's eventually set to 5.
This causes the loop to display nothing because it attempts to iterate
from 10 to 5.
command in REPORT GENERAL. Also, that change is from almost 7 years
ago. From personal experience, as such, if it was a general issue, it'd
have been uncovered widely by now.
I have to admit that I am also surprised. Maybe smp_discover is a very
low usage package. Or maybe all its users are stuck on older versions.
The reason I filed this bug was to raise awareness of this issue after
spending hours pinpointing the problematic code.
I personally wouldn't override the current code with your proposed MR
changes, for I have limited insight of its side-effects.
I can offer access to one or more machines with an expander where the
issue occurs if it helps in any way. Feel free to email me and I'll
grant you or your fellow developers access.
For you, I guess if this change is a blocker, you could continue to use
the older versions until the cause of this anomaly is sorted out.
Sure, I can do that as a last resort, but I feel like fixing this bug in
Debian would benefit everyone.
Cheers,
Thanks,
Ritesh
