Linux userland API discussions
 help / color / mirror / Atom feed
* Re: [PATCH v4 1/5] nohz_full: add support for "cpu_isolated" mode
From: Chris Metcalf @ 2015-07-24 20:21 UTC (permalink / raw)
  To: Frederic Weisbecker
  Cc: Gilad Ben Yossef, Steven Rostedt, Ingo Molnar, Peter Zijlstra,
	Andrew Morton, Rik van Riel, Tejun Heo, Thomas Gleixner,
	Paul E. McKenney, Christoph Lameter, Viresh Kumar,
	linux-doc-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150724132659.GA20091@lerouge>

On 07/24/2015 09:27 AM, Frederic Weisbecker wrote:
> On Mon, Jul 13, 2015 at 03:57:57PM -0400, Chris Metcalf wrote:
>> +{
>> +	struct clock_event_device *dev =
>> +		__this_cpu_read(tick_cpu_device.evtdev);
>> +	struct task_struct *task = current;
>> +	unsigned long start = jiffies;
>> +	bool warned = false;
>> +
>> +	/* Drain the pagevecs to avoid unnecessary IPI flushes later. */
>> +	lru_add_drain();
>> +
>> +	while (READ_ONCE(dev->next_event.tv64) != KTIME_MAX) {
>> +		if (!warned && (jiffies - start) >= (5 * HZ)) {
>> +			pr_warn("%s/%d: cpu %d: cpu_isolated task blocked for %ld seconds\n",
>> +				task->comm, task->pid, smp_processor_id(),
>> +				(jiffies - start) / HZ);
>> +			warned = true;
>> +		}
>> +		if (should_resched())
>> +			schedule();
>> +		if (test_thread_flag(TIF_SIGPENDING))
>> +			break;
>> +		tick_nohz_cpu_isolated_wait();
> If we call cpu_idle(), what is going to wake the CPU up if no further interrupt happen?
>
> We could either implement some sort of tick waiters with proper wake up once the CPU sees
> no tick to schedule. Arguably this is all risky because this involve a scheduler wake up
> and thus the risk for new noise. But it might work.
>
> Another possibility is an msleep() based wait. But that's about the same, maybe even worse
> due to repetitive wake ups.

The presumption here is that it is not possible to have
tick_cpu_device have a pending next_event without also
having a timer interrupt pending to go off.  That certainly
seems to be true on the architectures I have looked at.
Do we think that might ever not be the case?

We are running here with interrupts disabled, so this core won't
transition from "timer interrupt scheduled" to "no timer interrupt
scheduled" before we spin or idle, and presumably no other core
can reach across and turn off our timer interrupt either.

-- 
Chris Metcalf, EZChip Semiconductor
http://www.ezchip.com

^ permalink raw reply

* Re: [PATCH v4 1/5] nohz_full: add support for "cpu_isolated" mode
From: Chris Metcalf @ 2015-07-24 20:19 UTC (permalink / raw)
  To: Frederic Weisbecker
  Cc: Andy Lutomirski, Gilad Ben Yossef, Steven Rostedt, Ingo Molnar,
	Peter Zijlstra, Andrew Morton, Rik van Riel, Tejun Heo,
	Thomas Gleixner, Paul E. McKenney, Christoph Lameter,
	Viresh Kumar, linux-doc@vger.kernel.org, Linux API,
	linux-kernel@vger.kernel.org, Mike Galbraith
In-Reply-To: <20150724140308.GB20091@lerouge>

On 07/24/2015 10:03 AM, Frederic Weisbecker wrote:
> To summarize, lets first split nohz and isolation. Introduce
> CONFIG_CPU_ISOLATION and stuff all the isolation policies to
> kernel/cpu_isolation.c, lets try to implement hard isolation and see if that
> meets soft isolation workload users as well, if not we'll split that later.

I will do that for v5.

-- 
Chris Metcalf, EZChip Semiconductor
http://www.ezchip.com


^ permalink raw reply

* Re: [PATCH v4 3/5] pagemap: rework hugetlb and thp report
From: Mark Williamson @ 2015-07-24 18:19 UTC (permalink / raw)
  To: Konstantin Khlebnikov
  Cc: Naoya Horiguchi, Konstantin Khlebnikov, linux-mm@kvack.org,
	Andrew Morton, Kirill A. Shutemov, linux-kernel@vger.kernel.org,
	linux-api@vger.kernel.org
In-Reply-To: <CAEVpBaKTMs7JQrwHQVKF5ySb58QTm6Zr+GPgZevtjgStYBk8RQ@mail.gmail.com>

(my review on this patch comes with the caveat that the specifics of
hugetlb / thp are a bit outside my experience)

On Fri, Jul 24, 2015 at 7:17 PM, Mark Williamson
<mwilliamson@undo-software.com> wrote:
> Reviewed-by: Mark Williamson <mwilliamson@undo-software.com>
>
> On Tue, Jul 21, 2015 at 9:43 AM, Konstantin Khlebnikov <koct9i@gmail.com> wrote:
>> On Tue, Jul 21, 2015 at 11:00 AM, Naoya Horiguchi
>> <n-horiguchi@ah.jp.nec.com> wrote:
>>> On Tue, Jul 14, 2015 at 06:37:39PM +0300, Konstantin Khlebnikov wrote:
>>>> This patch moves pmd dissection out of reporting loop: huge pages
>>>> are reported as bunch of normal pages with contiguous PFNs.
>>>>
>>>> Add missing "FILE" bit in hugetlb vmas.
>>>>
>>>> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
>>>
>>> With reflecting Kirill's comment about #ifdef, I'm OK for this patch.
>>
>> That ifdef works most like documentation: "all thp magic happens here".
>> I'd like to keep it, if two redundant lines isn't a big deal.
>>
>>>
>>> Reviewed-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
>>> --
>>> 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 hrefmailto:"dont@kvack.org"> email@kvack.org </a>

--
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 v4 5/5] pagemap: add mmap-exclusive bit for marking pages mapped only here
From: Mark Williamson @ 2015-07-24 18:18 UTC (permalink / raw)
  To: Naoya Horiguchi
  Cc: Konstantin Khlebnikov, linux-mm@kvack.org, Andrew Morton,
	Kirill A. Shutemov, linux-kernel@vger.kernel.org,
	linux-api@vger.kernel.org
In-Reply-To: <20150721081755.GD4490@hori1.linux.bs1.fc.nec.co.jp>

Reviewed-by: Mark Williamson <mwilliamson@undo-software.com>

On Tue, Jul 21, 2015 at 9:17 AM, Naoya Horiguchi
<n-horiguchi@ah.jp.nec.com> wrote:
> On Tue, Jul 14, 2015 at 06:37:49PM +0300, Konstantin Khlebnikov wrote:
>> This patch sets bit 56 in pagemap if this page is mapped only once.
>> It allows to detect exclusively used pages without exposing PFN:
>>
>> present file exclusive state
>> 0       0    0         non-present
>> 1       1    0         file page mapped somewhere else
>> 1       1    1         file page mapped only here
>> 1       0    0         anon non-CoWed page (shared with parent/child)
>> 1       0    1         anon CoWed page (or never forked)
>>
>> CoWed pages in (MAP_FILE | MAP_PRIVATE) areas are anon in this context.
>>
>> MMap-exclusive bit doesn't reflect potential page-sharing via swapcache:
>> page could be mapped once but has several swap-ptes which point to it.
>> Application could detect that by swap bit in pagemap entry and touch
>> that pte via /proc/pid/mem to get real information.
>>
>> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
>> Requested-by: Mark Williamson <mwilliamson@undo-software.com>
>> Link: http://lkml.kernel.org/r/CAEVpBa+_RyACkhODZrRvQLs80iy0sqpdrd0AaP_-tgnX3Y9yNQ@mail.gmail.com
>
> Reviewed-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>

^ permalink raw reply

* Re: [PATCH v4 4/5] pagemap: hide physical addresses from non-privileged users
From: Mark Williamson @ 2015-07-24 18:18 UTC (permalink / raw)
  To: Konstantin Khlebnikov
  Cc: Naoya Horiguchi, Konstantin Khlebnikov, linux-mm@kvack.org,
	Andrew Morton, Kirill A. Shutemov, linux-kernel@vger.kernel.org,
	linux-api@vger.kernel.org
In-Reply-To: <CALYGNiNHooR0eqH7rYfzOj65_97H6EeF34Bfbgh50JK+k4yN7w@mail.gmail.com>

Reviewed-by: Mark Williamson <mwilliamson@undo-software.com>

On Tue, Jul 21, 2015 at 9:39 AM, Konstantin Khlebnikov <koct9i@gmail.com> wrote:
> On Tue, Jul 21, 2015 at 11:11 AM, Naoya Horiguchi
> <n-horiguchi@ah.jp.nec.com> wrote:
>> On Tue, Jul 14, 2015 at 06:37:47PM +0300, Konstantin Khlebnikov wrote:
>>> This patch makes pagemap readable for normal users and hides physical
>>> addresses from them. For some use-cases PFN isn't required at all.
>>>
>>> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
>>> Fixes: ab676b7d6fbf ("pagemap: do not leak physical addresses to non-privileged userspace")
>>> Link: http://lkml.kernel.org/r/1425935472-17949-1-git-send-email-kirill@shutemov.name
>>> ---
>>>  fs/proc/task_mmu.c |   25 ++++++++++++++-----------
>>>  1 file changed, 14 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
>>> index 040721fa405a..3a5d338ea219 100644
>>> --- a/fs/proc/task_mmu.c
>>> +++ b/fs/proc/task_mmu.c
>>> @@ -937,6 +937,7 @@ typedef struct {
>>>  struct pagemapread {
>>>       int pos, len;           /* units: PM_ENTRY_BYTES, not bytes */
>>>       pagemap_entry_t *buffer;
>>> +     bool show_pfn;
>>>  };
>>>
>>>  #define PAGEMAP_WALK_SIZE    (PMD_SIZE)
>>> @@ -1013,7 +1014,8 @@ static pagemap_entry_t pte_to_pagemap_entry(struct pagemapread *pm,
>>>       struct page *page = NULL;
>>>
>>>       if (pte_present(pte)) {
>>> -             frame = pte_pfn(pte);
>>> +             if (pm->show_pfn)
>>> +                     frame = pte_pfn(pte);
>>>               flags |= PM_PRESENT;
>>>               page = vm_normal_page(vma, addr, pte);
>>>               if (pte_soft_dirty(pte))
>>
>> Don't you need the same if (pm->show_pfn) check in is_swap_pte path, too?
>> (although I don't think that it can be exploited by row hammer attack ...)
>
> Yeah, but I see no reason for that.
> Probably except swap on ramdrive, but this too weird =)
>
>>
>> Thanks,
>> Naoya Horiguchi
>> --
>> 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 hrefmailto:"dont@kvack.org"> email@kvack.org </a>

--
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 v4 3/5] pagemap: rework hugetlb and thp report
From: Mark Williamson @ 2015-07-24 18:17 UTC (permalink / raw)
  To: Konstantin Khlebnikov
  Cc: Naoya Horiguchi, Konstantin Khlebnikov,
	linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org, Andrew Morton,
	Kirill A. Shutemov,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CALYGNiO9wpmnrqbKhRKvfHgnUC854accifKc3m6Bvqsv0LHqXQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Reviewed-by: Mark Williamson <mwilliamson-/4lU09Eg6ahx67MzidHQgQC/G2K4zDHf@public.gmane.org>

On Tue, Jul 21, 2015 at 9:43 AM, Konstantin Khlebnikov <koct9i-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Tue, Jul 21, 2015 at 11:00 AM, Naoya Horiguchi
> <n-horiguchi-PaJj6Psr51x8UrSeD/g0lQ@public.gmane.org> wrote:
>> On Tue, Jul 14, 2015 at 06:37:39PM +0300, Konstantin Khlebnikov wrote:
>>> This patch moves pmd dissection out of reporting loop: huge pages
>>> are reported as bunch of normal pages with contiguous PFNs.
>>>
>>> Add missing "FILE" bit in hugetlb vmas.
>>>
>>> Signed-off-by: Konstantin Khlebnikov <khlebnikov-XoJtRXgx1JseBXzfvpsJ4g@public.gmane.org>
>>
>> With reflecting Kirill's comment about #ifdef, I'm OK for this patch.
>
> That ifdef works most like documentation: "all thp magic happens here".
> I'd like to keep it, if two redundant lines isn't a big deal.
>
>>
>> Reviewed-by: Naoya Horiguchi <n-horiguchi-PaJj6Psr51x8UrSeD/g0lQ@public.gmane.org>
>> --
>> To unsubscribe, send a message with 'unsubscribe linux-mm' in
>> the body to majordomo-Bw31MaZKKs0EbZ0PF+XxCw@public.gmane.org  For more info on Linux MM,
>> see: http://www.linux-mm.org/ .
>> Don't email: <a hrefmailto:"dont-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org"> email-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org </a>

^ permalink raw reply

* Re: [PATCH v4 1/5] pagemap: check permissions and capabilities at open time
From: Mark Williamson @ 2015-07-24 18:16 UTC (permalink / raw)
  To: Naoya Horiguchi
  Cc: Konstantin Khlebnikov,
	linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org, Andrew Morton,
	Kirill A. Shutemov,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20150721080626.GB4490-CFvn+FByjrC01j0XV7yAGhjPlmGC1jup6qtp775pBPw@public.gmane.org>

(within the limits of my understanding of the mm code)
Reviewed-by: Mark Williamson <mwilliamson-/4lU09Eg6ahx67MzidHQgQC/G2K4zDHf@public.gmane.org>

On Tue, Jul 21, 2015 at 9:06 AM, Naoya Horiguchi
<n-horiguchi-PaJj6Psr51x8UrSeD/g0lQ@public.gmane.org> wrote:
> On Tue, Jul 14, 2015 at 06:37:35PM +0300, Konstantin Khlebnikov wrote:
>> This patch moves permission checks from pagemap_read() into pagemap_open().
>>
>> Pointer to mm is saved in file->private_data. This reference pins only
>> mm_struct itself. /proc/*/mem, maps, smaps already work in the same way.
>>
>> Signed-off-by: Konstantin Khlebnikov <khlebnikov-XoJtRXgx1JseBXzfvpsJ4g@public.gmane.org>
>> Link: http://lkml.kernel.org/r/CA+55aFyKpWrt_Ajzh1rzp_GcwZ4=6Y=kOv8hBz172CFJp6L8Tg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org
>
> Reviewed-by: Naoya Horiguchi <n-horiguchi-PaJj6Psr51x8UrSeD/g0lQ@public.gmane.org>

^ permalink raw reply

* Re: [PATCHSET v4 0/5] pagemap: make useable for non-privilege users
From: Mark Williamson @ 2015-07-24 17:34 UTC (permalink / raw)
  To: Konstantin Khlebnikov
  Cc: linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Andrew Morton, Naoya Horiguchi,
	Kirill A. Shutemov, kernel list, Linux API
In-Reply-To: <20150714152516.29844.69929.stgit@buzz>

Hi Konstantin,

Thank you for the further update - I tested this patchset against our
code and it allows our software to work correctly (with minor userland
changes, as before).

I'll follow up with review messages but there aren't really any
concerns that I can see.

Cheers,
Mark

On Tue, Jul 14, 2015 at 4:37 PM, Konstantin Khlebnikov
<khlebnikov-XoJtRXgx1JseBXzfvpsJ4g@public.gmane.org> wrote:
> This patchset makes pagemap useable again in the safe way (after row hammer
> bug it was made CAP_SYS_ADMIN-only). This patchset restores access for
> non-privileged users but hides PFNs from them.
>
> Also it adds bit 'map-exlusive' which is set if page is mapped only here:
> it helps in estimation of working set without exposing pfns and allows to
> distinguish CoWed and non-CoWed private anonymous pages.
>
> Second patch removes page-shift bits and completes migration to the new
> pagemap format: flags soft-dirty and mmap-exlusive are available only
> in the new format.
>
> Changes since v3:
> * patches reordered: cleanup now in second patch
> * update pagemap for hugetlb, add missing 'FILE' bit
> * fix PM_PFRAME_BITS: its 55 not 54 as was in previous versions
>
> ---
>
> Konstantin Khlebnikov (5):
>       pagemap: check permissions and capabilities at open time
>       pagemap: switch to the new format and do some cleanup
>       pagemap: rework hugetlb and thp report
>       pagemap: hide physical addresses from non-privileged users
>       pagemap: add mmap-exclusive bit for marking pages mapped only here
>
>
>  Documentation/vm/pagemap.txt |    3
>  fs/proc/task_mmu.c           |  267 ++++++++++++++++++------------------------
>  tools/vm/page-types.c        |   35 +++---
>  3 files changed, 137 insertions(+), 168 deletions(-)
>
> --
> Konstantin

^ permalink raw reply

* Re: [PATCH V4 2/6] mm: mlock: Add new mlock, munlock, and munlockall system calls
From: Eric B Munson @ 2015-07-24 15:53 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Ralf Baechle, Michael Ellerman, Andrew Morton, linux-mips,
	linux-m68k, linux-ia64, linux-sh, Catalin Marinas, Heiko Carstens,
	Michal Hocko, linux-mm, sparclinux, linux-arch, Stephen Rothwell,
	linux-am33-list, Geert Uytterhoeven, Vlastimil Babka,
	linux-xtensa, linux-s390, adi-buildroot-devel, linux-arm-kernel,
	linux-cris-kernel, linux-parisc, linux-api, linux-kernel,
	linux-alpha, linuxppc-dev
In-Reply-To: <55B25DDE.8090107@roeck-us.net>

[-- Attachment #1: Type: text/plain, Size: 1272 bytes --]

On Fri, 24 Jul 2015, Guenter Roeck wrote:

> On 07/24/2015 07:39 AM, Eric B Munson wrote:
> >On Thu, 23 Jul 2015, Ralf Baechle wrote:
> >
> >>On Wed, Jul 22, 2015 at 10:15:01AM -0400, Eric B Munson wrote:
> >>
> >>>>
> >>>>You haven't wired it up properly on powerpc, but I haven't mentioned it because
> >>>>I'd rather we did it.
> >>>>
> >>>>cheers
> >>>
> >>>It looks like I will be spinning a V5, so I will drop all but the x86
> >>>system calls additions in that version.
> >>
> >>The MIPS bits are looking good however, so
> >>
> >>Acked-by: Ralf Baechle <ralf@linux-mips.org>
> >>
> >>With my ack, will you keep them or maybe carry them as a separate patch?
> >
> >I will keep the MIPS additions as a separate patch in the series, though
> >I have dropped two of the new syscalls after some discussion.  So I will
> >not include your ack on the new patch.
> >
> >Eric
> >
> 
> Hi Eric,
> 
> next-20150724 still has some failures due to this patch set. Are those
> being looked at (I know parisc builds fail, but there may be others) ?
> 
> Thanks,
> Guenter

Guenter,

Yes, the next respin will drop all new arch syscall entries except
x86[_64] and MIPS.  I will leave it up to arch maintainers to add the
entries.

Eric

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH V4 2/6] mm: mlock: Add new mlock, munlock, and munlockall system calls
From: Guenter Roeck @ 2015-07-24 15:46 UTC (permalink / raw)
  To: Eric B Munson, Ralf Baechle
  Cc: Michael Ellerman, Andrew Morton,
	linux-mips-6z/3iImG2C8G8FEW9MqTrA,
	linux-m68k-u79uwXL29TY76Z2rM5mHXA,
	linux-ia64-u79uwXL29TY76Z2rM5mHXA,
	linux-sh-u79uwXL29TY76Z2rM5mHXA, Catalin Marinas, Heiko Carstens,
	Michal Hocko, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
	sparclinux-u79uwXL29TY76Z2rM5mHXA,
	linux-arch-u79uwXL29TY76Z2rM5mHXA, Stephen Rothwell,
	linux-am33-list-H+wXaHxf7aLQT0dZR+AlfA, Geert Uytterhoeven,
	Vlastimil Babka, linux-xtensa-PjhNF2WwrV/0Sa2dR60CXw,
	linux-s390-u79uwXL29TY76Z2rM5mHXA,
	adi-buildroot-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-cris-kernel-VrBV9hrLPhE,
	linux-parisc-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-alpha-u79uwXL29TY76Z2rM5mHXA,
	linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ
In-Reply-To: <20150724143936.GE9203-JqFfY2XvxFXQT0dZR+AlfA@public.gmane.org>

On 07/24/2015 07:39 AM, Eric B Munson wrote:
> On Thu, 23 Jul 2015, Ralf Baechle wrote:
>
>> On Wed, Jul 22, 2015 at 10:15:01AM -0400, Eric B Munson wrote:
>>
>>>>
>>>> You haven't wired it up properly on powerpc, but I haven't mentioned it because
>>>> I'd rather we did it.
>>>>
>>>> cheers
>>>
>>> It looks like I will be spinning a V5, so I will drop all but the x86
>>> system calls additions in that version.
>>
>> The MIPS bits are looking good however, so
>>
>> Acked-by: Ralf Baechle <ralf-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org>
>>
>> With my ack, will you keep them or maybe carry them as a separate patch?
>
> I will keep the MIPS additions as a separate patch in the series, though
> I have dropped two of the new syscalls after some discussion.  So I will
> not include your ack on the new patch.
>
> Eric
>

Hi Eric,

next-20150724 still has some failures due to this patch set. Are those
being looked at (I know parisc builds fail, but there may be others) ?

Thanks,
Guenter

^ permalink raw reply

* Re: [PATCH 00/14] Enhance twl4030_charger functionality. - V2
From: Sebastian Reichel @ 2015-07-24 14:44 UTC (permalink / raw)
  To: NeilBrown
  Cc: linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, GTA04 owners,
	inux-pm-u79uwXL29TY76Z2rM5mHXA, Pavel Machek,
	linux-omap-u79uwXL29TY76Z2rM5mHXA, Lee Jones
In-Reply-To: <20150322231635.23789.70149.stgit-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 372 bytes --]

Hi Neil,

On Mon, Mar 23, 2015 at 10:20:28AM +1100, NeilBrown wrote:
> here is V2 of my patches to improve the twl4030 charger.
> They include improved documentation and removal of changes
> to the CVENAC bit which I didn't properly understand.

I rebased and queued patches 1 and 2. Can you rebase the
other patches and take care of the remaining comments?

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH V4 2/6] mm: mlock: Add new mlock, munlock, and munlockall system calls
From: Eric B Munson @ 2015-07-24 14:39 UTC (permalink / raw)
  To: Ralf Baechle
  Cc: Michael Ellerman, Andrew Morton, linux-mips, linux-m68k,
	linux-ia64, linux-sh, Catalin Marinas, Heiko Carstens,
	Michal Hocko, linux-mm, sparclinux, linux-arch, Stephen Rothwell,
	linux-am33-list, Geert Uytterhoeven, Vlastimil Babka,
	Guenter Roeck, linux-xtensa, linux-s390, adi-buildroot-devel,
	linux-arm-kernel, linux-cris-kernel, linux-parisc, linux-api,
	linux-kernel, linux-alpha, linuxppc-dev
In-Reply-To: <20150723065830.GA5919@linux-mips.org>

[-- Attachment #1: Type: text/plain, Size: 766 bytes --]

On Thu, 23 Jul 2015, Ralf Baechle wrote:

> On Wed, Jul 22, 2015 at 10:15:01AM -0400, Eric B Munson wrote:
> 
> > > 
> > > You haven't wired it up properly on powerpc, but I haven't mentioned it because
> > > I'd rather we did it.
> > > 
> > > cheers
> > 
> > It looks like I will be spinning a V5, so I will drop all but the x86
> > system calls additions in that version.
> 
> The MIPS bits are looking good however, so
> 
> Acked-by: Ralf Baechle <ralf@linux-mips.org>
> 
> With my ack, will you keep them or maybe carry them as a separate patch?

I will keep the MIPS additions as a separate patch in the series, though
I have dropped two of the new syscalls after some discussion.  So I will
not include your ack on the new patch.

Eric

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH 02/14] twl4030_charger: use devres for power_supply_register and kzalloc.
From: Sebastian Reichel @ 2015-07-24 14:35 UTC (permalink / raw)
  To: NeilBrown
  Cc: NeilBrown, linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, GTA04 owners,
	inux-pm-u79uwXL29TY76Z2rM5mHXA, Pavel Machek,
	linux-omap-u79uwXL29TY76Z2rM5mHXA, Lee Jones
In-Reply-To: <20150322232028.23789.38570.stgit-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 447 bytes --]

Hi,

On Mon, Mar 23, 2015 at 10:20:28AM +1100, NeilBrown wrote:
> From: NeilBrown <neilb-l3A5Bk7waGM@public.gmane.org>
> 
> Final allocations/registrations are now managed by devres.
> 
> Signed-off-by: NeilBrown <neilb-l3A5Bk7waGM@public.gmane.org>

Thanks for the patch, rebased to current master and queued with
Pavel's ACK:

http://git.infradead.org/battery-2.6.git/commit/325b50aa5d1c7dae57d1e44defdbacd1e2bcabde

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH 01/14] twl4030_charger: use devm_request_threaded_irq
From: Sebastian Reichel @ 2015-07-24 14:34 UTC (permalink / raw)
  To: NeilBrown
  Cc: NeilBrown, linux-api, linux-kernel, GTA04 owners, inux-pm,
	Pavel Machek, linux-omap, Lee Jones
In-Reply-To: <20150322232028.23789.97241.stgit@notabene.brown>

[-- Attachment #1: Type: text/plain, Size: 366 bytes --]

Hi,

On Mon, Mar 23, 2015 at 10:20:28AM +1100, NeilBrown wrote:
> From: NeilBrown <neilb@suse.de>
> 
> This simplifies the error paths.
> 
> Signed-off-by: NeilBrown <neilb@suse.de>

I queued this with Pavel's ACK after a rebase to current master:

http://git.infradead.org/battery-2.6.git/commit/1b7cac23419e1398eac4c28368da6671c9c4ce9d

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH -mm v9 6/8] proc: add kpageidle file
From: Vladimir Davydov @ 2015-07-24 14:17 UTC (permalink / raw)
  To: Paul Gortmaker
  Cc: Andrew Morton, 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-u79uwXL29TY76Z2rM5mHXA, LKML doc,
	linux-mm-Bw31MaZKKs3YtjvyW6yDsg, cgroups-u79uwXL29TY76Z2rM5mHXA,
	LKML, linux-next-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAP=VYLqiNfQJ6oyQg2GszeHwdOmeY_uD3XPvw=++weJOKdx4_g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Fri, Jul 24, 2015 at 10:08:25AM -0400, Paul Gortmaker wrote:

> fs/proc/page.c:341:4: error: implicit declaration of function
> 'pmdp_clear_young_notify' [-Werror=implicit-function-declaration]
> fs/proc/page.c:347:4: error: implicit declaration of function
> 'ptep_clear_young_notify' [-Werror=implicit-function-declaration]
> cc1: some warnings being treated as errors
> make[3]: *** [fs/proc/page.o] Error 1
> make[2]: *** [fs/proc] Error 2

My bad, sorry.

It's already been reported by the kbuild-test-robot, see

  [linux-next:master 3983/4215] fs/proc/page.c:332:4: error: implicit declaration of function 'pmdp_clear_young_notify'

The fix is:

From: Vladimir Davydov <vdavydov-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
Subject: [PATCH] mmu_notifier: add missing stubs for clear_young

This is a compilation fix for !CONFIG_MMU_NOTIFIER.

Fixes: mmu-notifier-add-clear_young-callback
Signed-off-by: Vladimir Davydov <vdavydov-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>

diff --git a/include/linux/mmu_notifier.h b/include/linux/mmu_notifier.h
index a5b17137c683..a1a210d59961 100644
--- a/include/linux/mmu_notifier.h
+++ b/include/linux/mmu_notifier.h
@@ -471,6 +471,8 @@ static inline void mmu_notifier_mm_destroy(struct mm_struct *mm)
 
 #define ptep_clear_flush_young_notify ptep_clear_flush_young
 #define pmdp_clear_flush_young_notify pmdp_clear_flush_young
+#define ptep_clear_young_notify ptep_test_and_clear_young
+#define pmdp_clear_young_notify pmdp_test_and_clear_young
 #define	ptep_clear_flush_notify ptep_clear_flush
 #define pmdp_huge_clear_flush_notify pmdp_huge_clear_flush
 #define pmdp_huge_get_and_clear_notify pmdp_huge_get_and_clear

^ permalink raw reply related

* Re: [PATCH -mm v9 6/8] proc: add kpageidle file
From: Paul Gortmaker @ 2015-07-24 14:08 UTC (permalink / raw)
  To: Vladimir Davydov
  Cc: Andrew Morton, 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-u79uwXL29TY76Z2rM5mHXA, LKML doc,
	linux-mm-Bw31MaZKKs3YtjvyW6yDsg, cgroups-u79uwXL29TY76Z2rM5mHXA,
	LKML, linux-next-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <d7a78b72053cf529c0c9ff6cbc02ffbb3d58fe35.1437303956.git.vdavydov-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>

On Sun, Jul 19, 2015 at 8:31 AM, Vladimir Davydov
<vdavydov-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org> wrote:
> Knowing the portion of memory that is not used by a certain application
> or memory cgroup (idle memory) can be useful for partitioning the system
> efficiently, e.g. by setting memory cgroup limits appropriately.

The version of this commit currently in linux-next breaks cris and m68k
(and maybe others).   Fails with:

fs/proc/page.c:341:4: error: implicit declaration of function
'pmdp_clear_young_notify' [-Werror=implicit-function-declaration]
fs/proc/page.c:347:4: error: implicit declaration of function
'ptep_clear_young_notify' [-Werror=implicit-function-declaration]
cc1: some warnings being treated as errors
make[3]: *** [fs/proc/page.o] Error 1
make[2]: *** [fs/proc] Error 2

http://kisskb.ellerman.id.au/kisskb/buildresult/12470364/

Bisect says:

65525488fa86cda44fb6870f29e9859c974700cd is the first bad commit
commit 65525488fa86cda44fb6870f29e9859c974700cd
Author: Vladimir Davydov <vdavydov-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
Date:   Fri Jul 24 09:11:32 2015 +1000

    proc: add kpageidle file

Paul.
--

> Currently, the only means to estimate the amount of idle memory provided
> by the kernel is /proc/PID/{clear_refs,smaps}: the user can clear the
> access bit for all pages mapped to a particular process by writing 1 to
> clear_refs, wait for some time, and then count smaps:Referenced.
> However, this method has two serious shortcomings:
>
>  - it does not count unmapped file pages
>  - it affects the reclaimer logic
>
> To overcome these drawbacks, this patch introduces two new page flags,
> Idle and Young, and a new proc file, /proc/kpageidle. A page's Idle flag
> can only be set from userspace by setting bit in /proc/kpageidle at the
> offset corresponding to the page, and it is cleared whenever the page is
> accessed either through page tables (it is cleared in page_referenced()
> in this case) or using the read(2) system call (mark_page_accessed()).
> Thus by setting the Idle flag for pages of a particular workload, which
> can be found e.g. by reading /proc/PID/pagemap, waiting for some time to
> let the workload access its working set, and then reading the kpageidle
> file, one can estimate the amount of pages that are not used by the
> workload.
>
> The Young page flag is used to avoid interference with the memory
> reclaimer. A page's Young flag is set whenever the Access bit of a page
> table entry pointing to the page is cleared by writing to kpageidle. If
> page_referenced() is called on a Young page, it will add 1 to its return
> value, therefore concealing the fact that the Access bit was cleared.
>
> Note, since there is no room for extra page flags on 32 bit, this
> feature uses extended page flags when compiled on 32 bit.
>
> Signed-off-by: Vladimir Davydov <vdavydov-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
> ---
>  Documentation/vm/pagemap.txt |  12 ++-
>  fs/proc/page.c               | 218 +++++++++++++++++++++++++++++++++++++++++++
>  fs/proc/task_mmu.c           |   4 +-
>  include/linux/mm.h           |  98 +++++++++++++++++++
>  include/linux/page-flags.h   |  11 +++
>  include/linux/page_ext.h     |   4 +
>  mm/Kconfig                   |  12 +++
>  mm/debug.c                   |   4 +
>  mm/huge_memory.c             |  11 ++-
>  mm/migrate.c                 |   5 +
>  mm/page_ext.c                |   3 +
>  mm/rmap.c                    |   5 +
>  mm/swap.c                    |   2 +
>  13 files changed, 385 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/vm/pagemap.txt b/Documentation/vm/pagemap.txt
> index 3a37ed184258..34fe828c3007 100644
> --- a/Documentation/vm/pagemap.txt
> +++ b/Documentation/vm/pagemap.txt
> @@ -5,7 +5,7 @@ pagemap is a new (as of 2.6.25) set of interfaces in the kernel that allow
>  userspace programs to examine the page tables and related information by
>  reading files in /proc.
>
> -There are four components to pagemap:
> +There are five components to pagemap:
>
>   * /proc/pid/pagemap.  This file lets a userspace process find out which
>     physical frame each virtual page is mapped to.  It contains one 64-bit
> @@ -70,6 +70,16 @@ There are four components to pagemap:
>     memory cgroup each page is charged to, indexed by PFN. Only available when
>     CONFIG_MEMCG is set.
>
> + * /proc/kpageidle.  This file implements a bitmap where each bit corresponds
> +   to a page, indexed by PFN. When the bit is set, the corresponding page is
> +   idle. A page is considered idle if it has not been accessed since it was
> +   marked idle. 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.
> +
>  Short descriptions to the page flags:
>
>   0. LOCKED
> diff --git a/fs/proc/page.c b/fs/proc/page.c
> index 70d23245dd43..273537885ab4 100644
> --- a/fs/proc/page.c
> +++ b/fs/proc/page.c
> @@ -5,6 +5,8 @@
>  #include <linux/ksm.h>
>  #include <linux/mm.h>
>  #include <linux/mmzone.h>
> +#include <linux/rmap.h>
> +#include <linux/mmu_notifier.h>
>  #include <linux/huge_mm.h>
>  #include <linux/proc_fs.h>
>  #include <linux/seq_file.h>
> @@ -16,6 +18,7 @@
>
>  #define KPMSIZE sizeof(u64)
>  #define KPMMASK (KPMSIZE - 1)
> +#define KPMBITS (KPMSIZE * BITS_PER_BYTE)
>
>  /* /proc/kpagecount - an array exposing page counts
>   *
> @@ -275,6 +278,217 @@ static const struct file_operations proc_kpagecgroup_operations = {
>  };
>  #endif /* CONFIG_MEMCG */
>
> +#ifdef CONFIG_IDLE_PAGE_TRACKING
> +/*
> + * Idle page tracking only considers user memory pages, for other types of
> + * pages the idle flag is always unset and an attempt to set it is silently
> + * ignored.
> + *
> + * We treat a page as a user memory page if it is on an LRU list, because it is
> + * always safe to pass such a page to rmap_walk(), which is essential for idle
> + * page tracking. With such an indicator of user pages we can skip isolated
> + * pages, but since there are not usually many of them, it will hardly affect
> + * the overall result.
> + *
> + * This function tries to get a user memory page by pfn as described above.
> + */
> +static struct page *kpageidle_get_page(unsigned long pfn)
> +{
> +       struct page *page;
> +       struct zone *zone;
> +
> +       if (!pfn_valid(pfn))
> +               return NULL;
> +
> +       page = pfn_to_page(pfn);
> +       if (!page || !PageLRU(page) ||
> +           !get_page_unless_zero(page))
> +               return NULL;
> +
> +       zone = page_zone(page);
> +       spin_lock_irq(&zone->lru_lock);
> +       if (unlikely(!PageLRU(page))) {
> +               put_page(page);
> +               page = NULL;
> +       }
> +       spin_unlock_irq(&zone->lru_lock);
> +       return page;
> +}
> +
> +static int kpageidle_clear_pte_refs_one(struct page *page,
> +                                       struct vm_area_struct *vma,
> +                                       unsigned long addr, void *arg)
> +{
> +       struct mm_struct *mm = vma->vm_mm;
> +       spinlock_t *ptl;
> +       pmd_t *pmd;
> +       pte_t *pte;
> +       bool referenced = false;
> +
> +       if (unlikely(PageTransHuge(page))) {
> +               pmd = page_check_address_pmd(page, mm, addr,
> +                                            PAGE_CHECK_ADDRESS_PMD_FLAG, &ptl);
> +               if (pmd) {
> +                       referenced = pmdp_clear_young_notify(vma, addr, pmd);
> +                       spin_unlock(ptl);
> +               }
> +       } else {
> +               pte = page_check_address(page, mm, addr, &ptl, 0);
> +               if (pte) {
> +                       referenced = ptep_clear_young_notify(vma, addr, pte);
> +                       pte_unmap_unlock(pte, ptl);
> +               }
> +       }
> +       if (referenced) {
> +               clear_page_idle(page);
> +               /*
> +                * We cleared the referenced bit in a mapping to this page. To
> +                * avoid interference with page reclaim, mark it young so that
> +                * page_referenced() will return > 0.
> +                */
> +               set_page_young(page);
> +       }
> +       return SWAP_AGAIN;
> +}
> +
> +static void kpageidle_clear_pte_refs(struct page *page)
> +{
> +       struct rmap_walk_control rwc = {
> +               .rmap_one = kpageidle_clear_pte_refs_one,
> +               .anon_lock = page_lock_anon_vma_read,
> +       };
> +       bool need_lock;
> +
> +       if (!page_mapped(page) ||
> +           !page_rmapping(page))
> +               return;
> +
> +       need_lock = !PageAnon(page) || PageKsm(page);
> +       if (need_lock && !trylock_page(page))
> +               return;
> +
> +       rmap_walk(page, &rwc);
> +
> +       if (need_lock)
> +               unlock_page(page);
> +}
> +
> +static ssize_t kpageidle_read(struct file *file, char __user *buf,
> +                             size_t count, loff_t *ppos)
> +{
> +       u64 __user *out = (u64 __user *)buf;
> +       struct page *page;
> +       unsigned long pfn, end_pfn;
> +       ssize_t ret = 0;
> +       u64 idle_bitmap = 0;
> +       int bit;
> +
> +       if (*ppos & KPMMASK || count & KPMMASK)
> +               return -EINVAL;
> +
> +       pfn = *ppos * BITS_PER_BYTE;
> +       if (pfn >= max_pfn)
> +               return 0;
> +
> +       end_pfn = pfn + count * BITS_PER_BYTE;
> +       if (end_pfn > max_pfn)
> +               end_pfn = ALIGN(max_pfn, KPMBITS);
> +
> +       for (; pfn < end_pfn; pfn++) {
> +               bit = pfn % KPMBITS;
> +               page = kpageidle_get_page(pfn);
> +               if (page) {
> +                       if (page_is_idle(page)) {
> +                               /*
> +                                * The page might have been referenced via a
> +                                * pte, in which case it is not idle. Clear
> +                                * refs and recheck.
> +                                */
> +                               kpageidle_clear_pte_refs(page);
> +                               if (page_is_idle(page))
> +                                       idle_bitmap |= 1ULL << bit;
> +                       }
> +                       put_page(page);
> +               }
> +               if (bit == KPMBITS - 1) {
> +                       if (put_user(idle_bitmap, out)) {
> +                               ret = -EFAULT;
> +                               break;
> +                       }
> +                       idle_bitmap = 0;
> +                       out++;
> +               }
> +       }
> +
> +       *ppos += (char __user *)out - buf;
> +       if (!ret)
> +               ret = (char __user *)out - buf;
> +       return ret;
> +}
> +
> +static ssize_t kpageidle_write(struct file *file, const char __user *buf,
> +                              size_t count, loff_t *ppos)
> +{
> +       const u64 __user *in = (const u64 __user *)buf;
> +       struct page *page;
> +       unsigned long pfn, end_pfn;
> +       ssize_t ret = 0;
> +       u64 idle_bitmap = 0;
> +       int bit;
> +
> +       if (*ppos & KPMMASK || count & KPMMASK)
> +               return -EINVAL;
> +
> +       pfn = *ppos * BITS_PER_BYTE;
> +       if (pfn >= max_pfn)
> +               return -ENXIO;
> +
> +       end_pfn = pfn + count * BITS_PER_BYTE;
> +       if (end_pfn > max_pfn)
> +               end_pfn = ALIGN(max_pfn, KPMBITS);
> +
> +       for (; pfn < end_pfn; pfn++) {
> +               bit = pfn % KPMBITS;
> +               if (bit == 0) {
> +                       if (get_user(idle_bitmap, in)) {
> +                               ret = -EFAULT;
> +                               break;
> +                       }
> +                       in++;
> +               }
> +               if (idle_bitmap >> bit & 1) {
> +                       page = kpageidle_get_page(pfn);
> +                       if (page) {
> +                               kpageidle_clear_pte_refs(page);
> +                               set_page_idle(page);
> +                               put_page(page);
> +                       }
> +               }
> +       }
> +
> +       *ppos += (const char __user *)in - buf;
> +       if (!ret)
> +               ret = (const char __user *)in - buf;
> +       return ret;
> +}
> +
> +static const struct file_operations proc_kpageidle_operations = {
> +       .llseek = mem_lseek,
> +       .read = kpageidle_read,
> +       .write = kpageidle_write,
> +};
> +
> +#ifndef CONFIG_64BIT
> +static bool need_page_idle(void)
> +{
> +       return true;
> +}
> +struct page_ext_operations page_idle_ops = {
> +       .need = need_page_idle,
> +};
> +#endif
> +#endif /* CONFIG_IDLE_PAGE_TRACKING */
> +
>  static int __init proc_page_init(void)
>  {
>         proc_create("kpagecount", S_IRUSR, NULL, &proc_kpagecount_operations);
> @@ -282,6 +496,10 @@ static int __init proc_page_init(void)
>  #ifdef CONFIG_MEMCG
>         proc_create("kpagecgroup", S_IRUSR, NULL, &proc_kpagecgroup_operations);
>  #endif
> +#ifdef CONFIG_IDLE_PAGE_TRACKING
> +       proc_create("kpageidle", S_IRUSR | S_IWUSR, NULL,
> +                   &proc_kpageidle_operations);
> +#endif
>         return 0;
>  }
>  fs_initcall(proc_page_init);
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index 860bb0f30f14..7c9a17414106 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -459,7 +459,7 @@ static void smaps_account(struct mem_size_stats *mss, struct page *page,
>
>         mss->resident += size;
>         /* Accumulate the size in pages that have been accessed. */
> -       if (young || PageReferenced(page))
> +       if (young || page_is_young(page) || PageReferenced(page))
>                 mss->referenced += size;
>         mapcount = page_mapcount(page);
>         if (mapcount >= 2) {
> @@ -808,6 +808,7 @@ static int clear_refs_pte_range(pmd_t *pmd, unsigned long addr,
>
>                 /* Clear accessed and referenced bits. */
>                 pmdp_test_and_clear_young(vma, addr, pmd);
> +               test_and_clear_page_young(page);
>                 ClearPageReferenced(page);
>  out:
>                 spin_unlock(ptl);
> @@ -835,6 +836,7 @@ out:
>
>                 /* Clear accessed and referenced bits. */
>                 ptep_test_and_clear_young(vma, addr, pte);
> +               test_and_clear_page_young(page);
>                 ClearPageReferenced(page);
>         }
>         pte_unmap_unlock(pte - 1, ptl);
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index c3a2b37365f6..0e62be7d5138 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2202,5 +2202,103 @@ void __init setup_nr_node_ids(void);
>  static inline void setup_nr_node_ids(void) {}
>  #endif
>
> +#ifdef CONFIG_IDLE_PAGE_TRACKING
> +#ifdef CONFIG_64BIT
> +static inline bool page_is_young(struct page *page)
> +{
> +       return PageYoung(page);
> +}
> +
> +static inline void set_page_young(struct page *page)
> +{
> +       SetPageYoung(page);
> +}
> +
> +static inline bool test_and_clear_page_young(struct page *page)
> +{
> +       return TestClearPageYoung(page);
> +}
> +
> +static inline bool page_is_idle(struct page *page)
> +{
> +       return PageIdle(page);
> +}
> +
> +static inline void set_page_idle(struct page *page)
> +{
> +       SetPageIdle(page);
> +}
> +
> +static inline void clear_page_idle(struct page *page)
> +{
> +       ClearPageIdle(page);
> +}
> +#else /* !CONFIG_64BIT */
> +/*
> + * If there is not enough space to store Idle and Young bits in page flags, use
> + * page ext flags instead.
> + */
> +extern struct page_ext_operations page_idle_ops;
> +
> +static inline bool page_is_young(struct page *page)
> +{
> +       return test_bit(PAGE_EXT_YOUNG, &lookup_page_ext(page)->flags);
> +}
> +
> +static inline void set_page_young(struct page *page)
> +{
> +       set_bit(PAGE_EXT_YOUNG, &lookup_page_ext(page)->flags);
> +}
> +
> +static inline bool test_and_clear_page_young(struct page *page)
> +{
> +       return test_and_clear_bit(PAGE_EXT_YOUNG,
> +                                 &lookup_page_ext(page)->flags);
> +}
> +
> +static inline bool page_is_idle(struct page *page)
> +{
> +       return test_bit(PAGE_EXT_IDLE, &lookup_page_ext(page)->flags);
> +}
> +
> +static inline void set_page_idle(struct page *page)
> +{
> +       set_bit(PAGE_EXT_IDLE, &lookup_page_ext(page)->flags);
> +}
> +
> +static inline void clear_page_idle(struct page *page)
> +{
> +       clear_bit(PAGE_EXT_IDLE, &lookup_page_ext(page)->flags);
> +}
> +#endif /* CONFIG_64BIT */
> +#else /* !CONFIG_IDLE_PAGE_TRACKING */
> +static inline bool page_is_young(struct page *page)
> +{
> +       return false;
> +}
> +
> +static inline void set_page_young(struct page *page)
> +{
> +}
> +
> +static inline bool test_and_clear_page_young(struct page *page)
> +{
> +       return false;
> +}
> +
> +static inline bool page_is_idle(struct page *page)
> +{
> +       return false;
> +}
> +
> +static inline void set_page_idle(struct page *page)
> +{
> +}
> +
> +static inline void clear_page_idle(struct page *page)
> +{
> +}
> +#endif /* CONFIG_IDLE_PAGE_TRACKING */
> +
>  #endif /* __KERNEL__ */
>  #endif /* _LINUX_MM_H */
> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> index 91b7f9b2b774..478f2241f284 100644
> --- a/include/linux/page-flags.h
> +++ b/include/linux/page-flags.h
> @@ -109,6 +109,10 @@ enum pageflags {
>  #ifdef CONFIG_TRANSPARENT_HUGEPAGE
>         PG_compound_lock,
>  #endif
> +#if defined(CONFIG_IDLE_PAGE_TRACKING) && defined(CONFIG_64BIT)
> +       PG_young,
> +       PG_idle,
> +#endif
>         __NR_PAGEFLAGS,
>
>         /* Filesystems */
> @@ -363,6 +367,13 @@ PAGEFLAG_FALSE(HWPoison)
>  #define __PG_HWPOISON 0
>  #endif
>
> +#if defined(CONFIG_IDLE_PAGE_TRACKING) && defined(CONFIG_64BIT)
> +TESTPAGEFLAG(Young, young, PF_ANY)
> +SETPAGEFLAG(Young, young, PF_ANY)
> +TESTCLEARFLAG(Young, young, PF_ANY)
> +PAGEFLAG(Idle, idle, PF_ANY)
> +#endif
> +
>  /*
>   * On an anonymous page mapped into a user virtual memory area,
>   * page->mapping points to its anon_vma, not to a struct address_space;
> diff --git a/include/linux/page_ext.h b/include/linux/page_ext.h
> index c42981cd99aa..17f118a82854 100644
> --- a/include/linux/page_ext.h
> +++ b/include/linux/page_ext.h
> @@ -26,6 +26,10 @@ enum page_ext_flags {
>         PAGE_EXT_DEBUG_POISON,          /* Page is poisoned */
>         PAGE_EXT_DEBUG_GUARD,
>         PAGE_EXT_OWNER,
> +#if defined(CONFIG_IDLE_PAGE_TRACKING) && !defined(CONFIG_64BIT)
> +       PAGE_EXT_YOUNG,
> +       PAGE_EXT_IDLE,
> +#endif
>  };
>
>  /*
> diff --git a/mm/Kconfig b/mm/Kconfig
> index e79de2bd12cd..db817e2c2ec8 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -654,3 +654,15 @@ config DEFERRED_STRUCT_PAGE_INIT
>           when kswapd starts. This has a potential performance impact on
>           processes running early in the lifetime of the systemm until kswapd
>           finishes the initialisation.
> +
> +config IDLE_PAGE_TRACKING
> +       bool "Enable idle page tracking"
> +       select PROC_PAGE_MONITOR
> +       select PAGE_EXTENSION if !64BIT
> +       help
> +         This feature allows to estimate the amount of user pages that have
> +         not been touched during a given period of time. This information can
> +         be useful to tune memory cgroup limits and/or for job placement
> +         within a compute cluster.
> +
> +         See Documentation/vm/pagemap.txt for more details.
> diff --git a/mm/debug.c b/mm/debug.c
> index 76089ddf99ea..6c1b3ea61bfd 100644
> --- a/mm/debug.c
> +++ b/mm/debug.c
> @@ -48,6 +48,10 @@ static const struct trace_print_flags pageflag_names[] = {
>  #ifdef CONFIG_TRANSPARENT_HUGEPAGE
>         {1UL << PG_compound_lock,       "compound_lock" },
>  #endif
> +#if defined(CONFIG_IDLE_PAGE_TRACKING) && defined(CONFIG_64BIT)
> +       {1UL << PG_young,               "young"         },
> +       {1UL << PG_idle,                "idle"          },
> +#endif
>  };
>
>  static void dump_flags(unsigned long flags,
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 8f9a334a6c66..5ab46adca104 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -1806,6 +1806,11 @@ static void __split_huge_page_refcount(struct page *page,
>                 /* clear PageTail before overwriting first_page */
>                 smp_wmb();
>
> +               if (page_is_young(page))
> +                       set_page_young(page_tail);
> +               if (page_is_idle(page))
> +                       set_page_idle(page_tail);
> +
>                 /*
>                  * __split_huge_page_splitting() already set the
>                  * splitting bit in all pmd that could map this
> @@ -2311,7 +2316,8 @@ static int __collapse_huge_page_isolate(struct vm_area_struct *vma,
>                 VM_BUG_ON_PAGE(PageLRU(page), page);
>
>                 /* If there is no mapped pte young don't collapse the page */
> -               if (pte_young(pteval) || PageReferenced(page) ||
> +               if (pte_young(pteval) ||
> +                   page_is_young(page) || PageReferenced(page) ||
>                     mmu_notifier_test_young(vma->vm_mm, address))
>                         referenced = true;
>         }
> @@ -2738,7 +2744,8 @@ static int khugepaged_scan_pmd(struct mm_struct *mm,
>                  */
>                 if (page_count(page) != 1 + !!PageSwapCache(page))
>                         goto out_unmap;
> -               if (pte_young(pteval) || PageReferenced(page) ||
> +               if (pte_young(pteval) ||
> +                   page_is_young(page) || PageReferenced(page) ||
>                     mmu_notifier_test_young(vma->vm_mm, address))
>                         referenced = true;
>         }
> diff --git a/mm/migrate.c b/mm/migrate.c
> index d3529d620a5b..d86cec005aa6 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -524,6 +524,11 @@ void migrate_page_copy(struct page *newpage, struct page *page)
>                         __set_page_dirty_nobuffers(newpage);
>         }
>
> +       if (page_is_young(page))
> +               set_page_young(newpage);
> +       if (page_is_idle(page))
> +               set_page_idle(newpage);
> +
>         /*
>          * Copy NUMA information to the new page, to prevent over-eager
>          * future migrations of this same page.
> diff --git a/mm/page_ext.c b/mm/page_ext.c
> index d86fd2f5353f..e4b3af054bf2 100644
> --- a/mm/page_ext.c
> +++ b/mm/page_ext.c
> @@ -59,6 +59,9 @@ static struct page_ext_operations *page_ext_ops[] = {
>  #ifdef CONFIG_PAGE_OWNER
>         &page_owner_ops,
>  #endif
> +#if defined(CONFIG_IDLE_PAGE_TRACKING) && !defined(CONFIG_64BIT)
> +       &page_idle_ops,
> +#endif
>  };
>
>  static unsigned long total_usage;
> diff --git a/mm/rmap.c b/mm/rmap.c
> index 30812e9042ae..9e411aa03176 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -900,6 +900,11 @@ static int page_referenced_one(struct page *page, struct vm_area_struct *vma,
>                 pte_unmap_unlock(pte, ptl);
>         }
>
> +       if (referenced)
> +               clear_page_idle(page);
> +       if (test_and_clear_page_young(page))
> +               referenced++;
> +
>         if (referenced) {
>                 pra->referenced++;
>                 pra->vm_flags |= vma->vm_flags;
> diff --git a/mm/swap.c b/mm/swap.c
> index d398860badd1..04b6ce51bcf0 100644
> --- a/mm/swap.c
> +++ b/mm/swap.c
> @@ -623,6 +623,8 @@ void mark_page_accessed(struct page *page)
>         } else if (!PageReferenced(page)) {
>                 SetPageReferenced(page);
>         }
> +       if (page_is_idle(page))
> +               clear_page_idle(page);
>  }
>  EXPORT_SYMBOL(mark_page_accessed);
>
> --
> 2.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply

* Re: [PATCH v4 1/5] nohz_full: add support for "cpu_isolated" mode
From: Frederic Weisbecker @ 2015-07-24 14:03 UTC (permalink / raw)
  To: Chris Metcalf
  Cc: Andy Lutomirski, Gilad Ben Yossef, Steven Rostedt, Ingo Molnar,
	Peter Zijlstra, Andrew Morton, Rik van Riel, Tejun Heo,
	Thomas Gleixner, Paul E. McKenney, Christoph Lameter,
	Viresh Kumar, linux-doc@vger.kernel.org, Linux API,
	linux-kernel@vger.kernel.org, Mike Galbraith
In-Reply-To: <55AE993E.6040501@ezchip.com>

On Tue, Jul 21, 2015 at 03:10:54PM -0400, Chris Metcalf wrote:
> >>If you're arguing that the cpu_isolated semantic is really the only
> >>one that makes sense for nohz_full, my sense is that it might be
> >>surprising to many of the folks who do nohz_full work.  But, I'm happy
> >>to be wrong on this point, and maybe all the nohz_full community is
> >>interested in making the same tradeoffs for nohz_full generally that
> >>I've proposed in this patch series just for cpu_isolated?
> >nohz_full is currently dog slow for no particularly good reasons.  I
> >suspect that the interrupts you're seeing are also there for no
> >particularly good reasons as well.
> >
> >Let's fix them instead of adding new ABIs to work around them.
> 
> Well, in principle if we accepted my proposed patch series
> and then over time came to decide that it was reasonable
> for nohz_full to have these complete cpu isolation
> semantics, the one proposed ABI simply becomes a no-op.
> So it's not as problematic an ABI as some.
> 
> My issue is this: I'm totally happy with submitting a revised
> patch series that does all the stuff for pure nohz_full that
> I'm currently proposing for cpu_isolated.  But, is it what
> the community wants?  Should I propose it and see?
> 
> Frederic, do you have any insight here?  Thanks!

So you guys mean that if nohz_full was implemented fully like
we expect it to, we shouldn't be burdened at all by noise and that
whole patchset would therefore be pointless, right? And that would meet
the requirements for those who want hard isolation (critical noise-free
guarantee) as well as those who want soft isolation (less noise as
possible for performance).

Well first of all nohz is not isolation, it's a significant part of it
but it's not all isolatiion. We really want to separate these things and
not mess up isolation policies in the tick code.

Second, yes perhaps we can eventually have both soft and hard isolation
expectation eventually be implemented the same way through hard isolation.
But that will only work if we don't do that polling for noise-free before
resuming userspace, which might work for hard isolation that is ready to
sacrifice some warm-up before a run to meet guarantees, but it won't
work for soft isolation workloads.

So the only solution is to offline everything we can to housekeeping
CPUs. And if we still have stuff that can't be dealt with that way
and which need to be taken care of with some explicit operation
before resuming to userspace, then we can start to think about splitting
stuff in several isolation configs.

Similarly, offlining everything to housekeepers means that we sacrifice
a CPU that could have been used in performance oriented workloads so that
might not match soft isolation as well. But I think we'll see that all once
we manage to have pure noise-free CPUs (some patches are on the way to be
posted by Vatika Harlalka concerning the residual 1hz tick to kill).

To summarize, lets first split nohz and isolation. Introduce
CONFIG_CPU_ISOLATION and stuff all the isolation policies to
kernel/cpu_isolation.c, lets try to implement hard isolation and see if that
meets soft isolation workload users as well, if not we'll split that later.

And we can keep the prctl to tell the user when hard isolation has been
broken, through SIGKILL or whatever. I think we are doing a similar thing
with SCHED_DEADLINE when the task hasn't met deadline requirement. We might
want to do the same.

^ permalink raw reply

* Re: [PATCH v4 1/5] nohz_full: add support for "cpu_isolated" mode
From: Frederic Weisbecker @ 2015-07-24 13:27 UTC (permalink / raw)
  To: Chris Metcalf
  Cc: Gilad Ben Yossef, Steven Rostedt, Ingo Molnar, Peter Zijlstra,
	Andrew Morton, Rik van Riel, Tejun Heo, Thomas Gleixner,
	Paul E. McKenney, Christoph Lameter, Viresh Kumar, linux-doc,
	linux-api, linux-kernel
In-Reply-To: <1436817481-8732-2-git-send-email-cmetcalf@ezchip.com>

On Mon, Jul 13, 2015 at 03:57:57PM -0400, Chris Metcalf wrote:
> The existing nohz_full mode makes tradeoffs to minimize userspace
> interruptions while still attempting to avoid overheads in the
> kernel entry/exit path, to provide 100% kernel semantics, etc.
> 
> However, some applications require a stronger commitment from the
> kernel to avoid interruptions, in particular userspace device
> driver style applications, such as high-speed networking code.
> 
> This change introduces a framework to allow applications to elect
> to have the stronger semantics as needed, specifying
> prctl(PR_SET_CPU_ISOLATED, PR_CPU_ISOLATED_ENABLE) to do so.
> Subsequent commits will add additional flags and additional
> semantics.
> 
> The "cpu_isolated" state is indicated by setting a new task struct
> field, cpu_isolated_flags, to the value passed by prctl().  When the
> _ENABLE bit is set for a task, and it is returning to userspace
> on a nohz_full core, it calls the new tick_nohz_cpu_isolated_enter()
> routine to take additional actions to help the task avoid being
> interrupted in the future.
> 
> Initially, there are only two actions taken.  First, the task
> calls lru_add_drain() to prevent being interrupted by a subsequent
> lru_add_drain_all() call on another core.  Then, the code checks for
> pending timer interrupts and quiesces until they are no longer pending.
> As a result, sys calls (and page faults, etc.) can be inordinately slow.
> However, this quiescing guarantees that no unexpected interrupts will
> occur, even if the application intentionally calls into the kernel.
> 
> Signed-off-by: Chris Metcalf <cmetcalf@ezchip.com>
> ---
>  arch/tile/kernel/process.c |  9 ++++++++
>  include/linux/sched.h      |  3 +++
>  include/linux/tick.h       | 10 ++++++++
>  include/uapi/linux/prctl.h |  5 ++++
>  kernel/context_tracking.c  |  3 +++
>  kernel/sys.c               |  8 +++++++
>  kernel/time/tick-sched.c   | 57 ++++++++++++++++++++++++++++++++++++++++++++++
>  7 files changed, 95 insertions(+)
> 
> diff --git a/arch/tile/kernel/process.c b/arch/tile/kernel/process.c
> index e036c0aa9792..3625e839ad62 100644
> --- a/arch/tile/kernel/process.c
> +++ b/arch/tile/kernel/process.c
> @@ -70,6 +70,15 @@ void arch_cpu_idle(void)
>  	_cpu_idle();
>  }
>  
> +#ifdef CONFIG_NO_HZ_FULL

I think this goes way beyond nohz itself. We don't only want the tick to shutdown,
we want also the pending timers, workqueues, etc...

It's time to create the CONFIG_ISOLATION_foo stuffs.

> +void tick_nohz_cpu_isolated_wait(void)
> +{
> +	set_current_state(TASK_INTERRUPTIBLE);
> +	_cpu_idle();
> +	set_current_state(TASK_RUNNING);
> +}
> +#endif
> +
>  /*
>   * Release a thread_info structure
>   */
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index ae21f1591615..f350b0c20bbc 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -1778,6 +1778,9 @@ struct task_struct {
>  	unsigned long	task_state_change;
>  #endif
>  	int pagefault_disabled;
> +#ifdef CONFIG_NO_HZ_FULL
> +	unsigned int	cpu_isolated_flags;
> +#endif
>  };
>  
>  /* Future-safe accessor for struct task_struct's cpus_allowed. */
> diff --git a/include/linux/tick.h b/include/linux/tick.h
> index 3741ba1a652c..cb5569181359 100644
> --- a/include/linux/tick.h
> +++ b/include/linux/tick.h
> @@ -10,6 +10,7 @@
>  #include <linux/context_tracking_state.h>
>  #include <linux/cpumask.h>
>  #include <linux/sched.h>
> +#include <linux/prctl.h>
>  
>  #ifdef CONFIG_GENERIC_CLOCKEVENTS
>  extern void __init tick_init(void);
> @@ -144,11 +145,18 @@ static inline void tick_nohz_full_add_cpus_to(struct cpumask *mask)
>  		cpumask_or(mask, mask, tick_nohz_full_mask);
>  }
>  
> +static inline bool tick_nohz_is_cpu_isolated(void)
> +{
> +	return tick_nohz_full_cpu(smp_processor_id()) &&
> +		(current->cpu_isolated_flags & PR_CPU_ISOLATED_ENABLE);
> +}
> +
>  extern void __tick_nohz_full_check(void);
>  extern void tick_nohz_full_kick(void);
>  extern void tick_nohz_full_kick_cpu(int cpu);
>  extern void tick_nohz_full_kick_all(void);
>  extern void __tick_nohz_task_switch(struct task_struct *tsk);
> +extern void tick_nohz_cpu_isolated_enter(void);
>  #else
>  static inline bool tick_nohz_full_enabled(void) { return false; }
>  static inline bool tick_nohz_full_cpu(int cpu) { return false; }
> @@ -158,6 +166,8 @@ static inline void tick_nohz_full_kick_cpu(int cpu) { }
>  static inline void tick_nohz_full_kick(void) { }
>  static inline void tick_nohz_full_kick_all(void) { }
>  static inline void __tick_nohz_task_switch(struct task_struct *tsk) { }
> +static inline bool tick_nohz_is_cpu_isolated(void) { return false; }
> +static inline void tick_nohz_cpu_isolated_enter(void) { }
>  #endif
>  
>  static inline bool is_housekeeping_cpu(int cpu)
> diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
> index 31891d9535e2..edb40b6b84db 100644
> --- a/include/uapi/linux/prctl.h
> +++ b/include/uapi/linux/prctl.h
> @@ -190,4 +190,9 @@ struct prctl_mm_map {
>  # define PR_FP_MODE_FR		(1 << 0)	/* 64b FP registers */
>  # define PR_FP_MODE_FRE		(1 << 1)	/* 32b compatibility */
>  
> +/* Enable/disable or query cpu_isolated mode for NO_HZ_FULL kernels. */
> +#define PR_SET_CPU_ISOLATED	47
> +#define PR_GET_CPU_ISOLATED	48
> +# define PR_CPU_ISOLATED_ENABLE	(1 << 0)
> +
>  #endif /* _LINUX_PRCTL_H */
> diff --git a/kernel/context_tracking.c b/kernel/context_tracking.c
> index 0a495ab35bc7..f9de3ee12723 100644
> --- a/kernel/context_tracking.c
> +++ b/kernel/context_tracking.c
> @@ -20,6 +20,7 @@
>  #include <linux/hardirq.h>
>  #include <linux/export.h>
>  #include <linux/kprobes.h>
> +#include <linux/tick.h>
>  
>  #define CREATE_TRACE_POINTS
>  #include <trace/events/context_tracking.h>
> @@ -99,6 +100,8 @@ void context_tracking_enter(enum ctx_state state)
>  			 * on the tick.
>  			 */
>  			if (state == CONTEXT_USER) {
> +				if (tick_nohz_is_cpu_isolated())
> +					tick_nohz_cpu_isolated_enter();
>  				trace_user_enter(0);
>  				vtime_user_enter(current);
>  			}
> diff --git a/kernel/sys.c b/kernel/sys.c
> index 259fda25eb6b..36eb9a839f1f 100644
> --- a/kernel/sys.c
> +++ b/kernel/sys.c
> @@ -2267,6 +2267,14 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
>  	case PR_GET_FP_MODE:
>  		error = GET_FP_MODE(me);
>  		break;
> +#ifdef CONFIG_NO_HZ_FULL
> +	case PR_SET_CPU_ISOLATED:
> +		me->cpu_isolated_flags = arg2;
> +		break;
> +	case PR_GET_CPU_ISOLATED:
> +		error = me->cpu_isolated_flags;
> +		break;
> +#endif
>  	default:
>  		error = -EINVAL;
>  		break;
> diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
> index c792429e98c6..4cf093c012d1 100644
> --- a/kernel/time/tick-sched.c
> +++ b/kernel/time/tick-sched.c
> @@ -24,6 +24,7 @@
>  #include <linux/posix-timers.h>
>  #include <linux/perf_event.h>
>  #include <linux/context_tracking.h>
> +#include <linux/swap.h>
>  
>  #include <asm/irq_regs.h>
>  
> @@ -389,6 +390,62 @@ void __init tick_nohz_init(void)
>  	pr_info("NO_HZ: Full dynticks CPUs: %*pbl.\n",
>  		cpumask_pr_args(tick_nohz_full_mask));
>  }
> +
> +/*
> + * Rather than continuously polling for the next_event in the
> + * tick_cpu_device, architectures can provide a method to save power
> + * by sleeping until an interrupt arrives.
> + */
> +void __weak tick_nohz_cpu_isolated_wait(void)
> +{
> +	cpu_relax();
> +}
> +
> +/*
> + * We normally return immediately to userspace.
> + *
> + * In "cpu_isolated" mode we wait until no more interrupts are
> + * pending.  Otherwise we nap with interrupts enabled and wait for the
> + * next interrupt to fire, then loop back and retry.
> + *
> + * Note that if you schedule two "cpu_isolated" processes on the same
> + * core, neither will ever leave the kernel, and one will have to be
> + * killed manually.  Otherwise in situations where another process is
> + * in the runqueue on this cpu, this task will just wait for that
> + * other task to go idle before returning to user space.
> + */
> +void tick_nohz_cpu_isolated_enter(void)

Similarly, I'd rather see that in kernel/cpu_isolation.c and call it
cpu_isolation_enter().

> +{
> +	struct clock_event_device *dev =
> +		__this_cpu_read(tick_cpu_device.evtdev);
> +	struct task_struct *task = current;
> +	unsigned long start = jiffies;
> +	bool warned = false;
> +
> +	/* Drain the pagevecs to avoid unnecessary IPI flushes later. */
> +	lru_add_drain();
> +
> +	while (READ_ONCE(dev->next_event.tv64) != KTIME_MAX) {
> +		if (!warned && (jiffies - start) >= (5 * HZ)) {
> +			pr_warn("%s/%d: cpu %d: cpu_isolated task blocked for %ld seconds\n",
> +				task->comm, task->pid, smp_processor_id(),
> +				(jiffies - start) / HZ);
> +			warned = true;
> +		}
> +		if (should_resched())
> +			schedule();
> +		if (test_thread_flag(TIF_SIGPENDING))
> +			break;
> +		tick_nohz_cpu_isolated_wait();

If we call cpu_idle(), what is going to wake the CPU up if not further interrupt happen?

We could either implement some sort of tick waiters with proper wake up once the CPU sees
no tick to schedule. Arguably this is all risky because this involve a scheduler wake up
and thus the risk for new noise. But it might work.

Another possibility is an msleep() based wait. But that's about the same, maybe even worse
due to repetitive wake ups.

> +	}
> +	if (warned) {
> +		pr_warn("%s/%d: cpu %d: cpu_isolated task unblocked after %ld seconds\n",
> +			task->comm, task->pid, smp_processor_id(),
> +			(jiffies - start) / HZ);
> +		dump_stack();
> +	}
> +}
> +
>  #endif
>  
>  /*
> -- 
> 2.1.2
> 

^ permalink raw reply

* [PATCH v4] Input: Add userio module
From: Stephen Chandler Paul @ 2015-07-23 17:33 UTC (permalink / raw)
  To: Dmitry Torokhov, Andrew Morton, Mauro Carvalho Chehab, Greg KH,
	Arnd Bergmann, Joe Perches, Jiri Slaby, Vishnu Patekar,
	Sebastian Ott, linux-doc, linux-kernel, linux-input, linux-api
  Cc: Benjamin Tissoires, Hans de Goede, Stephen Chandler Paul

Debugging input devices, specifically laptop touchpads, can be tricky
without having the physical device handy. Here we try to remedy that
with userio. This module allows an application to connect to a character
device provided by the kernel, and emulate any serio device. In
combination with userspace programs that can record PS/2 devices and
replay them through the /dev/userio device, this allows developers to
debug driver issues on the PS/2 level with devices simply by requesting
a recording from the user experiencing the issue without having to have
the physical hardware in front of them.

Signed-off-by: Stephen Chandler Paul <cpaul@redhat.com>
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@gmail.com>

---
				    Changes
* Use one lock, not two
* Don't use devm_kzalloc()
* Add lock/unlock operations in userio_char_write()
* Handle mutex locking and head/tail checking the same way we do in evdev, wait
  until there's data available, attempt to grab the mutex but give up if the
  data's been taken by another thread and try again until we get the data
* Don't clear userio->serio->port_data, serio_port_unregister() does this for us
* Put userio->head == userio->tail test inside mutex lock
* Add comment making note of the fact that we don't have to free serio

 Documentation/input/userio.txt |  70 +++++++++++
 MAINTAINERS                    |   6 +
 drivers/input/serio/Kconfig    |  11 ++
 drivers/input/serio/Makefile   |   1 +
 drivers/input/serio/userio.c   | 274 +++++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/userio.h    |  42 +++++++
 6 files changed, 404 insertions(+)
 create mode 100644 Documentation/input/userio.txt
 create mode 100644 drivers/input/serio/userio.c
 create mode 100644 include/uapi/linux/userio.h

diff --git a/Documentation/input/userio.txt b/Documentation/input/userio.txt
new file mode 100644
index 0000000..0880c0f
--- /dev/null
+++ b/Documentation/input/userio.txt
@@ -0,0 +1,70 @@
+			      The userio Protocol
+	     (c) 2015 Stephen Chandler Paul <thatslyude@gmail.com>
+			      Sponsored by Red Hat
+--------------------------------------------------------------------------------
+
+1. Introduction
+~~~~~~~~~~~~~~~
+  This module is intended to try to make the lives of input driver developers
+easier by allowing them to test various serio devices (mainly the various
+touchpads found on laptops) without having to have the physical device in front
+of them. userio accomplishes this by allowing any privileged userspace program
+to directly interact with the kernel's serio driver and control a virtual serio
+port from there.
+
+2. Usage overview
+~~~~~~~~~~~~~~~~~
+  In order to interact with the userio kernel module, one simply opens the
+/dev/userio character device in their applications. Commands are sent to the
+kernel module by writing to the device, and any data received from the serio
+driver is read as-is from the /dev/userio device. All of the structures and
+macros you need to interact with the device are defined in <linux/userio.h> and
+<linux/serio.h>.
+
+3. Command Structure
+~~~~~~~~~~~~~~~~~~~~
+  The struct used for sending commands to /dev/userio is as follows:
+
+	struct userio_cmd {
+		__u8 type;
+		__u8 data;
+	};
+
+  "type" describes the type of command that is being sent. This can be any one
+of the USERIO_CMD macros defined in <linux/userio.h>. "data" is the argument
+that goes along with the command. In the event that the command doesn't have an
+argument, this field can be left untouched and will be ignored by the kernel.
+Each command should be sent by writing the struct directly to the character
+device. In the event that the command you send is invalid, an error will be
+returned by the character device and a more descriptive error will be printed
+to the kernel log. Only one command can be sent at a time, any additional data
+written to the character device after the initial command will be ignored.
+  To close the virtual serio port, just close /dev/userio.
+
+4. Commands
+~~~~~~~~~~~
+
+4.1 USERIO_CMD_REGISTER
+~~~~~~~~~~~~~~~~~~~~~~~
+  Registers the port with the serio driver and begins transmitting data back and
+forth. Registration can only be performed once a port type is set with
+USERIO_CMD_SET_PORT_TYPE. Has no argument.
+
+4.2 USERIO_CMD_SET_PORT_TYPE
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+  Sets the type of port we're emulating, where "data" is the port type being
+set. Can be any of the macros from <linux/serio.h>. For example: SERIO_8042
+would set the port type to be a normal PS/2 port.
+
+4.3 USERIO_CMD_SEND_INTERRUPT
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+  Sends an interrupt through the virtual serio port to the serio driver, where
+"data" is the interrupt data being sent.
+
+5. Userspace tools
+~~~~~~~~~~~~~~~~~~
+  The userio userspace tools are able to record PS/2 devices using some of the
+debugging information from i8042, and play back the devices on /dev/userio. The
+latest version of these tools can be found at:
+
+	https://github.com/Lyude/ps2emu
diff --git a/MAINTAINERS b/MAINTAINERS
index a226416..68a0977 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10877,6 +10877,12 @@ S:	Maintained
 F:	drivers/media/v4l2-core/videobuf2-*
 F:	include/media/videobuf2-*
 
+VIRTUAL PS/2 DEVICE DRIVER
+M:	Stephen Chandler Paul <thatslyude@gmail.com>
+S:	Maintained
+F:	drivers/input/serio/ps2emu.c
+F:	include/uapi/linux/ps2emu.h
+
 VIRTIO CONSOLE DRIVER
 M:	Amit Shah <amit.shah@redhat.com>
 L:	virtualization@lists.linux-foundation.org
diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
index 200841b..22b8b58 100644
--- a/drivers/input/serio/Kconfig
+++ b/drivers/input/serio/Kconfig
@@ -292,4 +292,15 @@ config SERIO_SUN4I_PS2
 	  To compile this driver as a module, choose M here: the
 	  module will be called sun4i-ps2.
 
+config USERIO
+	tristate "Virtual serio device support"
+	help
+	  Say Y here if you want to emulate serio devices using the userio
+	  kernel module.
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called userio.
+
+	  If you are unsure, say N.
+
 endif
diff --git a/drivers/input/serio/Makefile b/drivers/input/serio/Makefile
index c600089..2374ef9 100644
--- a/drivers/input/serio/Makefile
+++ b/drivers/input/serio/Makefile
@@ -30,3 +30,4 @@ obj-$(CONFIG_SERIO_APBPS2)	+= apbps2.o
 obj-$(CONFIG_SERIO_OLPC_APSP)	+= olpc_apsp.o
 obj-$(CONFIG_HYPERV_KEYBOARD)	+= hyperv-keyboard.o
 obj-$(CONFIG_SERIO_SUN4I_PS2)	+= sun4i-ps2.o
+obj-$(CONFIG_USERIO)		+= userio.o
diff --git a/drivers/input/serio/userio.c b/drivers/input/serio/userio.c
new file mode 100644
index 0000000..043fbbf
--- /dev/null
+++ b/drivers/input/serio/userio.c
@@ -0,0 +1,274 @@
+/*
+ * userio kernel serio device emulation module
+ * Copyright (C) 2015 Red Hat
+ * Copyright (C) 2015 Stephen Chandler Paul <thatslyude@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more
+ * details.
+ */
+#include <linux/circ_buf.h>
+#include <linux/mutex.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/serio.h>
+#include <linux/slab.h>
+#include <linux/fs.h>
+#include <linux/miscdevice.h>
+#include <linux/sched.h>
+#include <linux/poll.h>
+#include <uapi/linux/userio.h>
+
+#define USERIO_NAME "userio"
+#define USERIO_BUFSIZE 16
+
+static const struct file_operations userio_fops;
+static struct miscdevice userio_misc;
+
+struct userio_device {
+	struct serio *serio;
+	struct mutex lock;
+
+	bool running;
+
+	u8 head;
+	u8 tail;
+
+	unsigned char buf[USERIO_BUFSIZE];
+
+	wait_queue_head_t waitq;
+};
+
+/**
+ * userio_device_write - Write data from serio to a userio device in userspace
+ * @id: The serio port for the userio device
+ * @val: The data to write to the device
+ */
+static int userio_device_write(struct serio *id, unsigned char val)
+{
+	struct userio_device *userio = id->port_data;
+
+	if (!userio)
+		return -1;
+
+	mutex_lock(&userio->lock);
+
+	userio->buf[userio->head] = val;
+	userio->head = (userio->head + 1) % USERIO_BUFSIZE;
+
+	if (userio->head == userio->tail)
+		dev_warn(userio_misc.this_device,
+			 "Buffer overflowed, userio client isn't keeping up");
+
+	mutex_unlock(&userio->lock);
+
+	wake_up_interruptible(&userio->waitq);
+
+	return 0;
+}
+
+static int userio_char_open(struct inode *inode, struct file *file)
+{
+	struct userio_device *userio;
+
+	userio = kzalloc(sizeof(struct userio_device), GFP_KERNEL);
+	if (!userio)
+		return -ENOMEM;
+
+	mutex_init(&userio->lock);
+	init_waitqueue_head(&userio->waitq);
+
+	userio->serio = kzalloc(sizeof(struct serio), GFP_KERNEL);
+	if (!userio->serio) {
+		kfree(userio);
+		return -ENOMEM;
+	}
+
+	userio->serio->write = userio_device_write;
+	userio->serio->port_data = userio;
+
+	file->private_data = userio;
+
+	return 0;
+}
+
+static int userio_char_release(struct inode *inode, struct file *file)
+{
+	struct userio_device *userio = file->private_data;
+
+	if (userio->serio) {
+		/* Don't free the serio port here, serio_unregister_port() does
+		 * this for us */
+		if (userio->running)
+			serio_unregister_port(userio->serio);
+		else
+			kfree(userio->serio);
+	}
+
+	kfree(userio);
+
+	return 0;
+}
+
+static ssize_t userio_char_read(struct file *file, char __user *buffer,
+				size_t count, loff_t *ppos)
+{
+	struct userio_device *userio = file->private_data;
+	int ret;
+	size_t nonwrap_len, copylen;
+
+	if (!count)
+		return 0;
+
+	/*
+	 * By the time we get here, the data that was waiting might have been
+	 * taken by another thread. Grab the mutex and check if there's still
+	 * any data waiting, otherwise repeat repeat this process until we have
+	 * data (unless the file descriptor is non-blocking of course)
+	 */
+	for (;;) {
+		ret = mutex_lock_interruptible(&userio->lock);
+		if (ret)
+			return ret;
+
+		if (userio->head != userio->tail)
+			break;
+
+		mutex_unlock(&userio->lock);
+
+		if (file->f_flags & O_NONBLOCK)
+			return -EAGAIN;
+
+		ret = wait_event_interruptible(userio->waitq,
+					       userio->head != userio->tail);
+		if (ret)
+			return ret;
+	}
+
+	nonwrap_len = CIRC_CNT_TO_END(userio->head, userio->tail,
+				      USERIO_BUFSIZE);
+	copylen = min(nonwrap_len, count);
+
+	if (copy_to_user(buffer, &userio->buf[userio->tail], copylen)) {
+		mutex_unlock(&userio->lock);
+		return -EFAULT;
+	}
+
+	userio->tail = (userio->tail + copylen) % USERIO_BUFSIZE;
+
+	mutex_unlock(&userio->lock);
+
+	return copylen;
+}
+
+static ssize_t userio_char_write(struct file *file, const char __user *buffer,
+				 size_t count, loff_t *ppos)
+{
+	struct userio_device *userio = file->private_data;
+	struct userio_cmd cmd;
+	int ret;
+
+	if (count < sizeof(cmd))
+		return -EINVAL;
+
+	if (copy_from_user(&cmd, buffer, sizeof(cmd)))
+		return -EFAULT;
+
+	ret = mutex_lock_interruptible(&userio->lock);
+	if (ret)
+		return ret;
+
+	ret = -EINVAL;
+
+	switch (cmd.type) {
+	case USERIO_CMD_REGISTER:
+		if (!userio->serio->id.type) {
+			dev_warn(userio_misc.this_device,
+				 "No port type given on /dev/userio\n");
+
+			goto out;
+		}
+		if (userio->running) {
+			dev_warn(userio_misc.this_device,
+				 "Begin command sent, but we're already running\n");
+
+			goto out;
+		}
+
+		userio->running = true;
+		serio_register_port(userio->serio);
+		break;
+
+	case USERIO_CMD_SET_PORT_TYPE:
+		if (userio->running) {
+			dev_warn(userio_misc.this_device,
+				 "Can't change port type on an already running userio instance\n");
+
+			goto out;
+		}
+
+		userio->serio->id.type = cmd.data;
+		break;
+
+	case USERIO_CMD_SEND_INTERRUPT:
+		if (!userio->running) {
+			dev_warn(userio_misc.this_device,
+				 "The device must be registered before sending interrupts\n");
+
+			goto out;
+		}
+
+		serio_interrupt(userio->serio, cmd.data, 0);
+		break;
+
+	default:
+		goto out;
+	}
+
+	ret = sizeof(cmd);
+
+out:
+	mutex_unlock(&userio->lock);
+	return ret;
+}
+
+static unsigned int userio_char_poll(struct file *file, poll_table *wait)
+{
+	struct userio_device *userio = file->private_data;
+
+	poll_wait(file, &userio->waitq, wait);
+
+	if (userio->head != userio->tail)
+		return POLLIN | POLLRDNORM;
+
+	return 0;
+}
+
+static const struct file_operations userio_fops = {
+	.owner		= THIS_MODULE,
+	.open		= userio_char_open,
+	.release	= userio_char_release,
+	.read		= userio_char_read,
+	.write		= userio_char_write,
+	.poll		= userio_char_poll,
+	.llseek		= no_llseek,
+};
+
+static struct miscdevice userio_misc = {
+	.fops	= &userio_fops,
+	.minor	= MISC_DYNAMIC_MINOR,
+	.name	= USERIO_NAME,
+};
+
+MODULE_AUTHOR("Stephen Chandler Paul <thatslyude@gmail.com>");
+MODULE_DESCRIPTION("userio");
+MODULE_LICENSE("GPL");
+
+module_driver(userio_misc, misc_register, misc_deregister);
diff --git a/include/uapi/linux/userio.h b/include/uapi/linux/userio.h
new file mode 100644
index 0000000..da0a3d6
--- /dev/null
+++ b/include/uapi/linux/userio.h
@@ -0,0 +1,42 @@
+/*
+ * userio.h
+ * Copyright (C) 2015 Red Hat
+ * Copyright (C) 2015 Lyude (Stephen Chandler Paul) <cpaul@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more
+ * details.
+ *
+ * This is the public header used for user-space communication with the userio
+ * driver. __attribute__((__packed__)) is used  for all structs to keep ABI
+ * compatibility between all architectures.
+ */
+
+#ifndef _USERIO_H
+#define _USERIO_H
+
+#include <linux/types.h>
+
+#define USERIO_CMD_REGISTER		0
+#define USERIO_CMD_SET_PORT_TYPE	1
+#define USERIO_CMD_SEND_INTERRUPT	2
+
+/*
+ * userio Commands
+ * All commands sent to /dev/userio are encoded using this structure. The type
+ * field should contain a USERIO_CMD* value that indicates what kind of command
+ * is being sent to userio. The data field should contain the accompanying
+ * argument for the command, if there is one.
+ */
+struct userio_cmd {
+	__u8 type;
+	__u8 data;
+} __attribute__((__packed__));
+
+#endif /* !_USERIO_H */
-- 
2.4.3


^ permalink raw reply related

* Re: [PATCH v4 00/10] hugetlbfs: add fallocate support
From: Mike Kravetz @ 2015-07-23 17:29 UTC (permalink / raw)
  To: Eric B Munson
  Cc: Andrew Morton, Davidlohr Bueso, linux-mm, linux-kernel, linux-api,
	Dave Hansen, Naoya Horiguchi, David Rientjes, Hugh Dickins,
	Aneesh Kumar, Hillf Danton, Christoph Hellwig, Michal Hocko,
	emunson
In-Reply-To: <20150723171728.GC9203@akamai.com>

On 07/23/2015 10:17 AM, Eric B Munson wrote:
> On Thu, 23 Jul 2015, Mike Kravetz wrote:
>
>> On 07/23/2015 08:17 AM, Eric B Munson wrote:
>>> On Wed, 22 Jul 2015, Mike Kravetz wrote:
>>>
>>>> On 07/22/2015 03:30 PM, Andrew Morton wrote:
>>>>> On Wed, 22 Jul 2015 15:19:54 -0700 Davidlohr Bueso <dave@stgolabs.net> wrote:
>>>>>
>>>>>>>
>>>>>>> I didn't know that libhugetlbfs has tests.  I wonder if that makes
>>>>>>> tools/testing/selftests/vm's hugetlbfstest harmful?
>>>>>>
>>>>>> Why harmful? Redundant, maybe(?).
>>>>>
>>>>> The presence of the in-kernel tests will cause people to add stuff to
>>>>> them when it would be better if they were to apply that effort to
>>>>> making libhugetlbfs better.  Or vice versa.
>>>>>
>>>>> Mike's work is an example.  Someone later makes a change to hugetlbfs, runs
>>>>> the kernel selftest and says "yay, everything works", unaware that they
>>>>> just broke fallocate support.
>>>>>
>>>>>> Does anyone even use selftests for
>>>>>> hugetlbfs regression testing? Lets see, we also have these:
>>>>>>
>>>>>> - hugepage-{mmap,shm}.c
>>>>>> - map_hugetlb.c
>>>>>>
>>>>>> There's probably a lot of room for improvement here.
>>>>>
>>>>> selftests is a pretty scrappy place.  It's partly a dumping ground for
>>>>> things so useful test code doesn't just get lost and bitrotted.  Partly
>>>>> a framework so people who add features can easily test them. Partly to
>>>>> provide tools to architecture maintainers when they wire up new
>>>>> syscalls and the like.
>>>>>
>>>>> Unless there's some good reason to retain the hugetlb part of
>>>>> selftests, I'm thinking we should just remove it to avoid
>>>>> distracting/misleading people.  Or possibly move the libhugetlbfs test
>>>>> code into the kernel tree and maintain it there.
>>>>
>>>> Adding Eric as he is the libhugetlbfs maintainer.
>>>>
>>>> I think removing the hugetlb selftests in the kernel and pointing
>>>> people to libhugetlbfs is the way to go.  From a very quick scan
>>>> of the selftests, I would guess libhugetlbfs covers everything
>>>> in those tests.
>>>>
>>>> I'm willing to verify the testing provided by selftests is included
>>>> in libhugetlbfs, and remove selftests if that is the direction we
>>>> want to take.
>>>
>>> I would rather see the test suite stay in the library, there are a
>>> number of tests that rely on infrastructure in the library that is not
>>> available in selftests.
>>>
>>> I am happy to help with any tests that need to be added/modified in the
>>> library to cover.
>>
>> I thought about this some more and think there are two distinct
>> groups of users that should be considered.
>> 1) Application developers who simply want to use hugetlb
>> 2) Kernel developers who are modifying hugetlb related code
>>
>> The application developers will mostly want information in the
>> man pages, hugetlbpage.txt and hugetlb selftest programs to use
>> as sample code to get started.  They can also use libhugetlbfs
>> man pages/library if they desire.  Because of this, I do not
>> really want to remove the hugetlb selftest programs.  There are
>> no equivalent simple stand alone programs in libhugetlbfs.
>>
>> Kernel developers would be more concerned about introducing
>> regressions.  The selftest programs are of limited use for this
>> purpose.  The libhugetlbfs test suite is much more suited for
>> regression testing.
>>
>> With this in mind, I suggest:
>> - Keep the mmap man page reference to Documentation/vm/hugetlbpage.txt
>> - Small modification to hugetlbpage.txt saying the selftest code is
>>    good for application development examples.  And, kernel developers
>>    should use libhugetlbfs test suite for regression testing.  In any
>>    case, the sourceforge URL for libhugetlbfs is no longer valid and
>>    needs to be updated.
>> - Modify the run_vmtests selftest script to print out a message saying
>>    libhugetlbfs should be used for hugetlb regression testing.  This
>>    would help catch people who might think the few selftests are
>>    sufficient.
>>
>> Thoughts?
>
> There are a number of tests in the libhugetlbfs suite that cover kernel
> problems, are you suggesting that we move all these tests out of
> libhugetlbfs and into selftests?  I don't think we should separate the
> responsibility for testing kernel regressions so where ever they end up,
> they should all be together.  The libhugetlbfs suite has some nice
> features for setting up the test environment (consider that a plug to
> move tests in that direction).

No, not suggesting we move anything out of libhugetlbfs.  I believe
that should be the primary test suite for hugetlb.

However, the few programs in selftest do provide some value IMO.
They are examples of hugetlb usage without any of the libhugetlbfs
infrastructure present.  Ideally, there would be some place to put
this sample code.  I can not think of an ideal location.

-- 
Mike Kravetz

--
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 v4 00/10] hugetlbfs: add fallocate support
From: Eric B Munson @ 2015-07-23 17:17 UTC (permalink / raw)
  To: Mike Kravetz
  Cc: Andrew Morton, Davidlohr Bueso, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA, Dave Hansen, Naoya Horiguchi,
	David Rientjes, Hugh Dickins, Aneesh Kumar, Hillf Danton,
	Christoph Hellwig, Michal Hocko, emunson-CVBTeua0HjReoWH0uzbU5w
In-Reply-To: <55B11EEF.1070605-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 4501 bytes --]

