Linux userland API discussions
 help / color / mirror / Atom feed
* Re: [RFC PATCH] Add /proc/<pid>/numa_vamaps for numa node information
From: Michal Hocko @ 2018-05-04 11:12 UTC (permalink / raw)
  To: prakash.sangappa
  Cc: Christopher Lameter, linux-kernel, linux-mm, linux-api, akpm,
	kirill.shutemov, n-horiguchi, drepper, rientjes
In-Reply-To: <c80ee329-084b-367f-1937-3175c178e978@oracle.com>

On Thu 03-05-18 15:39:49, prakash.sangappa wrote:
> 
> 
> On 05/03/2018 11:03 AM, Christopher Lameter wrote:
> > On Tue, 1 May 2018, Prakash Sangappa wrote:
> > 
> > > For analysis purpose it is useful to have numa node information
> > > corresponding mapped address ranges of the process. Currently
> > > /proc/<pid>/numa_maps provides list of numa nodes from where pages are
> > > allocated per VMA of the process. This is not useful if an user needs to
> > > determine which numa node the mapped pages are allocated from for a
> > > particular address range. It would have helped if the numa node information
> > > presented in /proc/<pid>/numa_maps was broken down by VA ranges showing the
> > > exact numa node from where the pages have been allocated.
> > Cant you write a small script that scans the information in numa_maps and
> > then displays the total pages per NUMA node and then a list of which
> > ranges have how many pages on a particular node?
> 
> Don't think we can determine which numa node a given user process
> address range has pages from, based on the existing 'numa_maps' file.

yes we have. See move_pages...
 
> > > reading this file will not be restricted(i.e requiring CAP_SYS_ADMIN).
> > So a prime motivator here is security restricted access to numa_maps?
> No it is the opposite. A regular user should be able to determine
> numa node information.

Well, that breaks the layout randomization, doesn't it?
-- 
Michal Hocko
SUSE Labs

^ permalink raw reply

* Re: [RFC PATCH] Add /proc/<pid>/numa_vamaps for numa node information
From: Michal Hocko @ 2018-05-04 11:10 UTC (permalink / raw)
  To: prakash.sangappa
  Cc: Andrew Morton, linux-kernel, linux-mm, linux-api, kirill.shutemov,
	n-horiguchi, drepper, rientjes, Naoya Horiguchi, Dave Hansen
In-Reply-To: <40be68bb-8322-2bef-f454-22e1ab9029da@oracle.com>

On Thu 03-05-18 15:37:39, prakash.sangappa wrote:
> 
> 
> On 05/03/2018 01:57 AM, Michal Hocko wrote:
> > On Wed 02-05-18 16:43:58, prakash.sangappa wrote:
> > > 
> > > On 05/02/2018 02:33 PM, Andrew Morton wrote:
> > > > On Tue,  1 May 2018 22:58:06 -0700 Prakash Sangappa <prakash.sangappa@oracle.com> wrote:
> > > > 
> > > > > For analysis purpose it is useful to have numa node information
> > > > > corresponding mapped address ranges of the process. Currently
> > > > > /proc/<pid>/numa_maps provides list of numa nodes from where pages are
> > > > > allocated per VMA of the process. This is not useful if an user needs to
> > > > > determine which numa node the mapped pages are allocated from for a
> > > > > particular address range. It would have helped if the numa node information
> > > > > presented in /proc/<pid>/numa_maps was broken down by VA ranges showing the
> > > > > exact numa node from where the pages have been allocated.
> > > > > 
> > > > > The format of /proc/<pid>/numa_maps file content is dependent on
> > > > > /proc/<pid>/maps file content as mentioned in the manpage. i.e one line
> > > > > entry for every VMA corresponding to entries in /proc/<pids>/maps file.
> > > > > Therefore changing the output of /proc/<pid>/numa_maps may not be possible.
> > > > > 
> > > > > Hence, this patch proposes adding file /proc/<pid>/numa_vamaps which will
> > > > > provide proper break down of VA ranges by numa node id from where the mapped
> > > > > pages are allocated. For Address ranges not having any pages mapped, a '-'
> > > > > is printed instead of numa node id. In addition, this file will include most
> > > > > of the other information currently presented in /proc/<pid>/numa_maps. The
> > > > > additional information included is for convenience. If this is not
> > > > > preferred, the patch could be modified to just provide VA range to numa node
> > > > > information as the rest of the information is already available thru
> > > > > /proc/<pid>/numa_maps file.
> > > > > 
> > > > > Since the VA range to numa node information does not include page's PFN,
> > > > > reading this file will not be restricted(i.e requiring CAP_SYS_ADMIN).
> > > > > 
> > > > > Here is the snippet from the new file content showing the format.
> > > > > 
> > > > > 00400000-00401000 N0=1 kernelpagesize_kB=4 mapped=1 file=/tmp/hmap2
> > > > > 00600000-00601000 N0=1 kernelpagesize_kB=4 anon=1 dirty=1 file=/tmp/hmap2
> > > > > 00601000-00602000 N0=1 kernelpagesize_kB=4 anon=1 dirty=1 file=/tmp/hmap2
> > > > > 7f0215600000-7f0215800000 N0=1 kernelpagesize_kB=2048 dirty=1 file=/mnt/f1
> > > > > 7f0215800000-7f0215c00000 -  file=/mnt/f1
> > > > > 7f0215c00000-7f0215e00000 N0=1 kernelpagesize_kB=2048 dirty=1 file=/mnt/f1
> > > > > 7f0215e00000-7f0216200000 -  file=/mnt/f1
> > > > > ..
> > > > > 7f0217ecb000-7f0217f20000 N0=85 kernelpagesize_kB=4 mapped=85 mapmax=51
> > > > >      file=/usr/lib64/libc-2.17.so
> > > > > 7f0217f20000-7f0217f30000 -  file=/usr/lib64/libc-2.17.so
> > > > > 7f0217f30000-7f0217f90000 N0=96 kernelpagesize_kB=4 mapped=96 mapmax=51
> > > > >      file=/usr/lib64/libc-2.17.so
> > > > > 7f0217f90000-7f0217fb0000 -  file=/usr/lib64/libc-2.17.so
> > > > > ..
> > > > > 
> > > > > The 'pmap' command can be enhanced to include an option to show numa node
> > > > > information which it can read from this new proc file. This will be a
> > > > > follow on proposal.
> > > > I'd like to hear rather more about the use-cases for this new
> > > > interface.  Why do people need it, what is the end-user benefit, etc?
> > > This is mainly for debugging / performance analysis. Oracle Database
> > > team is looking to use this information.
> > But we do have an interface to query (e.g. move_pages) that your
> > application can use. I am really worried that the broken out per node
> > data can be really large (just take a large vma with interleaved policy
> > as an example). So is this really worth adding as a general purpose proc
> > interface?
> 
> I guess move_pages could be useful. There needs to be a tool or
> command which can read the numa node information using move_pages
> to be used to observe another process.

That should be trivial. You can get vma ranges of interest from /proc/maps
and then use move_pages to get a more detailed information.

> From an observability point of view, one of the use of the proposed
> new file 'numa_vamaps' was to modify  'pmap' command to display numa
> node information broken down by address ranges.  Would having pmap
> show numa node information be useful?

I do not have a usecase for that.
-- 
Michal Hocko
SUSE Labs

^ permalink raw reply

* Re: [PATCH v5 00/28] FPGA Device Feature List (DFL) Device Drivers
From: Wu Hao @ 2018-05-04  0:15 UTC (permalink / raw)
  To: Alan Tull
  Cc: Moritz Fischer, linux-fpga, linux-kernel, linux-api, Kang, Luwei,
	Zhang, Yi Z
In-Reply-To: <CANk1AXSFQzBoZWQu4R9aYQSXFbr76UQ_-39ENe5hSOncKuDq+A@mail.gmail.com>

On Thu, May 03, 2018 at 04:14:48PM -0500, Alan Tull wrote:
> On Tue, May 1, 2018 at 9:50 PM, Wu Hao <hao.wu@intel.com> wrote:
> > Hi All,
> >
> > Here is v5 patch-series adding drivers for FPGA DFL devices.
> >
> > This patch series provides a common framework to support FPGA Device Feature
> > List (DFL), and also feature dev drivers under this DFL framework to provide
> > interfaces for userspace applications to configure, enumerate, open, and
> > access FPGA accelerators on DFL based FPGA device and enables system level
> > management functions such as FPGA partial reconfiguration, power management
> > and virtualization.
> 
> Hi Hao,
> 

Hi Alan,

Thanks for your review on this patch set.

> I've started looking at your v5 here.  It's 5212 lines of code, so
> it's not tiny.
> 
> I see a lot of renaming from 'fpga' to 'dfl', thanks for doing that.
> 
> The main thing that stands out at first look is a new method of
> registering port ops.  It would be better if the fpga bridge framework
> were expanded or changed somehow to do that.  It's like we have two
> bridge frameworks now.

I only limit this change in DFL framework, because introduce a port ops
here to DFL framework is to resolve the hard dependency between DFL
driver modules (e.g DFL FME and port), this sounds like a DFL specific
thing, and I am not sure if there is some real need or benefit for other
non-DFL module, so i didn't modify the common fpga bridge code.

> 
> I see that there isn't a way of specifying which FPGA mgr driver to
> use, so dfl-fme-mgr.c is tied to dfl-fme-pr.c.  It would be better if
> the DFL could specify which fpga manager driver to use.

Hm.. I think we discussed this against v4. It could be a different sub
feature with different id to support different PR hardware.  In that
case, we could still reuse sub feature driver provided by dfl-fme-pr.c
(specify a new id which reuses the same pr_mgmt_ops)

        {
                 .id = FME_FEATURE_ID_PR_MGMT,
                 .ops = &pr_mgmt_ops,
        },

Inside dfl-fme-pr.c, it could parse the different id to create different
platform devices for different FPGA mgrs.

Could we leave this a later change? as i don't have a different hardware
like this at this moment (maybe future) and I don't see current framework
block us on reusing these code.

> 
> Maybe I'm repeating myself here, the biggest question I'm asking
> region, how much of this code can be reused?  I would hope that all of
> it could except for the fpga manager/bridge drivers.  That was the
> intent of the original FPGA framework.  

I fully understand that it's better to make code reusable, this is what
we are trying to do in these patchsets. But it's still not very clear that
who others or which products will use this DFL framework and how, what's
the difficulty they will face and what kind of help or changes we could
provide. so maybe we could defer some work when people really need it, if
there is no hard blocking issue on architecture level.

> I'll have more time next week to look in more depth.

Looking forward to your feedback. Thanks again for your review. :)

Hao

> 
> Alan
> 

^ permalink raw reply

* [PATCH v2 4/4] mm/hugetlb: use find_alloc_contig_pages() to allocate gigantic pages
From: Mike Kravetz @ 2018-05-03 23:29 UTC (permalink / raw)
  To: linux-mm, linux-kernel, linux-api
  Cc: Reinette Chatre, Michal Hocko, Christopher Lameter, Guy Shattah,
	Anshuman Khandual, Michal Nazarewicz, Vlastimil Babka,
	David Nellans, Laura Abbott, Pavel Machek, Dave Hansen,
	Andrew Morton, Mike Kravetz
In-Reply-To: <20180503232935.22539-1-mike.kravetz@oracle.com>

Use the new find_alloc_contig_pages() interface for the allocation of
gigantic pages and remove associated code in hugetlb.c.

Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
---
 mm/hugetlb.c | 87 +++++-------------------------------------------------------
 1 file changed, 6 insertions(+), 81 deletions(-)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index c81072ce7510..91edf1df3592 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -1053,91 +1053,16 @@ static void destroy_compound_gigantic_page(struct page *page,
 	__ClearPageHead(page);
 }
 
