From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf0-f197.google.com (mail-pf0-f197.google.com [209.85.192.197]) by kanga.kvack.org (Postfix) with ESMTP id EC34E6B0038 for ; Wed, 10 May 2017 18:42:53 -0400 (EDT) Received: by mail-pf0-f197.google.com with SMTP id h87so7253125pfh.2 for ; Wed, 10 May 2017 15:42:53 -0700 (PDT) Received: from mail.kernel.org (mail.kernel.org. [198.145.29.136]) by mx.google.com with ESMTPS id b75si323399pfe.49.2017.05.10.15.42.53 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 10 May 2017 15:42:53 -0700 (PDT) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id A3CE52034A for ; Wed, 10 May 2017 22:42:51 +0000 (UTC) Received: from mail-ua0-f170.google.com (mail-ua0-f170.google.com [209.85.217.170]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 3EE45202EC for ; Wed, 10 May 2017 22:42:49 +0000 (UTC) Received: by mail-ua0-f170.google.com with SMTP id e28so11801707uah.0 for ; Wed, 10 May 2017 15:42:49 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <20170510082425.5ks5okbjne7xgjtv@gmail.com> References: <1a124281c99741606f1789140f9805beebb119da.1494160201.git.luto@kernel.org> <20170510055727.g6wojjiis36a6nvm@gmail.com> <20170510082425.5ks5okbjne7xgjtv@gmail.com> From: Andy Lutomirski Date: Wed, 10 May 2017 15:42:27 -0700 Message-ID: Subject: Re: [RFC 09/10] x86/mm: Rework lazy TLB to track the actual loaded mm Content-Type: text/plain; charset=UTF-8 Sender: owner-linux-mm@kvack.org List-ID: To: Ingo Molnar Cc: Thomas Gleixner , Andy Lutomirski , X86 ML , "linux-kernel@vger.kernel.org" , Borislav Petkov , Linus Torvalds , Andrew Morton , Mel Gorman , "linux-mm@kvack.org" , Rik van Riel , Dave Hansen , Nadav Amit , Michal Hocko , Arjan van de Ven On Wed, May 10, 2017 at 1:24 AM, Ingo Molnar wrote: > > * Thomas Gleixner wrote: > >> On Wed, 10 May 2017, Ingo Molnar wrote: >> > >> > * Thomas Gleixner wrote: >> > >> > > On Sun, 7 May 2017, Andy Lutomirski wrote: >> > > > /* context.lock is held for us, so we don't need any locking. */ >> > > > static void flush_ldt(void *current_mm) >> > > > { >> > > > + struct mm_struct *mm = current_mm; >> > > > mm_context_t *pc; >> > > > >> > > > - if (current->active_mm != current_mm) >> > > > + if (this_cpu_read(cpu_tlbstate.loaded_mm) != current_mm) >> > > >> > > While functional correct, this really should compare against 'mm'. >> > > >> > > > return; >> > > > >> > > > - pc = ¤t->active_mm->context; >> > > > + pc = &mm->context; >> > >> > So this appears to be the function: >> > >> > static void flush_ldt(void *current_mm) >> > { >> > struct mm_struct *mm = current_mm; >> > mm_context_t *pc; >> > >> > if (this_cpu_read(cpu_tlbstate.loaded_mm) != current_mm) >> > return; >> > >> > pc = &mm->context; >> > set_ldt(pc->ldt->entries, pc->ldt->size); >> > } >> > >> > why not rename 'current_mm' to 'mm' and remove the 'mm' local variable? >> >> Because you cannot dereference a void pointer, i.e. &mm->context .... > > Indeed, doh! The naming totally confused me. The way I'd write it is the canonical > form for such callbacks: > > static void flush_ldt(void *data) > { > struct mm_struct *mm = data; > > ... which beyond unconfusing me would probably also have prevented any accidental > use of the 'current_mm' callback argument. > > void *data and void *info both seem fairly common in the kernel. How about my personal favorite for non-kernel work, though: void *mm_void? It documents what the parameter means and avoids the confusion. --Andy -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org