On Thu, 23 Jul 2015, Mike Kravetz wrote:

> On 07/23/2015 08:17 AM, Eric B Munson wrote:
> >On Wed, 22 Jul 2015, Mike Kravetz wrote:
> >
> >>On 07/22/2015 03:30 PM, Andrew Morton wrote:
> >>>On Wed, 22 Jul 2015 15:19:54 -0700 Davidlohr Bueso <dave-h16yJtLeMjHk1uMJSBkQmQ@public.gmane.org> wrote:
> >>>
> >>>>>
> >>>>>I didn't know that libhugetlbfs has tests.  I wonder if that makes
> >>>>>tools/testing/selftests/vm's hugetlbfstest harmful?
> >>>>
> >>>>Why harmful? Redundant, maybe(?).
> >>>
> >>>The presence of the in-kernel tests will cause people to add stuff to
> >>>them when it would be better if they were to apply that effort to
> >>>making libhugetlbfs better.  Or vice versa.
> >>>
> >>>Mike's work is an example.  Someone later makes a change to hugetlbfs, runs
> >>>the kernel selftest and says "yay, everything works", unaware that they
> >>>just broke fallocate support.
> >>>
> >>>>Does anyone even use selftests for
> >>>>hugetlbfs regression testing? Lets see, we also have these:
> >>>>
> >>>>- hugepage-{mmap,shm}.c
> >>>>- map_hugetlb.c
> >>>>
> >>>>There's probably a lot of room for improvement here.
> >>>
> >>>selftests is a pretty scrappy place.  It's partly a dumping ground for
> >>>things so useful test code doesn't just get lost and bitrotted.  Partly
> >>>a framework so people who add features can easily test them. Partly to
> >>>provide tools to architecture maintainers when they wire up new
> >>>syscalls and the like.
> >>>
> >>>Unless there's some good reason to retain the hugetlb part of
> >>>selftests, I'm thinking we should just remove it to avoid
> >>>distracting/misleading people.  Or possibly move the libhugetlbfs test
> >>>code into the kernel tree and maintain it there.
> >>
> >>Adding Eric as he is the libhugetlbfs maintainer.
> >>
> >>I think removing the hugetlb selftests in the kernel and pointing
> >>people to libhugetlbfs is the way to go.  From a very quick scan
> >>of the selftests, I would guess libhugetlbfs covers everything
> >>in those tests.
> >>
> >>I'm willing to verify the testing provided by selftests is included
> >>in libhugetlbfs, and remove selftests if that is the direction we
> >>want to take.
> >
> >I would rather see the test suite stay in the library, there are a
> >number of tests that rely on infrastructure in the library that is not
> >available in selftests.
> >
> >I am happy to help with any tests that need to be added/modified in the
> >library to cover.
> 
> I thought about this some more and think there are two distinct
> groups of users that should be considered.
> 1) Application developers who simply want to use hugetlb
> 2) Kernel developers who are modifying hugetlb related code
> 
> The application developers will mostly want information in the
> man pages, hugetlbpage.txt and hugetlb selftest programs to use
> as sample code to get started.  They can also use libhugetlbfs
> man pages/library if they desire.  Because of this, I do not
> really want to remove the hugetlb selftest programs.  There are
> no equivalent simple stand alone programs in libhugetlbfs.
> 
> Kernel developers would be more concerned about introducing
> regressions.  The selftest programs are of limited use for this
> purpose.  The libhugetlbfs test suite is much more suited for
> regression testing.
> 
> With this in mind, I suggest:
> - Keep the mmap man page reference to Documentation/vm/hugetlbpage.txt
> - Small modification to hugetlbpage.txt saying the selftest code is
>   good for application development examples.  And, kernel developers
>   should use libhugetlbfs test suite for regression testing.  In any
>   case, the sourceforge URL for libhugetlbfs is no longer valid and
>   needs to be updated.
> - Modify the run_vmtests selftest script to print out a message saying
>   libhugetlbfs should be used for hugetlb regression testing.  This
>   would help catch people who might think the few selftests are
>   sufficient.
> 
> Thoughts?

