On 16/05/14 19:39, Mark Brown wrote:
On Fri, May 16, 2014 at 05:34:04PM +0100, Sudeep Holla wrote:
This is broken, IIRC Lorenzo commented on this in the previous version of the patch.
Could you please be specific? Lorenzo was concerned about overflow but that ought to be addressed here.
Ah, my bad. You are right, I took his comment on the shift to be different from overflow which is not the case.
Take a simple example of system with 2 Quad core clusters. The mpidr_hash.shift_aff[1] will be 6 as you need minimum 2 bits to represent aff[0]. So you will end up with second cluster with id = 4 instead of 1.
This isn't a problem, the clusters can have any numbers so long as they are distinct. There is no other requirement.
IIUC these topology information is exposed via sysfs. So it's good to have uniformity though they can have any number. As mentioned in the example, if the linearisation depend on aff[0], then this factor will not be uniform.
I am not sure if we need this serialization, but even if we need it you can't simply use the hash bits generated for MPIDR.Aff{3..0} serialization directly as is for serializing parts of it.
Ah, now I look at what the hash is doing that is indeed directly useful
- we can mask or shift out the bits we don't want. Equally well it just
looks like a preference?
Yes we can use the hash bits, but the way it's done in this patch needs fixing so that we can be more uniform(as its exposed via sysfs)
This does seem like something that could be dealt with incrementally.
Sorry, I didn't mean to block this patch, I am just mentioning the possible issue with this patch.
Regards, Sudeep