Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1497272

Re: [PATCH] mm: check VMA flags to avoid invalid PROT_NONE NUMA balancing

Show all headers | View raw

On Fri, Oct 7, 2016 at 3:07 AM, Lorenzo Stoakes <lstoakes@gmail.com> wrote:
>
> So I've experimented with this a little locally, removing FOLL_FORCE altogether
> and tracking places where it is used (it seems to be a fair few places
> actually.)

I'm actually a bit worried that it is used too much simply because
it's so easy to use.

In paticular, there's a class of functions that (for legacy reasons)
get the "int force" and "int write" arguments, and they both tend to
just use a very non-descript 0/1 in the callers. Then at some point
you have code like

    if (write)
        flags |= FOLL_WRITE;
    if (force)
        flags |= FOLL_FORCE;

(I'm paraphrasing from memory), and then subsequent calls use that FOLL_FORCE.

And the problem with that it's that it's *really* hard to see where
people actually use FOLL_FORCE, and it's also really easy to use it
without thinking (because it doesn't say "FOLL_FORCE", it just
randomly says "1" in some random argument list).

So the *first* step I'd do is to actually get rid of all the "write"
and "force" arguments, and just pass in "flags" instead, and use
FOLL_FORCE and FOLL_WRITE explicitly in the caller. That way it
suddenly becomes *much* easier to grep for FOLL_FORCE and see who it
is that actually really wants it.

(In fact, I think some functions get *both* "flags" and the separate
write/force argument).

Would you be willing to look at doing that kind of purely syntactic,
non-semantic cleanup first?

> I've rather naively replaced the FOLL_FORCE check in check_vma_flags() with a
> check against 'tsk && tsk->ptrace && tsk->parent == current', I'm not sure how
> valid or sane this is, however, but a quick check against gdb proves that it is
> able to do its thing in this configuration. Is this a viable path, or is this
> way off the mark here?

I think that if we end up having the FOLL_FORCE semantics, we're
actually better off having an explicit FOLL_FORCE flag, and *not* do
some kind of implicit "under these magical circumstances we'll force
it anyway". The implicit thing is what we used to do long long ago, we
definitely don't want to.

So I'd rather see all the callers that actually use FOLL_FORCE, and
then we can start looking at whether it's really a requirement. Some
of them may not strictly *need* it in actual use, they just have it
because of historical reasons (ie exactly those implicit users that
just got mindlessly converted to maintain same semantics).

                     Linus

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

Re: [PATCH] mm: check VMA flags to avoid invalid PROT_NONE NUMA  balancing Lorenzo Stoakes <lstoakes@gmail.com> - 2016-09-25 20:50 +0200
  Re: [PATCH] mm: check VMA flags to avoid invalid PROT_NONE NUMA balancing Linus Torvalds <torvalds@linux-foundation.org> - 2016-09-25 23:00 +0200
    Re: [PATCH] mm: check VMA flags to avoid invalid PROT_NONE NUMA balancing Linus Torvalds <torvalds@linux-foundation.org> - 2016-09-26 00:30 +0200
    Re: [PATCH] mm: check VMA flags to avoid invalid PROT_NONE NUMA  balancing Rik van Riel <riel@redhat.com> - 2016-09-26 00:40 +0200
      Re: [PATCH] mm: check VMA flags to avoid invalid PROT_NONE NUMA balancing Linus Torvalds <torvalds@linux-foundation.org> - 2016-09-26 01:00 +0200
        Re: [PATCH] mm: check VMA flags to avoid invalid PROT_NONE NUMA  balancing Hugh Dickins <hughd@google.com> - 2016-09-26 01:30 +0200
        Re: [PATCH] mm: check VMA flags to avoid invalid PROT_NONE NUMA  balancing Rik van Riel <riel@redhat.com> - 2016-09-26 02:50 +0200
          Re: [PATCH] mm: check VMA flags to avoid invalid PROT_NONE NUMA balancing Linus Torvalds <torvalds@linux-foundation.org> - 2016-09-26 03:10 +0200
        Re: [PATCH] mm: check VMA flags to avoid invalid PROT_NONE NUMA  balancing Lorenzo Stoakes <lstoakes@gmail.com> - 2016-10-07 12:10 +0200
          Re: [PATCH] mm: check VMA flags to avoid invalid PROT_NONE NUMA balancing Linus Torvalds <torvalds@linux-foundation.org> - 2016-10-07 17:40 +0200
            Re: [PATCH] mm: check VMA flags to avoid invalid PROT_NONE NUMA  balancing Lorenzo Stoakes <lstoakes@gmail.com> - 2016-10-07 18:30 +0200
              Re: [PATCH] mm: check VMA flags to avoid invalid PROT_NONE NUMA  balancing Hugh Dickins <hughd@google.com> - 2016-10-07 20:20 +0200
                Re: [PATCH] mm: check VMA flags to avoid invalid PROT_NONE NUMA  balancing Lorenzo Stoakes <lstoakes@gmail.com> - 2016-10-07 20:30 +0200
                Re: [PATCH] mm: check VMA flags to avoid invalid PROT_NONE NUMA  balancing Lorenzo Stoakes <lstoakes@gmail.com> - 2016-10-10 10:30 +0200
                Re: [PATCH] mm: check VMA flags to avoid invalid PROT_NONE NUMA  balancing Jan Kara <jack@suse.cz> - 2016-10-10 18:40 +0200
                Re: [PATCH] mm: check VMA flags to avoid invalid PROT_NONE NUMA  balancing Jan Kara <jack@suse.cz> - 2016-10-10 10:40 +0200

csiph-web