Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20200327225102.25061-1-walken@google.com> <20200327225102.25061-7-walken@google.com>
 <20200401134234.GR20696@hirez.programming.kicks-ass.net>
In-Reply-To: <20200401134234.GR20696@hirez.programming.kicks-ass.net>
From:   Michel Lespinasse <walken@google.com>
Date:   Mon, 6 Apr 2020 08:35:03 -0700
Message-ID: <CANN689H1VjW4aC8HuLEtb8YrVyAJ0t6OVM6w2_o1+P2=9Mt6Gw@mail.gmail.com>
Subject: Re: [PATCH v3 06/10] mmap locking API: convert nested write lock sites
To:     Peter Zijlstra <peterz@infradead.org>
Cc:     Andrew Morton <akpm@linux-foundation.org>,
        linux-mm <linux-mm@kvack.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Laurent Dufour <ldufour@linux.ibm.com>,
        Vlastimil Babka <vbabka@suse.cz>,
        Matthew Wilcox <willy@infradead.org>,
        Liam Howlett <Liam.Howlett@oracle.com>,
        Jerome Glisse <jglisse@redhat.com>,
        Davidlohr Bueso <dave@stgolabs.net>,
        David Rientjes <rientjes@google.com>,
        Hugh Dickins <hughd@google.com>, Ying Han <yinghan@google.com>,
        Jason Gunthorpe <jgg@ziepe.ca>,
        Markus Elfring <Markus.Elfring@web.de>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, Apr 1, 2020 at 6:42 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Fri, Mar 27, 2020 at 03:50:58PM -0700, Michel Lespinasse wrote:
>
> > @@ -26,6 +31,12 @@ static inline void mmap_write_unlock(struct mm_struct *mm)
> >       up_write(&mm->mmap_sem);
> >  }
> >
> > +/* Pairs with mmap_write_lock_nested() */
> > +static inline void mmap_write_unlock_nested(struct mm_struct *mm)
> > +{
> > +     up_write(&mm->mmap_sem);
> > +}
> > +
> >  static inline void mmap_downgrade_write_lock(struct mm_struct *mm)
> >  {
> >       downgrade_write(&mm->mmap_sem);
>
> Why does unlock_nested() make sense ?

I thought it would make things more explicit to match the nested lock
with the corresponding unlock site; however this information is not
used at the moment (i.e. the nested unlock is implemented identically
to the regular unlock).

Having the matching sites explicitly identified may help when
implementing lock instrumentation, or when changing the lock type
(another patchset I am working on needs to pass an explicit lock range
to the nested lock and unlock sites).

I'll admit this is not a super strong argument, and can be deferred to
when an actual need shows up in the future.