qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH v3 4/4] intel_iommu: implement mru list for iotl


From: Peter Xu
Subject: Re: [Qemu-devel] [PATCH v3 4/4] intel_iommu: implement mru list for iotlb
Date: Thu, 27 Jul 2017 10:19:17 +0800
User-agent: Mutt/1.5.24 (2015-08-30)

On Wed, Jul 26, 2017 at 11:37:13PM +0300, Michael S. Tsirkin wrote:
> 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.

Ok. I can split the patch into two (one to introduce MRU list, one to
replace the iterations of hash tables) if it will be needed in the
future. Currently since we don't have plan to merge it, I'll put it
aside for now. Thanks.

-- 
Peter Xu



reply via email to

[Prev in Thread] Current Thread [Next in Thread]