On (03/24/15 09:21), Benjamin Herrenschmidt wrote: > > So we have two choices here that I can see: > > - Keep that old platform use the old/simpler allocator Problem with that approach is that the base "struct iommu" structure for sparc gets a split personality: the older one is used with the older allocator, and other ugly things ensue. (alternatively, you end up duplicating a version of the code with the flush_all inlined). > - Try to regain the bulk of that benefit with the new one > > 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 (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. > 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 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. --Sowmini -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html