On Wed, 23 Dec 2020 14:49:57 -0800 Shakeel Butt <shakeelb@xxxxxxxxxx> wrote: > On Wed, Dec 23, 2020 at 8:34 AM SeongJae Park <sjpark@xxxxxxxxxx> wrote: > [snip] > > > Overall the patch looks good to me. Two concerns I have are if we > > > should damon_callback here or with the real user and the regions part > > > of primitive abstraction. For the first one, I don't have any strong > > > opinion but for the second one I do. > > > > I'd like to keep 'damon_callback' part here, to let API users know how the > > monitoring result will be available to them. > > > > For the 'regions' part, I will rename relevant things as below in the next > > version, to reduce any confusion. > > > > init_target_regions() -> init() > > update_target_regions() -> update() > > regions_update_interval -> update_interval > > last_regions_update -> last_update > > > > > > > > More specifically the question is if sampling and adaptive region > > > adjustment are general enough to be part of core monitoring context? > > > Can you give an example of a different primitive/use-case where these > > > would be beneficial. > > > > I think all adress spaces having some spatial locality and monitoring requests > > that need to have upper-bound overhead and best-effort accuracy could get > > benefit from it. The primitives targetting 'virtual address spaces' and the > > 'physical address space' clearly showed the benefit. > > I am still not much convinced on the 'physical address space' use-case > or the way you are presenting it. I understand the concern. I also once thought the mechanism might not work well for the physical address space because we cannot expect much spatial locality in the space. However, it turned out that there is some (temporal) spatial locality that enough to make DAMON work reasonably well. The word, 'reasonably well' might be controversial. With the mechanism, DAMON provides only 'best-effort' accuracy, rather than 100% accuracy. Our goal is to make the information accurate enough only for DRAM-centric optimizations. I'd like to also note that there are knobs that you can use to make minimum quality higher (nr_min_regions) while setting the upperbound of the monitoring overhead (nr_max_regions). What I can say for now is that we ran DAMON for physical address space of our production systems (shared detail in the 'Real-workd User Story' section of coverletter[1]) and the result was reasonable enough to convince the owner of the systems. [1] https://lore.kernel.org/linux-mm/20201215115448.25633-1-sjpark@xxxxxxxxxx/ > Anyways I think we start with what you have and if in future there is a > use-case where regions adjustment does not make sense, we can change it then. 100% agreed, and thank you for understanding my argument. Thanks, SeongJae Park