There are a number of tests in the libhugetlbfs suite that cover kernel
problems, are you suggesting that we move all these tests out of
libhugetlbfs and into selftests?  I don't think we should separate the
responsibility for testing kernel regressions so where ever they end up,
they should all be together.  The libhugetlbfs suite has some nice
features for setting up the test environment (consider that a plug to
move tests in that direction).


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH v4 00/10] hugetlbfs: add fallocate support
From: Mike Kravetz @ 2015-07-23 17:05 UTC (permalink / raw)
  To: Eric B Munson
  Cc: Andrew Morton, Davidlohr Bueso, linux-mm, linux-kernel, linux-api,
	Dave Hansen, Naoya Horiguchi, David Rientjes, Hugh Dickins,
	Aneesh Kumar, Hillf Danton, Christoph Hellwig, Michal Hocko,
	emunson
In-Reply-To: <20150723151707.GB7795@akamai.com>

On 07/23/2015 08:17 AM, Eric B Munson wrote:
> On Wed, 22 Jul 2015, Mike Kravetz wrote:
>
>> On 07/22/2015 03:30 PM, Andrew Morton wrote:
>>> On Wed, 22 Jul 2015 15:19:54 -0700 Davidlohr Bueso <dave@stgolabs.net> wrote:
>>>
>>>>>
>>>>> I didn't know that libhugetlbfs has tests.  I wonder if that makes
>>>>> tools/testing/selftests/vm's hugetlbfstest harmful?
>>>>
>>>> Why harmful? Redundant, maybe(?).
>>>
>>> The presence of the in-kernel tests will cause people to add stuff to
>>> them when it would be better if they were to apply that effort to
>>> making libhugetlbfs better.  Or vice versa.
>>>
>>> Mike's work is an example.  Someone later makes a change to hugetlbfs, runs
>>> the kernel selftest and says "yay, everything works", unaware that they
>>> just broke fallocate support.
>>>
>>>> Does anyone even use selftests for
>>>> hugetlbfs regression testing? Lets see, we also have these:
>>>>
>>>> - hugepage-{mmap,shm}.c
>>>> - map_hugetlb.c
>>>>
>>>> There's probably a lot of room for improvement here.
>>>
>>> selftests is a pretty scrappy place.  It's partly a dumping ground for
>>> things so useful test code doesn't just get lost and bitrotted.  Partly
>>> a framework so people who add features can easily test them. Partly to
>>> provide tools to architecture maintainers when they wire up new
>>> syscalls and the like.
>>>
>>> Unless there's some good reason to retain the hugetlb part of
>>> selftests, I'm thinking we should just remove it to avoid
>>> distracting/misleading people.  Or possibly move the libhugetlbfs test
>>> code into the kernel tree and maintain it there.
>>
>> Adding Eric as he is the libhugetlbfs maintainer.
>>
>> I think removing the hugetlb selftests in the kernel and pointing
>> people to libhugetlbfs is the way to go.  From a very quick scan
>> of the selftests, I would guess libhugetlbfs covers everything
>> in those tests.
>>
>> I'm willing to verify the testing provided by selftests is included
>> in libhugetlbfs, and remove selftests if that is the direction we
>> want to take.
>
> I would rather see the test suite stay in the library, there are a
> number of tests that rely on infrastructure in the library that is not
> available in selftests.
>
> I am happy to help with any tests that need to be added/modified in the
> library to cover.

