lkml.org 
[lkml]   [2020]   [Mar]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH v2 07/10] mmap locking API: add mmap_read_release() and mmap_read_unlock_non_owner()
On Thu, Mar 26, 2020 at 9:48 PM Davidlohr Bueso <dave@stgolabs.net> wrote:
>
> On Thu, 26 Mar 2020, Michel Lespinasse wrote:
>
> >Add a couple APIs to allow splitting mmap_read_unlock() into two calls:
> >- mmap_read_release(), called by the task that had taken the mmap lock;
> >- mmap_read_unlock_non_owner(), called from a work queue.
> >
> >These apis are used by kernel/bpf/stackmap.c only.
>
> I'm not crazy about the idea generalizing such calls into an mm api.
> We try to stay away from non-owner semantics in locking - granted
> the IS_ENABLED(CONFIG_PREEMPT_RT) warning, but still.
>
> Could this give future users the wrong impression? What about just
> using rwsem calls directly in bpf?

I see what you mean and I certainly don't want to encourage any new
non-owner call sites to appear.... This bpf stackmap site is a small
pain point in my larger range locking patchset too.

I am not sure what is the proper response to it; the opposite side of
your argument could be that using a direct rwsem call there hides the
issue and makes it less likely for someone to fix it ? I don't have a
very strong opinion on this, as I think it can be argued either way...

But at a minimum, I think it'd be worth adding a comment asking people
not to add new call sites to the mmap_read_release() and
mmap_read_unlock_non_owner() APIs ?

--
Michel "Walken" Lespinasse
A program is never fully debugged until the last user dies.

\
 
 \ /
  Last update: 2020-03-27 06:10    [W:0.097 / U:3.288 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site