On 03/07/24 21:27, Alex Rousskov wrote:
On 2024-07-03 09:27, Nishant Sharma wrote:
I was able to compile by replacing `uint64_t` to `uint32_t` and squid
worked with workers > 1.
Where did you replace uint64_t with uint32_t? In IdSet::Node typedef?
Any other changes? AFAICT, changing just IdSet::Node badly breaks the
corresponding binary tree code because we hard-code the number of bits
per leaf node (at least!) to be 64. I did not audit code for other
dependencies.
In the code for squid-6.10, in ipc/mem/PageStack.h there is just one
occurrence of `uint64_t` at line 79 inside class IdSet. I had changed
that. But now I understand that I don't have to, rolled it back.
class IdSet
{
public:
using size_type = IdSetMeasurement::size_type;
using Position = IdSetPosition;
...
...
...
private:
typedef uint64_t Node; ///< either leaf or intermediate node
typedef std::atomic<Node> StoredNode; ///< a Node stored in shared
memory
...
...
// No more data members should follow! See FlexibleArray<> for details.
};
Further discussion on Openwrt issue tracker suggested [1] the following:
It is possible that the above comment was negatively influenced by the
previous misleading statement about Squid v4 having no uint64_t: "In
code for version 4.x, there is no mention of uint64_t. It was introduced
with 5.x." In reality, Squid has been using 64-bit integers since before
Squid v3. It is neither practical nor necessary to remove uint64_t from
Squid so there will be no corresponding conditionals in ./configure.
That was due to my comment there. I actually meant to convey that I
couldn't find a reference to `uint64_t` in the PageStack.cc file as I
simply knew that the assert error message is being generated from the
code in this file.
The shared binary tree (that contains the assertion) did not exist in
v4, as we discussed earlier.
Ack. This is the correct statement that I should have used while
replying on Openwrt issue tracker.
Is there any change that we need to do in the configure script to
check for the availability of 64 bit atomic lock and use 32 bit lock
if not available?
It is technically possible (perhaps even without ./configure checks),
but it would require adjusting complex shared tree code in the abcense
of comprehensive ready-to-use tests. It is trivial to break that code.
It is difficult to detect bugs. IMO, we should not expose ourselves to
such risks in this case, especially since Squid uses 64-bit atomics in
many other places: Supporting 32 bits in shared binary tree nodes is not
going to remove the last frequently used 64-bit lock.
Just being curious here, if a certain platform (mips32 in this case) is
unable to guarantee a 64 bit atomic lock, other functions except SMP
mode might get affected as well?
Or may be document the fact that it is not advisable / possible to run
squid in SMP mode on such platforms that are not able to provide 64
bit lock ID.
I believe your experiments with removing the assertion point in a rather
different direction: If your tests do not suggest otherwise, we should
downgrade that assertion to a startup warning. Let folks run Squid on
platforms without 64-bit atomic locks (if they wish to do so), but warn
them about an uncertain impact. Perhaps we can even convince ourselves
that the impact can only be on performance (i.e., there can be no
deadlocks due to mutexes).
Disclaimer: I do not know what "lock ID" is in this context.
I am not a programmer and not very well versed with a lot of these
terms, so I have mixed / messed up while passing messages between the
two forums.
"lock ID" term was used on Openwrt issue tracker where it was suggested
that "The assertion assumes 64bit lock id.". [1]
Let me experiment with squid-6.x on these devices and also use them in
the live environment.
The only change being commenting out the following line from
ipc/mem/PageStack.c:
`assertion(StoredNode().is_lock_free());`
I will report back with success or any failures encountered.
Regards,
Nishant
[1]
"https://github.com/openwrt/packages/issues/24469#issuecomment-2202322703";
_______________________________________________
squid-users mailing list
squid-users@lists.squid-cache.org
https://lists.squid-cache.org/listinfo/squid-users
