Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 21 Apr 2023 00:41:20 +0000
From:   Carlos Llamas <cmllamas@google.com>
To:     Suren Baghdasaryan <surenb@google.com>
Cc:     linux-mm@kvack.org, tkjos@google.com, linux-kernel@vger.kernel.org,
        kernel-team@android.com
Subject: Re: Using page-fault handler in binder
Message-ID: <ZEHbsOmokBGlzNjo@google.com>
References: <ZEGh4mliGHvyWIvo@google.com>
 <CAJuCfpEGBquOjEBFD87DEHqWc0TZ8TkLHRh6RAjR=1wFPbMNHw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CAJuCfpEGBquOjEBFD87DEHqWc0TZ8TkLHRh6RAjR=1wFPbMNHw@mail.gmail.com>
Precedence: bulk

On Thu, Apr 20, 2023 at 02:35:00PM -0700, Suren Baghdasaryan wrote:
> On Thu, Apr 20, 2023 at 1:34 PM Carlos Llamas <cmllamas@google.com> wrote:
> >
> > Instead of caching page pointers, I believe binder could make use of
> > get_user_pages_remote() and trigger a page fault as needed. Basically,
> > implement something similar to access_remote_vm() but that instead does
> > a copy_from_user(). However, I don't see many in-tree users of these
> > routines, so I wonder if I'm overlooking something in this approach?
> 
> Sounds doable but there might be GUP details I'm missing...
> 
> >
> > One more reason for the page caching in binder is being able to free
> > pages from the shrinker's callback. Unused pages are added to an LRU
> > list and we manually zap/free them when they are reclaimed. It is not
> > evident to me how can binder mark a range within the vma region as
> > reclaimable?
> 
> I think you would have to release the individual pages using
> put_page(). Also note that get_user_pages_remote() is being
> deprecated, see:
> https://elixir.bootlin.com/linux/latest/source/mm/gup.c#L2171

Yeah, I remember reading that comment but it doesn't make sense since
the suggested alternatives do not support operations on _remote_ mm.

I actually tracked this down and it seems the original comment was made
for get_user_pages() instead, in commit f0818f472d8d ("mm: gup: add
get_user_pages_locked and get_user_pages_unlocked"). The whole comment
block was then carried over without update into get_user_pages_remote()
in commit 1e9877902dc7 ("mm/gup: Introduce get_user_pages_remote()").
The comment block was finally switched to use the "_remote" portion in
commit adc8cb406e52 ("mm/gup.c: update the documentation"). In reality,
this comment should be relocated to the get_user_pages() section.

> 
> >
> > Any pointers are appreciated!
> >
> > --
> > Carlos Llamas