-static void free_gigantic_page(struct page *page, unsigned int order)
+static void free_gigantic_page(struct page *page, struct hstate *h)
 {
-	free_contig_range(page_to_pfn(page), 1UL << order);
-}
-
-static int __alloc_gigantic_page(unsigned long start_pfn,
-				unsigned long nr_pages, gfp_t gfp_mask)
-{
-	unsigned long end_pfn = start_pfn + nr_pages;
-	return alloc_contig_range(start_pfn, end_pfn, MIGRATE_MOVABLE,
-				  gfp_mask);
-}
-
-static bool pfn_range_valid_gigantic(struct zone *z,
-			unsigned long start_pfn, unsigned long nr_pages)
-{
-	unsigned long i, end_pfn = start_pfn + nr_pages;
-	struct page *page;
-
-	for (i = start_pfn; i < end_pfn; i++) {
-		if (!pfn_valid(i))
-			return false;
-
-		page = pfn_to_page(i);
-
-		if (page_zone(page) != z)
-			return false;
-
-		if (PageReserved(page))
-			return false;
-
-		if (page_count(page) > 0)
-			return false;
-
-		if (PageHuge(page))
-			return false;
-	}
-
-	return true;
-}
-
-static bool zone_spans_last_pfn(const struct zone *zone,
-			unsigned long start_pfn, unsigned long nr_pages)
-{
-	unsigned long last_pfn = start_pfn + nr_pages - 1;
-	return zone_spans_pfn(zone, last_pfn);
+	free_contig_pages(page, (unsigned long)pages_per_huge_page(h));
 }
 
 static struct page *alloc_gigantic_page(struct hstate *h, gfp_t gfp_mask,
 		int nid, nodemask_t *nodemask)
 {
-	unsigned int order = huge_page_order(h);
-	unsigned long nr_pages = 1 << order;
-	unsigned long ret, pfn, flags;
-	struct zonelist *zonelist;
-	struct zone *zone;
-	struct zoneref *z;
-
-	zonelist = node_zonelist(nid, gfp_mask);
-	for_each_zone_zonelist_nodemask(zone, z, zonelist, gfp_zone(gfp_mask), nodemask) {
-		spin_lock_irqsave(&zone->lock, flags);
-
-		pfn = ALIGN(zone->zone_start_pfn, nr_pages);
-		while (zone_spans_last_pfn(zone, pfn, nr_pages)) {
-			if (pfn_range_valid_gigantic(zone, pfn, nr_pages)) {
-				/*
-				 * We release the zone lock here because
-				 * alloc_contig_range() will also lock the zone
-				 * at some point. If there's an allocation
-				 * spinning on this lock, it may win the race
-				 * and cause alloc_contig_range() to fail...
-				 */
-				spin_unlock_irqrestore(&zone->lock, flags);
-				ret = __alloc_gigantic_page(pfn, nr_pages, gfp_mask);
-				if (!ret)
-					return pfn_to_page(pfn);
-				spin_lock_irqsave(&zone->lock, flags);
-			}
-			pfn += nr_pages;
-		}
-
-		spin_unlock_irqrestore(&zone->lock, flags);
-	}
-
-	return NULL;
+	return find_alloc_contig_pages((unsigned long)pages_per_huge_page(h),
+					gfp_mask, nid, nodemask);
 }
 
 static void prep_new_huge_page(struct hstate *h, struct page *page, int nid);
@@ -1147,7 +1072,7 @@ static void prep_compound_gigantic_page(struct page *page, unsigned int order);
 static inline bool gigantic_page_supported(void) { return false; }
 static struct page *alloc_gigantic_page(struct hstate *h, gfp_t gfp_mask,
 		int nid, nodemask_t *nodemask) { return NULL; }
-static inline void free_gigantic_page(struct page *page, unsigned int order) { }
+static inline void free_gigantic_page(struct page *page, struct hstate *h) { }
 static inline void destroy_compound_gigantic_page(struct page *page,
 						unsigned int order) { }
 #endif
@@ -1172,7 +1097,7 @@ static void update_and_free_page(struct hstate *h, struct page *page)
 	set_page_refcounted(page);
 	if (hstate_is_gigantic(h)) {
 		destroy_compound_gigantic_page(page, huge_page_order(h));
-		free_gigantic_page(page, huge_page_order(h));
+		free_gigantic_page(page, h);
 	} else {
 		__free_pages(page, huge_page_order(h));
 	}
-- 
2.13.6

^ permalink raw reply related

* [PATCH v2 3/4] mm: add find_alloc_contig_pages() interface
From: Mike Kravetz @ 2018-05-03 23:29 UTC (permalink / raw)
  To: linux-mm, linux-kernel, linux-api
  Cc: Reinette Chatre, Michal Hocko, Christopher Lameter, Guy Shattah,
	Anshuman Khandual, Michal Nazarewicz, Vlastimil Babka,
	David Nellans, Laura Abbott, Pavel Machek, Dave Hansen,
	Andrew Morton, Mike Kravetz
In-Reply-To: <20180503232935.22539-1-mike.kravetz@oracle.com>

find_alloc_contig_pages() is a new interface that attempts to locate
and allocate a contiguous range of pages.  It is provided as a more
convenient interface than alloc_contig_range() which is currently
used by CMA and gigantic huge pages.

When attempting to allocate a range of pages, migration is employed
if possible.  There is no guarantee that the routine will succeed.
So, the user must be prepared for failure and have a fall back plan.

Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
---
 include/linux/gfp.h |  12 +++++
 mm/page_alloc.c     | 136 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 146 insertions(+), 2 deletions(-)

diff --git a/include/linux/gfp.h b/include/linux/gfp.h
index 86a0d06463ab..b0d11777d487 100644
--- a/include/linux/gfp.h
+++ b/include/linux/gfp.h
@@ -573,6 +573,18 @@ static inline bool pm_suspended_storage(void)
 extern int alloc_contig_range(unsigned long start, unsigned long end,
 			      unsigned migratetype, gfp_t gfp_mask);
 extern void free_contig_range(unsigned long pfn, unsigned long nr_pages);
+extern struct page *find_alloc_contig_pages(unsigned long nr_pages, gfp_t gfp,
+						int nid, nodemask_t *nodemask);
+extern void free_contig_pages(struct page *page, unsigned long nr_pages);
+#else
+static inline struct page *find_alloc_contig_pages(unsigned long nr_pages,
+				gfp_t gfp, int nid, nodemask_t *nodemask)
+{
+	return NULL;
+}
+static inline void free_contig_pages(struct page *page, unsigned long nr_pages)
+{
+}
 #endif
 
 #ifdef CONFIG_CMA
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index cb1a5e0be6ee..d0a2d0da9eae 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -67,6 +67,7 @@
 #include <linux/ftrace.h>
 #include <linux/lockdep.h>
 #include <linux/nmi.h>
+#include <linux/mmzone.h>
 
 #include <asm/sections.h>
 #include <asm/tlbflush.h>
@@ -7913,8 +7914,12 @@ int alloc_contig_range(unsigned long start, unsigned long end,
 
 	/* Make sure the range is really isolated. */
 	if (test_pages_isolated(outer_start, end, false)) {
-		pr_info_ratelimited("%s: [%lx, %lx) PFNs busy\n",
-			__func__, outer_start, end);
+#ifdef MIGRATE_CMA
+		/* Only print messages for CMA allocations */
+		if (migratetype == MIGRATE_CMA)
+			pr_info_ratelimited("%s: [%lx, %lx) PFNs busy\n",
+				__func__, outer_start, end);
+#endif
 		ret = -EBUSY;
 		goto done;
 	}
@@ -7950,6 +7955,133 @@ void free_contig_range(unsigned long pfn, unsigned long nr_pages)
 	}
 	WARN(count != 0, "%ld pages are still in use!\n", count);
 }
+
+/*
+ * Only check for obvious pfn/pages which can not be used/migrated.  The
+ * migration code will do the final check.  Under stress, this minimal set
+ * has been observed to provide the best results.  The checks can be expanded
+ * if needed.
+ */
+static bool contig_pfn_range_valid(struct zone *z, unsigned long start_pfn,
+					unsigned long nr_pages)
+{
+	unsigned long i, end_pfn = start_pfn + nr_pages;
+	struct page *page;
+
+	for (i = start_pfn; i < end_pfn; i++) {
+		if (!pfn_valid(i))
+			return false;
+
+		page = pfn_to_online_page(i);
+
+		if (page_zone(page) != z)
+			return false;
+
+	}
+
+	return true;
+}
+
+/*
+ * Search for and attempt to allocate contiguous allocations greater than
+ * MAX_ORDER.
+ */
+static struct page *__alloc_contig_pages_nodemask(gfp_t gfp,
+						unsigned long order,
+						int nid, nodemask_t *nodemask)
+{
+	unsigned long nr_pages, pfn, flags;
+	struct page *ret_page = NULL;
+	struct zonelist *zonelist;
+	struct zoneref *z;
+	struct zone *zone;
+	int rc;
+
+	nr_pages = 1 << order;
+	zonelist = node_zonelist(nid, gfp);
+	for_each_zone_zonelist_nodemask(zone, z, zonelist, gfp_zone(gfp),
+					nodemask) {
+		pgdat_resize_lock(zone->zone_pgdat, &flags);
+		pfn = ALIGN(zone->zone_start_pfn, nr_pages);
+		while (zone_spans_pfn(zone, pfn + nr_pages - 1)) {
+			if (contig_pfn_range_valid(zone, pfn, nr_pages)) {
+				struct page *page = pfn_to_online_page(pfn);
+				unsigned int migratetype;
+
+				/*
+				 * All pageblocks in range must be of same
+				 * migrate type.
+				 */
+				migratetype = get_pageblock_migratetype(page);
+				pgdat_resize_unlock(zone->zone_pgdat, &flags);
+
+				rc = alloc_contig_range(pfn, pfn + nr_pages,
+						migratetype, gfp);
+				if (!rc) {
+					ret_page = pfn_to_page(pfn);
+					return ret_page;
+				}
+				pgdat_resize_lock(zone->zone_pgdat, &flags);
+			}
+			pfn += nr_pages;
+		}
+		pgdat_resize_unlock(zone->zone_pgdat, &flags);
+	}
+
+	return ret_page;
+}
+
+/**
+ * find_alloc_contig_pages() -- attempt to find and allocate a contiguous
+ *				range of pages
+ * @nr_pages:	number of pages to find/allocate
+ * @gfp:	gfp mask used to limit search as well as during compaction
+ * @nid:	target node
+ * @nodemask:	mask of other possible nodes
+ *
+ * Pages can be freed with a call to free_contig_pages(), or by manually
+ * calling __free_page() for each page allocated.
+ *
+ * Return: pointer to 'order' pages on success, or NULL if not successful.
+ */
+struct page *find_alloc_contig_pages(unsigned long nr_pages, gfp_t gfp,
+					int nid, nodemask_t *nodemask)
+{
+	unsigned long i, alloc_order, order_pages;
+	struct page *pages;
+
+	/*
+	 * Underlying allocators perform page order sized allocations.
+	 */
+	alloc_order = get_count_order(nr_pages);
+	if (alloc_order < MAX_ORDER) {
+		pages = __alloc_pages_nodemask(gfp, (unsigned int)alloc_order,
+						nid, nodemask);
+		split_page(pages, alloc_order);
+	} else {
+		pages = __alloc_contig_pages_nodemask(gfp, alloc_order, nid,
+							nodemask);
+	}
+
+	if (pages) {
+		/*
+		 * More pages than desired could have been allocated due to
+		 * rounding up to next page order.  Free any excess pages.
+		 */
+		order_pages = 1UL << alloc_order;
+		for (i = nr_pages; i < order_pages; i++)
+			__free_page(pages + i);
+	}
+
+	return pages;
+}
+EXPORT_SYMBOL_GPL(find_alloc_contig_pages);
+
+void free_contig_pages(struct page *page, unsigned long nr_pages)
+{
+	free_contig_range(page_to_pfn(page), nr_pages);
+}
+EXPORT_SYMBOL_GPL(free_contig_pages);
 #endif
 
 #if defined CONFIG_MEMORY_HOTPLUG || defined CONFIG_CMA
