* Re: [PATCH V5 0/7] Allow user to request memory to be locked on page fault
From: Michal Hocko @ 2015-07-28 11:17 UTC (permalink / raw)
To: Eric B Munson
Cc: Vlastimil Babka, Andrew Morton, Shuah Khan, Michael Kerrisk,
Jonathan Corbet, Ralf Baechle, linux-alpha, linux-kernel,
linux-mips, linux-parisc, linuxppc-dev, sparclinux, linux-xtensa,
linux-mm, linux-arch, linux-api
In-Reply-To: <20150727145409.GB21664@akamai.com>
[I am sorry but I didn't get to this sooner.]
On Mon 27-07-15 10:54:09, Eric B Munson wrote:
> Now that VM_LOCKONFAULT is a modifier to VM_LOCKED and
> cannot be specified independentally, it might make more sense to mirror
> that relationship to userspace. Which would lead to soemthing like the
> following:
A modifier makes more sense.
> To lock and populate a region:
> mlock2(start, len, 0);
>
> To lock on fault a region:
> mlock2(start, len, MLOCK_ONFAULT);
>
> If LOCKONFAULT is seen as a modifier to mlock, then having the flags
> argument as 0 mean do mlock classic makes more sense to me.
>
> To mlock current on fault only:
> mlockall(MCL_CURRENT | MCL_ONFAULT);
>
> To mlock future on fault only:
> mlockall(MCL_FUTURE | MCL_ONFAULT);
>
> To lock everything on fault:
> mlockall(MCL_CURRENT | MCL_FUTURE | MCL_ONFAULT);
Makes sense to me. The only remaining and still tricky part would be
the munlock{all}(flags) behavior. What should munlock(MLOCK_ONFAULT)
do? Keep locked and poppulate the range or simply ignore the flag an
just unlock?
I can see some sense to allow munlockall(MCL_FUTURE[|MLOCK_ONFAULT]),
munlockall(MCL_CURRENT) resp. munlockall(MCL_CURRENT|MCL_FUTURE) but
other combinations sound weird to me.
Anyway munlock with flags opens new doors of trickiness.
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* [PATCH RFC 1/1] UAPI,x86: export syscall numbers for all x86 archs
From: David Drysdale @ 2015-07-28 8:05 UTC (permalink / raw)
To: x86, Thomas Gleixner, Ingo Molnar, Andy Lutomirski,
H. Peter Anvin
Cc: Michael Kerrisk, Kees Cook, Paul Moore, Eric Paris, linux-api,
linux-kernel, David Drysdale
In-Reply-To: <1438070731-17764-1-git-send-email-drysdale@google.com>
Some userspace code occasionally has a need to reference the syscall
numbers for different-but-compatible architectures, so explicitly
export the generated files that contain the __NR_ia32_<name> and
__NR_x32_<name> constants.
Also, add a new generated unistd_64_amd64.h file, holding amd64
system call numbers in form __NR_amd64_<name>.
For example, this allows a seccomp-bpf filter to include filtering
for multiple architectures, and (in particular) to include x32
syscalls in an x86_64 filter. (Programmatic control of the audit
framework is another possible use case.)
Signed-off-by: David Drysdale <drysdale@google.com>
---
arch/x86/entry/syscalls/Makefile | 11 ++++++++---
arch/x86/include/uapi/asm/Kbuild | 3 +++
2 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/arch/x86/entry/syscalls/Makefile b/arch/x86/entry/syscalls/Makefile
index 57aa59fd140c..ec6811d0db0b 100644
--- a/arch/x86/entry/syscalls/Makefile
+++ b/arch/x86/entry/syscalls/Makefile
@@ -28,7 +28,7 @@ $(uapi)/unistd_32.h: $(syscall32) $(syshdr)
syshdr_abi_unistd_32_ia32 := i386
syshdr_pfx_unistd_32_ia32 := ia32_
-$(out)/unistd_32_ia32.h: $(syscall32) $(syshdr)
+$(uapi)/unistd_32_ia32.h: $(syscall32) $(syshdr)
$(call if_changed,syshdr)
syshdr_abi_unistd_x32 := common,x32
@@ -42,7 +42,12 @@ $(uapi)/unistd_64.h: $(syscall64) $(syshdr)
syshdr_abi_unistd_64_x32 := x32
syshdr_pfx_unistd_64_x32 := x32_
-$(out)/unistd_64_x32.h: $(syscall64) $(syshdr)
+$(uapi)/unistd_64_x32.h: $(syscall64) $(syshdr)
+ $(call if_changed,syshdr)
+
+syshdr_abi_unistd_64_amd64 := common,64
+syshdr_pfx_unistd_64_amd64 := amd64_
+$(uapi)/unistd_64_amd64.h: $(syscall64) $(syshdr)
$(call if_changed,syshdr)
$(out)/syscalls_32.h: $(syscall32) $(systbl)
@@ -56,8 +61,8 @@ $(out)/xen-hypercalls.h: $(srctree)/scripts/xen-hypercalls.sh
$(out)/xen-hypercalls.h: $(srctree)/include/xen/interface/xen*.h
uapisyshdr-y += unistd_32.h unistd_64.h unistd_x32.h
+uapisyshdr-$(CONFIG_X86_64) += unistd_32_ia32.h unistd_64_amd64.h unistd_64_x32.h
syshdr-y += syscalls_32.h
-syshdr-$(CONFIG_X86_64) += unistd_32_ia32.h unistd_64_x32.h
syshdr-$(CONFIG_X86_64) += syscalls_64.h
syshdr-$(CONFIG_XEN) += xen-hypercalls.h
diff --git a/arch/x86/include/uapi/asm/Kbuild b/arch/x86/include/uapi/asm/Kbuild
index 3dec769cadf7..68805cba2fad 100644
--- a/arch/x86/include/uapi/asm/Kbuild
+++ b/arch/x86/include/uapi/asm/Kbuild
@@ -4,6 +4,9 @@ include include/uapi/asm-generic/Kbuild.asm
genhdr-y += unistd_32.h
genhdr-y += unistd_64.h
genhdr-y += unistd_x32.h
+genhdr-y += unistd_64_amd64.h
+genhdr-y += unistd_64_x32.h
+genhdr-y += unistd_32_ia32.h
header-y += a.out.h
header-y += auxvec.h
header-y += bitsperlong.h
--
2.4.3.573.g4eafbef
^ permalink raw reply related
* [PATCH RFC 0/1] UAPI,x86: export syscall numbers for all x86 archs
From: David Drysdale @ 2015-07-28 8:05 UTC (permalink / raw)
To: x86, Thomas Gleixner, Ingo Molnar, Andy Lutomirski,
H. Peter Anvin
Cc: Michael Kerrisk, Kees Cook, Paul Moore, Eric Paris, linux-api,
linux-kernel, David Drysdale
A while ago I was trying to build a seccomp-bpf filter program that would
survive a change of x86 architecture. This was complicated for all sorts of
reasons, but one of the problems was that the different syscall numbers aren't
all available at the same time -- hence this patch.
Naming-wise, Andy Lutomirski has indicated he'd prefer the prefixes to be
__NR_x86_64_, __NR_x86_64_x32_ and __NR_i386_; however, for the latter two
sets of numbers there are existing headers that use different prefixes
(__NR_x32_ and __NR_ia32_), so altering those would involve a change and/or an
additional set of definitions. For the new constants I've left in my
original suggestion (__NR_amd64_) for the time being.
What are folks' thoughts about the preferred naming for these?
David Drysdale (1):
UAPI,x86: export syscall numbers for all x86 archs
arch/x86/entry/syscalls/Makefile | 11 ++++++++---
arch/x86/include/uapi/asm/Kbuild | 3 +++
2 files changed, 11 insertions(+), 3 deletions(-)
--
2.4.3.573.g4eafbef
^ permalink raw reply
* Re: Revised futex(2) man page for review
From: Michael Kerrisk (man-pages) @ 2015-07-28 7:44 UTC (permalink / raw)
To: Davidlohr Bueso
Cc: mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w, Peter Zijlstra,
Thomas Gleixner, Darren Hart, Carlos O'Donell, Ingo Molnar,
Jakub Jelinek, linux-man-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
lkml, Arnd Bergmann, Steven Rostedt, Linux API, Torvald Riegel,
Roland McGrath, Darren Hart, Anton Blanchard, Eric Dumazet,
bill o gallmeister, Jan Kiszka, Daniel Wagner, Rich Felker,
Andy Lutomirski, bert hubert, Rusty Russell
In-Reply-To: <1438053377.2249.25.camel-h16yJtLeMjHk1uMJSBkQmQ@public.gmane.org>
Hi David,
On 07/28/2015 05:16 AM, Davidlohr Bueso wrote:
> On Mon, 2015-07-27 at 13:10 +0200, Michael Kerrisk (man-pages) wrote:
>> Hi David,
>>
>> On 03/31/2015 04:45 PM, Davidlohr Bueso wrote:
>>> On Sat, 2015-03-28 at 12:47 +0100, Peter Zijlstra wrote:
>>>
>>>> The condition is represented by the futex word, which is an address in
>>>> memory supplied to the futex() system call, and the value at this mem‐
>>>> ory location. (While the virtual addresses for the same memory in sep‐
>>>> arate processes may not be equal, the kernel maps them internally so
>>>> that the same memory mapped in different locations will correspond for
>>>> futex() calls.)
>>>>
>>>> When executing a futex operation that requests to block a thread, the
>>>> kernel will only block if the futex word has the value that the calling
>>>
>>> Given the use of "word", you should probably state right away that
>>> futexes are only 32bit.
>>
>> So, I made the opening sentence here:
>>
>> The condition is represented by the futex word, which is an
>> address in memory supplied to the futex() system call, and the
>> 32-bit value at this memory location.
>>
>> Okay?
>
> I think we can still improve :)
>
> I've re-read the whole first paragraphs, and have a few comments that
> touch upon this specific wording. Lets see. You have:
>
>> The futex() system call provides a method for waiting until a certain
>> condition becomes true. It is typically used as a blocking construct
>> in the context of shared-memory synchronization: The program implements
>> the majority of the synchronization in user space, and uses one of
>> operations of the system call when it is likely that it has to block
>> for a longer time until the condition becomes true. The program uses
>> another operation of the system call to wake anyone waiting for a par‐
>> ticular condition.
>
> I've rephrased the next paragraph. How about adding this to follow?
>
> A futex is in essence a 32-bit user-space address. All futex operations and
> conditions are governed by this variable, from now on referred to as 'futex
> word'. As such, a futex is identified by the address in shared memory, which
> may or may not be shared between different processes. For virtual memory, the
> kernel will internally handle and resolve the later. This futex word, along
> with the value at its the memory location, are supplied to the futex() system
> call.
>
> Feel free to reword however you think is better.
I agree with you that that second paragraph is a bit broken. But, like Heinrich,
I'm confused by this term "32-bit ... address".
I've rewritten the paragraph as:
A futex is a 32-bit value—referred to below as a futex word—whose
address is supplied to the futex() system call. (Futexes are
32-bits in size on all platforms, including 64-bit systems.) All
futex operations are governed by this value. In order to share a
futex between processes, the futex is placed in a region of
shared memory, created using (for example) mmap(2) or shmat(2).
(Thus the futex word may have different virtual addresses in dif‐
ferent processes, but these addresses all refer to the same loca‐
tion in physical memory.)
Maybe you still have some further improvements for the paragraph?
Cheers,
Michael
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
^ permalink raw reply
* Re: Aw: Re: Revised futex(2) man page for review
From: Davidlohr Bueso @ 2015-07-28 6:50 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: Michael Kerrisk (man-pages), Peter Zijlstra, Thomas Gleixner,
Darren Hart, Carlos O'Donell, Ingo Molnar, Jakub Jelinek,
linux-man-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, lkml,
Arnd Bergmann, Steven Rostedt, Linux API, Torvald Riegel,
Roland McGrath, Darren Hart, Anton Blanchard, Eric Dumazet,
bill o gallmeister, Jan Kiszka, Daniel Wagner, Rich Felker,
Andy Lutomirski, bert hubert
In-Reply-To: <trinity-4b369924-3445-4e0e-a00e-95d87f708bd5-1438062240689@msvc-mesg-gmx003>
On Tue, 2015-07-28 at 07:44 +0200, Heinrich Schuchardt wrote:
> Hello David,
>
> >> A futex is in essence a 32-bit user-space address.
> I know what a 32 bit integer is.
> I am not aware of 32 bit addresses on my 64 bit operating system.
Well I am referring to in the context of a user-space address, such as a
32-bit lock ('int'), but yes, my text is misleading. In fact we
obviously need to cast to the word size for doing gup_fast, among other
tasks.
Thanks,
Davidlohr
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: Revised futex(2) man page for review
From: Michael Kerrisk (man-pages) @ 2015-07-28 6:39 UTC (permalink / raw)
To: Davidlohr Bueso, Peter Zijlstra
Cc: mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w, Thomas Gleixner, Darren Hart,
Carlos O'Donell, Ingo Molnar, Jakub Jelinek,
linux-man-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, lkml,
Arnd Bergmann, Steven Rostedt, Linux API, Torvald Riegel,
Roland McGrath, Darren Hart, Anton Blanchard, Eric Dumazet,
bill o gallmeister, Jan Kiszka, Daniel Wagner, Rich Felker,
Andy Lutomirski, bert hubert, Rusty Russell,
Heinrich Schuchardt <xypron>
In-Reply-To: <1438051964.2249.9.camel-h16yJtLeMjHk1uMJSBkQmQ@public.gmane.org>
On 07/28/2015 04:52 AM, Davidlohr Bueso wrote:
> On Sat, 2015-03-28 at 12:47 +0100, Peter Zijlstra wrote:
>> SEE ALSO
>> get_robust_list(2), restart_syscall(2), futex(7)
>
> For pi futexes, I also suggest pthread_mutexattr_getprotocol(3), which
> is a common entry point.
Thanks. Added.
Cheers,
Michael
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
^ permalink raw reply
* Re: Revised futex(2) man page for review
From: Davidlohr Bueso @ 2015-07-28 3:16 UTC (permalink / raw)
To: Michael Kerrisk (man-pages)
Cc: Peter Zijlstra, Thomas Gleixner, Darren Hart, Carlos O'Donell,
Ingo Molnar, Jakub Jelinek,
linux-man-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, lkml,
Arnd Bergmann, Steven Rostedt, Linux API, Torvald Riegel,
Roland McGrath, Darren Hart, Anton Blanchard, Eric Dumazet,
bill o gallmeister, Jan Kiszka, Daniel Wagner, Rich Felker,
Andy Lutomirski, bert hubert, Rusty Russell, Heinrich
In-Reply-To: <55B611B3.7030005-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Mon, 2015-07-27 at 13:10 +0200, Michael Kerrisk (man-pages) wrote:
> Hi David,
>
> On 03/31/2015 04:45 PM, Davidlohr Bueso wrote:
> > On Sat, 2015-03-28 at 12:47 +0100, Peter Zijlstra wrote:
> >
> >> The condition is represented by the futex word, which is an address in
> >> memory supplied to the futex() system call, and the value at this mem‐
> >> ory location. (While the virtual addresses for the same memory in sep‐
> >> arate processes may not be equal, the kernel maps them internally so
> >> that the same memory mapped in different locations will correspond for
> >> futex() calls.)
> >>
> >> When executing a futex operation that requests to block a thread, the
> >> kernel will only block if the futex word has the value that the calling
> >
> > Given the use of "word", you should probably state right away that
> > futexes are only 32bit.
>
> So, I made the opening sentence here:
>
> The condition is represented by the futex word, which is an
> address in memory supplied to the futex() system call, and the
> 32-bit value at this memory location.
>
> Okay?
I think we can still improve :)
I've re-read the whole first paragraphs, and have a few comments that
touch upon this specific wording. Lets see. You have:
> The futex() system call provides a method for waiting until a certain
> condition becomes true. It is typically used as a blocking construct
> in the context of shared-memory synchronization: The program implements
> the majority of the synchronization in user space, and uses one of
> operations of the system call when it is likely that it has to block
> for a longer time until the condition becomes true. The program uses
> another operation of the system call to wake anyone waiting for a par‐
> ticular condition.
I've rephrased the next paragraph. How about adding this to follow?
A futex is in essence a 32-bit user-space address. All futex operations and
conditions are governed by this variable, from now on referred to as 'futex
word'. As such, a futex is identified by the address in shared memory, which
may or may not be shared between different processes. For virtual memory, the
kernel will internally handle and resolve the later. This futex word, along
with the value at its the memory location, are supplied to the futex() system
call.
Feel free to reword however you think is better.
Thanks,
Davidlohr
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: Revised futex(2) man page for review
From: Davidlohr Bueso @ 2015-07-28 2:52 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Michael Kerrisk (man-pages), Thomas Gleixner, Darren Hart,
Carlos O'Donell, Ingo Molnar, Jakub Jelinek,
linux-man-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, lkml,
Arnd Bergmann, Steven Rostedt, Linux API, Torvald Riegel,
Roland McGrath, Darren Hart, Anton Blanchard, Eric Dumazet,
bill o gallmeister, Jan Kiszka, Daniel Wagner, Rich Felker,
Andy Lutomirski, bert hubert, Rusty Russell
In-Reply-To: <20150328114725.GJ27490-IIpfhp3q70z/8w/KjCw3T+5/BudmfyzbbVWyRVo5IupeoWH0uzbU5w@public.gmane.org>
On Sat, 2015-03-28 at 12:47 +0100, Peter Zijlstra wrote:
> SEE ALSO
> get_robust_list(2), restart_syscall(2), futex(7)
For pi futexes, I also suggest pthread_mutexattr_getprotocol(3), which
is a common entry point.
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 4/4] mm: remove direct calling of migration
From: Gioh Kim @ 2015-07-28 0:26 UTC (permalink / raw)
To: Vlastimil Babka, jlayton, bfields, iamjoonsoo.kim, viro, mst,
koct9i, minchan, aquini, linux-fsdevel, virtualization,
linux-kernel, linux-api, linux-mm
Cc: dri-devel, akpm, Gioh Kim
In-Reply-To: <55B638F1.9090407@suse.cz>
2015-07-27 오후 10:58에 Vlastimil Babka 이(가) 쓴 글:
> On 07/13/2015 10:35 AM, Gioh Kim wrote:
>> From: Gioh Kim <gurugio@hanmail.net>
>>
>> Migration is completely generalized so that migrating mobile page
>> is processed with lru-pages in move_to_new_page.
>>
>> Signed-off-by: Gioh Kim <gioh.kim@lge.com>
>> Acked-by: Rafael Aquini <aquini@redhat.com>
>
> Why not just fold this to Patch 3? You already modify this hunk there, and prior to patch 3, the hunk was balloon-pages specific. You made it look generic only to remove it, which is unneeded code churn and I don't think it adds anything wrt e.g. bisectability.
Yes, you have a point.
I'll be merged into 3/4 at next spin.
I wanted to show the process how migration is generalized with this patch.
On second thought it is not necessary.
>
>> ---
>> mm/migrate.c | 15 ---------------
>> 1 file changed, 15 deletions(-)
>>
>> diff --git a/mm/migrate.c b/mm/migrate.c
>> index 53f0081d..e6644ac 100644
>> --- a/mm/migrate.c
>> +++ b/mm/migrate.c
>> @@ -844,21 +844,6 @@ static int __unmap_and_move(struct page *page, struct page *newpage,
>> }
>> }
>>
>> - if (unlikely(mobile_page(page))) {
>> - /*
>> - * A mobile page does not need any special attention from
>> - * physical to virtual reverse mapping procedures.
>> - * Skip any attempt to unmap PTEs or to remap swap cache,
>> - * in order to avoid burning cycles at rmap level, and perform
>> - * the page migration right away (proteced by page lock).
>> - */
>> - lock_page(newpage);
>> - rc = page->mapping->a_ops->migratepage(page->mapping,
>> - newpage, page, mode);
>> - unlock_page(newpage);
>> - goto out_unlock;
>> - }
>> -
>> /*
>> * Corner case handling:
>> * 1. When a new swap-cache page is read into, it is added to the LRU
>>
>
>
^ permalink raw reply
* Re: [PATCH 2/4] mm/compaction: enable mobile-page migration
From: Gioh Kim @ 2015-07-28 0:21 UTC (permalink / raw)
To: Konstantin Khlebnikov, Vlastimil Babka
Cc: Jeff Layton, Bruce Fields, Joonsoo Kim, Al Viro,
Michael S. Tsirkin, Minchan Kim, Rafael Aquini, linux-fsdevel,
virtualization, Linux Kernel Mailing List, Linux API,
linux-mm@kvack.org, dri-devel, Andrew Morton, Gioh Kim
In-Reply-To: <CALYGNiMJ79HmLkoV_c2BR+ENig0h5aNzeY5YdS7kEYpFbLe=xA@mail.gmail.com>
> On Mon, Jul 27, 2015 at 4:55 PM, Vlastimil Babka <vbabka@suse.cz> wrote:
>> On 07/13/2015 10:35 AM, Gioh Kim wrote:
>>>
>>> From: Gioh Kim <gurugio@hanmail.net>
>>>
>>> Add framework to register callback functions and check page mobility.
>>> There are some modes for page isolation so that isolate interface
>>> has arguments of page address and isolation mode while putback
>>> interface has only page address as argument.
>>
>>
>> Note that unlike what subject suggest, this doesn't really enable
>> mobile-page migration inside compaction, since that only happens with patch
>> 3. This might theoretically affect some cherry-pick backports that don't
>> care about balloon pages. I can imagine that can easily happen in the world
>> of mobile devices?
>> It would thus be somewhat cleaner if this patch was complete in that sense.
You have a point.
Current 2/4 patch is lack of calling migrate/isolate.
It is not complete without 3/4.
I'll add calling migrate/isolate() at next spin.
>>
>>> Signed-off-by: Gioh Kim <gioh.kim@lge.com>
>>> Acked-by: Rafael Aquini <aquini@redhat.com>
>>> ---
>>> fs/proc/page.c | 3 ++
>>> include/linux/compaction.h | 80
>>> ++++++++++++++++++++++++++++++++++
>>> include/linux/fs.h | 2 +
>>> include/linux/page-flags.h | 19 ++++++++
>>> include/uapi/linux/kernel-page-flags.h | 1 +
>>> 5 files changed, 105 insertions(+)
>>>
>>> diff --git a/fs/proc/page.c b/fs/proc/page.c
>>> index 7eee2d8..a4f5a00 100644
>>> --- a/fs/proc/page.c
>>> +++ b/fs/proc/page.c
>>> @@ -146,6 +146,9 @@ u64 stable_page_flags(struct page *page)
>>> if (PageBalloon(page))
>>> u |= 1 << KPF_BALLOON;
>>>
>>> + if (PageMobile(page))
>>> + u |= 1 << KPF_MOBILE;
>>
>>
>> PageMovable() would probably be as good a name and correspond to
>> MIGRATE_MOVABLE somewhat, unlike a completely new term. Whatever driver
>> starts to using this should probably change allocation flags to allocate
>> MIGRATE_MOVABLE, so that it works fine with what fragmentation avoidance
>> expects. Guess I should have said that earlier, but can you still
>> reconsider?
>
> Well, I've suggested to name it "mobile" because there's already a lot of things
> called "movable". Mobile pages are special subset of movable pages: they
> are non-lru pages and define their own rules of moving in address
> space operations.
>
> Also there's a little pun: I guess main user will zram which is used
> mostly in embedded/mobile devices.
I like "mobile".
"movable" is for pages allocated with GFP_MOVABLE.
I think "movable" is a little ambiguous in this situation.
>
>>
>>> +
>>> u |= kpf_copy_bit(k, KPF_LOCKED, PG_locked);
>>>
>>> u |= kpf_copy_bit(k, KPF_SLAB, PG_slab);
>>> diff --git a/include/linux/compaction.h b/include/linux/compaction.h
>>> index aa8f61c..f693072 100644
>>> --- a/include/linux/compaction.h
>>> +++ b/include/linux/compaction.h
>>> @@ -1,6 +1,9 @@
>>> #ifndef _LINUX_COMPACTION_H
>>> #define _LINUX_COMPACTION_H
>>>
>>> +#include <linux/page-flags.h>
>>> +#include <linux/pagemap.h>
>>> +
>>> /* Return values for compact_zone() and try_to_compact_pages() */
>>> /* compaction didn't start as it was deferred due to past failures */
>>> #define COMPACT_DEFERRED 0
>>> @@ -51,6 +54,70 @@ extern void compaction_defer_reset(struct zone *zone,
>>> int order,
>>> bool alloc_success);
>>> extern bool compaction_restarting(struct zone *zone, int order);
>>>
>>> +static inline bool mobile_page(struct page *page)
>>> +{
>>> + return page->mapping && (PageMobile(page) || PageBalloon(page));
>>> +}
>>
>>
>> I would put this definition to linux/page-flags.h and rename it to
>> page_mobile (or better page_movable()), which is more common ordering.
>>
>>> +
>>> +static inline bool isolate_mobilepage(struct page *page, isolate_mode_t
>>> mode)
>>
>>
>> Does this have to be in compaction.h? The only user is compaction.c so
>> probably move it there, and if there ever is another module using this in
>> the future, we can move it to a more appropriate place and declare it in
>> e.g. mm/internal.h.
I think compaction.c is suitable.
>>
>>
>>> +{
>>> + bool ret = false;
>>> +
>>> + /*
>>> + * Avoid burning cycles with pages that are yet under
>>> __free_pages(),
>>> + * or just got freed under us.
>>> + *
>>> + * In case we 'win' a race for a mobile page being freed under us
>>> and
>>> + * raise its refcount preventing __free_pages() from doing its job
>>> + * the put_page() at the end of this block will take care of
>>> + * release this page, thus avoiding a nasty leakage.
>>> + */
>>> + if (unlikely(!get_page_unless_zero(page)))
>>> + goto out;
>>> +
>>> + /*
>>> + * As mobile pages are not isolated from LRU lists, concurrent
>>> + * compaction threads can race against page migration functions
>>> + * as well as race against the releasing a page.
>>> + *
>>> + * In order to avoid having an already isolated mobile page
>>> + * being (wrongly) re-isolated while it is under migration,
>>> + * or to avoid attempting to isolate pages being released,
>>> + * lets be sure we have the page lock
>>> + * before proceeding with the mobile page isolation steps.
>>> + */
>>> + if (unlikely(!trylock_page(page)))
>>> + goto out_putpage;
>>> +
>>> + if (!(mobile_page(page) && page->mapping->a_ops->isolatepage))
>>> + goto out_not_isolated;
>>> + ret = page->mapping->a_ops->isolatepage(page, mode);
>>> + if (!ret)
>>> + goto out_not_isolated;
>>> + unlock_page(page);
>>> + return ret;
>>> +
>>> +out_not_isolated:
>>> + unlock_page(page);
>>> +out_putpage:
>>> + put_page(page);
>>> +out:
>>> + return ret;
>>> +}
>>> +
>>> +static inline void putback_mobilepage(struct page *page)
>>
>>
>> Likewise, this could go to migrate.c. Or maybe together with
>> isolate_mobilepage() if you don't want to split them.
I got it.
>>
>>
>>> +{
>>> + /*
>>> + * 'lock_page()' stabilizes the page and prevents races against
>>> + * concurrent isolation threads attempting to re-isolate it.
>>> + */
>>> + lock_page(page);
>>> + if (page->mapping && page->mapping->a_ops->putbackpage)
>>> + page->mapping->a_ops->putbackpage(page);
>>> + unlock_page(page);
>>> + /* drop the extra ref count taken for mobile page isolation */
>>> + put_page(page);
>>> +}
>>> #else
>>> static inline unsigned long try_to_compact_pages(gfp_t gfp_mask,
>>> unsigned int order, int alloc_flags,
>>> @@ -83,6 +150,19 @@ static inline bool compaction_deferred(struct zone
>>> *zone, int order)
>>> return true;
>>> }
>>>
>>> +static inline bool mobile_page(struct page *page)
>>> +{
>>> + return false;
>>> +}
>>> +
>>> +static inline bool isolate_mobilepage(struct page *page, isolate_mode_t
>>> mode)
>>> +{
>>> + return false;
>>> +}
>>> +
>>> +static inline void putback_mobilepage(struct page *page)
>>> +{
>>> +}
>>> #endif /* CONFIG_COMPACTION */
>>>
>>> #if defined(CONFIG_COMPACTION) && defined(CONFIG_SYSFS) &&
>>> defined(CONFIG_NUMA)
>>> diff --git a/include/linux/fs.h b/include/linux/fs.h
>>> index a0653e5..2cc4b24 100644
>>> --- a/include/linux/fs.h
>>> +++ b/include/linux/fs.h
>>> @@ -396,6 +396,8 @@ struct address_space_operations {
>>> */
>>> int (*migratepage) (struct address_space *,
>>> struct page *, struct page *, enum migrate_mode);
>>> + bool (*isolatepage) (struct page *, isolate_mode_t);
>>> + void (*putbackpage) (struct page *);
>>> int (*launder_page) (struct page *);
>>> int (*is_partially_uptodate) (struct page *, unsigned long,
>>> unsigned long);
>>> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
>>> index f34e040..abef145 100644
>>> --- a/include/linux/page-flags.h
>>> +++ b/include/linux/page-flags.h
>>> @@ -582,6 +582,25 @@ static inline void __ClearPageBalloon(struct page
>>> *page)
>>> atomic_set(&page->_mapcount, -1);
>>> }
>>>
>>> +#define PAGE_MOBILE_MAPCOUNT_VALUE (-255)
>>> +
>>> +static inline int PageMobile(struct page *page)
>>> +{
>>> + return atomic_read(&page->_mapcount) ==
>>> PAGE_MOBILE_MAPCOUNT_VALUE;
>>> +}
>>> +
>>> +static inline void __SetPageMobile(struct page *page)
>>> +{
>>> + VM_BUG_ON_PAGE(atomic_read(&page->_mapcount) != -1, page);
>>> + atomic_set(&page->_mapcount, PAGE_MOBILE_MAPCOUNT_VALUE);
>>> +}
>>> +
>>> +static inline void __ClearPageMobile(struct page *page)
>>> +{
>>> + VM_BUG_ON_PAGE(!PageMobile(page), page);
>>> + atomic_set(&page->_mapcount, -1);
>>> +}
>>> +
>>> /*
>>> * If network-based swap is enabled, sl*b must keep track of whether
>>> pages
>>> * were allocated from pfmemalloc reserves.
>>> diff --git a/include/uapi/linux/kernel-page-flags.h
>>> b/include/uapi/linux/kernel-page-flags.h
>>> index a6c4962..d50d9e8 100644
>>> --- a/include/uapi/linux/kernel-page-flags.h
>>> +++ b/include/uapi/linux/kernel-page-flags.h
>>> @@ -33,6 +33,7 @@
>>> #define KPF_THP 22
>>> #define KPF_BALLOON 23
>>> #define KPF_ZERO_PAGE 24
>>> +#define KPF_MOBILE 25
>>>
>>>
>>> #endif /* _UAPILINUX_KERNEL_PAGE_FLAGS_H */
>>>
>>
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH v7 4/6] block: loop: prepare for supporing direct IO
From: Dave Chinner @ 2015-07-27 22:06 UTC (permalink / raw)
To: Ming Lei
Cc: Christoph Hellwig, Jens Axboe, Linux Kernel Mailing List,
Justin M. Forbes, Jeff Moyer, Tejun Heo,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CACVXFVMKycx768HtAJXMgEZNjsQrNm_f3UzW9kUysSHAMM5FPQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Mon, Jul 27, 2015 at 05:53:33AM -0400, Ming Lei wrote:
> On Mon, Jul 27, 2015 at 5:45 AM, Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote:
> > On Mon, Jul 27, 2015 at 05:41:57AM -0400, Ming Lei wrote:
> >> > Why the hardcoded value? I suspect this should be more like:
> >> >
> >> > if (dio && inode->i_sb->s_bdev &&
> >> > (lo->lo_offset & (bdev_io_min(inode->i_sb->s_bdev) - 1)) != 0)
> >> > dio = false;
> >>
> >> The above can't work if the backing device has a bigger sector size
> >> (such as 4K), that is why loop's direct-io requires 512 min_io_size of
> >> backing device.
> >
> > Why doesn't it work? If the backing device sector size is 4k
> > and lo_offset is 0 or a multiple of 4k it should allow direct I/O,
> > and my code sniplet will allow that.
>
> Because size has to be 4k aligned too.
So check that, too. Any >= 4k block size filesystem should be doing
mostly 4k aligned and sized IO...
Cheers,
Dave.
--
Dave Chinner
david-FqsqvQoI3Ljby3iVrkZq2A@public.gmane.org
^ permalink raw reply
* Re: [PATCH -mm v9 0/8] idle memory tracking
From: Andrew Morton @ 2015-07-27 19:25 UTC (permalink / raw)
To: Kees Cook
Cc: Vladimir Davydov, Andres Lagar-Cavilla, Minchan Kim,
Raghavendra K T, Johannes Weiner, Michal Hocko, Greg Thelen,
Michel Lespinasse, David Rientjes, Pavel Emelyanov,
Cyrill Gorcunov, Jonathan Corbet, Linux API,
linux-doc@vger.kernel.org, Linux-MM, Cgroups, LKML
In-Reply-To: <CAGXu5jLPT-2c_H3kjCzbVgRKQO0xMskVd7JcAMmWZSmFgzZ4ng@mail.gmail.com>
On Mon, 27 Jul 2015 12:18:57 -0700 Kees Cook <keescook@chromium.org> wrote:
> > Why were these put in /proc anyway? Rather than under /sys/fs/cgroup
> > somewhere? Presumably because /proc/kpageidle is useful in non-memcg
> > setups.
>
> Do we need a /proc/vm/ for holding these kinds of things? We're
> collecting a lot there. Or invent some way for this to be sensible in
> /sys?
/proc is the traditional place for such things (/proc/kpagecount,
/proc/kpageflags, /proc/pagetypeinfo). But that was probably a
mistake.
/proc/sys/vm is rather a dumping ground of random tunables and
statuses, but yes, I do think that moving the kpageidle stuff into there
would be better.
^ permalink raw reply
* Re: [PATCH -mm v9 0/8] idle memory tracking
From: Kees Cook @ 2015-07-27 19:18 UTC (permalink / raw)
To: Andrew Morton
Cc: Vladimir Davydov, Andres Lagar-Cavilla, Minchan Kim,
Raghavendra K T, Johannes Weiner, Michal Hocko, Greg Thelen,
Michel Lespinasse, David Rientjes, Pavel Emelyanov,
Cyrill Gorcunov, Jonathan Corbet, Linux API,
linux-doc@vger.kernel.org, Linux-MM, Cgroups, LKML
In-Reply-To: <20150721163402.43ad2527d9b8caa476a1c9e1@linux-foundation.org>
On Tue, Jul 21, 2015 at 4:34 PM, Andrew Morton
<akpm@linux-foundation.org> wrote:
> On Sun, 19 Jul 2015 15:31:09 +0300 Vladimir Davydov <vdavydov@parallels.com> wrote:
>> To mark a page idle one should set the bit corresponding to the
>> page by writing to the file. A value written to the file is OR-ed with the
>> current bitmap value. Only user memory pages can be marked idle, for other
>> page types input is silently ignored. Writing to this file beyond max PFN
>> results in the ENXIO error. Only available when CONFIG_IDLE_PAGE_TRACKING is
>> set.
>>
>> This file can be used to estimate the amount of pages that are not
>> used by a particular workload as follows:
>>
>> 1. mark all pages of interest idle by setting corresponding bits in the
>> /proc/kpageidle bitmap
>> 2. wait until the workload accesses its working set
>> 3. read /proc/kpageidle and count the number of bits set
>
> Security implications. This interface could be used to learn about a
> sensitive application by poking data at it and then observing its
> memory access patterns. Perhaps this is why the proc files are
> root-only (whcih I assume is sufficient). Some words here about the
> security side of things and the reasoning behind the chosen permissions
> would be good to have.
As long as this stays true-root-only, I think it should be safe enough.
>> * /proc/kpagecgroup. This file contains a 64-bit inode number of the
>> memory cgroup each page is charged to, indexed by PFN.
>
> Actually "closest online ancestor". This also should be in the
> interface documentation.
>
>> Only available when CONFIG_MEMCG is set.
>
> CONFIG_MEMCG and CONFIG_IDLE_PAGE_TRACKING I assume?
>
>>
>> This file can be used to find all pages (including unmapped file
>> pages) accounted to a particular cgroup. Using /proc/kpageidle, one
>> can then estimate the cgroup working set size.
>>
>> For an example of using these files for estimating the amount of unused
>> memory pages per each memory cgroup, please see the script attached
>> below.
>
> Why were these put in /proc anyway? Rather than under /sys/fs/cgroup
> somewhere? Presumably because /proc/kpageidle is useful in non-memcg
> setups.
Do we need a /proc/vm/ for holding these kinds of things? We're
collecting a lot there. Or invent some way for this to be sensible in
/sys?
-Kees
--
Kees Cook
Chrome OS Security
^ permalink raw reply
* Re: [PATCH 2/4] mm/compaction: enable mobile-page migration
From: Konstantin Khlebnikov @ 2015-07-27 18:56 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Gioh Kim, Jeff Layton, Bruce Fields, Joonsoo Kim, Al Viro,
Michael S. Tsirkin, Minchan Kim, Rafael Aquini, linux-fsdevel,
virtualization-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
Linux Kernel Mailing List, Linux API,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org, dri-devel,
Andrew Morton, Gioh Kim
In-Reply-To: <55B63851.1080100-AlSwsSmVLrQ@public.gmane.org>
On Mon, Jul 27, 2015 at 4:55 PM, Vlastimil Babka <vbabka-AlSwsSmVLrQ@public.gmane.org> wrote:
> On 07/13/2015 10:35 AM, Gioh Kim wrote:
>>
>> From: Gioh Kim <gurugio-A7HHaXTFBl7R7s880joybQ@public.gmane.org>
>>
>> Add framework to register callback functions and check page mobility.
>> There are some modes for page isolation so that isolate interface
>> has arguments of page address and isolation mode while putback
>> interface has only page address as argument.
>
>
> Note that unlike what subject suggest, this doesn't really enable
> mobile-page migration inside compaction, since that only happens with patch
> 3. This might theoretically affect some cherry-pick backports that don't
> care about balloon pages. I can imagine that can easily happen in the world
> of mobile devices?
> It would thus be somewhat cleaner if this patch was complete in that sense.
>
>> Signed-off-by: Gioh Kim <gioh.kim-Hm3cg6mZ9cc@public.gmane.org>
>> Acked-by: Rafael Aquini <aquini-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>> ---
>> fs/proc/page.c | 3 ++
>> include/linux/compaction.h | 80
>> ++++++++++++++++++++++++++++++++++
>> include/linux/fs.h | 2 +
>> include/linux/page-flags.h | 19 ++++++++
>> include/uapi/linux/kernel-page-flags.h | 1 +
>> 5 files changed, 105 insertions(+)
>>
>> diff --git a/fs/proc/page.c b/fs/proc/page.c
>> index 7eee2d8..a4f5a00 100644
>> --- a/fs/proc/page.c
>> +++ b/fs/proc/page.c
>> @@ -146,6 +146,9 @@ u64 stable_page_flags(struct page *page)
>> if (PageBalloon(page))
>> u |= 1 << KPF_BALLOON;
>>
>> + if (PageMobile(page))
>> + u |= 1 << KPF_MOBILE;
>
>
> PageMovable() would probably be as good a name and correspond to
> MIGRATE_MOVABLE somewhat, unlike a completely new term. Whatever driver
> starts to using this should probably change allocation flags to allocate
> MIGRATE_MOVABLE, so that it works fine with what fragmentation avoidance
> expects. Guess I should have said that earlier, but can you still
> reconsider?
Well, I've suggested to name it "mobile" because there's already a lot of things
called "movable". Mobile pages are special subset of movable pages: they
are non-lru pages and define their own rules of moving in address
space operations.
Also there's a little pun: I guess main user will zram which is used
mostly in embedded/mobile devices.
>
>> +
>> u |= kpf_copy_bit(k, KPF_LOCKED, PG_locked);
>>
>> u |= kpf_copy_bit(k, KPF_SLAB, PG_slab);
>> diff --git a/include/linux/compaction.h b/include/linux/compaction.h
>> index aa8f61c..f693072 100644
>> --- a/include/linux/compaction.h
>> +++ b/include/linux/compaction.h
>> @@ -1,6 +1,9 @@
>> #ifndef _LINUX_COMPACTION_H
>> #define _LINUX_COMPACTION_H
>>
>> +#include <linux/page-flags.h>
>> +#include <linux/pagemap.h>
>> +
>> /* Return values for compact_zone() and try_to_compact_pages() */
>> /* compaction didn't start as it was deferred due to past failures */
>> #define COMPACT_DEFERRED 0
>> @@ -51,6 +54,70 @@ extern void compaction_defer_reset(struct zone *zone,
>> int order,
>> bool alloc_success);
>> extern bool compaction_restarting(struct zone *zone, int order);
>>
>> +static inline bool mobile_page(struct page *page)
>> +{
>> + return page->mapping && (PageMobile(page) || PageBalloon(page));
>> +}
>
>
> I would put this definition to linux/page-flags.h and rename it to
> page_mobile (or better page_movable()), which is more common ordering.
>
>> +
>> +static inline bool isolate_mobilepage(struct page *page, isolate_mode_t
>> mode)
>
>
> Does this have to be in compaction.h? The only user is compaction.c so
> probably move it there, and if there ever is another module using this in
> the future, we can move it to a more appropriate place and declare it in
> e.g. mm/internal.h.
>
>
>> +{
>> + bool ret = false;
>> +
>> + /*
>> + * Avoid burning cycles with pages that are yet under
>> __free_pages(),
>> + * or just got freed under us.
>> + *
>> + * In case we 'win' a race for a mobile page being freed under us
>> and
>> + * raise its refcount preventing __free_pages() from doing its job
>> + * the put_page() at the end of this block will take care of
>> + * release this page, thus avoiding a nasty leakage.
>> + */
>> + if (unlikely(!get_page_unless_zero(page)))
>> + goto out;
>> +
>> + /*
>> + * As mobile pages are not isolated from LRU lists, concurrent
>> + * compaction threads can race against page migration functions
>> + * as well as race against the releasing a page.
>> + *
>> + * In order to avoid having an already isolated mobile page
>> + * being (wrongly) re-isolated while it is under migration,
>> + * or to avoid attempting to isolate pages being released,
>> + * lets be sure we have the page lock
>> + * before proceeding with the mobile page isolation steps.
>> + */
>> + if (unlikely(!trylock_page(page)))
>> + goto out_putpage;
>> +
>> + if (!(mobile_page(page) && page->mapping->a_ops->isolatepage))
>> + goto out_not_isolated;
>> + ret = page->mapping->a_ops->isolatepage(page, mode);
>> + if (!ret)
>> + goto out_not_isolated;
>> + unlock_page(page);
>> + return ret;
>> +
>> +out_not_isolated:
>> + unlock_page(page);
>> +out_putpage:
>> + put_page(page);
>> +out:
>> + return ret;
>> +}
>> +
>> +static inline void putback_mobilepage(struct page *page)
>
>
> Likewise, this could go to migrate.c. Or maybe together with
> isolate_mobilepage() if you don't want to split them.
>
>
>> +{
>> + /*
>> + * 'lock_page()' stabilizes the page and prevents races against
>> + * concurrent isolation threads attempting to re-isolate it.
>> + */
>> + lock_page(page);
>> + if (page->mapping && page->mapping->a_ops->putbackpage)
>> + page->mapping->a_ops->putbackpage(page);
>> + unlock_page(page);
>> + /* drop the extra ref count taken for mobile page isolation */
>> + put_page(page);
>> +}
>> #else
>> static inline unsigned long try_to_compact_pages(gfp_t gfp_mask,
>> unsigned int order, int alloc_flags,
>> @@ -83,6 +150,19 @@ static inline bool compaction_deferred(struct zone
>> *zone, int order)
>> return true;
>> }
>>
>> +static inline bool mobile_page(struct page *page)
>> +{
>> + return false;
>> +}
>> +
>> +static inline bool isolate_mobilepage(struct page *page, isolate_mode_t
>> mode)
>> +{
>> + return false;
>> +}
>> +
>> +static inline void putback_mobilepage(struct page *page)
>> +{
>> +}
>> #endif /* CONFIG_COMPACTION */
>>
>> #if defined(CONFIG_COMPACTION) && defined(CONFIG_SYSFS) &&
>> defined(CONFIG_NUMA)
>> diff --git a/include/linux/fs.h b/include/linux/fs.h
>> index a0653e5..2cc4b24 100644
>> --- a/include/linux/fs.h
>> +++ b/include/linux/fs.h
>> @@ -396,6 +396,8 @@ struct address_space_operations {
>> */
>> int (*migratepage) (struct address_space *,
>> struct page *, struct page *, enum migrate_mode);
>> + bool (*isolatepage) (struct page *, isolate_mode_t);
>> + void (*putbackpage) (struct page *);
>> int (*launder_page) (struct page *);
>> int (*is_partially_uptodate) (struct page *, unsigned long,
>> unsigned long);
>> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
>> index f34e040..abef145 100644
>> --- a/include/linux/page-flags.h
>> +++ b/include/linux/page-flags.h
>> @@ -582,6 +582,25 @@ static inline void __ClearPageBalloon(struct page
>> *page)
>> atomic_set(&page->_mapcount, -1);
>> }
>>
>> +#define PAGE_MOBILE_MAPCOUNT_VALUE (-255)
>> +
>> +static inline int PageMobile(struct page *page)
>> +{
>> + return atomic_read(&page->_mapcount) ==
>> PAGE_MOBILE_MAPCOUNT_VALUE;
>> +}
>> +
>> +static inline void __SetPageMobile(struct page *page)
>> +{
>> + VM_BUG_ON_PAGE(atomic_read(&page->_mapcount) != -1, page);
>> + atomic_set(&page->_mapcount, PAGE_MOBILE_MAPCOUNT_VALUE);
>> +}
>> +
>> +static inline void __ClearPageMobile(struct page *page)
>> +{
>> + VM_BUG_ON_PAGE(!PageMobile(page), page);
>> + atomic_set(&page->_mapcount, -1);
>> +}
>> +
>> /*
>> * If network-based swap is enabled, sl*b must keep track of whether
>> pages
>> * were allocated from pfmemalloc reserves.
>> diff --git a/include/uapi/linux/kernel-page-flags.h
>> b/include/uapi/linux/kernel-page-flags.h
>> index a6c4962..d50d9e8 100644
>> --- a/include/uapi/linux/kernel-page-flags.h
>> +++ b/include/uapi/linux/kernel-page-flags.h
>> @@ -33,6 +33,7 @@
>> #define KPF_THP 22
>> #define KPF_BALLOON 23
>> #define KPF_ZERO_PAGE 24
>> +#define KPF_MOBILE 25
>>
>>
>> #endif /* _UAPILINUX_KERNEL_PAGE_FLAGS_H */
>>
>
^ permalink raw reply
* Re: [PATCH v7 4/6] block: loop: prepare for supporing direct IO
From: Christoph Hellwig @ 2015-07-27 17:33 UTC (permalink / raw)
To: Ming Lei
Cc: Christoph Hellwig, Dave Chinner, Jens Axboe,
Linux Kernel Mailing List, Justin M. Forbes, Jeff Moyer,
Tejun Heo, linux-api
In-Reply-To: <CACVXFVMKycx768HtAJXMgEZNjsQrNm_f3UzW9kUysSHAMM5FPQ@mail.gmail.com>
On Mon, Jul 27, 2015 at 05:53:33AM -0400, Ming Lei wrote:
> Because size has to be 4k aligned too.
Yes. But again I don't see any reason to limit us to a hardcoded 512
byte block size here, especially considering the patches to finally
allow enabling other block sizes from userspace.
^ permalink raw reply
* Re: [PATCH] pci_regs: reintroduce PCI_MSIX_FLAGS_BIRMASK
From: Bjorn Helgaas @ 2015-07-27 16:13 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Rajat Jain,
Chen, Gong, Guenter Roeck, Rafał Miłecki,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20150727175412-mutt-send-email-mst-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Mon, Jul 27, 2015 at 9:55 AM, Michael S. Tsirkin <mst-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On Tue, Jul 14, 2015 at 06:29:24PM -0500, Bjorn Helgaas wrote:
>> On Mon, Jul 13, 2015 at 06:11:38PM +0300, Michael S. Tsirkin wrote:
>> > On Wed, Jul 01, 2015 at 12:14:10PM +0200, Michael S. Tsirkin wrote:
>> > > This partially reverts commit 09a2c73ddfc7f173237fc7209a65b34dd5bcb5ed.
>> > > PCI: Remove unused PCI_MSIX_FLAGS_BIRMASK definition
>> > >
>> > > That commit dropped a symbol from an exported header claiming "no one
>> > > uses it". This isn't how Linux normally approaches userspace API though,
>> > > and in fact QEMU build fails if trying to use updated headers from linux
>> > > 3.12 and up.
>> > >
>> > > Sure, userspace can be fixed to use the new symbol, but the cost
>> > > of keeping the old one around is fairly low, too.
>> > >
>> > > Signed-off-by: Michael S. Tsirkin <mst-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>>
>> Sorry, I lost the original message somehow, so I can't reply to it. I
>> applied this by hand to my for-linus branch for v4.2, thanks!
>
>
>
> Did you mean 4.2? time's running out on this one ...
> Just making sure - it's still not upstream, right?
It's on my for-linus branch and it's been in -next for a couple weeks.
We're at -rc4, and I will send a pull request to Linus before v4.2
releases. This is a really minor fix, so I'm holding it in case there
are other fixes that I can include in the same request.
>> commit c9ddbac9c89110f77cb0fa07e634aaf1194899aa
>> Author: Michael S. Tsirkin <mst-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>> Date: Tue Jul 14 18:27:46 2015 -0500
>>
>> PCI: Restore PCI_MSIX_FLAGS_BIRMASK definition
>>
>> 09a2c73ddfc7 ("PCI: Remove unused PCI_MSIX_FLAGS_BIRMASK definition")
>> removed PCI_MSIX_FLAGS_BIRMASK from an exported header because it was
>> unused in the kernel. But that breaks user programs that were using it
>> (QEMU in particular).
>>
>> Restore the PCI_MSIX_FLAGS_BIRMASK definition.
>>
>> [bhelgaas: changelog]
>> Signed-off-by: Michael S. Tsirkin <mst-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>> Signed-off-by: Bjorn Helgaas <bhelgaas-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
>> CC: stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org # v3.13+
>>
>> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
>> index efe3443..413417f 100644
>> --- a/include/uapi/linux/pci_regs.h
>> +++ b/include/uapi/linux/pci_regs.h
>> @@ -319,6 +319,7 @@
>> #define PCI_MSIX_PBA 8 /* Pending Bit Array offset */
>> #define PCI_MSIX_PBA_BIR 0x00000007 /* BAR index */
>> #define PCI_MSIX_PBA_OFFSET 0xfffffff8 /* Offset into specified BAR */
>> +#define PCI_MSIX_FLAGS_BIRMASK PCI_MSIX_PBA_BIR /* deprecated */
>> #define PCI_CAP_MSIX_SIZEOF 12 /* size of MSIX registers */
>>
>> /* MSI-X Table entry format */
^ permalink raw reply
* Re: [PATCH V5 0/7] Allow user to request memory to be locked on page fault
From: Vlastimil Babka @ 2015-07-27 15:40 UTC (permalink / raw)
To: Eric B Munson
Cc: Andrew Morton, Shuah Khan, Michal Hocko, Michael Kerrisk,
Jonathan Corbet, Ralf Baechle, linux-alpha, linux-kernel,
linux-mips, linux-parisc, linuxppc-dev, sparclinux, linux-xtensa,
linux-mm, linux-arch, linux-api
In-Reply-To: <20150727145409.GB21664@akamai.com>
On 07/27/2015 04:54 PM, Eric B Munson wrote:
> On Mon, 27 Jul 2015, Vlastimil Babka wrote:
>>
>>> We do actually have an MCL_LOCKED, we just call it MCL_CURRENT. Would
>>> you prefer that I match the name in mlock2() (add MLOCK_CURRENT
>>> instead)?
>>
>> Hm it's similar but not exactly the same, because MCL_FUTURE is not
>> the same as MLOCK_ONFAULT :) So MLOCK_CURRENT would be even more
>> confusing. Especially if mlockall(MCL_CURRENT | MCL_FUTURE) is OK,
>> but mlock2(MLOCK_LOCKED | MLOCK_ONFAULT) is invalid.
>
> MLOCK_ONFAULT isn't meant to be the same as MCL_FUTURE, rather it is
> meant to be the same as MCL_ONFAULT. MCL_FUTURE only controls if the
> locking policy will be applied to any new mappings made by this process,
> not the locking policy itself. The better comparison is MCL_CURRENT to
> MLOCK_LOCK and MCL_ONFAULT to MLOCK_ONFAULT. MCL_CURRENT and
> MLOCK_LOCK do the same thing, only one requires a specific range of
> addresses while the other works process wide. This is why I suggested
> changing MLOCK_LOCK to MLOCK_CURRENT. It is an error to call
> mlock2(MLOCK_LOCK | MLOCK_ONFAULT) just like it is an error to call
> mlockall(MCL_CURRENT | MCL_ONFAULT). The combinations do no make sense.
How is it an error to call mlockall(MCL_CURRENT | MCL_ONFAULT)? How else
would you apply mlock2(MCL_ONFAULT) to all current mappings? Later below
you use the same example and I don't think it's different by removing
MLOCK_LOCKED flag.
> This was all decided when VM_LOCKONFAULT was a separate state from
> VM_LOCKED. Now that VM_LOCKONFAULT is a modifier to VM_LOCKED and
> cannot be specified independentally, it might make more sense to mirror
> that relationship to userspace. Which would lead to soemthing like the
> following:
>
> To lock and populate a region:
> mlock2(start, len, 0);
>
> To lock on fault a region:
> mlock2(start, len, MLOCK_ONFAULT);
>
> If LOCKONFAULT is seen as a modifier to mlock, then having the flags
> argument as 0 mean do mlock classic makes more sense to me.
Yup that's what I was trying to suggest.
> To mlock current on fault only:
> mlockall(MCL_CURRENT | MCL_ONFAULT);
>
> To mlock future on fault only:
> mlockall(MCL_FUTURE | MCL_ONFAULT);
>
> To lock everything on fault:
> mlockall(MCL_CURRENT | MCL_FUTURE | MCL_ONFAULT);
>
> I think I have talked myself into rewriting the set again :/
Sorry :) You could also wait a bit for more input than just from me...
>>
>>> Finally, on the question of MAP_LOCKONFAULT, do you just dislike
>>> MAP_LOCKED and do not want to see it extended, or is this a NAK on the
>>> set if that patch is included. I ask because I have to spin a V6 to get
>>> the MLOCK flag declarations right, but I would prefer not to do a V7+.
>>> If this is a NAK with, I can drop that patch and rework the tests to
>>> cover without the mmap flag. Otherwise I want to keep it, I have an
>>> internal user that would like to see it added.
>>
>> I don't want to NAK that patch if you think it's useful.
>>
>>
^ permalink raw reply
* [GIT PULL] Kselftest fixes for 4.2-rc5
From: Shuah Khan @ 2015-07-27 15:01 UTC (permalink / raw)
To: Linus Torvalds
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
open list:KERNEL SELFTEST F..., Shuah Khan
Hi Linus,
Please pull the following Kselftest fixes for 4.2-rc5
thanks,
-- Shuah
The following changes since commit d770e558e21961ad6cfdf0ff7df0eb5d7d4f0754:
Linux 4.2-rc1 (2015-07-05 11:01:52 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest
tags/linux-kselftest-4.2-rc5
for you to fetch changes up to fee50f3c8427aacabfb603229bf0a9056c3f2638:
selftests/futex: Fix futex_cmp_requeue_pi() error handling (2015-07-20
18:29:38 -0600)
----------------------------------------------------------------
linux-kselftest-4.2-rc5
Kselftest fixes for 4.2-rc5
----------------------------------------------------------------
Darren Hart (1):
selftests/futex: Fix futex_cmp_requeue_pi() error handling
.../selftests/futex/functional/futex_requeue_pi_signal_restart.c
| 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org | (970) 217-8978
^ permalink raw reply
* Re: [PATCH] pci_regs: reintroduce PCI_MSIX_FLAGS_BIRMASK
From: Michael S. Tsirkin @ 2015-07-27 14:55 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-kernel, Rajat Jain, Chen, Gong, Guenter Roeck,
=?UTF-8?q?Rafa=C5=82=20Mi=C5=82ecki?=, linux-api, linux-pci
In-Reply-To: <20150714232924.GS24416@google.com>
On Tue, Jul 14, 2015 at 06:29:24PM -0500, Bjorn Helgaas wrote:
> On Mon, Jul 13, 2015 at 06:11:38PM +0300, Michael S. Tsirkin wrote:
> > On Wed, Jul 01, 2015 at 12:14:10PM +0200, Michael S. Tsirkin wrote:
> > > This partially reverts commit 09a2c73ddfc7f173237fc7209a65b34dd5bcb5ed.
> > > PCI: Remove unused PCI_MSIX_FLAGS_BIRMASK definition
> > >
> > > That commit dropped a symbol from an exported header claiming "no one
> > > uses it". This isn't how Linux normally approaches userspace API though,
> > > and in fact QEMU build fails if trying to use updated headers from linux
> > > 3.12 and up.
> > >
> > > Sure, userspace can be fixed to use the new symbol, but the cost
> > > of keeping the old one around is fairly low, too.
> > >
> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>
> Sorry, I lost the original message somehow, so I can't reply to it. I
> applied this by hand to my for-linus branch for v4.2, thanks!
Did you mean 4.2? time's running out on this one ...
Just making sure - it's still not upstream, right?
>
> commit c9ddbac9c89110f77cb0fa07e634aaf1194899aa
> Author: Michael S. Tsirkin <mst@redhat.com>
> Date: Tue Jul 14 18:27:46 2015 -0500
>
> PCI: Restore PCI_MSIX_FLAGS_BIRMASK definition
>
> 09a2c73ddfc7 ("PCI: Remove unused PCI_MSIX_FLAGS_BIRMASK definition")
> removed PCI_MSIX_FLAGS_BIRMASK from an exported header because it was
> unused in the kernel. But that breaks user programs that were using it
> (QEMU in particular).
>
> Restore the PCI_MSIX_FLAGS_BIRMASK definition.
>
> [bhelgaas: changelog]
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> CC: stable@vger.kernel.org # v3.13+
>
> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> index efe3443..413417f 100644
> --- a/include/uapi/linux/pci_regs.h
> +++ b/include/uapi/linux/pci_regs.h
> @@ -319,6 +319,7 @@
> #define PCI_MSIX_PBA 8 /* Pending Bit Array offset */
> #define PCI_MSIX_PBA_BIR 0x00000007 /* BAR index */
> #define PCI_MSIX_PBA_OFFSET 0xfffffff8 /* Offset into specified BAR */
> +#define PCI_MSIX_FLAGS_BIRMASK PCI_MSIX_PBA_BIR /* deprecated */
> #define PCI_CAP_MSIX_SIZEOF 12 /* size of MSIX registers */
>
> /* MSI-X Table entry format */
^ permalink raw reply
* Re: [PATCH V5 0/7] Allow user to request memory to be locked on page fault
From: Eric B Munson @ 2015-07-27 14:54 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Andrew Morton, Shuah Khan, Michal Hocko, Michael Kerrisk,
Jonathan Corbet, Ralf Baechle, linux-alpha, linux-kernel,
linux-mips, linux-parisc, linuxppc-dev, sparclinux, linux-xtensa,
linux-mm, linux-arch, linux-api
In-Reply-To: <55B63D37.20303@suse.cz>
[-- Attachment #1: Type: text/plain, Size: 5469 bytes --]
On Mon, 27 Jul 2015, Vlastimil Babka wrote:
> On 07/27/2015 03:35 PM, Eric B Munson wrote:
> >On Mon, 27 Jul 2015, Vlastimil Babka wrote:
> >
> >>On 07/24/2015 11:28 PM, Eric B Munson wrote:
> >>
> >>...
> >>
> >>>Changes from V4:
> >>>Drop all architectures for new sys call entries except x86[_64] and MIPS
> >>>Drop munlock2 and munlockall2
> >>>Make VM_LOCKONFAULT a modifier to VM_LOCKED only to simplify book keeping
> >>>Adjust tests to match
> >>
> >>Hi, thanks for considering my suggestions. Well, I do hope there
> >>were correct as API's are hard and I'm no API expert. But since
> >>API's are also impossible to change after merging, I'm sorry but
> >>I'll keep pestering for one last thing. Thanks again for persisting,
> >>I do believe it's for the good thing!
> >>
> >>The thing is that I still don't like that one has to call
> >>mlock2(MLOCK_LOCKED) to get the equivalent of the old mlock(). Why
> >>is that flag needed? We have two modes of locking now, and v5 no
> >>longer treats them separately in vma flags. But having two flags
> >>gives us four possible combinations, so two of them would serve
> >>nothing but to confuse the programmer IMHO. What will mlock2()
> >>without flags do? What will mlock2(MLOCK_LOCKED | MLOCK_ONFAULT) do?
> >>(Note I haven't studied the code yet, as having agreed on the API
> >>should come first. But I did suggest documenting these things more
> >>thoroughly too...)
> >>OK I checked now and both cases above seem to return EINVAL.
> >>
> >>So about the only point I see in MLOCK_LOCKED flag is parity with
> >>MAP_LOCKED for mmap(). But as Kirill said (and me before as well)
> >>MAP_LOCKED is broken anyway so we shouldn't twist the rest just of
> >>the API to keep the poor thing happier in its misery.
> >>
> >>Also note that AFAICS you don't have MCL_LOCKED for mlockall() so
> >>there's no full parity anyway. But please don't fix that by adding
> >>MCL_LOCKED :)
> >>
> >>Thanks!
> >
> >
> >I have an MLOCK_LOCKED flag because I prefer an interface to be
> >explicit.
>
> I think it's already explicit enough that the user calls mlock2(),
> no? He obviously wants the range mlocked. An optional flag says that
> there should be no pre-fault.
>
> >The caller of mlock2() will be required to fill in the flags
> >argument regardless.
>
> I guess users not caring about MLOCK_ONFAULT will continue using
> plain mlock() without flags anyway.
>
> I can drop the MLOCK_LOCKED flag with 0 being the
> >value for LOCKED, but I thought it easier to make clear what was going
> >on at any call to mlock2(). If user space defines a MLOCK_LOCKED that
> >happens to be 0, I suppose that would be okay.
>
> Yeah that would remove the weird 4-states-of-which-2-are-invalid
> problem I mentioned, but at the cost of glibc wrapper behaving
> differently than the kernel syscall itself. For little gain.
>
> >We do actually have an MCL_LOCKED, we just call it MCL_CURRENT. Would
> >you prefer that I match the name in mlock2() (add MLOCK_CURRENT
> >instead)?
>
> Hm it's similar but not exactly the same, because MCL_FUTURE is not
> the same as MLOCK_ONFAULT :) So MLOCK_CURRENT would be even more
> confusing. Especially if mlockall(MCL_CURRENT | MCL_FUTURE) is OK,
> but mlock2(MLOCK_LOCKED | MLOCK_ONFAULT) is invalid.
MLOCK_ONFAULT isn't meant to be the same as MCL_FUTURE, rather it is
meant to be the same as MCL_ONFAULT. MCL_FUTURE only controls if the
locking policy will be applied to any new mappings made by this process,
not the locking policy itself. The better comparison is MCL_CURRENT to
MLOCK_LOCK and MCL_ONFAULT to MLOCK_ONFAULT. MCL_CURRENT and
MLOCK_LOCK do the same thing, only one requires a specific range of
addresses while the other works process wide. This is why I suggested
changing MLOCK_LOCK to MLOCK_CURRENT. It is an error to call
mlock2(MLOCK_LOCK | MLOCK_ONFAULT) just like it is an error to call
mlockall(MCL_CURRENT | MCL_ONFAULT). The combinations do no make sense.
This was all decided when VM_LOCKONFAULT was a separate state from
VM_LOCKED. Now that VM_LOCKONFAULT is a modifier to VM_LOCKED and
cannot be specified independentally, it might make more sense to mirror
that relationship to userspace. Which would lead to soemthing like the
following:
To lock and populate a region:
mlock2(start, len, 0);
To lock on fault a region:
mlock2(start, len, MLOCK_ONFAULT);
If LOCKONFAULT is seen as a modifier to mlock, then having the flags
argument as 0 mean do mlock classic makes more sense to me.
To mlock current on fault only:
mlockall(MCL_CURRENT | MCL_ONFAULT);
To mlock future on fault only:
mlockall(MCL_FUTURE | MCL_ONFAULT);
To lock everything on fault:
mlockall(MCL_CURRENT | MCL_FUTURE | MCL_ONFAULT);
I think I have talked myself into rewriting the set again :/
>
> >Finally, on the question of MAP_LOCKONFAULT, do you just dislike
> >MAP_LOCKED and do not want to see it extended, or is this a NAK on the
> >set if that patch is included. I ask because I have to spin a V6 to get
> >the MLOCK flag declarations right, but I would prefer not to do a V7+.
> >If this is a NAK with, I can drop that patch and rework the tests to
> >cover without the mmap flag. Otherwise I want to keep it, I have an
> >internal user that would like to see it added.
>
> I don't want to NAK that patch if you think it's useful.
>
>
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: Next round: revised futex(2) man page for review
From: Michael Kerrisk (man-pages) @ 2015-07-27 14:44 UTC (permalink / raw)
To: Heinrich Schuchardt, Thomas Gleixner, Darren Hart, Torvald Riegel
Cc: mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w, Carlos O'Donell,
Ingo Molnar, Jakub Jelinek, linux-man, lkml, Davidlohr Bueso,
Arnd Bergmann, Steven Rostedt, Peter Zijlstra, Linux API,
Roland McGrath, Anton Blanchard, Eric Dumazet, bill o gallmeister,
Jan Kiszka, Daniel Wagner, Rich Felker, Andy Lutomirski,
bert hubert, Rusty Russell
In-Reply-To: <trinity-f9a7a25f-09e9-4409-b656-0e035dfca0b5-1438006633780@msvc-mesg-gmx010>
On 07/27/2015 04:17 PM, Heinrich Schuchardt wrote:
> instruction. A thread maybe unable
>
> to << missing word
>
> acquire a lock because it is
> already acquired by another thread. It then may pass the lock's
> flag as futex word and the value representing the acquired state
> as the expected value to a futex() wait operation.
Thanks, Heinrich. Fixed.
Cheers,
Michael
^ permalink raw reply
* Re: [PATCH V5 0/7] Allow user to request memory to be locked on page fault
From: Vlastimil Babka @ 2015-07-27 14:16 UTC (permalink / raw)
To: Eric B Munson
Cc: Andrew Morton, Shuah Khan, Michal Hocko, Michael Kerrisk,
Jonathan Corbet, Ralf Baechle, linux-alpha, linux-kernel,
linux-mips, linux-parisc, linuxppc-dev, sparclinux, linux-xtensa,
linux-mm, linux-arch, linux-api
In-Reply-To: <20150727133555.GA17133@akamai.com>
On 07/27/2015 03:35 PM, Eric B Munson wrote:
> On Mon, 27 Jul 2015, Vlastimil Babka wrote:
>
>> On 07/24/2015 11:28 PM, Eric B Munson wrote:
>>
>> ...
>>
>>> Changes from V4:
>>> Drop all architectures for new sys call entries except x86[_64] and MIPS
>>> Drop munlock2 and munlockall2
>>> Make VM_LOCKONFAULT a modifier to VM_LOCKED only to simplify book keeping
>>> Adjust tests to match
>>
>> Hi, thanks for considering my suggestions. Well, I do hope there
>> were correct as API's are hard and I'm no API expert. But since
>> API's are also impossible to change after merging, I'm sorry but
>> I'll keep pestering for one last thing. Thanks again for persisting,
>> I do believe it's for the good thing!
>>
>> The thing is that I still don't like that one has to call
>> mlock2(MLOCK_LOCKED) to get the equivalent of the old mlock(). Why
>> is that flag needed? We have two modes of locking now, and v5 no
>> longer treats them separately in vma flags. But having two flags
>> gives us four possible combinations, so two of them would serve
>> nothing but to confuse the programmer IMHO. What will mlock2()
>> without flags do? What will mlock2(MLOCK_LOCKED | MLOCK_ONFAULT) do?
>> (Note I haven't studied the code yet, as having agreed on the API
>> should come first. But I did suggest documenting these things more
>> thoroughly too...)
>> OK I checked now and both cases above seem to return EINVAL.
>>
>> So about the only point I see in MLOCK_LOCKED flag is parity with
>> MAP_LOCKED for mmap(). But as Kirill said (and me before as well)
>> MAP_LOCKED is broken anyway so we shouldn't twist the rest just of
>> the API to keep the poor thing happier in its misery.
>>
>> Also note that AFAICS you don't have MCL_LOCKED for mlockall() so
>> there's no full parity anyway. But please don't fix that by adding
>> MCL_LOCKED :)
>>
>> Thanks!
>
>
> I have an MLOCK_LOCKED flag because I prefer an interface to be
> explicit.
I think it's already explicit enough that the user calls mlock2(), no?
He obviously wants the range mlocked. An optional flag says that there
should be no pre-fault.
> The caller of mlock2() will be required to fill in the flags
> argument regardless.
I guess users not caring about MLOCK_ONFAULT will continue using plain
mlock() without flags anyway.
I can drop the MLOCK_LOCKED flag with 0 being the
> value for LOCKED, but I thought it easier to make clear what was going
> on at any call to mlock2(). If user space defines a MLOCK_LOCKED that
> happens to be 0, I suppose that would be okay.
Yeah that would remove the weird 4-states-of-which-2-are-invalid problem
I mentioned, but at the cost of glibc wrapper behaving differently than
the kernel syscall itself. For little gain.
> We do actually have an MCL_LOCKED, we just call it MCL_CURRENT. Would
> you prefer that I match the name in mlock2() (add MLOCK_CURRENT
> instead)?
Hm it's similar but not exactly the same, because MCL_FUTURE is not the
same as MLOCK_ONFAULT :) So MLOCK_CURRENT would be even more confusing.
Especially if mlockall(MCL_CURRENT | MCL_FUTURE) is OK, but
mlock2(MLOCK_LOCKED | MLOCK_ONFAULT) is invalid.
> Finally, on the question of MAP_LOCKONFAULT, do you just dislike
> MAP_LOCKED and do not want to see it extended, or is this a NAK on the
> set if that patch is included. I ask because I have to spin a V6 to get
> the MLOCK flag declarations right, but I would prefer not to do a V7+.
> If this is a NAK with, I can drop that patch and rework the tests to
> cover without the mmap flag. Otherwise I want to keep it, I have an
> internal user that would like to see it added.
I don't want to NAK that patch if you think it's useful.
^ permalink raw reply
* Re: [PATCH V5 5/7] mm: mmap: Add mmap flag to request VM_LOCKONFAULT
From: Eric B Munson @ 2015-07-27 14:11 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: Andrew Morton, Michal Hocko, Vlastimil Babka, Paul Gortmaker,
Chris Metcalf, Guenter Roeck, linux-alpha, linux-kernel,
linux-mips, linux-parisc, linuxppc-dev, sparclinux, linux-xtensa,
linux-mm, linux-arch, linux-api
In-Reply-To: <20150727140355.GA11360@node.dhcp.inet.fi>
[-- Attachment #1: Type: text/plain, Size: 2135 bytes --]
On Mon, 27 Jul 2015, Kirill A. Shutemov wrote:
> On Mon, Jul 27, 2015 at 09:41:26AM -0400, Eric B Munson wrote:
> > On Mon, 27 Jul 2015, Kirill A. Shutemov wrote:
> >
> > > On Fri, Jul 24, 2015 at 05:28:43PM -0400, Eric B Munson wrote:
> > > > The cost of faulting in all memory to be locked can be very high when
> > > > working with large mappings. If only portions of the mapping will be
> > > > used this can incur a high penalty for locking.
> > > >
> > > > Now that we have the new VMA flag for the locked but not present state,
> > > > expose it as an mmap option like MAP_LOCKED -> VM_LOCKED.
> > >
> > > As I mentioned before, I don't think this interface is justified.
> > >
> > > MAP_LOCKED has known issues[1]. The MAP_LOCKED problem is not necessary
> > > affects MAP_LOCKONFAULT, but still.
> > >
> > > Let's not add new interface unless it's demonstrably useful.
> > >
> > > [1] http://lkml.kernel.org/g/20150114095019.GC4706@dhcp22.suse.cz
> >
> > I understand and should have been more explicit. This patch is still
> > included becuase I have an internal user that wants to see it added.
> > The problem discussed in the thread you point out does not affect
> > MAP_LOCKONFAULT because we do not attempt to populate the region with
> > MAP_LOCKONFAULT.
> >
> > As I told Vlastimil, if this is a hard NAK with the patch I can work
> > with that. Otherwise I prefer it stays.
>
> That's not how it works.
I am not sure what you mean here. I have a user that will find this
useful and MAP_LOCKONFAULT does not suffer from the problem you point
out. I do not understand your NAK but thank you for explicit about it.
>
> Once an ABI added to the kernel it stays there practically forever.
> Therefore it must be useful to justify maintenance cost. I don't see it
> demonstrated.
I understand this, and I get that you do not like MAP_LOCKED, but I do
not see how your dislike for MAP_LOCKED means that this would not be
useful.
>
> So, NAK.
>
V6 will not have the new mmap flag unless there is someone else that
speaks up in favor of keeping it.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH V5 5/7] mm: mmap: Add mmap flag to request VM_LOCKONFAULT
From: Kirill A. Shutemov @ 2015-07-27 14:03 UTC (permalink / raw)
To: Eric B Munson
Cc: Andrew Morton, Michal Hocko, Vlastimil Babka, Paul Gortmaker,
Chris Metcalf, Guenter Roeck, linux-alpha, linux-kernel,
linux-mips, linux-parisc, linuxppc-dev, sparclinux, linux-xtensa,
linux-mm, linux-arch, linux-api
In-Reply-To: <20150727134126.GB17133@akamai.com>
On Mon, Jul 27, 2015 at 09:41:26AM -0400, Eric B Munson wrote:
> On Mon, 27 Jul 2015, Kirill A. Shutemov wrote:
>
> > On Fri, Jul 24, 2015 at 05:28:43PM -0400, Eric B Munson wrote:
> > > The cost of faulting in all memory to be locked can be very high when
> > > working with large mappings. If only portions of the mapping will be
> > > used this can incur a high penalty for locking.
> > >
> > > Now that we have the new VMA flag for the locked but not present state,
> > > expose it as an mmap option like MAP_LOCKED -> VM_LOCKED.
> >
> > As I mentioned before, I don't think this interface is justified.
> >
> > MAP_LOCKED has known issues[1]. The MAP_LOCKED problem is not necessary
> > affects MAP_LOCKONFAULT, but still.
> >
> > Let's not add new interface unless it's demonstrably useful.
> >
> > [1] http://lkml.kernel.org/g/20150114095019.GC4706@dhcp22.suse.cz
>
> I understand and should have been more explicit. This patch is still
> included becuase I have an internal user that wants to see it added.
> The problem discussed in the thread you point out does not affect
> MAP_LOCKONFAULT because we do not attempt to populate the region with
> MAP_LOCKONFAULT.
>
> As I told Vlastimil, if this is a hard NAK with the patch I can work
> with that. Otherwise I prefer it stays.
That's not how it works.
Once an ABI added to the kernel it stays there practically forever.
Therefore it must be useful to justify maintenance cost. I don't see it
demonstrated.
So, NAK.
--
Kirill A. Shutemov
^ permalink raw reply
* Re: [PATCH 4/4] mm: remove direct calling of migration
From: Vlastimil Babka @ 2015-07-27 13:58 UTC (permalink / raw)
To: Gioh Kim, jlayton, bfields, iamjoonsoo.kim, viro, mst, koct9i,
minchan, aquini, linux-fsdevel, virtualization, linux-kernel,
linux-api, linux-mm
Cc: dri-devel, akpm, Gioh Kim
In-Reply-To: <1436776519-17337-5-git-send-email-gioh.kim@lge.com>
On 07/13/2015 10:35 AM, Gioh Kim wrote:
> From: Gioh Kim <gurugio@hanmail.net>
>
> Migration is completely generalized so that migrating mobile page
> is processed with lru-pages in move_to_new_page.
>
> Signed-off-by: Gioh Kim <gioh.kim@lge.com>
> Acked-by: Rafael Aquini <aquini@redhat.com>
Why not just fold this to Patch 3? You already modify this hunk there,
and prior to patch 3, the hunk was balloon-pages specific. You made it
look generic only to remove it, which is unneeded code churn and I don't
think it adds anything wrt e.g. bisectability.
> ---
> mm/migrate.c | 15 ---------------
> 1 file changed, 15 deletions(-)
>
> diff --git a/mm/migrate.c b/mm/migrate.c
> index 53f0081d..e6644ac 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -844,21 +844,6 @@ static int __unmap_and_move(struct page *page, struct page *newpage,
> }
> }
>
> - if (unlikely(mobile_page(page))) {
> - /*
> - * A mobile page does not need any special attention from
> - * physical to virtual reverse mapping procedures.
> - * Skip any attempt to unmap PTEs or to remap swap cache,
> - * in order to avoid burning cycles at rmap level, and perform
> - * the page migration right away (proteced by page lock).
> - */
> - lock_page(newpage);
> - rc = page->mapping->a_ops->migratepage(page->mapping,
> - newpage, page, mode);
> - unlock_page(newpage);
> - goto out_unlock;
> - }
> -
> /*
> * Corner case handling:
> * 1. When a new swap-cache page is read into, it is added to the LRU
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox