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;
Content-Type: text/plain;
        charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 11.2 \(3445.5.20\))
Subject: Re: [RFC PATCH] Take mmap_min_addr into account while choosing
 unmapped address for x86-64.
From:   Ilya Smith <blackzert@gmail.com>
In-Reply-To: <20180227085726.GC15357@dhcp22.suse.cz>
Date:   Tue, 27 Feb 2018 16:27:29 +0300
Cc:     tglx@linutronix.de, mingo@redhat.com, hpa@zytor.com,
        x86@kernel.org, kirill.shutemov@linux.intel.com,
        dsafonov@virtuozzo.com, hughd@google.com,
        gregkh@linuxfoundation.org, craigb@google.com, oleg@redhat.com,
        linux-kernel@vger.kernel.org
Content-Transfer-Encoding: quoted-printable
Message-Id: <B6A5E9B0-4986-4CD4-B01A-B6218D859579@gmail.com>
References: <20180226211257.63067-1-blackzert@gmail.com>
 <20180227085726.GC15357@dhcp22.suse.cz>
To:     Michal Hocko <mhocko@kernel.org>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

>=20
> mmap_min_addr handling is a bit mess... As you say, we would return
> EPERM rather than ENOMEM which can be confusing but depleting the
> address space like that is quite unlikely on 64b unless I am missing.
> It is good to be in sync here with the generic implementation though,
> IMO.
>=20

If we take a look on mm/mmap.c:
#ifndef HAVE_ARCH_UNMAPPED_AREA_TOPDOWN
unsigned long
arch_get_unmapped_area_topdown(
=E2=80=A6
	if (len > TASK_SIZE - mmap_min_addr)
		return -ENOMEM;
=E2=80=A6
	info.low_limit =3D max(PAGE_SIZE, mmap_min_addr);

And this one looks like a generic implementation.
But for many other architectures like arch/parisc/kernel/sys_parisc.c=20
or arch/x86/kernel/sys_x86_64.c

	info.low_limit =3D PAGE_SIZE;

What is looks like an issue for me.

Here is C code could be used as test-case:

#include <errno.h>
#include <stdio.h>
#include <sys/mman.h>

int main() {
  char buffer[1024];

  unsigned long len =3D 1ULL << 46;
  while(len) {
    void *ptr =3D mmap(4096, len, 0, MAP_PRIVATE | MAP_ANONYMOUS , -1, =
0);
    if (ptr =3D=3D MAP_FAILED) {
      if (errno =3D=3D EPERM)
        break;
      if (errno =3D=3D ENOMEM) {
        len >>=3D 1;
        continue;
      }
      return -1;
    }
  }
  if (errno =3D=3D EPERM) {
    printf("Test failed, you have wrong ret code EPERM\n");
    sprintf(buffer, "cat /proc/%d/maps", getpid());
    system(buffer);
    return -1;
  }
  return 0;
}


>>=20
>> +	if (addr < mmap_min_addr)
>> +		return false;
>> +
>> 	return (addr > DEFAULT_MAP_WINDOW) =3D=3D (addr + len > =
DEFAULT_MAP_WINDOW);
>=20
> But is this one necessary? We do sanitze hint address before going to
> get_unmapped_address AFAIR.
>=20
I=E2=80=99m agree, looks like I was trying to fix something that already =
fine.

Thanks,
Ilya