-- 
2.13.6

^ permalink raw reply related

* [PATCH v2 2/4] mm: check for proper migrate type during isolation
From: Mike Kravetz @ 2018-05-03 23:29 UTC (permalink / raw)
  To: linux-mm, linux-kernel, linux-api
  Cc: Reinette Chatre, Michal Hocko, Christopher Lameter, Guy Shattah,
	Anshuman Khandual, Michal Nazarewicz, Vlastimil Babka,
	David Nellans, Laura Abbott, Pavel Machek, Dave Hansen,
	Andrew Morton, Mike Kravetz
In-Reply-To: <20180503232935.22539-1-mike.kravetz@oracle.com>

The routine start_isolate_page_range and alloc_contig_range have
comments saying that migratetype must be either MIGRATE_MOVABLE or
MIGRATE_CMA.  However, this is not enforced.  What is important is
that that all pageblocks in the range are of type migratetype.
This is because blocks will be set to migratetype on error.

Add a boolean argument enforce_migratetype to the routine
start_isolate_page_range.  If set, it will check that all pageblocks
in the range have the passed migratetype.  Return -EINVAL is pageblock
is wrong type is found in range.

A boolean is used for enforce_migratetype as there are two primary
users.  Contiguous range allocation which wants to enforce migration
type checking.  Memory offline (hotplug) which is not concerned about
type checking.

Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
---
 include/linux/page-isolation.h |  8 +++-----
 mm/memory_hotplug.c            |  2 +-
 mm/page_alloc.c                | 17 +++++++++--------
 mm/page_isolation.c            | 40 ++++++++++++++++++++++++++++++----------
 4 files changed, 43 insertions(+), 24 deletions(-)

