Search squid archive

Re: FATAL: assertion failed: mem/PageStack.cc:159: "StoredNode().is_lock_free()"

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxxxxxxxxx
https://lists.squid-cache.org/listinfo/squid-users



[Index of Archives]     [Linux Audio Users]     [Samba]     [Big List of Linux Books]     [Linux USB]     [Yosemite News]

  Powered by Linux