Re: [PATCH 3/3] samples/damon: fix bugs for damon sample for start failures

[Honggyu Kim <honggyu.kim@sk.com>]

Hi SeongJae,

On 6/23/2025 1:29 AM, SeongJae Park wrote:
> Hi Honggyu,
> 
> On Sun, 22 Jun 2025 21:09:25 +0900 Honggyu Kim <honggyu.kim@sk.com> wrote:
> 
>> The damon_sample_{wsse,prcl,mtier}_start() can fail so we must reset the
>> "enabled" parameter to "false" again for proper rollback.
>>
>> In such cases, setting Y to "enabled" then N triggers the following
>> crash because damon sample start failed but the "enabled" stays as Y.
>>
>>    [ 2441.419649] damon_sample_prcl: start
>>    [ 2454.146817] damon_sample_prcl: stop
>>    [ 2454.146862] ------------[ cut here ]------------
>>    [ 2454.146865] kernel BUG at mm/slub.c:546!
>>    [ 2454.148183] Oops: invalid opcode: 0000 [#1] SMP NOPTI
>>        ...
>>    [ 2454.167555] Call Trace:
>>    [ 2454.167822]  <TASK>
>>    [ 2454.168061]  damon_destroy_ctx+0x78/0x140
>>    [ 2454.168454]  damon_sample_prcl_enable_store+0x8d/0xd0
>>    [ 2454.168932]  param_attr_store+0xa1/0x120
>>    [ 2454.169315]  module_attr_store+0x20/0x50
>>    [ 2454.169695]  sysfs_kf_write+0x72/0x90
>>    [ 2454.170065]  kernfs_fop_write_iter+0x150/0x1e0
>>    [ 2454.170491]  vfs_write+0x315/0x440
>>    [ 2454.170833]  ksys_write+0x69/0xf0
>>    [ 2454.171162]  __x64_sys_write+0x19/0x30
>>    [ 2454.171525]  x64_sys_call+0x18b2/0x2700
>>    [ 2454.171900]  do_syscall_64+0x7f/0x680
>>    [ 2454.172258]  ? exit_to_user_mode_loop+0xf6/0x180
>>    [ 2454.172694]  ? clear_bhb_loop+0x30/0x80
>>    [ 2454.173067]  ? clear_bhb_loop+0x30/0x80
>>    [ 2454.173439]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
> 
> Thank you for finding and fixing this!
> 
>>
>> Since it's just a sample module, no need to add it to stable tree.
> 
> I agree this is not a real bug.  But, even a bug in DAMON unit test was
> deserved to get a CVE id.  Hence I'd suggest adding at least Fixes: tag here,
> and let stable kernels maintainers to decide whether to pick this up to stable
> kernels or not.  I can help backporting if needed.
> 
> If you agree, I think we could add
> 
> Fixes: b757c6cfc696d ("samples/damon/wsse: start and stop DAMON as the user requests")
> 
> If we want to make stable kernel maintainers' life easier, we would better to
> split this into three patches, for each module, and add appropriate Fixes, as
> below.

Spliting this into three patches is okay.

> 
> For wsse,
> Fixes: b757c6cfc696d ("samples/damon/wsse: start and stop DAMON as the user requests")
> 
> For prcl,
> 
> Fixes: 2aca254620a8 ("samples/damon: introduce a skeleton of a smaple DAMON module for proactive reclamation")
> 
> For mtier,
> 
> Fixes: 82a08bde3cf7 ("samples/damon: implement a DAMON module for memory tiering")
> 
> Could you please do so?

If we do, then this patchset has to come before "enabled" renaming patch.

In addition, I remember mixing stable and non-stable patches together in the
same patchset isn't welcomed by Andrew or other stable tree maintainers.  So I
might be better to split those three fix patches into a separate patchset.

The only thing that bothers me in that case is writing cover letter for simple 2
or 3 patches as they are relatively simple.

But it isn't that hard so can manage those in the next spin.

> 
>>
>> Signed-off-by: Honggyu Kim <honggyu.kim@sk.com>
>> Cc: Yunjeong Mun <yunjeong.mun@sk.com>
> 
> Regardless of your agreement on adding Fixes:
> 
> Reviewed-by: SeongJae Park <sj@kernel.org>
> 
> 
> Thanks,
> SJ

Thanks,
Honggyu

> 
> [...]