<html><head><meta http-equiv="content-type" content="text/html; charset=utf-8"></head><body style="overflow-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;"><br><div><br><blockquote type="cite"><div>2024年6月25日 16:25，Jan Beulich <jbeulich@suse.com> 写道：</div><br class="Apple-interchange-newline"><div><div>On 25.06.2024 06:31, Nelson Chu wrote:<br><blockquote type="cite">On Fri, Jun 21, 2024 at 9:25 AM <lifang_xia@linux.alibaba.com> wrote:<br><br><blockquote type="cite">From: Lifang Xia <lifang_xia@linux.alibaba.com><br><br>When the same address across different segments needs to be recorded, it<br>will<br>overwrite the slot, leading to a memory leak. To ensure uniqueness, the<br>segment ID needs to be included in the hash key calculation.<br><br></blockquote><br>Yeah, this makes sense.<br><br><br><blockquote type="cite">@@ -1798,7 +1799,13 @@ static hashval_t<br> riscv_pcrel_fixup_hash (const void *entry)<br> {<br> const riscv_pcrel_hi_fixup *e = entry;<br>- return (hashval_t) (e->address);<br>+<br>+ /* the pcrel_hi with same address may reside in different segments,<br>+ to ensure uniqueness, the segment ID needs to be included in the<br>+ hash key calculation.<br>+ There is no hash for two integer, refer to ctf_hash_integer. */<br>+ return htab_hash_pointer ((void *)(uintptr_t)e->seg->id)<br>+ + 59 * htab_hash_pointer ((void *)(uintptr_t)e->address);<br> }<br><br></blockquote><br>The riscv_segment_info_type is used to record target stuff for special<br>works. We used to record mapping symbol stuff there. Maybe we can have a<br>pcrel_hi hash table with address as key, in each riscv_segment_info_type if<br>there is/are pcrel_hi instructions, and have another summarized hash table<br>with segment id as key. I am not familiar with the ctf_hash_integer stuff,<br>so not sure which solution is better. I think Alan, Nick and Jan know<br>these better since they are experts, so cc them, and hope they may have<br>time to give us some suggestions ;)<br><br><br>Hi Alan, Nick, Jan,<br><br>We met the problem that there are no internal hash tables that can use two<br>keys - one key is the segment id, the other is the riscv auipc (pcrel hi)<br>instruction address in the segment. Do you think it's fine to refer to how<br>ctf_hash_integer works in this patch? Or we should have multiple hash<br>tables for each segment in riscv_segment_info_type if there are auipc<br>instructions (auipc address of segment as key), and have another one hash<br>table to collect the segment (segment id as key) which has any auipc.<br></blockquote><br>In principle that's fine, I think, just that the reference to<br>ctf_hash_integer() seems misleading. The two similar CTF functions are<br>ctf_hash_type_key() and ctf_hash_type_id_key(), afaics.<br></div></div></blockquote>Em.. Sorry, It’s a mistake. I went to comment with “ctf_hash_type_key”.</div><div><br></div><div><blockquote type="cite"><div><div><br>I don't really understand, though, why htab_hash_pointer() wants / needs<br>using here. Wouldn't (deriving from what was there before)<br>"e->address + 59 * e->seg->id" suffice (multiplier subject to possible<br>improvement), and be less of a behavioral change?<br></div></div></blockquote><div><span style="color: rgba(0, 0, 0, 0.78); font-family: "DM Sans", -apple-system, system-ui, "Segoe UI", Helvetica, Arial, sans-serif, "Apple Color Emoji", "Segoe UI Emoji"; font-size: 14px; font-variant-ligatures: normal; orphans: 2; widows: 2; background-color: rgb(255, 255, 255); text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial;">"e->address + 59 * e->seg->id” would be fine. It's just necessary </span></div><div><span style="color: rgba(0, 0, 0, 0.78); font-family: "DM Sans", -apple-system, system-ui, "Segoe UI", Helvetica, Arial, sans-serif, "Apple Color Emoji", "Segoe UI Emoji"; font-size: 14px; font-variant-ligatures: normal; orphans: 2; widows: 2; background-color: rgb(255, 255, 255); text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial;">to ensure uniqueness within a finite range. </span></div><div><span style="color: rgba(0, 0, 0, 0.78); font-family: "DM Sans", -apple-system, system-ui, "Segoe UI", Helvetica, Arial, sans-serif, "Apple Color Emoji", "Segoe UI Emoji"; font-size: 14px; font-variant-ligatures: normal; orphans: 2; widows: 2; background-color: rgb(255, 255, 255); text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial;">As for the multiplier, do you have any suggestions? I feel that 59 might be</span></div><div><span style="color: rgba(0, 0, 0, 0.78); font-family: "DM Sans", -apple-system, system-ui, "Segoe UI", Helvetica, Arial, sans-serif, "Apple Color Emoji", "Segoe UI Emoji"; font-size: 14px; font-variant-ligatures: normal; orphans: 2; widows: 2; background-color: rgb(255, 255, 255); text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial;">too small. </span><span style="color: rgba(0, 0, 0, 0.78); font-family: "DM Sans", -apple-system, system-ui, "Segoe UI", Helvetica, Arial, sans-serif, "Apple Color Emoji", "Segoe UI Emoji"; font-size: 14px; orphans: 2; widows: 2; background-color: rgb(255, 255, 255);">Is 127 a better choice?</span></div><blockquote type="cite"><div><div>Jan</div></div></blockquote></div><br></body></html>