diff --git a/include/linux/page-isolation.h b/include/linux/page-isolation.h
index 4ae347cbc36d..2ab7e5a399ac 100644
--- a/include/linux/page-isolation.h
+++ b/include/linux/page-isolation.h
@@ -38,8 +38,6 @@ int move_freepages_block(struct zone *zone, struct page *page,
 
 /*
  * Changes migrate type in [start_pfn, end_pfn) to be MIGRATE_ISOLATE.
- * If specified range includes migrate types other than MOVABLE or CMA,
- * this will fail with -EBUSY.
  *
  * For isolating all pages in the range finally, the caller have to
  * free all pages in the range. test_page_isolated() can be used for
@@ -47,11 +45,11 @@ int move_freepages_block(struct zone *zone, struct page *page,
  */
 int
 start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
-			 unsigned migratetype, bool skip_hwpoisoned_pages);
+			 unsigned migratetype, bool skip_hwpoisoned_pages,
+			 bool enforce_migratetype);
 
 /*
- * Changes MIGRATE_ISOLATE to MIGRATE_MOVABLE.
- * target range is [start_pfn, end_pfn)
+ * Changes MIGRATE_ISOLATE to migratetype for range [start_pfn, end_pfn)
  */
 int
 undo_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index f74826cdceea..ebc1c8c330e2 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1601,7 +1601,7 @@ static int __ref __offline_pages(unsigned long start_pfn,
 
 	/* set above range as isolated */
 	ret = start_isolate_page_range(start_pfn, end_pfn,
-				       MIGRATE_MOVABLE, true);
+				       MIGRATE_MOVABLE, true, false);
 	if (ret)
 		return ret;
 
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 0fd5e8e2456e..cb1a5e0be6ee 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -7787,9 +7787,10 @@ static int __alloc_contig_migrate_range(struct compact_control *cc,
  * alloc_contig_range() -- tries to allocate given range of pages
  * @start:	start PFN to allocate
  * @end:	one-past-the-last PFN to allocate
- * @migratetype:	migratetype of the underlaying pageblocks (either
- *			#MIGRATE_MOVABLE or #MIGRATE_CMA).  All pageblocks
- *			in range must have the same migratetype and it must
+ * @migratetype:	migratetype of the underlaying pageblocks.  All
+ *			pageblocks in range must have the same migratetype.
+ *			migratetype is typically MIGRATE_MOVABLE or
+ *			MIGRATE_CMA, but this is not a requirement.
  *			be either of the two.
  * @gfp_mask:	GFP mask to use during compaction
  *
@@ -7840,15 +7841,15 @@ int alloc_contig_range(unsigned long start, unsigned long end,
 	 * allocator removing them from the buddy system.  This way
 	 * page allocator will never consider using them.
 	 *
-	 * This lets us mark the pageblocks back as
-	 * MIGRATE_CMA/MIGRATE_MOVABLE so that free pages in the
-	 * aligned range but not in the unaligned, original range are
-	 * put back to page allocator so that buddy can use them.
+	 * This lets us mark the pageblocks back as their original
+	 * migrate type so that free pages in the  aligned range but
+	 * not in the unaligned, original range are put back to page
+	 * allocator so that buddy can use them.
 	 */
 
 	ret = start_isolate_page_range(pfn_max_align_down(start),
 				       pfn_max_align_up(end), migratetype,
-				       false);
+				       false, true);
 	if (ret)
 		return ret;
 
diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index 43e085608846..472191cc1909 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -16,7 +16,8 @@
 #include <trace/events/page_isolation.h>
 
 static int set_migratetype_isolate(struct page *page, int migratetype,
-				bool skip_hwpoisoned_pages)
+				bool skip_hwpoisoned_pages,
+				bool enforce_migratetype)
 {
 	struct zone *zone;
 	unsigned long flags, pfn;
@@ -36,6 +37,17 @@ static int set_migratetype_isolate(struct page *page, int migratetype,
 	if (is_migrate_isolate_page(page))
 		goto out;
 
+	/*
+	 * If requested, check migration type of pageblock and make sure
+	 * it matches migratetype
+	 */
+	if (enforce_migratetype) {
+		if (get_pageblock_migratetype(page) != migratetype) {
+			ret = -EINVAL;
+			goto out;
+		}
+	}
+
 	pfn = page_to_pfn(page);
 	arg.start_pfn = pfn;
 	arg.nr_pages = pageblock_nr_pages;
@@ -167,14 +179,16 @@ __first_valid_page(unsigned long pfn, unsigned long nr_pages)
  * to be MIGRATE_ISOLATE.
  * @start_pfn: The lower PFN of the range to be isolated.
  * @end_pfn: The upper PFN of the range to be isolated.
- * @migratetype: migrate type to set in error recovery.
+ * @migratetype: migrate type of all blocks in range.
  *
  * Making page-allocation-type to be MIGRATE_ISOLATE means free pages in
  * the range will never be allocated. Any free pages and pages freed in the
  * future will not be allocated again.
  *
  * start_pfn/end_pfn must be aligned to pageblock_order.
- * Return 0 on success and -EBUSY if any part of range cannot be isolated.
+ * Return 0 on success or error returned by set_migratetype_isolate.  Typical
+ * errors are -EBUSY if any part of range cannot be isolated or -EINVAL if
+ * any page block is not of migratetype.
  *
  * There is no high level synchronization mechanism that prevents two threads
  * from trying to isolate overlapping ranges.  If this happens, one thread
@@ -185,11 +199,13 @@ __first_valid_page(unsigned long pfn, unsigned long nr_pages)
  * prevents two threads from simultaneously working on overlapping ranges.
  */
 int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
-			     unsigned migratetype, bool skip_hwpoisoned_pages)
+			     unsigned migratetype, bool skip_hwpoisoned_pages,
+			     bool enforce_migratetype)
 {
 	unsigned long pfn;
 	unsigned long undo_pfn;
 	struct page *page;
+	int ret = 0;
 
 	BUG_ON(!IS_ALIGNED(start_pfn, pageblock_nr_pages));
 	BUG_ON(!IS_ALIGNED(end_pfn, pageblock_nr_pages));
@@ -198,13 +214,17 @@ int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
 	     pfn < end_pfn;
 	     pfn += pageblock_nr_pages) {
 		page = __first_valid_page(pfn, pageblock_nr_pages);
-		if (page &&
-		    set_migratetype_isolate(page, migratetype, skip_hwpoisoned_pages)) {
-			undo_pfn = pfn;
-			goto undo;
+		if (page) {
+			ret = set_migratetype_isolate(page, migratetype,
+							skip_hwpoisoned_pages,
+							enforce_migratetype);
+			if (ret) {
+				undo_pfn = pfn;
+				goto undo;
+			}
 		}
 	}
-	return 0;
+	return ret;
 undo:
 	for (pfn = start_pfn;
 	     pfn < undo_pfn;
@@ -215,7 +235,7 @@ int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
 		unset_migratetype_isolate(page, migratetype);
 	}
 
-	return -EBUSY;
+	return ret;
 }
 
 /*
-- 
2.13.6

^ permalink raw reply related

* [PATCH v2 1/4] mm: change type of free_contig_range(nr_pages) to unsigned long
From: Mike Kravetz @ 2018-05-03 23:29 UTC (permalink / raw)
  To: linux-mm, linux-kernel, linux-api
  Cc: Reinette Chatre, Michal Hocko, Christopher Lameter, Guy Shattah,
	Anshuman Khandual, Michal Nazarewicz, Vlastimil Babka,
	David Nellans, Laura Abbott, Pavel Machek, Dave Hansen,
	Andrew Morton, Mike Kravetz
In-Reply-To: <20180503232935.22539-1-mike.kravetz@oracle.com>

free_contig_range() is currently defined as:
void free_contig_range(unsigned long pfn, unsigned nr_pages);
change to,
void free_contig_range(unsigned long pfn, unsigned long nr_pages);

Some callers are passing a truncated unsigned long today.  It is
highly unlikely that these values will overflow an unsigned int.
However, this should be changed to an unsigned long to be consistent
with other page counts.

Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
---
 include/linux/gfp.h | 2 +-
 mm/cma.c            | 2 +-
 mm/hugetlb.c        | 2 +-
 mm/page_alloc.c     | 6 +++---
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/linux/gfp.h b/include/linux/gfp.h
index 1a4582b44d32..86a0d06463ab 100644
--- a/include/linux/gfp.h
+++ b/include/linux/gfp.h
@@ -572,7 +572,7 @@ static inline bool pm_suspended_storage(void)
 /* The below functions must be run on a range from a single zone. */
 extern int alloc_contig_range(unsigned long start, unsigned long end,
 			      unsigned migratetype, gfp_t gfp_mask);
-extern void free_contig_range(unsigned long pfn, unsigned nr_pages);
+extern void free_contig_range(unsigned long pfn, unsigned long nr_pages);
 #endif
 
 #ifdef CONFIG_CMA
diff --git a/mm/cma.c b/mm/cma.c
index aa40e6c7b042..f473fc2b7cbd 100644
--- a/mm/cma.c
+++ b/mm/cma.c
@@ -563,7 +563,7 @@ bool cma_release(struct cma *cma, const struct page *pages, unsigned int count)
 
 	VM_BUG_ON(pfn + count > cma->base_pfn + cma->count);
 
-	free_contig_range(pfn, count);
+	free_contig_range(pfn, (unsigned long)count);
 	cma_clear_bitmap(cma, pfn, count);
 	trace_cma_release(pfn, pages, count);
 
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 218679138255..c81072ce7510 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -1055,7 +1055,7 @@ static void destroy_compound_gigantic_page(struct page *page,
 
 static void free_gigantic_page(struct page *page, unsigned int order)
 {
-	free_contig_range(page_to_pfn(page), 1 << order);
+	free_contig_range(page_to_pfn(page), 1UL << order);
 }
 
 static int __alloc_gigantic_page(unsigned long start_pfn,
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 905db9d7962f..0fd5e8e2456e 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -7937,9 +7937,9 @@ int alloc_contig_range(unsigned long start, unsigned long end,
 	return ret;
 }
 
-void free_contig_range(unsigned long pfn, unsigned nr_pages)
+void free_contig_range(unsigned long pfn, unsigned long nr_pages)
 {
-	unsigned int count = 0;
+	unsigned long count = 0;
 
 	for (; nr_pages--; pfn++) {
 		struct page *page = pfn_to_page(pfn);
@@ -7947,7 +7947,7 @@ void free_contig_range(unsigned long pfn, unsigned nr_pages)
 		count += page_count(page) != 1;
 		__free_page(page);
 	}
-	WARN(count != 0, "%d pages are still in use!\n", count);
+	WARN(count != 0, "%ld pages are still in use!\n", count);
 }
 #endif
 
-- 
2.13.6

^ permalink raw reply related

* [PATCH v2 0/4] Interface for higher order contiguous allocations
From: Mike Kravetz @ 2018-05-03 23:29 UTC (permalink / raw)
  To: linux-mm, linux-kernel, linux-api
  Cc: Reinette Chatre, Michal Hocko, Christopher Lameter, Guy Shattah,
	Anshuman Khandual, Michal Nazarewicz, Vlastimil Babka,
	David Nellans, Laura Abbott, Pavel Machek, Dave Hansen,
	Andrew Morton, Mike Kravetz

A respin of of the series to address these issues:

- fix issues found by kbuild
- find_alloc_contig_pages() should take nr_pages as argument instead of
  page order (Vlastimil and Michal).
- Cleaned up migratetype handling (Vlastimil and Michal).
- Use pfn_to_online_page instead of pfn_to_page as suggested by Michal.
  Also added comment about minimal number of conditions checked in
  contig_pfn_range_valid().
- When scanning pfns in zone, take pgdat_resize_lock() instead of
  zone->lock (Michal)

Also, 
- Separate patch to change type of free_contig_range(nr_pages) to an
  unsigned long so that it is consistent with other uses of nr_pages.
- Separate patch to optionally validate migratetype during pageblock
  isolation.
- Make find_alloc_contig_pages() work for smaller size allocation by
  simply calling __alloc_pages_nodemask().

Vlastimil and Michal brought up the issue of allocation alignment.  The
routine will currently align to 'nr_pages' (which is the requested size
argument).  It does this by examining and trying to allocate the first
nr_pages aligned/nr_pages sized range.  If this fails, it moves on to the
next nr_pages aligned/nr_pages sized range until success or all potential
ranges are exhausted.  If we allow an alignment to be specified, we will
need to potentially check all alignment aligned/nr_pages sized ranges.
In the worst case where alignment = PAGE_SIZE, this could result in huge
increase in the number of ranges to check.
To help cut down on the number of ranges to check, we could identify the
first page that causes a range allocation failure and start the next
range at the next aligned boundary.  I tried this, and we still end up
with a huge number of ranges and wasted CPU cycles.
This series did not add an alignment option.  Allocations are aligned to
nr_pages as described above.  If someone can thing of a good way to support
an alignment argument, I am open to implementing/adding it.

As described before,
These patches came out of the "[RFC] mmap(MAP_CONTIG)" discussions at:
http://lkml.kernel.org/r/21f1ec96-2822-1189-1c95-79a2bb491571@oracle.com

One suggestion in that thread was to create a friendlier interface that
could be used by drivers and others outside core mm code to allocate a
contiguous set of pages.  The alloc_contig_range() interface is used for
this purpose today by CMA and gigantic page allocation.  However, this is
not a general purpose interface.  So, wrap alloc_contig_range() in the
more general interface:

struct page *find_alloc_contig_pages(unsigned long nr_pages, gfp_t gfp,
					int nid, nodemask_t *nodemask)

This interface is essentially the same functionality provided by the
hugetlb specific routine alloc_gigantic_page().  After creating the
interface, change alloc_gigantic_page() to call find_alloc_contig_pages()
and delete all the supporting code in hugetlb.c.

A new use case for allocating contiguous memory has been identified in
Intel(R) Resource Director Technology Cache Pseudo-Locking.

Mike Kravetz (4):
  mm: change type of free_contig_range(nr_pages) to unsigned long
  mm: check for proper migrate type during isolation
  mm: add find_alloc_contig_pages() interface
  mm/hugetlb: use find_alloc_contig_pages() to allocate gigantic pages

 include/linux/gfp.h            |  14 +++-
 include/linux/page-isolation.h |   8 +--
 mm/cma.c                       |   2 +-
 mm/hugetlb.c                   |  87 ++--------------------
 mm/memory_hotplug.c            |   2 +-
 mm/page_alloc.c                | 159 +++++++++++++++++++++++++++++++++++++----
 mm/page_isolation.c            |  40 ++++++++---
 7 files changed, 200 insertions(+), 112 deletions(-)

-- 
2.13.6

^ permalink raw reply

* Re: [PATCH v2] fs: Add aio iopriority support for block_dev
From: Jens Axboe @ 2018-05-03 22:53 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Adam Manzanares, viro@zeniv.linux.org.uk, bcrl@kvack.org,
	linux-fsdevel@vger.kernel.org, linux-aio@kvack.org,
	linux-api@vger.kernel.org, linux-block@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <20180503224308.GA30823@bombadil.infradead.org>

On 5/3/18 4:43 PM, Matthew Wilcox wrote:
> On Thu, May 03, 2018 at 02:24:58PM -0600, Jens Axboe wrote:
>> On 5/3/18 2:15 PM, Adam Manzanares wrote:
>>> On 5/3/18 11:33 AM, Matthew Wilcox wrote:
>>>> Or we could just make ki_hint a u8 or u16 ... seems unlikely we'll need
>>>> 32 bits of ki_hint.  (currently defined values are 1-5)
>>>
>>> I like the approach of using a u16 for the ki_hint. I'll update and 
>>> resubmit.
>>
>> It's intended to be a mask. If you do shrink it for now, then we need some
>> guard code to ensure it can always carry what it needs to.
> 
> ummm ...
> 
> enum rw_hint {
>         WRITE_LIFE_NOT_SET      = 0,
>         WRITE_LIFE_NONE         = RWH_WRITE_LIFE_NONE,
>         WRITE_LIFE_SHORT        = RWH_WRITE_LIFE_SHORT,
> ...
> 
>                 .ki_hint = file_write_hint(filp),
> 
> static inline enum rw_hint file_write_hint(struct file *file)
> 
> #define RWF_WRITE_LIFE_NOT_SET  0
> #define RWH_WRITE_LIFE_NONE     1
> #define RWH_WRITE_LIFE_SHORT    2
> #define RWH_WRITE_LIFE_MEDIUM   3
> #define RWH_WRITE_LIFE_LONG     4
> #define RWH_WRITE_LIFE_EXTREME  5
> 
> It doesn't look like it's being used as a mask.

Right, currently it only supports the life time hint. I'm saying the
intent is, for when it's being expanded to cover other hints, is to
split up the value into maskable sections.

-- 
Jens Axboe

--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org.  For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>

^ permalink raw reply

* Re: [PATCH v2] fs: Add aio iopriority support for block_dev
From: Matthew Wilcox @ 2018-05-03 22:43 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Adam Manzanares, viro@zeniv.linux.org.uk, bcrl@kvack.org,
	linux-fsdevel@vger.kernel.org, linux-aio@kvack.org,
	linux-api@vger.kernel.org, linux-block@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <18300bdb-a12f-0b6c-1317-6db3e4391d57@kernel.dk>

On Thu, May 03, 2018 at 02:24:58PM -0600, Jens Axboe wrote:
> On 5/3/18 2:15 PM, Adam Manzanares wrote:
> > On 5/3/18 11:33 AM, Matthew Wilcox wrote:
> >> Or we could just make ki_hint a u8 or u16 ... seems unlikely we'll need
> >> 32 bits of ki_hint.  (currently defined values are 1-5)
> > 
> > I like the approach of using a u16 for the ki_hint. I'll update and 
> > resubmit.
> 
> It's intended to be a mask. If you do shrink it for now, then we need some
> guard code to ensure it can always carry what it needs to.

ummm ...

enum rw_hint {
        WRITE_LIFE_NOT_SET      = 0,
        WRITE_LIFE_NONE         = RWH_WRITE_LIFE_NONE,
        WRITE_LIFE_SHORT        = RWH_WRITE_LIFE_SHORT,
...

                .ki_hint = file_write_hint(filp),

static inline enum rw_hint file_write_hint(struct file *file)

#define RWF_WRITE_LIFE_NOT_SET  0
#define RWH_WRITE_LIFE_NONE     1
#define RWH_WRITE_LIFE_SHORT    2
#define RWH_WRITE_LIFE_MEDIUM   3
#define RWH_WRITE_LIFE_LONG     4
#define RWH_WRITE_LIFE_EXTREME  5

It doesn't look like it's being used as a mask.

--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org.  For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>

^ permalink raw reply

* Re: [RFC PATCH] Add /proc/<pid>/numa_vamaps for numa node information
From: prakash.sangappa @ 2018-05-03 22:39 UTC (permalink / raw)
  To: Christopher Lameter
  Cc: linux-kernel, linux-mm, linux-api, akpm, mhocko, kirill.shutemov,
	n-horiguchi, drepper, rientjes
In-Reply-To: <alpine.DEB.2.21.1805031259210.7831@nuc-kabylake>



On 05/03/2018 11:03 AM, Christopher Lameter wrote:
> On Tue, 1 May 2018, Prakash Sangappa wrote:
>
>> For analysis purpose it is useful to have numa node information
>> corresponding mapped address ranges of the process. Currently
>> /proc/<pid>/numa_maps provides list of numa nodes from where pages are
>> allocated per VMA of the process. This is not useful if an user needs to
>> determine which numa node the mapped pages are allocated from for a
>> particular address range. It would have helped if the numa node information
>> presented in /proc/<pid>/numa_maps was broken down by VA ranges showing the
>> exact numa node from where the pages have been allocated.
> Cant you write a small script that scans the information in numa_maps and
> then displays the total pages per NUMA node and then a list of which
> ranges have how many pages on a particular node?

Don't think we can determine which numa node a given user process
address range has pages from, based on the existing 'numa_maps' file.

>> reading this file will not be restricted(i.e requiring CAP_SYS_ADMIN).
> So a prime motivator here is security restricted access to numa_maps?
No it is the opposite. A regular user should be able to determine
numa node information.

^ permalink raw reply

* Re: [RFC PATCH] Add /proc/<pid>/numa_vamaps for numa node information
From: prakash.sangappa @ 2018-05-03 22:37 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Andrew Morton, linux-kernel, linux-mm, linux-api, kirill.shutemov,
	n-horiguchi, drepper, rientjes, Naoya Horiguchi, Dave Hansen
In-Reply-To: <20180503085741.GD4535@dhcp22.suse.cz>



On 05/03/2018 01:57 AM, Michal Hocko wrote:
> On Wed 02-05-18 16:43:58, prakash.sangappa wrote:
>>
>> On 05/02/2018 02:33 PM, Andrew Morton wrote:
>>> On Tue,  1 May 2018 22:58:06 -0700 Prakash Sangappa <prakash.sangappa@oracle.com> wrote:
>>>
>>>> For analysis purpose it is useful to have numa node information
>>>> corresponding mapped address ranges of the process. Currently
>>>> /proc/<pid>/numa_maps provides list of numa nodes from where pages are
>>>> allocated per VMA of the process. This is not useful if an user needs to
>>>> determine which numa node the mapped pages are allocated from for a
>>>> particular address range. It would have helped if the numa node information
>>>> presented in /proc/<pid>/numa_maps was broken down by VA ranges showing the
>>>> exact numa node from where the pages have been allocated.
>>>>
>>>> The format of /proc/<pid>/numa_maps file content is dependent on
>>>> /proc/<pid>/maps file content as mentioned in the manpage. i.e one line
>>>> entry for every VMA corresponding to entries in /proc/<pids>/maps file.
>>>> Therefore changing the output of /proc/<pid>/numa_maps may not be possible.
>>>>
>>>> Hence, this patch proposes adding file /proc/<pid>/numa_vamaps which will
>>>> provide proper break down of VA ranges by numa node id from where the mapped
>>>> pages are allocated. For Address ranges not having any pages mapped, a '-'
>>>> is printed instead of numa node id. In addition, this file will include most
>>>> of the other information currently presented in /proc/<pid>/numa_maps. The
>>>> additional information included is for convenience. If this is not
>>>> preferred, the patch could be modified to just provide VA range to numa node
>>>> information as the rest of the information is already available thru
>>>> /proc/<pid>/numa_maps file.
>>>>
>>>> Since the VA range to numa node information does not include page's PFN,
>>>> reading this file will not be restricted(i.e requiring CAP_SYS_ADMIN).
>>>>
>>>> Here is the snippet from the new file content showing the format.
>>>>
>>>> 00400000-00401000 N0=1 kernelpagesize_kB=4 mapped=1 file=/tmp/hmap2
>>>> 00600000-00601000 N0=1 kernelpagesize_kB=4 anon=1 dirty=1 file=/tmp/hmap2
>>>> 00601000-00602000 N0=1 kernelpagesize_kB=4 anon=1 dirty=1 file=/tmp/hmap2
>>>> 7f0215600000-7f0215800000 N0=1 kernelpagesize_kB=2048 dirty=1 file=/mnt/f1
>>>> 7f0215800000-7f0215c00000 -  file=/mnt/f1
>>>> 7f0215c00000-7f0215e00000 N0=1 kernelpagesize_kB=2048 dirty=1 file=/mnt/f1
>>>> 7f0215e00000-7f0216200000 -  file=/mnt/f1
>>>> ..
>>>> 7f0217ecb000-7f0217f20000 N0=85 kernelpagesize_kB=4 mapped=85 mapmax=51
>>>>      file=/usr/lib64/libc-2.17.so
>>>> 7f0217f20000-7f0217f30000 -  file=/usr/lib64/libc-2.17.so
>>>> 7f0217f30000-7f0217f90000 N0=96 kernelpagesize_kB=4 mapped=96 mapmax=51
>>>>      file=/usr/lib64/libc-2.17.so
>>>> 7f0217f90000-7f0217fb0000 -  file=/usr/lib64/libc-2.17.so
>>>> ..
>>>>
>>>> The 'pmap' command can be enhanced to include an option to show numa node
>>>> information which it can read from this new proc file. This will be a
>>>> follow on proposal.
>>> I'd like to hear rather more about the use-cases for this new
>>> interface.  Why do people need it, what is the end-user benefit, etc?
>> This is mainly for debugging / performance analysis. Oracle Database
>> team is looking to use this information.
> But we do have an interface to query (e.g. move_pages) that your
> application can use. I am really worried that the broken out per node
> data can be really large (just take a large vma with interleaved policy
> as an example). So is this really worth adding as a general purpose proc
> interface?

I guess move_pages could be useful. There needs to be a tool or
command which can read the numa node information using move_pages
to be used to observe another process.

 From an observability point of view, one of the use of the proposed
new file 'numa_vamaps' was to modify  'pmap' command to display numa
node information broken down by address ranges.  Would having pmap
show numa node information be useful?

^ permalink raw reply

* Re: [RFC PATCH] Add /proc/<pid>/numa_vamaps for numa node information
From: prakash.sangappa @ 2018-05-03 22:27 UTC (permalink / raw)
  To: Anshuman Khandual, Dave Hansen, Andrew Morton
  Cc: linux-kernel, linux-mm, linux-api, mhocko, kirill.shutemov,
	n-horiguchi, drepper, rientjes, Naoya Horiguchi
In-Reply-To: <33f96879-351f-674a-ca23-43f233f4eb1d@linux.vnet.ibm.com>



On 05/03/2018 01:46 AM, Anshuman Khandual wrote:
> On 05/03/2018 03:58 AM, Dave Hansen wrote:
>> On 05/02/2018 02:33 PM, Andrew Morton wrote:
>>> On Tue,  1 May 2018 22:58:06 -0700 Prakash Sangappa <prakash.sangappa@oracle.com> wrote:
>>>> For analysis purpose it is useful to have numa node information
>>>> corresponding mapped address ranges of the process. Currently
>>>> /proc/<pid>/numa_maps provides list of numa nodes from where pages are
>>>> allocated per VMA of the process. This is not useful if an user needs to
>>>> determine which numa node the mapped pages are allocated from for a
>>>> particular address range. It would have helped if the numa node information
>>>> presented in /proc/<pid>/numa_maps was broken down by VA ranges showing the
>>>> exact numa node from where the pages have been allocated.
>> I'm finding myself a little lost in figuring out what this does.  Today,
>> numa_maps might us that a 3-page VMA has 1 page from Node 0 and 2 pages
>> from Node 1.  We group *entirely* by VMA:
>>
>> 1000-4000 N0=1 N1=2
>>
>> We don't want that.  We want to tell exactly where each node's memory is
>> despite if they are in the same VMA, like this:
>>
>> 1000-2000 N1=1
>> 2000-3000 N0=1
>> 3000-4000 N1=1
> I am kind of wondering on a big memory system how many lines of output
> we might have for a large (consuming lets say 80 % of system RAM) VMA
> in interleave policy. Is not that a problem ?
>
If each consecutive page comes from different node, yes in
the extreme case is this file will have a lot of lines. All the lines
are generated at the time file is read. The amount of data read will be
limited to the user read buffer size used in the read.

/proc/<pid>/pagemap also has kind of  similar issue. There is 1 64 bit 
value
for each user page.

^ permalink raw reply

* Re: [RFC PATCH] Add /proc/<pid>/numa_vamaps for numa node information
From: Dave Hansen @ 2018-05-03 22:26 UTC (permalink / raw)
  To: prakash.sangappa, Anshuman Khandual, Andrew Morton
  Cc: linux-kernel, linux-mm, linux-api, mhocko, kirill.shutemov,
	n-horiguchi, drepper, rientjes, Naoya Horiguchi
In-Reply-To: <82d2b35c-272a-ad02-692f-2c109aacdfb6@oracle.com>

On 05/03/2018 03:27 PM, prakash.sangappa wrote:
>>
> If each consecutive page comes from different node, yes in
> the extreme case is this file will have a lot of lines. All the lines
> are generated at the time file is read. The amount of data read will be
> limited to the user read buffer size used in the read.
> 
> /proc/<pid>/pagemap also has kind of  similar issue. There is 1 64
> bit value for each user page.
But nobody reads it sequentially.  Everybody lseek()s because it has a
fixed block size.  You can't do that in text.

^ permalink raw reply

* Re: [PATCH v2] fs: Add aio iopriority support for block_dev
From: Jens Axboe @ 2018-05-03 21:20 UTC (permalink / raw)
  To: Adam Manzanares, Matthew Wilcox
  Cc: viro@zeniv.linux.org.uk, bcrl@kvack.org,
	linux-fsdevel@vger.kernel.org, linux-aio@kvack.org,
	linux-api@vger.kernel.org, linux-block@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <e81c74fd-a4a2-80e0-476c-6ff35f158abf@wdc.com>

On 5/3/18 2:58 PM, Adam Manzanares wrote:
> 
> 
> On 5/3/18 1:24 PM, Jens Axboe wrote:
>> On 5/3/18 2:15 PM, Adam Manzanares wrote:
>>>
>>>
>>> On 5/3/18 11:33 AM, Matthew Wilcox wrote:
>>>> On Thu, May 03, 2018 at 11:21:14AM -0700, adam.manzanares@wdc.com wrote:
>>>>> If we want to avoid bloating struct kiocb, I suggest we turn the private field
>>>>> into a union of the private and ki_ioprio field. It seems like the users of
>>>>> the private field all use it at a point where we can yank the priority from
>>>>> the kiocb before the private field is used. Comments and suggestions welcome.
>>>>
>>>> Or we could just make ki_hint a u8 or u16 ... seems unlikely we'll need
>>>> 32 bits of ki_hint.  (currently defined values are 1-5)
>>>
>>> I like the approach of using a u16 for the ki_hint. I'll update and
>>> resubmit.
>>
>> It's intended to be a mask. If you do shrink it for now, then we need some
>> guard code to ensure it can always carry what it needs to.
>>
> 
> Got it, I'll add the guard to rw_hint_valid along with a comment about 
> being limited by the size of ki_hint in case we get to a situation where 
> 16 bits is not enough.

Other way around - the API should not be limited by the fact that some
smaller type was chosen for an internal structure. Hence the guard/check
should not be in rw_hint_valid, but rather around where you assign
ki_hint. If/when we do extend the read/write hints, then we'll
potentially need to bump ki_hint to accommodate it.

-- 
Jens Axboe

^ permalink raw reply

* Re: [PATCH v5 00/28] FPGA Device Feature List (DFL) Device Drivers
From: Alan Tull @ 2018-05-03 21:14 UTC (permalink / raw)
  To: Wu Hao
  Cc: Moritz Fischer, linux-fpga, linux-kernel, linux-api, Kang, Luwei,
	Zhang, Yi Z
In-Reply-To: <1525229431-3087-1-git-send-email-hao.wu@intel.com>

On Tue, May 1, 2018 at 9:50 PM, Wu Hao <hao.wu@intel.com> wrote:
> Hi All,
>
> Here is v5 patch-series adding drivers for FPGA DFL devices.
>
> This patch series provides a common framework to support FPGA Device Feature
> List (DFL), and also feature dev drivers under this DFL framework to provide
> interfaces for userspace applications to configure, enumerate, open, and
> access FPGA accelerators on DFL based FPGA device and enables system level
> management functions such as FPGA partial reconfiguration, power management
> and virtualization.

Hi Hao,

I've started looking at your v5 here.  It's 5212 lines of code, so
it's not tiny.

I see a lot of renaming from 'fpga' to 'dfl', thanks for doing that.

The main thing that stands out at first look is a new method of
registering port ops.  It would be better if the fpga bridge framework
were expanded or changed somehow to do that.  It's like we have two
bridge frameworks now.

I see that there isn't a way of specifying which FPGA mgr driver to
use, so dfl-fme-mgr.c is tied to dfl-fme-pr.c.  It would be better if
the DFL could specify which fpga manager driver to use.

Maybe I'm repeating myself here, the biggest question I'm asking
myself is: If someone had a different FPGA or a different static
region, how much of this code can be reused?  I would hope that all of
it could except for the fpga manager/bridge drivers.  That was the
intent of the original FPGA framework.  I'll have more time next week
to look in more depth.

Alan

>
> This patch series only adds the basic functions for FPGA accelerators and
> partial reconfiguration. Patches for more functions, e.g power management
> and virtualization, will be submitted after this series gets reviewed.
>
> Note this patch series is only verified on DFL based Intel(R) FPGA PCIe
> devices (e.g Intel Server Platform with In-package FPGA and Intel FPGA PCIe
> Acceleration Cards).
>
> Patch 1: add a document for FPGA DFL framework driver overview, including
> Device Feature List (DFL) introduction, the HW architecture, driver
> organization, device enumeration and opens.
>
> Patch 2: add region_id for fpga_image_info data structure, which allows
> driver to pass region id information to fpga-mgr for FPGA reconfiguration
> function. (Used by Patch 17)
>
> Patch 3: add a 'status' sysfs interface to fpga-mgr class, it reflects
> the status of the fpga-mgr including reconfiguration errors. (Used by
> Patch 18)
>
> Patch 4-5: add compat_id support in fpga manager and region, this compat
> id is used for compatibility check before further actions (e.g partial
> reconfiguration). (Used by Patch 19 and 22)
>
> Patch 6-11: add FPGA device feature list support, it provides common
> enumeration interfaces which creates container device (FPGA base region)
> and all feature devices by walking through all the 'Device Feature Lists'
> provided low level drivers. A global list is added to DFL framework to
> manage port ops from different ports.
>
> Patch 12-13: implement FPGA PCIe device driver. It locates all 'Device
> Feature Lists' in PCIe device memory and invokes common interfaces from
> above device feature list framework to finish the enumeration.
>
> Patch 14-17: implement FPGA Management Engine (FME) driver. It's a
> platform driver matching with the FME platform device created by above
> device feature list framework during enumeration. Sysfs and device file
> ioctls are exposed as user interfaces to allow partial reconfiguration
> to Accelerated Function Units (AFUs) from user space applications.
>
> Patch 18-22: implement FPGA manager/bridge/region platform drivers for
> Intel FPGA Management Engine (FME). These platform drivers match with
> platform devices created by above FME driver, they use the generic
> fpga-mgr/bridge/region class infrastructure to implement FPGA partial
> reconfiguration function.
>
> Patch 23-28: implement FPGA Accelerated Function Unit (AFU) driver.
> It's a platform driver matching with AFU platform device created by above
> device feature list framework during enumeration. It provides user
> interfaces to expose the AFU MMIO region, map/unmap dma buffer, and
> control the port which AFU connects to.
>
> Changes from v4:
> - Update the dfl.txt documentation, remove descriptions for the APIs and
>   features which are not implemented in this patch series.
> - Add DFL_ / dfl_ prefix for APIs and data structure, to avoid directly
>   using DFL_ / fpga_xxx naming.
> - Use "static region" and "PR bistream" instead of "blue bitstream" and
>   "green bistream" terminology in description to avoid misunderstanding.
> - Fix building issues caused by BIT() on 64bit register definition and
>   missing correct header file for readq and writeq.
> - Remove port specific code in DFL framework, and introduce port ops support
>   to resolve the dependency issue between FME driver module and Port driver
>   module. (more details in Patch 10).
> - Add compat id to fpga manager, as in case some hardware implements the
>   compat id in fpga manager's register, not register belongs to fpga region,
>   and its value is shared by all related fpga regions.
> - Pass mapped ioaddr to fme manager platform device from dfl-fme-pr via pdata.
> - Move PR hardware register definition to dfl-fme-mgr.c.
> - Fix other comments from Alan and Moritz, including description improvement,
>   coding style issue and etc.
>
> Changes from v3:
> - Fix SPDX license issue.
> - Rename documentation to dfl.txt, add introduction for Device Feature List
>   (DFL) and re-organize the content.
> - Rename to FPGA Device Feature List (DFL) drivers from Intel FPGA device
>   drivers for better reuse purposes. Unified driver and files to dfl-*.*
> - Remove static feature_info table from common enumeration code, and switch
>   to use feature id for sub feature driver matching.
> - Remove next_afu register checking for AFU from common enumeration code.
> - Remove interface_id sysfs for dfl-fme-mgr and use per fpga-region
>   compat_id instead. (new patch 13, 15, 19).
> - Add more comments for driver data structures and functions.
> - Fix typos, issues in debug message/commit message and other places.
>
> Changes from v2:
> - Split common enumeration code from pcie driver to a separated module
>   which for device feature list support.
> - Drop fpga-dev class and switch to use fpga base region as container.
> - Update the intel-fpga.txt documentation for new driver organization.
> - Rename feature device drivers for future code reuse.
> - Rebase code due to fpga APIs changes
> - replace bitfields with marco and shift.
> - fix typos, checkpatch issue and other comments.
>
> Changes from v1:
> - Use GPLv2 license instead of Dual BSD/GPL.
> - Move the code to drivers/fpga folder.
> - Update the intel-fpga.txt documentation for new driver organization.
> - Add documentation for new sysfs interfaces.
> - Switch to use common fpga-region interface for partial reconfiguration
>   (PR) function in FME. It creates fpga-region/fpga-mgr/fpga-bridge
>   platform devices and leave the implementation to their platform drivers.
> - Add platform drivers for FME fpga-mgr/bridge/region platform devices.
> - Fix kbuild warnings, typos and other comments.
>
> This patch series depends on the below patchset from Alan Tull.
> [PATCH v4 0/4] fpga: change api, don't use drive data[1]
>
> [1] https://marc.info/?l=linux-fpga&m=152356504900466&w=2
>
> Kang Luwei (3):
>   fpga: dfl: add FPGA Management Engine driver basic framework
>   fpga: dfl: fme: add header sub feature support
>   fpga: dfl: fme: add partial reconfiguration sub feature support
>
> Wu Hao (22):
>   docs: fpga: add a document for FPGA Device Feature List (DFL)
>     Framework Overview
>   fpga: mgr: add region_id to fpga_image_info
>   fpga: mgr: add status for fpga-manager
>   fpga: mgr: add compat_id support
>   fpga: region: add compat_id support
>   fpga: add device feature list support
>   fpga: dfl: add chardev support for feature devices
>   fpga: dfl: add dfl_fpga_cdev_find_port
>   fpga: dfl: add dfl_fpga_port_ops support.
>   fpga: dfl: add dfl_fpga_check_port_id function.
>   fpga: dfl-pci: add enumeration for feature devices
>   fpga: dfl: fme: add DFL_FPGA_GET_API_VERSION/CHECK_EXTENSION ioctls
>     support
>   fpga: dfl: add fpga manager platform driver for FME
>   fpga: dfl: fme-mgr: add compat_id support
>   fpga: dfl: add fpga bridge platform driver for FME
>   fpga: dfl: add fpga region platform driver for FME
>   fpga: dfl: fme-region: add support for compat_id
>   fpga: dfl: add FPGA Accelerated Function Unit driver basic framework
>   fpga: dfl: afu: add port ops support
>   fpga: dfl: afu: add header sub feature support
>   fpga: dfl: afu: add DFL_FPGA_GET_API_VERSION/CHECK_EXTENSION ioctls
>     support
>   fpga: dfl: afu: add DFL_FPGA_PORT_DMA_MAP/UNMAP ioctls support
>
> Xiao Guangrong (2):
>   fpga: dfl: add feature device infrastructure
>   fpga: dfl: afu: add afu sub feature support
>
> Zhang Yi (1):
>   fpga: add FPGA DFL PCIe device driver
>
>  Documentation/ABI/testing/sysfs-class-fpga-manager |   24 +
>  Documentation/ABI/testing/sysfs-class-fpga-region  |    9 +
>  Documentation/ABI/testing/sysfs-platform-dfl-fme   |   23 +
>  Documentation/ABI/testing/sysfs-platform-dfl-port  |   16 +
>  Documentation/fpga/dfl.txt                         |  285 ++++++
>  Documentation/ioctl/ioctl-number.txt               |    1 +
>  drivers/fpga/Kconfig                               |   68 ++
>  drivers/fpga/Makefile                              |   14 +
>  drivers/fpga/dfl-afu-dma-region.c                  |  463 +++++++++
>  drivers/fpga/dfl-afu-main.c                        |  636 +++++++++++++
>  drivers/fpga/dfl-afu-region.c                      |  166 ++++
>  drivers/fpga/dfl-afu.h                             |  100 ++
>  drivers/fpga/dfl-fme-br.c                          |  114 +++
>  drivers/fpga/dfl-fme-main.c                        |  279 ++++++
>  drivers/fpga/dfl-fme-mgr.c                         |  349 +++++++
>  drivers/fpga/dfl-fme-pr.c                          |  474 ++++++++++
>  drivers/fpga/dfl-fme-pr.h                          |   84 ++
>  drivers/fpga/dfl-fme-region.c                      |   89 ++
>  drivers/fpga/dfl-fme.h                             |   38 +
>  drivers/fpga/dfl-pci.c                             |  310 ++++++
>  drivers/fpga/dfl.c                                 | 1000 ++++++++++++++++++++
>  drivers/fpga/dfl.h                                 |  416 ++++++++
>  drivers/fpga/fpga-mgr.c                            |   28 +
>  drivers/fpga/fpga-region.c                         |   22 +
>  include/linux/fpga/fpga-mgr.h                      |   24 +
>  include/linux/fpga/fpga-region.h                   |    2 +
>  include/uapi/linux/fpga-dfl.h                      |  178 ++++
>  27 files changed, 5212 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-class-fpga-region
>  create mode 100644 Documentation/ABI/testing/sysfs-platform-dfl-fme
>  create mode 100644 Documentation/ABI/testing/sysfs-platform-dfl-port
>  create mode 100644 Documentation/fpga/dfl.txt
>  create mode 100644 drivers/fpga/dfl-afu-dma-region.c
>  create mode 100644 drivers/fpga/dfl-afu-main.c
>  create mode 100644 drivers/fpga/dfl-afu-region.c
>  create mode 100644 drivers/fpga/dfl-afu.h
>  create mode 100644 drivers/fpga/dfl-fme-br.c
>  create mode 100644 drivers/fpga/dfl-fme-main.c
>  create mode 100644 drivers/fpga/dfl-fme-mgr.c
>  create mode 100644 drivers/fpga/dfl-fme-pr.c
>  create mode 100644 drivers/fpga/dfl-fme-pr.h
>  create mode 100644 drivers/fpga/dfl-fme-region.c
>  create mode 100644 drivers/fpga/dfl-fme.h
>  create mode 100644 drivers/fpga/dfl-pci.c
>  create mode 100644 drivers/fpga/dfl.c
>  create mode 100644 drivers/fpga/dfl.h
>  create mode 100644 include/uapi/linux/fpga-dfl.h
>
> --
> 1.8.3.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v2] fs: Add aio iopriority support for block_dev
From: Adam Manzanares @ 2018-05-03 20:58 UTC (permalink / raw)
  To: Jens Axboe, Matthew Wilcox
  Cc: viro@zeniv.linux.org.uk, bcrl@kvack.org,
	linux-fsdevel@vger.kernel.org, linux-aio@kvack.org,
	linux-api@vger.kernel.org, linux-block@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <18300bdb-a12f-0b6c-1317-6db3e4391d57@kernel.dk>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1209 bytes --]



On 5/3/18 1:24 PM, Jens Axboe wrote:
> On 5/3/18 2:15 PM, Adam Manzanares wrote:
>>
>>
>> On 5/3/18 11:33 AM, Matthew Wilcox wrote:
>>> On Thu, May 03, 2018 at 11:21:14AM -0700, adam.manzanares@wdc.com wrote:
>>>> If we want to avoid bloating struct kiocb, I suggest we turn the private field
>>>> into a union of the private and ki_ioprio field. It seems like the users of
>>>> the private field all use it at a point where we can yank the priority from
>>>> the kiocb before the private field is used. Comments and suggestions welcome.
>>>
>>> Or we could just make ki_hint a u8 or u16 ... seems unlikely we'll need
>>> 32 bits of ki_hint.  (currently defined values are 1-5)
>>
>> I like the approach of using a u16 for the ki_hint. I'll update and
>> resubmit.
> 
> It's intended to be a mask. If you do shrink it for now, then we need some
> guard code to ensure it can always carry what it needs to.
> 

Got it, I'll add the guard to rw_hint_valid along with a comment about 
being limited by the size of ki_hint in case we get to a situation where 
16 bits is not enough.
N‹§²æìr¸›zǧu©ž²Æ {\b­†éì¹»\x1c®&Þ–)îŨ¨Š{ayº\x1dÊÚ&j:+v‰¨’öœ’Šà\x16Šæ¢·¢ú(œ¸§»\x10\b:Çž†Ûiÿü0ÂKÚrJ+ƒö¢£ðèž×¦j)Z†·Ÿ

^ permalink raw reply

* Re: [PATCH v2] fs: Add aio iopriority support for block_dev
From: Jens Axboe @ 2018-05-03 20:24 UTC (permalink / raw)
  To: Adam Manzanares, Matthew Wilcox
  Cc: viro@zeniv.linux.org.uk, bcrl@kvack.org,
	linux-fsdevel@vger.kernel.org, linux-aio@kvack.org,
	linux-api@vger.kernel.org, linux-block@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <47e0a519-37b4-f5e7-0616-8659d11c2b69@wdc.com>

On 5/3/18 2:15 PM, Adam Manzanares wrote:
> 
> 
> On 5/3/18 11:33 AM, Matthew Wilcox wrote:
>> On Thu, May 03, 2018 at 11:21:14AM -0700, adam.manzanares@wdc.com wrote:
>>> If we want to avoid bloating struct kiocb, I suggest we turn the private field
>>> into a union of the private and ki_ioprio field. It seems like the users of
>>> the private field all use it at a point where we can yank the priority from
>>> the kiocb before the private field is used. Comments and suggestions welcome.
>>
>> Or we could just make ki_hint a u8 or u16 ... seems unlikely we'll need
>> 32 bits of ki_hint.  (currently defined values are 1-5)
> 
> I like the approach of using a u16 for the ki_hint. I'll update and 
> resubmit.

It's intended to be a mask. If you do shrink it for now, then we need some
guard code to ensure it can always carry what it needs to.

-- 
Jens Axboe

--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org.  For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>

^ permalink raw reply

* Re: [PATCH v2] fs: Add aio iopriority support for block_dev
From: Adam Manzanares @ 2018-05-03 20:24 UTC (permalink / raw)
  To: Jeff Moyer
  Cc: viro@zeniv.linux.org.uk, bcrl@kvack.org,
	linux-fsdevel@vger.kernel.org, linux-aio@kvack.org,
	linux-api@vger.kernel.org, linux-block@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <x49po2cr3hc.fsf@segfault.boston.devel.redhat.com>



On 5/3/18 11:36 AM, Jeff Moyer wrote:
> Hi, Adam,

Hello Jeff,

> 
> adam.manzanares@wdc.com writes:
> 
>> From: Adam Manzanares <adam.manzanares@wdc.com>
>>
>> This is the per-I/O equivalent of the ioprio_set system call.
>>
>> When IOCB_FLAG_IOPRIO is set on the iocb aio_flags field, then we set the
>> newly added kiocb ki_ioprio field to the value in the iocb aio_reqprio field.
>>
>> When a bio is created for an aio request by the block dev we set the priority
>> value of the bio to the user supplied value.
>>
>> See the following link for performance implications on a SATA HDD:
>> https://lkml.org/lkml/2016/12/6/495
>>
>> Given that WRR support for NVME devices has patches floating around and it was
>> discussed at LSFMM, we may soon have a lower latency storage device that can
>> make use of iopriorities. A per command iopriority interface seems timely
>> given these developments.
>>
>> If we want to avoid bloating struct kiocb, I suggest we turn the private field
>> into a union of the private and ki_ioprio field. It seems like the users of
>> the private field all use it at a point where we can yank the priority from
>> the kiocb before the private field is used. Comments and suggestions welcome.
> 
> The ioprio_set system call requires CAP_SYS_ADMIN for setting
> IOPRIO_CLASS_RT.  I think we need similar checks here.

I forgot how dangerous IOPRIO_CLASS_RT can be :). I will make a new 
function based on the checks in ioprio.c and reuse.

Thanks,
Adam

> 
> -Jeff
> 
>>
>> v2: merge patches
>>      use IOCB_FLAG_IOPRIO
>>      validate intended use with IOCB_IOPRIO
>>      add linux-api and linux-block to cc
>>
>> Signed-off-by: Adam Manzanares <adam.manzanares@wdc.com>
>> ---
>>   fs/aio.c                     | 10 ++++++++++
>>   fs/block_dev.c               |  2 ++
>>   include/linux/fs.h           |  2 ++
>>   include/uapi/linux/aio_abi.h |  1 +
>>   4 files changed, 15 insertions(+)
>>
>> diff --git a/fs/aio.c b/fs/aio.c
>> index 88d7927ffbc6..f36636d8ff2c 100644
>> --- a/fs/aio.c
>> +++ b/fs/aio.c
>> @@ -1597,6 +1597,16 @@ static int io_submit_one(struct kioctx *ctx, struct iocb __user *user_iocb,
>>   		req->common.ki_flags |= IOCB_EVENTFD;
>>   	}
>>   
>> +	if (iocb->aio_flags & IOCB_FLAG_IOPRIO) {
>> +		/*
>> +		 * If the IOCB_FLAG_IOPRIO flag of aio_flags is set, then
>> +		 * aio_reqprio is interpreted as an I/O scheduling
>> +		 * class and priority.
>> +		 */
>> +		req->common.ki_ioprio = iocb->aio_reqprio;
>> +		req->common.ki_flags |= IOCB_IOPRIO;
>> +	}
>> +
>>   	ret = kiocb_set_rw_flags(&req->common, iocb->aio_rw_flags);
>>   	if (unlikely(ret)) {
>>   		pr_debug("EINVAL: aio_rw_flags\n");
>> diff --git a/fs/block_dev.c b/fs/block_dev.c
>> index 7ec920e27065..970bef79caa6 100644
>> --- a/fs/block_dev.c
>> +++ b/fs/block_dev.c
>> @@ -355,6 +355,8 @@ __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter, int nr_pages)
>>   		bio->bi_write_hint = iocb->ki_hint;
>>   		bio->bi_private = dio;
>>   		bio->bi_end_io = blkdev_bio_end_io;
>> +		if (iocb->ki_flags & IOCB_IOPRIO)
>> +			bio->bi_ioprio = iocb->ki_ioprio;
>>   
>>   		ret = bio_iov_iter_get_pages(bio, iter);
>>   		if (unlikely(ret)) {
>> diff --git a/include/linux/fs.h b/include/linux/fs.h
>> index 760d8da1b6c7..ab63ce720305 100644
>> --- a/include/linux/fs.h
>> +++ b/include/linux/fs.h
>> @@ -292,6 +292,7 @@ enum rw_hint {
>>   #define IOCB_SYNC		(1 << 5)
>>   #define IOCB_WRITE		(1 << 6)
>>   #define IOCB_NOWAIT		(1 << 7)
>> +#define IOCB_IOPRIO		(1 << 8)
>>   
>>   struct kiocb {
>>   	struct file		*ki_filp;
>> @@ -300,6 +301,7 @@ struct kiocb {
>>   	void			*private;
>>   	int			ki_flags;
>>   	enum rw_hint		ki_hint;
>> +	u16			ki_ioprio; /* See linux/ioprio.h */
>>   } __randomize_layout;
>>   
>>   static inline bool is_sync_kiocb(struct kiocb *kiocb)
>> diff --git a/include/uapi/linux/aio_abi.h b/include/uapi/linux/aio_abi.h
>> index a04adbc70ddf..d4593a6062ef 100644
>> --- a/include/uapi/linux/aio_abi.h
>> +++ b/include/uapi/linux/aio_abi.h
>> @@ -54,6 +54,7 @@ enum {
>>    *                   is valid.
>>    */
>>   #define IOCB_FLAG_RESFD		(1 << 0)
>> +#define IOCB_FLAG_IOPRIO	(1 << 1)
>>   
>>   /* read() from /dev/aio returns these structures. */
>>   struct io_event {

^ permalink raw reply

* Re: [PATCH v2] fs: Add aio iopriority support for block_dev
From: Adam Manzanares @ 2018-05-03 20:15 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: viro@zeniv.linux.org.uk, bcrl@kvack.org,
	linux-fsdevel@vger.kernel.org, linux-aio@kvack.org,
	linux-api@vger.kernel.org, linux-block@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <20180503183353.GC1562@bombadil.infradead.org>



On 5/3/18 11:33 AM, Matthew Wilcox wrote:
> On Thu, May 03, 2018 at 11:21:14AM -0700, adam.manzanares@wdc.com wrote:
>> If we want to avoid bloating struct kiocb, I suggest we turn the private field
>> into a union of the private and ki_ioprio field. It seems like the users of
>> the private field all use it at a point where we can yank the priority from
>> the kiocb before the private field is used. Comments and suggestions welcome.
> 
> Or we could just make ki_hint a u8 or u16 ... seems unlikely we'll need
> 32 bits of ki_hint.  (currently defined values are 1-5)

I like the approach of using a u16 for the ki_hint. I'll update and 
resubmit.

>> @@ -300,6 +301,7 @@ struct kiocb {
>>   	void			*private;
>>   	int			ki_flags;
>>   	enum rw_hint		ki_hint;
>> +	u16			ki_ioprio; /* See linux/ioprio.h */
>>   } __randomize_layout;
>>   
>>   static inline bool is_sync_kiocb(struct kiocb *kiocb)

^ permalink raw reply

* Re: [PATCH v2] fs: Add aio iopriority support for block_dev
From: Jeff Moyer @ 2018-05-03 18:36 UTC (permalink / raw)
  To: adam.manzanares
  Cc: viro, bcrl, linux-fsdevel, linux-aio, linux-api, linux-block,
	linux-kernel
In-Reply-To: <20180503182114.2797-1-adam.manzanares@wdc.com>

Hi, Adam,

adam.manzanares@wdc.com writes:

> From: Adam Manzanares <adam.manzanares@wdc.com>
>
> This is the per-I/O equivalent of the ioprio_set system call.
>
> When IOCB_FLAG_IOPRIO is set on the iocb aio_flags field, then we set the
> newly added kiocb ki_ioprio field to the value in the iocb aio_reqprio field.
>
> When a bio is created for an aio request by the block dev we set the priority
> value of the bio to the user supplied value.
>
> See the following link for performance implications on a SATA HDD:
> https://lkml.org/lkml/2016/12/6/495
>
> Given that WRR support for NVME devices has patches floating around and it was
> discussed at LSFMM, we may soon have a lower latency storage device that can 
> make use of iopriorities. A per command iopriority interface seems timely 
> given these developments. 
>
> If we want to avoid bloating struct kiocb, I suggest we turn the private field 
> into a union of the private and ki_ioprio field. It seems like the users of 
> the private field all use it at a point where we can yank the priority from 
> the kiocb before the private field is used. Comments and suggestions welcome.

The ioprio_set system call requires CAP_SYS_ADMIN for setting
IOPRIO_CLASS_RT.  I think we need similar checks here.

-Jeff

>
> v2: merge patches
>     use IOCB_FLAG_IOPRIO
>     validate intended use with IOCB_IOPRIO
>     add linux-api and linux-block to cc
>
> Signed-off-by: Adam Manzanares <adam.manzanares@wdc.com>
> ---
>  fs/aio.c                     | 10 ++++++++++
>  fs/block_dev.c               |  2 ++
>  include/linux/fs.h           |  2 ++
>  include/uapi/linux/aio_abi.h |  1 +
>  4 files changed, 15 insertions(+)
>
> diff --git a/fs/aio.c b/fs/aio.c
> index 88d7927ffbc6..f36636d8ff2c 100644
> --- a/fs/aio.c
> +++ b/fs/aio.c
> @@ -1597,6 +1597,16 @@ static int io_submit_one(struct kioctx *ctx, struct iocb __user *user_iocb,
>  		req->common.ki_flags |= IOCB_EVENTFD;
>  	}
>  
> +	if (iocb->aio_flags & IOCB_FLAG_IOPRIO) {
> +		/*
> +		 * If the IOCB_FLAG_IOPRIO flag of aio_flags is set, then
> +		 * aio_reqprio is interpreted as an I/O scheduling
> +		 * class and priority.
> +		 */
> +		req->common.ki_ioprio = iocb->aio_reqprio;
> +		req->common.ki_flags |= IOCB_IOPRIO;
> +	}
> +
>  	ret = kiocb_set_rw_flags(&req->common, iocb->aio_rw_flags);
>  	if (unlikely(ret)) {
>  		pr_debug("EINVAL: aio_rw_flags\n");
> diff --git a/fs/block_dev.c b/fs/block_dev.c
> index 7ec920e27065..970bef79caa6 100644
> --- a/fs/block_dev.c
> +++ b/fs/block_dev.c
> @@ -355,6 +355,8 @@ __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter, int nr_pages)
>  		bio->bi_write_hint = iocb->ki_hint;
>  		bio->bi_private = dio;
>  		bio->bi_end_io = blkdev_bio_end_io;
> +		if (iocb->ki_flags & IOCB_IOPRIO)
> +			bio->bi_ioprio = iocb->ki_ioprio;
>  
>  		ret = bio_iov_iter_get_pages(bio, iter);
>  		if (unlikely(ret)) {
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 760d8da1b6c7..ab63ce720305 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -292,6 +292,7 @@ enum rw_hint {
>  #define IOCB_SYNC		(1 << 5)
>  #define IOCB_WRITE		(1 << 6)
>  #define IOCB_NOWAIT		(1 << 7)
> +#define IOCB_IOPRIO		(1 << 8)
>  
>  struct kiocb {
>  	struct file		*ki_filp;
> @@ -300,6 +301,7 @@ struct kiocb {
>  	void			*private;
>  	int			ki_flags;
>  	enum rw_hint		ki_hint;
> +	u16			ki_ioprio; /* See linux/ioprio.h */
>  } __randomize_layout;
>  
>  static inline bool is_sync_kiocb(struct kiocb *kiocb)
> diff --git a/include/uapi/linux/aio_abi.h b/include/uapi/linux/aio_abi.h
> index a04adbc70ddf..d4593a6062ef 100644
> --- a/include/uapi/linux/aio_abi.h
> +++ b/include/uapi/linux/aio_abi.h
> @@ -54,6 +54,7 @@ enum {
>   *                   is valid.
>   */
>  #define IOCB_FLAG_RESFD		(1 << 0)
> +#define IOCB_FLAG_IOPRIO	(1 << 1)
>  
>  /* read() from /dev/aio returns these structures. */
>  struct io_event {

--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org.  For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>

^ permalink raw reply

* Re: [PATCH v2] fs: Add aio iopriority support for block_dev
From: Matthew Wilcox @ 2018-05-03 18:33 UTC (permalink / raw)
  To: adam.manzanares
  Cc: viro, bcrl, linux-fsdevel, linux-aio, linux-api, linux-block,
	linux-kernel
In-Reply-To: <20180503182114.2797-1-adam.manzanares@wdc.com>

On Thu, May 03, 2018 at 11:21:14AM -0700, adam.manzanares@wdc.com wrote:
> If we want to avoid bloating struct kiocb, I suggest we turn the private field 
> into a union of the private and ki_ioprio field. It seems like the users of 
> the private field all use it at a point where we can yank the priority from 
> the kiocb before the private field is used. Comments and suggestions welcome.

Or we could just make ki_hint a u8 or u16 ... seems unlikely we'll need
32 bits of ki_hint.  (currently defined values are 1-5)

> @@ -300,6 +301,7 @@ struct kiocb {
>  	void			*private;
>  	int			ki_flags;
>  	enum rw_hint		ki_hint;
> +	u16			ki_ioprio; /* See linux/ioprio.h */
>  } __randomize_layout;
>  
>  static inline bool is_sync_kiocb(struct kiocb *kiocb)

--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org.  For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>

^ permalink raw reply

* [PATCH v2] fs: Add aio iopriority support for block_dev
From: adam.manzanares @ 2018-05-03 18:21 UTC (permalink / raw)
  To: viro, bcrl
  Cc: linux-fsdevel, linux-aio, linux-api, linux-block, linux-kernel,
	Adam Manzanares

From: Adam Manzanares <adam.manzanares@wdc.com>

This is the per-I/O equivalent of the ioprio_set system call.

When IOCB_FLAG_IOPRIO is set on the iocb aio_flags field, then we set the
newly added kiocb ki_ioprio field to the value in the iocb aio_reqprio field.

When a bio is created for an aio request by the block dev we set the priority
value of the bio to the user supplied value.

See the following link for performance implications on a SATA HDD:
https://lkml.org/lkml/2016/12/6/495

Given that WRR support for NVME devices has patches floating around and it was
discussed at LSFMM, we may soon have a lower latency storage device that can 
make use of iopriorities. A per command iopriority interface seems timely 
given these developments. 

If we want to avoid bloating struct kiocb, I suggest we turn the private field 
into a union of the private and ki_ioprio field. It seems like the users of 
the private field all use it at a point where we can yank the priority from 
the kiocb before the private field is used. Comments and suggestions welcome.

v2: merge patches
    use IOCB_FLAG_IOPRIO
    validate intended use with IOCB_IOPRIO
    add linux-api and linux-block to cc

Signed-off-by: Adam Manzanares <adam.manzanares@wdc.com>
---
 fs/aio.c                     | 10 ++++++++++
 fs/block_dev.c               |  2 ++
 include/linux/fs.h           |  2 ++
 include/uapi/linux/aio_abi.h |  1 +
 4 files changed, 15 insertions(+)

diff --git a/fs/aio.c b/fs/aio.c
index 88d7927ffbc6..f36636d8ff2c 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -1597,6 +1597,16 @@ static int io_submit_one(struct kioctx *ctx, struct iocb __user *user_iocb,
 		req->common.ki_flags |= IOCB_EVENTFD;
 	}
 
+	if (iocb->aio_flags & IOCB_FLAG_IOPRIO) {
+		/*
+		 * If the IOCB_FLAG_IOPRIO flag of aio_flags is set, then
+		 * aio_reqprio is interpreted as an I/O scheduling
+		 * class and priority.
+		 */
+		req->common.ki_ioprio = iocb->aio_reqprio;
+		req->common.ki_flags |= IOCB_IOPRIO;
+	}
+
 	ret = kiocb_set_rw_flags(&req->common, iocb->aio_rw_flags);
 	if (unlikely(ret)) {
 		pr_debug("EINVAL: aio_rw_flags\n");
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 7ec920e27065..970bef79caa6 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -355,6 +355,8 @@ __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter, int nr_pages)
 		bio->bi_write_hint = iocb->ki_hint;
 		bio->bi_private = dio;
 		bio->bi_end_io = blkdev_bio_end_io;
+		if (iocb->ki_flags & IOCB_IOPRIO)
+			bio->bi_ioprio = iocb->ki_ioprio;
 
 		ret = bio_iov_iter_get_pages(bio, iter);
 		if (unlikely(ret)) {
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 760d8da1b6c7..ab63ce720305 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -292,6 +292,7 @@ enum rw_hint {
 #define IOCB_SYNC		(1 << 5)
 #define IOCB_WRITE		(1 << 6)
 #define IOCB_NOWAIT		(1 << 7)
+#define IOCB_IOPRIO		(1 << 8)
 
 struct kiocb {
 	struct file		*ki_filp;
@@ -300,6 +301,7 @@ struct kiocb {
 	void			*private;
 	int			ki_flags;
 	enum rw_hint		ki_hint;
+	u16			ki_ioprio; /* See linux/ioprio.h */
 } __randomize_layout;
 
 static inline bool is_sync_kiocb(struct kiocb *kiocb)
diff --git a/include/uapi/linux/aio_abi.h b/include/uapi/linux/aio_abi.h
index a04adbc70ddf..d4593a6062ef 100644
--- a/include/uapi/linux/aio_abi.h
+++ b/include/uapi/linux/aio_abi.h
@@ -54,6 +54,7 @@ enum {
  *                   is valid.
  */
 #define IOCB_FLAG_RESFD		(1 << 0)
+#define IOCB_FLAG_IOPRIO	(1 << 1)
 
 /* read() from /dev/aio returns these structures. */
 struct io_event {
-- 
2.15.1

--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org.  For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>

^ permalink raw reply related

* Re: [RFC PATCH for 4.18 00/14] Restartable Sequences
From: Mathieu Desnoyers @ 2018-05-03 18:04 UTC (permalink / raw)
  To: Daniel Colascione
  Cc: Peter Zijlstra, Paul E. McKenney, Boqun Feng, Andy Lutomirski,
	Dave Watson, linux-kernel, linux-api, Paul Turner, Andrew Morton,
	Russell King, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Andrew Hunter, Andi Kleen, Chris Lameter, Ben Maurer, rostedt,
	Josh Triplett, Linus Torvalds, Catalin Marinas <cata>
In-Reply-To: <CAKOZuetW5NEvbROjuquQ_AxrW7BqR5Qfp6BYfy+zUrnPkZDaTw@mail.gmail.com>

----- On May 3, 2018, at 12:22 PM, Daniel Colascione dancol@google.com wrote:

> On Thu, May 3, 2018 at 9:12 AM Mathieu Desnoyers <
> mathieu.desnoyers@efficios.com> wrote:
>> By the way, if we eventually find a way to enhance user-space mutexes in
> the
>> fashion you describe here, it would belong to another TLS area, and would
>> be registered by another system call than rseq. I proposed a more generic
>> "TLS area registration" system call a few years ago, but Linus told me he
>> wanted a system call that was specific to rseq. If we need to implement
>> other use-cases in a TLS area shared between kernel and user-space in a
>> similar fashion, the plan is to do it in a distinct system call.
> 
> If we proliferate TLS areas; we'd have to register each one upon thread
> creation, adding to the overall thread creation path. There's already a
> provision for versioning the TLS area. What's the benefit of splitting the
> registration over multiple system calls?

See the original discussion thread at

https://lkml.org/lkml/2016/4/7/502

Thanks,

Mathieu

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

^ permalink raw reply

* Re: [RFC PATCH] Add /proc/<pid>/numa_vamaps for numa node information
From: Christopher Lameter @ 2018-05-03 18:03 UTC (permalink / raw)
  To: Prakash Sangappa
  Cc: linux-kernel, linux-mm, linux-api, akpm, mhocko, kirill.shutemov,
	n-horiguchi, drepper, rientjes
In-Reply-To: <1525240686-13335-1-git-send-email-prakash.sangappa@oracle.com>

On Tue, 1 May 2018, Prakash Sangappa wrote:

> For analysis purpose it is useful to have numa node information
> corresponding mapped address ranges of the process. Currently
> /proc/<pid>/numa_maps provides list of numa nodes from where pages are
> allocated per VMA of the process. This is not useful if an user needs to
> determine which numa node the mapped pages are allocated from for a
> particular address range. It would have helped if the numa node information
> presented in /proc/<pid>/numa_maps was broken down by VA ranges showing the
> exact numa node from where the pages have been allocated.

Cant you write a small script that scans the information in numa_maps and
then displays the total pages per NUMA node and then a list of which
ranges have how many pages on a particular node?

> reading this file will not be restricted(i.e requiring CAP_SYS_ADMIN).

So a prime motivator here is security restricted access to numa_maps?

^ 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