On Mon, Jul 17, 2017 at 09:53:27AM +0800, Peter Xu wrote: > On Fri, Jul 14, 2017 at 03:28:09PM +0800, Jason Wang wrote: > > > > > > On 2017年07月14日 12:32, Peter Xu wrote: > > >On Thu, Jul 13, 2017 at 04:48:42PM +0800, Jason Wang wrote: > > >> > > >>On 2017年07月12日 16:13, Peter Xu wrote: > > >>>It is not wise to disgard all the IOTLB cache when cache size reaches > > >>>max, but that's what we do now. A slightly better (but still simple) way > > >>>to do this is, we just throw away the least recent used cache entry. > > >>> > > >>>This patch implemented MRU list algorithm for VT-d IOTLB. The main logic > > >>>is to maintain a Most Recently Used list for the IOTLB entries. The hash > > >>>table is still used for lookup, though a new list field is added to each > > >>>IOTLB entry for a iotlb MRU list. For each active IOTLB entry, it's both > > >>>in the hash table in s->iotlb, and also linked into the MRU list of > > >>>s->iotlb_head. The hash helps in fast lookup, and the MRU list helps in > > >>>knowing whether the cache is still hot. > > >>> > > >>>After we have such a MRU list, replacing all the iterators of IOTLB > > >>>entries by using list iterations rather than hash table iterations. > > >>Any reason of doing this, I thought hashtable was even a little bit > > >>faster? > > >Could I ask why? > > > > > >I thought they are merely the same (when iterating all the items)? > > > > > > > Ok, looks like I was wrong, but it they are merely the same, why bother? > > Because imho looping over list needs fewer LOCs and is also more > direct. E.g., for domain flush, hash needs this: > > static gboolean vtd_hash_remove_by_domain(gpointer key, gpointer value, > gpointer user_data) > { > VTDIOTLBEntry *entry = (VTDIOTLBEntry *)value; > uint16_t domain_id = *(uint16_t *)user_data; > return entry->domain_id == domain_id; > } > > Then: > > g_hash_table_foreach_remove(s->iotlb, vtd_hash_remove_by_domain, > &domain_id); > > For list it is only: > > FOREACH_IOTLB_SAFE(entry, s, entry_n) { > if (entry->domain_id == domain_id) { > vtd_iotlb_remove_entry(s, entry); > } > } > > Thanks,
Well the LOC seems to have gone up with this patch. If we are trying to simplify code, please state this in commit log. > -- > Peter Xu