I thought about this some more and think there are two distinct
groups of users that should be considered.
1) Application developers who simply want to use hugetlb
2) Kernel developers who are modifying hugetlb related code

The application developers will mostly want information in the
man pages, hugetlbpage.txt and hugetlb selftest programs to use
as sample code to get started.  They can also use libhugetlbfs
man pages/library if they desire.  Because of this, I do not
really want to remove the hugetlb selftest programs.  There are
no equivalent simple stand alone programs in libhugetlbfs.

Kernel developers would be more concerned about introducing
regressions.  The selftest programs are of limited use for this
purpose.  The libhugetlbfs test suite is much more suited for
regression testing.

With this in mind, I suggest:
- Keep the mmap man page reference to Documentation/vm/hugetlbpage.txt
- Small modification to hugetlbpage.txt saying the selftest code is
   good for application development examples.  And, kernel developers
   should use libhugetlbfs test suite for regression testing.  In any
   case, the sourceforge URL for libhugetlbfs is no longer valid and
   needs to be updated.
- Modify the run_vmtests selftest script to print out a message saying
   libhugetlbfs should be used for hugetlb regression testing.  This
   would help catch people who might think the few selftests are
   sufficient.

Thoughts?
-- 
Mike Kravetz

--
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 v8 1/9] nvmem: Add a simple NVMEM framework for nvmem providers
From: Srinivas Kandagatla @ 2015-07-23 15:50 UTC (permalink / raw)
  To: Stefan Wahren, linux-arm-kernel, Greg Kroah-Hartman
  Cc: Rob Herring, Mark Brown, s.hauer, linux-api, linux-kernel,
	devicetree, linux-arm-msm, arnd, sboyd, pantelis.antoniou,
	mporter, wxt, Maxime Ripard
