I noticed that it is advised to access task command name with
[gs]et_task_comm() in the comment of task_struct->comm,
which is safer with the protection of task_lock().
Relative comment in include/linux/sched.h is as follows:
/*
* executable name, excluding path.
*
* - normally initialized setup_new_exec()
* - access it with [gs]et_task_comm()
* - lock it with task_lock()
*/
Signed-off-by: Yixuan Cao <[email protected]>
---
mm/page_owner.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/mm/page_owner.c b/mm/page_owner.c
index 2743062e92c2..bda8fe2660c0 100644
--- a/mm/page_owner.c
+++ b/mm/page_owner.c
@@ -171,8 +171,7 @@ static inline void __set_page_owner_handle(struct page_ext *page_ext,
page_owner->pid = current->pid;
page_owner->tgid = current->tgid;
page_owner->ts_nsec = local_clock();
- strlcpy(page_owner->comm, current->comm,
- sizeof(page_owner->comm));
+ get_task_comm(page_owner->comm, current);
__set_bit(PAGE_EXT_OWNER, &page_ext->flags);
__set_bit(PAGE_EXT_OWNER_ALLOCATED, &page_ext->flags);
--
2.17.1
On Wed, Apr 20, 2022 at 08:28:17PM +0800, Yixuan Cao wrote:
> I noticed that it is advised to access task command name with
> [gs]et_task_comm() in the comment of task_struct->comm,
> which is safer with the protection of task_lock().
>
> Relative comment in include/linux/sched.h is as follows:
>
> /*
> * executable name, excluding path.
> *
> * - normally initialized setup_new_exec()
> * - access it with [gs]et_task_comm()
> * - lock it with task_lock()
> */
>
> Signed-off-by: Yixuan Cao <[email protected]>
> ---
> mm/page_owner.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/mm/page_owner.c b/mm/page_owner.c
> index 2743062e92c2..bda8fe2660c0 100644
> --- a/mm/page_owner.c
> +++ b/mm/page_owner.c
> @@ -171,8 +171,7 @@ static inline void __set_page_owner_handle(struct page_ext *page_ext,
> page_owner->pid = current->pid;
> page_owner->tgid = current->tgid;
> page_owner->ts_nsec = local_clock();
> - strlcpy(page_owner->comm, current->comm,
> - sizeof(page_owner->comm));
> + get_task_comm(page_owner->comm, current);
We can't call that thing here.
WARNING: inconsistent lock state
5.18.0-rc3-next-20220421-dirty #22 Not tainted
--------------------------------
inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage.
swapper/4/0 [HC0[0]:SC1[1]:HE1:SE0] takes:
ffff07ff89ad87f8 (&p->alloc_lock){+.?.}-{2:2}, at: __get_task_comm
{SOFTIRQ-ON-W} state was registered at:
__lock_acquire
lock_acquire.part.0
lock_acquire
_raw_spin_lock
__set_task_comm
kthreadd
ret_from_fork
irq event stamp: 50532
hardirqs last enabled at (50532): seqcount_lockdep_reader_access
hardirqs last disabled at (50531): seqcount_lockdep_reader_access
softirqs last enabled at (50306): __do_softirq
softirqs last disabled at (50313): __irq_exit_rcu
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0
----
lock(&p->alloc_lock);
<Interrupt>
lock(&p->alloc_lock);
*** DEADLOCK ***
1 lock held by swapper/4/0:
#0: ffffce91c3d81da0 (rcu_callback){....}-{0:0}, at: rcu_do_batch
stack backtrace:
CPU: 4 PID: 0 Comm: swapper/4 Not tainted 5.18.0-rc3-next-20220421-dirty #22
Call trace:
dump_backtrace
show_stack
dump_stack_lvl
dump_stack
print_usage_bug.part.0
mark_lock_irq
mark_lock
mark_usage
__lock_acquire
lock_acquire.part.0
lock_acquire
_raw_spin_lock
__get_task_comm
__get_task_comm at fs/exec.c:1221
__set_page_owner
arch___set_bit at include/asm-generic/bitops/non-atomic.h:22
(inlined by) __set_page_owner_handle at mm/page_owner.c:175
(inlined by) __set_page_owner at mm/page_owner.c:192
post_alloc_hook
get_page_from_freelist
__alloc_pages
alloc_pages
allocate_slab
new_slab
___slab_alloc
__slab_alloc.constprop.0
kmem_cache_alloc
fill_pool
__debug_object_init
debug_object_activate
call_rcu
put_object
__delete_object
kmemleak_free
slab_free_freelist_hook
kmem_cache_free
file_free_rcu
rcu_do_batch
rcu_core
rcu_core_si
__do_softirq
__irq_exit_rcu
irq_exit_rcu
el1_interrupt
el1h_64_irq_handler
el1h_64_irq
arch_local_irq_enable
default_idle_call
cpuidle_idle_call
do_idle
cpu_startup_entry
secondary_start_kernel
__secondary_switched