lkml.org 
[lkml]   [2024]   [Mar]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [RFC 03/14] fork: Clean-up naming of vm_strack/vm_struct variables in vmap stacks code
    From
    Le 11/03/2024 à 17:46, Pasha Tatashin a écrit :
    > There are two data types: "struct vm_struct" and "struct vm_stack" that
    > have the same local variable names: vm_stack, or vm, or s, which makes
    > code confusing to read.
    >
    > Change the code so the naming is consisent:

    Nit: consistent

    >
    > struct vm_struct is always called vm_area
    > struct vm_stack is always called vm_stack
    >
    > Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>
    > ---
    > kernel/fork.c | 38 ++++++++++++++++++--------------------
    > 1 file changed, 18 insertions(+), 20 deletions(-)
    >
    > diff --git a/kernel/fork.c b/kernel/fork.c
    > index 32600bf2422a..60e812825a7a 100644
    > --- a/kernel/fork.c
    > +++ b/kernel/fork.c
    > @@ -192,12 +192,12 @@ struct vm_stack {
    > struct vm_struct *stack_vm_area;
    > };
    >
    > -static bool try_release_thread_stack_to_cache(struct vm_struct *vm)
    > +static bool try_release_thread_stack_to_cache(struct vm_struct *vm_area)
    > {
    > unsigned int i;
    >
    > for (i = 0; i < NR_CACHED_STACKS; i++) {
    > - if (this_cpu_cmpxchg(cached_stacks[i], NULL, vm) != NULL)
    > + if (this_cpu_cmpxchg(cached_stacks[i], NULL, vm_area) != NULL)
    > continue;
    > return true;
    > }
    > @@ -207,11 +207,12 @@ static bool try_release_thread_stack_to_cache(struct vm_struct *vm)
    > static void thread_stack_free_rcu(struct rcu_head *rh)
    > {
    > struct vm_stack *vm_stack = container_of(rh, struct vm_stack, rcu);
    > + struct vm_struct *vm_area = vm_stack->stack_vm_area;
    >
    > if (try_release_thread_stack_to_cache(vm_stack->stack_vm_area))
    > return;
    >
    > - vfree(vm_stack);
    > + vfree(vm_area->addr);

    This does not look like only a renaming of a variable. Is it?

    If no, should there be a Fixes tag and should it be detailed in the
    commit description?

    CJ

    > }
    >
    > static void thread_stack_delayed_free(struct task_struct *tsk)
    > @@ -228,12 +229,12 @@ static int free_vm_stack_cache(unsigned int cpu)
    > int i;
    >
    > for (i = 0; i < NR_CACHED_STACKS; i++) {
    > - struct vm_struct *vm_stack = cached_vm_stacks[i];
    > + struct vm_struct *vm_area = cached_vm_stacks[i];
    >
    > - if (!vm_stack)
    > + if (!vm_area)
    > continue;
    >
    > - vfree(vm_stack->addr);
    > + vfree(vm_area->addr);
    > cached_vm_stacks[i] = NULL;
    > }
    >
    > @@ -263,32 +264,29 @@ static int memcg_charge_kernel_stack(struct vm_struct *vm)
    >
    > static int alloc_thread_stack_node(struct task_struct *tsk, int node)
    > {
    > - struct vm_struct *vm;
    > + struct vm_struct *vm_area;
    > void *stack;
    > int i;
    >
    > for (i = 0; i < NR_CACHED_STACKS; i++) {
    > - struct vm_struct *s;
    > -
    > - s = this_cpu_xchg(cached_stacks[i], NULL);
    > -
    > - if (!s)
    > + vm_area = this_cpu_xchg(cached_stacks[i], NULL);
    > + if (!vm_area)
    > continue;
    >
    > /* Reset stack metadata. */
    > - kasan_unpoison_range(s->addr, THREAD_SIZE);
    > + kasan_unpoison_range(vm_area->addr, THREAD_SIZE);
    >
    > - stack = kasan_reset_tag(s->addr);
    > + stack = kasan_reset_tag(vm_area->addr);
    >
    > /* Clear stale pointers from reused stack. */
    > memset(stack, 0, THREAD_SIZE);
    >
    > - if (memcg_charge_kernel_stack(s)) {
    > - vfree(s->addr);
    > + if (memcg_charge_kernel_stack(vm_area)) {
    > + vfree(vm_area->addr);
    > return -ENOMEM;
    > }
    >
    > - tsk->stack_vm_area = s;
    > + tsk->stack_vm_area = vm_area;
    > tsk->stack = stack;
    > return 0;
    > }
    > @@ -306,8 +304,8 @@ static int alloc_thread_stack_node(struct task_struct *tsk, int node)
    > if (!stack)
    > return -ENOMEM;
    >
    > - vm = find_vm_area(stack);
    > - if (memcg_charge_kernel_stack(vm)) {
    > + vm_area = find_vm_area(stack);
    > + if (memcg_charge_kernel_stack(vm_area)) {
    > vfree(stack);
    > return -ENOMEM;
    > }
    > @@ -316,7 +314,7 @@ static int alloc_thread_stack_node(struct task_struct *tsk, int node)
    > * free_thread_stack() can be called in interrupt context,
    > * so cache the vm_struct.
    > */
    > - tsk->stack_vm_area = vm;
    > + tsk->stack_vm_area = vm_area;
    > stack = kasan_reset_tag(stack);
    > tsk->stack = stack;
    > return 0;


    \
     
     \ /
      Last update: 2024-05-27 15:52    [W:4.056 / U:0.348 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site