In-Reply-To: <55B107B0.5010706@i2se.com>



On 23/07/15 16:26, Stefan Wahren wrote:
> Hi Srinivas,
>
> Am 20.07.2015 um 16:43 schrieb Srinivas Kandagatla:
>> This patch adds just providers part of the framework just to enable easy
>> review.
>>
>> Up until now, NVMEM drivers like eeprom were stored in drivers/misc,
>> where they all had to duplicate pretty much the same code to register
>> a sysfs file, allow in-kernel users to access the content of the devices
>> they were driving, etc.
>>
>> This was also a problem as far as other in-kernel users were involved,
>> since the solutions used were pretty much different from on driver to
>> another, there was a rather big abstraction leak.
>>
>> This introduction of this framework aims at solving this. It also
>> introduces DT representation for consumer devices to go get the data
>> they require (MAC Addresses, SoC/Revision ID, part numbers, and so on)
>> from the nvmems.
>>
>> Having regmap interface to this framework would give much better
>> abstraction for nvmems on different buses.
>>
>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>> [Maxime Ripard: intial version of eeprom framework]
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>> ---
>>   drivers/Kconfig                |   2 +
>>   drivers/Makefile               |   1 +
>>   drivers/nvmem/Kconfig          |  13 ++
>>   drivers/nvmem/Makefile         |   6 +
>>   drivers/nvmem/core.c           | 384 +++++++++++++++++++++++++++++++++++++++++
>>   include/linux/nvmem-consumer.h |  23 +++
>>   include/linux/nvmem-provider.h |  47 +++++
>>   7 files changed, 476 insertions(+)
>>   create mode 100644 drivers/nvmem/Kconfig
>>   create mode 100644 drivers/nvmem/Makefile
>>   create mode 100644 drivers/nvmem/core.c
>>   create mode 100644 include/linux/nvmem-consumer.h
>>   create mode 100644 include/linux/nvmem-provider.h
>
> i've tested this patch with my mxs-ocotp driver [1].
>
> So you can add
>
> Tested-by: Stefan Wahren <stefan.wahren@i2se.com>
>
Thanks for tested-by, That helps.

