Yep, I agree. While the new thing can be inspired by the ChunkGenerator,
unless it shares a significant chunk of implementation (not likely, there
isn't a ton of code in ChunkGenerator), they don't have to actually be
related to each other. I think particularly since in cases like the TLB
example, the returned object will likely be a temporary and so the type is
mostly irrelevant anyway.
Gabe
On Tue, Sep 8, 2020 at 9:34 AM Steve Reinhardt wrote:
> Thanks for the effort, Gabe!
>
> I totally agree that making the interface support range-based for loops
> makes sense. That would be a nice improvement.
>
> Having TLBs (or page tables?) support some kind of variable-size chunk
> loop range object that deals with multiple page sizes, maybe automatically
> merges contiguous mappings, etc. also seems pretty reasonable.
>
> I'm less sure about the need to have those two classes be related to each
> other. They would have similar (maybe even the same) interface(s), and
> conceptually similar purposes, but that's about it; would there be a use
> case where you'd have (say) a function that takes one of these "iterators"
> as an argument, and would need to use the base class to accept either
> type? On the other hand, if they were separate, you could specialize the
> interface; for example, I can see integrating the translation with the
> chunking for the TLB version and having both vaddr() and paddr() members,
> making your example above being something like:
>
> for (const auto : tlb->translateRegionByChunks(vregion_start,
> vregion_end) {
> read(buffer, chunk.paddr(), chunk.size());
> }
>
> while the simple version (for iterating over things like cache blocks)
> would just have addr() as currently.
>
> Thoughts?
>
> Steve
>
>
> On Mon, Sep 7, 2020 at 1:20 AM Gabe Black wrote:
>
>> Hi folks. In gem5, there is a simple but useful utility class called the
>> ChunkGenerator which takes a region of memory and a size, and breaks the
>> region into chunks which are broken on that size aligned boundaries.
>>
>> So for instance, if you wanted to translate every page that some big
>> array was located in in memory, you would use a ChunkGenerator with the
>> page size as the chunk size.
>>
>>
>> The problem here is that this class assumes that there is "the" page
>> size, or "the" block size to put into it, known ahead of time by the
>> consumer. This may not be true for page sizes for example, since a region
>> may fall across pages with different sizes. Or it may be a waste of effort
>> if, for instance, a region was contiguously mapped. There may also be
>> reasons to chunk up a region which are also not based on pages or other
>> fixed size boundaries.
>>
>> I think it would be a fairly simple extension to make the ChunkGenerator
>> where it would be created by some other entity which knows how big any
>> given chunk can be. For instance, there might be a call like this:
>>
>> chunk_generator = tlb->chunkRegion(region_start, region_end);
>>
>> Then you could walk through the chunks basically as you do now, but the
>> TLB would be involved and would know what boundaries to stop you at. I
>> think the biggest change that would require would be to make the "next()"
>> function virtual. It might be a good idea to break the class up into a base
>> class with generic bits, and then make the current, fixed chunk size
>> version inherit from it. Fancy variable sized versions could inherit from
>> the base without breaking existing usage.
>>
>> Also, the chunk generator has a very iterator like design currently. I
>> think it would be fairly straightforward to go all the way and give it real
>> iterators so that it can be used in a range based for loop, something like
>> this:
>>
>> for (const auto : tlb->chunkRegion(region_start, region_end) {
>> Addr paddr = translate(chunk.addr());
>> read(buffer, paddr, chunk.size());
>> }
>>
>> I think with these two changes, this class could be both more correct,
>> and a little easier to use. What do you think?
>>
>> Gabe
>>
>
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
