Re: [PATCH 1/2] mm/damon/sysfs: Implement recording feature

[cuiyangpei]

On Mon, Jan 22, 2024 at 09:56:11AM -0800, SeongJae Park wrote:
> Hi cuiyangpei,
> 
> On Mon, 22 Jan 2024 13:46:31 +0800 cuiyangpei <cuiyangpei@xxxxxxxxx> wrote:
> 
> > On Sun, Dec 03, 2023 at 07:37:45PM +0000, SeongJae Park wrote:
> > > On 2023-12-03T13:43:13+08:00 cuiyangpei <cuiyangpei@xxxxxxxxx> wrote:
> > > 
> > > > On Fri, Dec 01, 2023 at 05:31:12PM +0000, SeongJae Park wrote:
> > > > > Hi Cuiyangpei,
> > > > > 
> > > > > On Fri, 1 Dec 2023 20:25:07 +0800 cuiyangpei <cuiyangpei@xxxxxxxxx> wrote:
> > > > > 
> > > > > > On Thu, Nov 30, 2023 at 07:44:20PM +0000, SeongJae Park wrote:
> > > > > > > Hi Cuiyangpei,
> > > > > > > 
> > > > > > > On Thu, 30 Nov 2023 17:14:26 +0800 cuiyangpei <cuiyangpei@xxxxxxxxx> wrote:
> [...]
> > 
> > Hi SeongJae,
> > 
> > We set 'access_pattern' and 'stat' action in schemes when apps are on
> > foreground, record apps' memory access pattern when they are switched
> > to the background with 'update_schemes_tried_regions' state. But it
> > catch the snapshot after next aggregation interval. DAMON is still
> > sampling during the app switches to the background and the next
> > aggregation time, which can cause the value of "age" to change. The
> > sampling results during this period cannot accurately reflect the app's
> > foreground access pattern.
> > 
> > Is there any way to catch sampling result immediately after setting the
> > "update_schemes_tried_regions" state?
> 
> There is no way for exactly doing this.  You would need to proactively collect
> snapshots while the app is foreground, and use the latest one that collected
> before the app goes background, like recording-based approach would do.
> 
> I think recent DAMON changes might make an alternative approach available,
> though.  From v6.7, DAMON provides pseudo-moving-average monitoring result in
> sampling interval granualrity, since patchset "mm/damon: provide pseudo-moving
> sum based access rate".  And a followup patchset, namely "mm/damon: implement
> DAMOS apply intervals", has made DAMOS works in the sampling interval
> granualrity.  Both patchsets are merged into v6.7-rc1.
> 
> Hence, I think you could use 'update_schemes_tried_regions' after you noticed
> the app's state transition, with DAMOS apply interval of one sampling interval.
> Then you will get the monitoring results after one sampling interval.  Of
> course, the snapshot may contain some of background access pattern, but
> wouldn't made it changed significantly, unless you set aggregation interval too
> short.

All other actions will apply at one sampling interval except for the
`stat` action.

We use 'update_schemes_tried_regions' after switch to the background. The 
before_damos_apply callback function will only be set when the next aggregation
interval arrives. The `tried_regions` will only be updated after setting the 
callback function. 

DAMON is still sampling during setting 'update_schemes_tried_regions' to the next
aggregation time, which is not what we expected. The pseudo-moving-average
monitoring result can reduce nr_accesees inaccuracy, but age is still being modified
during this time, so it can't improve this issue.

Please let me know if my understanding is incorrect. Thank you.
> 
> > Alternatively, can it return the "last_nr_accesses" and "last_age" values in
> > tried_regions/<N> directory?
> 
> This could also be a good alternative in my think.  Nice idea.  But, because
> the previously mentioned alternative is already available while this require a
> bit small but additional changes, could we check if the previously one make
> sense and works first?  We could revisit this idea if it turns out the previous
> alternative is not suffice in my opinion.
> 
Can you consider adding "last_nr_accesses" and "last_age"  two files in
'tried_regions/<N>' directory?

Thanks.
> > 
> > Do you have any other suggestions?
> 
> As I mentioned above, I'd suggest the DAMOS apply interval of single sampling
> interval for now.
> 
> 
> Thanks,
> SJ
> 
> > 
> > Thanks.