--srini




> Regards
> Stefan
>

^ permalink raw reply

* Re: [PATCH v8 8/9] nvmem: sunxi: Move the SID driver to the nvmem framework
From: Srinivas Kandagatla @ 2015-07-23 15:48 UTC (permalink / raw)
  To: Stefan Wahren
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Greg Kroah-Hartman, devicetree-u79uwXL29TY76Z2rM5mHXA,
	arnd-r2nGTMty4D4, linux-api-u79uwXL29TY76Z2rM5mHXA,
	s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ, sboyd-sgV2jX0FEOL9JmXXK+q4OQ,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
	pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w, Mark Brown,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
	mporter-OWPKS81ov/FWk0Htik3J/w, Maxime Ripard,
	wxt-TNX95d0MmH7DzftRWevZcw
In-Reply-To: <55B105B6.7000502-eS4NqCHxEME@public.gmane.org>



On 23/07/15 16:18, Stefan Wahren wrote:
> Hi Srinivas,
>
> Am 20.07.2015 um 16:44 schrieb Srinivas Kandagatla:
>> From: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>
>> Now that we have the nvmem framework, we can consolidate the common
>> driver code. Move the driver to the framework, and hopefully, it will
>> fix the sysfs file creation race.
>>
>> Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>> [srinivas.kandagatla: Moved to regmap based EEPROM framework]
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> ---
>>   Documentation/ABI/testing/sysfs-driver-sunxi-sid   |  22 ---
>>   .../bindings/misc/allwinner,sunxi-sid.txt          |  17 ---
>>   .../bindings/nvmem/allwinner,sunxi-sid.txt         |  21 +++
>>   drivers/misc/eeprom/Kconfig                        |  13 --
>>   drivers/misc/eeprom/Makefile                       |   1 -
>>   drivers/misc/eeprom/sunxi_sid.c                    | 156 --------------------
>>   drivers/nvmem/Kconfig                              |  11 ++
>>   drivers/nvmem/Makefile                             |   2 +
>>   drivers/nvmem/sunxi_sid.c                          | 159 +++++++++++++++++++++
>>   9 files changed, 193 insertions(+), 209 deletions(-)
>>   delete mode 100644 Documentation/ABI/testing/sysfs-driver-sunxi-sid
>>   delete mode 100644 Documentation/devicetree/bindings/misc/allwinner,sunxi-sid.txt
>>   create mode 100644 Documentation/devicetree/bindings/nvmem/allwinner,sunxi-sid.txt
>>   delete mode 100644 drivers/misc/eeprom/sunxi_sid.c
>>   create mode 100644 drivers/nvmem/sunxi_sid.c
>>
>> [...]
>> diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
>> index ff44fe9..4328b93 100644
>> --- a/drivers/nvmem/Makefile
>> +++ b/drivers/nvmem/Makefile
>> @@ -8,3 +8,5 @@ nvmem_core-y			:= core.o
>>   # Devices
>>   obj-$(CONFIG_QCOM_QFPROM)	+= nvmem_qfprom.o
>>   nvmem_qfprom-y			:= qfprom.o
>> +obj-$(CONFIG_NVMEM_SUNXI_SID)	+= nvmem_sunxi_sid.o
>> +nvmem_sunxi_sid-y		:= sunxi_sid.o
>
> is it really necessary to have 2 lines for a single driver?
>
> Why not the following line?
>
> obj-$(CONFIG_NVMEM_SUNXI_SID)	+= sunxi_sid.o
>
We can do that, but only reason i did it this way is to make the module 
naming consistent like nvmem_*.ko

