The current code always picks slot 0 in the reuse case. (This doesn't seem to matter in the cases I've tested, though.)
Signed-off-by: Alyssa Milburn fuzzie@fuzzie.org --- arch/ppc/qemu/ofmem.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/arch/ppc/qemu/ofmem.c b/arch/ppc/qemu/ofmem.c index 6f346a3..c9fe154 100644 --- a/arch/ppc/qemu/ofmem.c +++ b/arch/ppc/qemu/ofmem.c @@ -359,9 +359,11 @@ hash_page_64(unsigned long ea, phys_addr_t phys, ucell mode) found = 1;
/* otherwise use a free slot */ - for (i = 0; !found && i < 8; i++) - if (!pp[i].v) - found = 1; + if (!found) { + for (i = 0; !found && i < 8; i++) + if (!pp[i].v) + found = 1; + }
/* out of slots, just evict one */ if (!found) @@ -414,9 +416,11 @@ hash_page_32(unsigned long ea, phys_addr_t phys, ucell mode) found = 1;
/* otherwise use a free slot */ - for (i = 0; !found && i < 8; i++) - if (!pp[i].v) - found = 1; + if (!found) { + for (i = 0; !found && i < 8; i++) + if (!pp[i].v) + found = 1; + }
/* out of slots, just evict one */ if (!found)
On 10/02/16 23:25, Alyssa Milburn wrote:
The current code always picks slot 0 in the reuse case. (This doesn't seem to matter in the cases I've tested, though.)
Signed-off-by: Alyssa Milburn fuzzie@fuzzie.org
arch/ppc/qemu/ofmem.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/arch/ppc/qemu/ofmem.c b/arch/ppc/qemu/ofmem.c index 6f346a3..c9fe154 100644 --- a/arch/ppc/qemu/ofmem.c +++ b/arch/ppc/qemu/ofmem.c @@ -359,9 +359,11 @@ hash_page_64(unsigned long ea, phys_addr_t phys, ucell mode) found = 1;
/* otherwise use a free slot */
- for (i = 0; !found && i < 8; i++)
if (!pp[i].v)found = 1;
if (!found) {
for (i = 0; !found && i < 8; i++)if (!pp[i].v)found = 1;}
/* out of slots, just evict one */ if (!found)
@@ -414,9 +416,11 @@ hash_page_32(unsigned long ea, phys_addr_t phys, ucell mode) found = 1;
/* otherwise use a free slot */
- for (i = 0; !found && i < 8; i++)
if (!pp[i].v)found = 1;
if (!found) {
for (i = 0; !found && i < 8; i++)if (!pp[i].v)found = 1;}
/* out of slots, just evict one */ if (!found)
Hmmm yes I think you're right there is a bug here. Assuming that we locate the index of an old translation, then i should be set to the slot number to replace. But even though found has been set, the subsequent for() will always reset i to zero, even though it exits without iterating.
In theory this could mean that we have multiple entries in the hash table for the same translation which can't be a good thing.
Alex - does this look like the right solution to you?
ATB,
Mark.
On 02/12/2016 12:38 PM, Mark Cave-Ayland wrote:
On 10/02/16 23:25, Alyssa Milburn wrote:
The current code always picks slot 0 in the reuse case. (This doesn't seem to matter in the cases I've tested, though.)
Signed-off-by: Alyssa Milburn fuzzie@fuzzie.org
arch/ppc/qemu/ofmem.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/arch/ppc/qemu/ofmem.c b/arch/ppc/qemu/ofmem.c index 6f346a3..c9fe154 100644 --- a/arch/ppc/qemu/ofmem.c +++ b/arch/ppc/qemu/ofmem.c @@ -359,9 +359,11 @@ hash_page_64(unsigned long ea, phys_addr_t phys, ucell mode) found = 1;
/* otherwise use a free slot */
- for (i = 0; !found && i < 8; i++)
if (!pp[i].v)found = 1;
if (!found) {
for (i = 0; !found && i < 8; i++)if (!pp[i].v)found = 1;}
/* out of slots, just evict one */ if (!found)
@@ -414,9 +416,11 @@ hash_page_32(unsigned long ea, phys_addr_t phys, ucell mode) found = 1;
/* otherwise use a free slot */
- for (i = 0; !found && i < 8; i++)
if (!pp[i].v)found = 1;
if (!found) {
for (i = 0; !found && i < 8; i++)if (!pp[i].v)found = 1;}
/* out of slots, just evict one */ if (!found)
Hmmm yes I think you're right there is a bug here. Assuming that we locate the index of an old translation, then i should be set to the slot number to replace. But even though found has been set, the subsequent for() will always reset i to zero, even though it exits without iterating.
In theory this could mean that we have multiple entries in the hash table for the same translation which can't be a good thing.
Yes, on real hardware machine checks can result from this.
Alex - does this look like the right solution to you?
Looks reasonable to me.
Alex
On 13/02/16 20:09, Alexander Graf wrote:
On 02/12/2016 12:38 PM, Mark Cave-Ayland wrote:
On 10/02/16 23:25, Alyssa Milburn wrote:
The current code always picks slot 0 in the reuse case. (This doesn't seem to matter in the cases I've tested, though.)
Signed-off-by: Alyssa Milburn fuzzie@fuzzie.org
arch/ppc/qemu/ofmem.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/arch/ppc/qemu/ofmem.c b/arch/ppc/qemu/ofmem.c index 6f346a3..c9fe154 100644 --- a/arch/ppc/qemu/ofmem.c +++ b/arch/ppc/qemu/ofmem.c @@ -359,9 +359,11 @@ hash_page_64(unsigned long ea, phys_addr_t phys, ucell mode) found = 1; /* otherwise use a free slot */
- for (i = 0; !found && i < 8; i++)
if (!pp[i].v)found = 1;
- if (!found) {
for (i = 0; !found && i < 8; i++)if (!pp[i].v)found = 1;- } /* out of slots, just evict one */ if (!found)
@@ -414,9 +416,11 @@ hash_page_32(unsigned long ea, phys_addr_t phys, ucell mode) found = 1; /* otherwise use a free slot */
- for (i = 0; !found && i < 8; i++)
if (!pp[i].v)found = 1;
- if (!found) {
for (i = 0; !found && i < 8; i++)if (!pp[i].v)found = 1;- } /* out of slots, just evict one */ if (!found)
Hmmm yes I think you're right there is a bug here. Assuming that we locate the index of an old translation, then i should be set to the slot number to replace. But even though found has been set, the subsequent for() will always reset i to zero, even though it exits without iterating.
In theory this could mean that we have multiple entries in the hash table for the same translation which can't be a good thing.
Yes, on real hardware machine checks can result from this.
Alex - does this look like the right solution to you?
Looks reasonable to me.
Thanks! Would you like to commit for a change? I get bored of seeing my own name in the commit logs all the time ;)
ATB,
Mark.
-
Alexander Graf -
Alyssa Milburn -
Mark Cave-Ayland