On Mon, 2015-03-23 at 19:08 -0400, Sowmini Varadhan wrote: > > Sowmini, I see various options for the second choice. We could stick to > > 1 pool, and basically do as before, ie, if we fail on the first pass of > > alloc, it means we wrap around and do a flush, I don't think that will > > cause a significant degradation from today, do you ? We might have an > > occasional additional flush but I would expect it to be in the noise. > > Isn't this essentially what I have in patch v5 here: > http://www.spinics.net/lists/sparclinux/msg13534.html
Possibly, I haven't looked at it in details yet. My point is that while we might flush a bit more often than the original code, it should remain in the noise performance-wise and thus David might rescind his objection, but we need to prove this with a measurement. > (the ops->reset is the flushall indirection, can be renamed if the > latter is preferred) > > > Dave, what's your feeling there ? Does anybody around still have some > > HW that we can test with ? > > I actually tested this on a V440 and a ultra45 (had a heck of a > time finding these, since the owners keep them turned off because > they are too noisy and consume too much power :-). Thus while I > have no opinion, I would not shed any tears if we lost this extra > perf-tweak in the interest of being earth-friendly :-)) > > so testing it is not a problem, though I dont have any perf > benchmarks for them either. That's what we'll need unfortunately to confirm our "gut feeling". It might be sufficient to add a flush counter and compare it between runs if actual wall-clock benchmarks are too hard to do (especially if you don't have things like very fast network cards at hand). Number of flush / number of packets might be a sufficient metric, it depends on whether it makes David happy :-) > > Sowmini, I think we can still kill the ops and have a separate data > > structure exclusively concerned by allocations by having the alloc > > functions take the lazy flush function as an argument (which can be > > NULL), I don't think we should bother with ops. > > I dont quite follow what you have in mind? The caller would somehow > have to specify a flush_all indirection for the legacy platforms Yes, pass a function pointer argument that can be NULL or just make it a member of the iommu_allocator struct (or whatever you call it) passed to the init function and that can be NULL. My point is we don't need a separate "ops" structure. > Also, you mention > > > You must hold the lock until you do the flush, otherwise somebody > > else might allocate the not-yet-flushed areas and try to use them... > > kaboom. However if that's the only callback left, pass it as an > > argument. > > Passing in a function pointer to the flushall to iommu_tbl_range_alloc > would work, or we could pass it in as an arg to iommu_tbl_init, > and stash it in the struct iommu_table. Pass it to init and stash it in the table but don't call it "iommu_table", let's use a name that conveys better the fact that this is purely a DMA space allocator (to be embedded by the arch specific iommu table). Something like iommu_alloc_zone or whatever you want to call it. I keep finding new names whenever I think of it :-) Cheers, Ben. _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