--srini

> Regards
> Stefan
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" 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 v8 8/9] nvmem: sunxi: Move the SID driver to the nvmem framework
From: Srinivas Kandagatla @ 2015-07-23 15:48 UTC (permalink / raw)
  To: Stefan Wahren, Greg Kroah-Hartman,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
  Cc: wxt-TNX95d0MmH7DzftRWevZcw, linux-api-u79uwXL29TY76Z2rM5mHXA,
	Rob Herring, sboyd-sgV2jX0FEOL9JmXXK+q4OQ, arnd-r2nGTMty4D4,
	s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	mporter-OWPKS81ov/FWk0Htik3J/w,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, Maxime Ripard,
	pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w, Mark Brown,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <2114639619.71276.1437496724946.JavaMail.open-xchange-0SF9iQWekqLc2Cnrm9MUdsgmgJlYmuWJ@public.gmane.org>



On 21/07/15 17:38, Stefan Wahren wrote:
> Hi Srinivas,
>
>> Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> hat am 20. Juli 2015 um
>> 16:44 geschrieben:
>>
>>
>> From: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>
>> Now that we have the nvmem framework, we can consolidate the common
>> driver code. Move the driver to the framework, and hopefully, it will
>> fix the sysfs file creation race.
>>
>> Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>> [srinivas.kandagatla: Moved to regmap based EEPROM framework]
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> ---
>> Documentation/ABI/testing/sysfs-driver-sunxi-sid | 22 ---
>> .../bindings/misc/allwinner,sunxi-sid.txt | 17 ---
>> .../bindings/nvmem/allwinner,sunxi-sid.txt | 21 +++
>> drivers/misc/eeprom/Kconfig | 13 --
>> drivers/misc/eeprom/Makefile | 1 -
>> drivers/misc/eeprom/sunxi_sid.c | 156 --------------------
>> drivers/nvmem/Kconfig | 11 ++
>> drivers/nvmem/Makefile | 2 +
>> drivers/nvmem/sunxi_sid.c | 159 +++++++++++++++++++++
>> 9 files changed, 193 insertions(+), 209 deletions(-)
>> delete mode 100644 Documentation/ABI/testing/sysfs-driver-sunxi-sid
>> delete mode 100644
>> Documentation/devicetree/bindings/misc/allwinner,sunxi-sid.txt
>> create mode 100644
>> Documentation/devicetree/bindings/nvmem/allwinner,sunxi-sid.txt
>> delete mode 100644 drivers/misc/eeprom/sunxi_sid.c
>> create mode 100644 drivers/nvmem/sunxi_sid.c
>>
>> [...]
>> -static int sunxi_sid_probe(struct platform_device *pdev)
>> -{
>> - struct sunxi_sid_data *sid_data;
>> - struct resource *res;
>> - const struct of_device_id *of_dev_id;
>> - u8 *entropy;
>> - unsigned int i;
>> -
>> - sid_data = devm_kzalloc(&pdev->dev, sizeof(struct sunxi_sid_data),
>> - GFP_KERNEL);
>> - if (!sid_data)
>> - return -ENOMEM;
>> -
>> - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> - sid_data->reg_base = devm_ioremap_resource(&pdev->dev, res);
>> - if (IS_ERR(sid_data->reg_base))
>> - return PTR_ERR(sid_data->reg_base);
>> -
>> - of_dev_id = of_match_device(sunxi_sid_of_match, &pdev->dev);
>> - if (!of_dev_id)
>> - return -ENODEV;
>> - sid_data->keysize = (int)of_dev_id->data;
>> -
>> - platform_set_drvdata(pdev, sid_data);
>> -
>> - sid_bin_attr.size = sid_data->keysize;
>> - if (device_create_bin_file(&pdev->dev, &sid_bin_attr))
>> - return -ENODEV;
>> -
>> - entropy = kzalloc(sizeof(u8) * sid_data->keysize, GFP_KERNEL);
>> - for (i = 0; i < sid_data->keysize; i++)
>> - entropy[i] = sunxi_sid_read_byte(sid_data, i);
>> - add_device_randomness(entropy, sid_data->keysize);
>> - kfree(entropy);
>> -
>
> in case of porting a driver to a new framework, i would expect the same
> features.
> The ported driver do not add the Security ID to the device randomness.
>
> What's the reason for this difference?
Not sure why it was discarded in the first place, but I can put it back 
which will atleast add some data to random pool.

--srini
>
> Regards
> Stefan
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox