Linux wireless drivers development
 help / color / mirror / Atom feed
* [PATCH 16/16] dma-mapping: use exact allocation in dma_alloc_contiguous
From: Christoph Hellwig @ 2019-06-14 13:47 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Sean Paul, David Airlie,
	Daniel Vetter, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
	Ian Abbott, H Hartley Sweeten
  Cc: Intel Linux Wireless, moderated list:ARM PORT, dri-devel,
	intel-gfx, linux-rdma, linux-media, netdev, linux-wireless,
	linux-s390, devel, linux-mm, iommu, linux-kernel
In-Reply-To: <20190614134726.3827-1-hch@lst.de>

Many architectures (e.g. arm, m68 and sh) have always used exact
allocation in their dma coherent allocator, which avoids a lot of
memory waste especially for larger allocations.  Lift this behavior
into the generic allocator so that dma-direct and the generic IOMMU
code benefit from this behavior as well.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 include/linux/dma-contiguous.h |  8 +++++---
 kernel/dma/contiguous.c        | 17 +++++++++++------
 2 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/include/linux/dma-contiguous.h b/include/linux/dma-contiguous.h
index c05d4e661489..2e542e314acf 100644
--- a/include/linux/dma-contiguous.h
+++ b/include/linux/dma-contiguous.h
@@ -161,15 +161,17 @@ static inline struct page *dma_alloc_contiguous(struct device *dev, size_t size,
 		gfp_t gfp)
 {
 	int node = dev ? dev_to_node(dev) : NUMA_NO_NODE;
-	size_t align = get_order(PAGE_ALIGN(size));
+	void *cpu_addr = alloc_pages_exact_node(node, size, gfp);
 
-	return alloc_pages_node(node, gfp, align);
+	if (!cpu_addr)
+		return NULL;
+	return virt_to_page(p);
 }
 
 static inline void dma_free_contiguous(struct device *dev, struct page *page,
 		size_t size)
 {
-	__free_pages(page, get_order(size));
+	free_pages_exact(page_address(page), get_order(size));
 }
 
 #endif
diff --git a/kernel/dma/contiguous.c b/kernel/dma/contiguous.c
index bfc0c17f2a3d..84f41eea2741 100644
--- a/kernel/dma/contiguous.c
+++ b/kernel/dma/contiguous.c
@@ -232,9 +232,8 @@ struct page *dma_alloc_contiguous(struct device *dev, size_t size, gfp_t gfp)
 {
 	int node = dev ? dev_to_node(dev) : NUMA_NO_NODE;
 	size_t count = PAGE_ALIGN(size) >> PAGE_SHIFT;
-	size_t align = get_order(PAGE_ALIGN(size));
-	struct page *page = NULL;
 	struct cma *cma = NULL;
+	void *cpu_addr;
 
 	if (dev && dev->cma_area)
 		cma = dev->cma_area;
@@ -243,14 +242,20 @@ struct page *dma_alloc_contiguous(struct device *dev, size_t size, gfp_t gfp)
 
 	/* CMA can be used only in the context which permits sleeping */
 	if (cma && gfpflags_allow_blocking(gfp)) {
+		size_t align = get_order(PAGE_ALIGN(size));
+		struct page *page;
+
 		align = min_t(size_t, align, CONFIG_CMA_ALIGNMENT);
 		page = cma_alloc(cma, count, align, gfp & __GFP_NOWARN);
+		if (page)
+			return page;
 	}
 
 	/* Fallback allocation of normal pages */
-	if (!page)
-		page = alloc_pages_node(node, gfp, align);
-	return page;
+	cpu_addr = alloc_pages_exact_node(node, size, gfp);
+	if (!cpu_addr)
+		return NULL;
+	return virt_to_page(cpu_addr);
 }
 
 /**
@@ -267,7 +272,7 @@ struct page *dma_alloc_contiguous(struct device *dev, size_t size, gfp_t gfp)
 void dma_free_contiguous(struct device *dev, struct page *page, size_t size)
 {
 	if (!cma_release(dev_get_cma_area(dev), page, size >> PAGE_SHIFT))
-		__free_pages(page, get_order(size));
+		free_pages_exact(page_address(page), get_order(size));
 }
 
 /*
-- 
2.20.1


^ permalink raw reply related

* [PATCH 14/16] mm: use alloc_pages_exact_node to implement alloc_pages_exact
From: Christoph Hellwig @ 2019-06-14 13:47 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Sean Paul, David Airlie,
	Daniel Vetter, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
	Ian Abbott, H Hartley Sweeten
  Cc: Intel Linux Wireless, moderated list:ARM PORT, dri-devel,
	intel-gfx, linux-rdma, linux-media, netdev, linux-wireless,
	linux-s390, devel, linux-mm, iommu, linux-kernel
In-Reply-To: <20190614134726.3827-1-hch@lst.de>

No need to duplicate the logic over two functions that are almost the
same.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 include/linux/gfp.h |  5 +++--
 mm/page_alloc.c     | 39 +++++++--------------------------------
 2 files changed, 10 insertions(+), 34 deletions(-)

diff --git a/include/linux/gfp.h b/include/linux/gfp.h
index 4274ea6bc72b..c616a23a3f81 100644
--- a/include/linux/gfp.h
+++ b/include/linux/gfp.h
@@ -530,9 +530,10 @@ extern struct page *alloc_pages_vma(gfp_t gfp_mask, int order,
 extern unsigned long __get_free_pages(gfp_t gfp_mask, unsigned int order);
 extern unsigned long get_zeroed_page(gfp_t gfp_mask);
 
-void *alloc_pages_exact(size_t size, gfp_t gfp_mask);
 void free_pages_exact(void *virt, size_t size);
-void * __meminit alloc_pages_exact_node(int nid, size_t size, gfp_t gfp_mask);
+void *alloc_pages_exact_node(int nid, size_t size, gfp_t gfp_mask);
+#define alloc_pages_exact(size, gfp_mask) \
+	alloc_pages_exact_node(NUMA_NO_NODE, size, gfp_mask)
 
 #define __get_free_page(gfp_mask) \
 		__get_free_pages((gfp_mask), 0)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index dd2fed66b656..dec68bd21a71 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -4859,34 +4859,6 @@ static void *make_alloc_exact(unsigned long addr, unsigned int order,
 	return (void *)addr;
 }
 
-/**
- * alloc_pages_exact - allocate an exact number physically-contiguous pages.
- * @size: the number of bytes to allocate
- * @gfp_mask: GFP flags for the allocation, must not contain __GFP_COMP
- *
- * This function is similar to alloc_pages(), except that it allocates the
- * minimum number of pages to satisfy the request.  alloc_pages() can only
- * allocate memory in power-of-two pages.
- *
- * This function is also limited by MAX_ORDER.
- *
- * Memory allocated by this function must be released by free_pages_exact().
- *
- * Return: pointer to the allocated area or %NULL in case of error.
- */
-void *alloc_pages_exact(size_t size, gfp_t gfp_mask)
-{
-	unsigned int order = get_order(size);
-	unsigned long addr;
-
-	if (WARN_ON_ONCE(gfp_mask & __GFP_COMP))
-		gfp_mask &= ~__GFP_COMP;
-
-	addr = __get_free_pages(gfp_mask, order);
-	return make_alloc_exact(addr, order, size);
-}
-EXPORT_SYMBOL(alloc_pages_exact);
-
 /**
  * alloc_pages_exact_node - allocate an exact number of physically-contiguous
  *			   pages on a node.
@@ -4894,12 +4866,15 @@ EXPORT_SYMBOL(alloc_pages_exact);
  * @size: the number of bytes to allocate
  * @gfp_mask: GFP flags for the allocation, must not contain __GFP_COMP
  *
- * Like alloc_pages_exact(), but try to allocate on node nid first before falling
- * back.
+ * This function is similar to alloc_pages_node(), except that it allocates the
+ * minimum number of pages to satisfy the request while alloc_pages() can only
+ * allocate memory in power-of-two pages.  This function is also limited by
+ * MAX_ORDER.
  *
- * Return: pointer to the allocated area or %NULL in case of error.
+ * Returns a pointer to the allocated area or %NULL in case of error, memory
+ * allocated by this function must be released by free_pages_exact().
  */
-void * __meminit alloc_pages_exact_node(int nid, size_t size, gfp_t gfp_mask)
+void *alloc_pages_exact_node(int nid, size_t size, gfp_t gfp_mask)
 {
 	unsigned int order = get_order(size);
 	struct page *p;
-- 
2.20.1


^ permalink raw reply related

* [PATCH 15/16] dma-mapping: clear __GFP_COMP in dma_alloc_attrs
From: Christoph Hellwig @ 2019-06-14 13:47 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Sean Paul, David Airlie,
	Daniel Vetter, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
	Ian Abbott, H Hartley Sweeten
  Cc: Intel Linux Wireless, moderated list:ARM PORT, dri-devel,
	intel-gfx, linux-rdma, linux-media, netdev, linux-wireless,
	linux-s390, devel, linux-mm, iommu, linux-kernel
In-Reply-To: <20190614134726.3827-1-hch@lst.de>

Lift the code to clear __GFP_COMP from arm into the common DMA
allocator path.  For one this fixes the various other patches that
call alloc_pages_exact or split_page in case a bogus driver passes
the argument, and it also prepares for doing exact allocation in
the generic dma-direct allocator.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/arm/mm/dma-mapping.c | 17 -----------------
 kernel/dma/mapping.c      |  9 +++++++++
 2 files changed, 9 insertions(+), 17 deletions(-)

diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 0a75058c11f3..86135feb2c05 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -759,14 +759,6 @@ static void *__dma_alloc(struct device *dev, size_t size, dma_addr_t *handle,
 	if (mask < 0xffffffffULL)
 		gfp |= GFP_DMA;
 
-	/*
-	 * Following is a work-around (a.k.a. hack) to prevent pages
-	 * with __GFP_COMP being passed to split_page() which cannot
-	 * handle them.  The real problem is that this flag probably
-	 * should be 0 on ARM as it is not supported on this
-	 * platform; see CONFIG_HUGETLBFS.
-	 */
-	gfp &= ~(__GFP_COMP);
 	args.gfp = gfp;
 
 	*handle = DMA_MAPPING_ERROR;
@@ -1527,15 +1519,6 @@ static void *__arm_iommu_alloc_attrs(struct device *dev, size_t size,
 		return __iommu_alloc_simple(dev, size, gfp, handle,
 					    coherent_flag, attrs);
 
-	/*
-	 * Following is a work-around (a.k.a. hack) to prevent pages
-	 * with __GFP_COMP being passed to split_page() which cannot
-	 * handle them.  The real problem is that this flag probably
-	 * should be 0 on ARM as it is not supported on this
-	 * platform; see CONFIG_HUGETLBFS.
-	 */
-	gfp &= ~(__GFP_COMP);
-
 	pages = __iommu_alloc_buffer(dev, size, gfp, attrs, coherent_flag);
 	if (!pages)
 		return NULL;
diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
index f7afdadb6770..4b618e1abbc1 100644
--- a/kernel/dma/mapping.c
+++ b/kernel/dma/mapping.c
@@ -252,6 +252,15 @@ void *dma_alloc_attrs(struct device *dev, size_t size, dma_addr_t *dma_handle,
 	/* let the implementation decide on the zone to allocate from: */
 	flag &= ~(__GFP_DMA | __GFP_DMA32 | __GFP_HIGHMEM);
 
+	/*
+	 * __GFP_COMP interacts badly with splitting up a larger order
+	 * allocation.  But as our allocations might not even come from the
+	 * page allocator, the callers can't rely on the fact that they
+	 * even get pages, never mind which kind.
+	 */
+	if (WARN_ON_ONCE(flag & __GFP_COMP))
+		flag &= ~__GFP_COMP;
+
 	if (dma_is_direct(ops))
 		cpu_addr = dma_direct_alloc(dev, size, dma_handle, flag, attrs);
 	else if (ops->alloc)
-- 
2.20.1


^ permalink raw reply related

* [PATCH 12/16] staging/comedi: mark as broken
From: Christoph Hellwig @ 2019-06-14 13:47 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Sean Paul, David Airlie,
	Daniel Vetter, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
	Ian Abbott, H Hartley Sweeten
  Cc: Intel Linux Wireless, moderated list:ARM PORT, dri-devel,
	intel-gfx, linux-rdma, linux-media, netdev, linux-wireless,
	linux-s390, devel, linux-mm, iommu, linux-kernel
In-Reply-To: <20190614134726.3827-1-hch@lst.de>

comedi_buf.c abuse the DMA API in gravely broken ways, as it assumes it
can call virt_to_page on the result, and the just remap it as uncached
using vmap.  Disable the driver until this API abuse has been fixed.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/staging/comedi/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/comedi/Kconfig b/drivers/staging/comedi/Kconfig
index 049b659fa6ad..e7c021d76cfa 100644
--- a/drivers/staging/comedi/Kconfig
+++ b/drivers/staging/comedi/Kconfig
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 config COMEDI
 	tristate "Data acquisition support (comedi)"
+	depends on BROKEN
 	help
 	  Enable support for a wide range of data acquisition devices
 	  for Linux.
-- 
2.20.1


^ permalink raw reply related

* [PATCH 13/16] mm: rename alloc_pages_exact_nid to alloc_pages_exact_node
From: Christoph Hellwig @ 2019-06-14 13:47 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Sean Paul, David Airlie,
	Daniel Vetter, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
	Ian Abbott, H Hartley Sweeten
  Cc: Intel Linux Wireless, moderated list:ARM PORT, dri-devel,
	intel-gfx, linux-rdma, linux-media, netdev, linux-wireless,
	linux-s390, devel, linux-mm, iommu, linux-kernel
In-Reply-To: <20190614134726.3827-1-hch@lst.de>

This fits in with the naming scheme used by alloc_pages_node.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 include/linux/gfp.h | 2 +-
 mm/page_alloc.c     | 4 ++--
 mm/page_ext.c       | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/linux/gfp.h b/include/linux/gfp.h
index fb07b503dc45..4274ea6bc72b 100644
--- a/include/linux/gfp.h
+++ b/include/linux/gfp.h
@@ -532,7 +532,7 @@ extern unsigned long get_zeroed_page(gfp_t gfp_mask);
 
 void *alloc_pages_exact(size_t size, gfp_t gfp_mask);
 void free_pages_exact(void *virt, size_t size);
-void * __meminit alloc_pages_exact_nid(int nid, size_t size, gfp_t gfp_mask);
+void * __meminit alloc_pages_exact_node(int nid, size_t size, gfp_t gfp_mask);
 
 #define __get_free_page(gfp_mask) \
 		__get_free_pages((gfp_mask), 0)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index d66bc8abe0af..dd2fed66b656 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -4888,7 +4888,7 @@ void *alloc_pages_exact(size_t size, gfp_t gfp_mask)
 EXPORT_SYMBOL(alloc_pages_exact);
 
 /**
- * alloc_pages_exact_nid - allocate an exact number of physically-contiguous
+ * alloc_pages_exact_node - allocate an exact number of physically-contiguous
  *			   pages on a node.
  * @nid: the preferred node ID where memory should be allocated
  * @size: the number of bytes to allocate
@@ -4899,7 +4899,7 @@ EXPORT_SYMBOL(alloc_pages_exact);
  *
  * Return: pointer to the allocated area or %NULL in case of error.
  */
-void * __meminit alloc_pages_exact_nid(int nid, size_t size, gfp_t gfp_mask)
+void * __meminit alloc_pages_exact_node(int nid, size_t size, gfp_t gfp_mask)
 {
 	unsigned int order = get_order(size);
 	struct page *p;
diff --git a/mm/page_ext.c b/mm/page_ext.c
index d8f1aca4ad43..bca6bb316714 100644
--- a/mm/page_ext.c
+++ b/mm/page_ext.c
@@ -215,7 +215,7 @@ static void *__meminit alloc_page_ext(size_t size, int nid)
 	gfp_t flags = GFP_KERNEL | __GFP_ZERO | __GFP_NOWARN;
 	void *addr = NULL;
 
-	addr = alloc_pages_exact_nid(nid, size, flags);
+	addr = alloc_pages_exact_node(nid, size, flags);
 	if (addr) {
 		kmemleak_alloc(addr, size, 1, flags);
 		return addr;
-- 
2.20.1


^ permalink raw reply related

* [PATCH 11/16] s390/ism: stop passing bogus gfp flags arguments to dma_alloc_coherent
From: Christoph Hellwig @ 2019-06-14 13:47 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Sean Paul, David Airlie,
	Daniel Vetter, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
	Ian Abbott, H Hartley Sweeten
  Cc: Intel Linux Wireless, moderated list:ARM PORT, dri-devel,
	intel-gfx, linux-rdma, linux-media, netdev, linux-wireless,
	linux-s390, devel, linux-mm, iommu, linux-kernel
In-Reply-To: <20190614134726.3827-1-hch@lst.de>

dma_alloc_coherent is not just the page allocator.  The only valid
arguments to pass are either GFP_ATOMIC or GFP_ATOMIC with possible
modifiers of __GFP_NORETRY or __GFP_NOWARN.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/s390/net/ism_drv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/s390/net/ism_drv.c b/drivers/s390/net/ism_drv.c
index 4fc2056bd227..4ff5506fa4c6 100644
--- a/drivers/s390/net/ism_drv.c
+++ b/drivers/s390/net/ism_drv.c
@@ -241,7 +241,8 @@ static int ism_alloc_dmb(struct ism_dev *ism, struct smcd_dmb *dmb)
 
 	dmb->cpu_addr = dma_alloc_coherent(&ism->pdev->dev, dmb->dmb_len,
 					   &dmb->dma_addr,
-					   GFP_KERNEL | __GFP_NOWARN | __GFP_NOMEMALLOC | __GFP_COMP | __GFP_NORETRY);
+					   GFP_KERNEL | __GFP_NOWARN |
+					   __GFP_NORETRY);
 	if (!dmb->cpu_addr)
 		clear_bit(dmb->sba_idx, ism->sba_bitmap);
 
-- 
2.20.1


^ permalink raw reply related

* [PATCH 07/16] IB/hfi1: stop passing bogus gfp flags arguments to dma_alloc_coherent
From: Christoph Hellwig @ 2019-06-14 13:47 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Sean Paul, David Airlie,
	Daniel Vetter, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
	Ian Abbott, H Hartley Sweeten
  Cc: Intel Linux Wireless, moderated list:ARM PORT, dri-devel,
	intel-gfx, linux-rdma, linux-media, netdev, linux-wireless,
	linux-s390, devel, linux-mm, iommu, linux-kernel
In-Reply-To: <20190614134726.3827-1-hch@lst.de>

dma_alloc_coherent is not just the page allocator.  The only valid
arguments to pass are either GFP_ATOMIC or GFP_ATOMIC with possible
modifiers of __GFP_NORETRY or __GFP_NOWARN.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/infiniband/hw/hfi1/init.c | 22 +++-------------------
 1 file changed, 3 insertions(+), 19 deletions(-)

diff --git a/drivers/infiniband/hw/hfi1/init.c b/drivers/infiniband/hw/hfi1/init.c
index 71cb9525c074..ff9d106ee784 100644
--- a/drivers/infiniband/hw/hfi1/init.c
+++ b/drivers/infiniband/hw/hfi1/init.c
@@ -1846,17 +1846,10 @@ int hfi1_create_rcvhdrq(struct hfi1_devdata *dd, struct hfi1_ctxtdata *rcd)
 	u64 reg;
 
 	if (!rcd->rcvhdrq) {
-		gfp_t gfp_flags;
-
 		amt = rcvhdrq_size(rcd);
 
-		if (rcd->ctxt < dd->first_dyn_alloc_ctxt || rcd->is_vnic)
-			gfp_flags = GFP_KERNEL;
-		else
-			gfp_flags = GFP_USER;
 		rcd->rcvhdrq = dma_alloc_coherent(&dd->pcidev->dev, amt,
-						  &rcd->rcvhdrq_dma,
-						  gfp_flags | __GFP_COMP);
+						  &rcd->rcvhdrq_dma, GFP_KERNEL);
 
 		if (!rcd->rcvhdrq) {
 			dd_dev_err(dd,
@@ -1870,7 +1863,7 @@ int hfi1_create_rcvhdrq(struct hfi1_devdata *dd, struct hfi1_ctxtdata *rcd)
 			rcd->rcvhdrtail_kvaddr = dma_alloc_coherent(&dd->pcidev->dev,
 								    PAGE_SIZE,
 								    &rcd->rcvhdrqtailaddr_dma,
-								    gfp_flags);
+								    GFP_KERNEL);
 			if (!rcd->rcvhdrtail_kvaddr)
 				goto bail_free;
 		}
@@ -1926,19 +1919,10 @@ int hfi1_setup_eagerbufs(struct hfi1_ctxtdata *rcd)
 {
 	struct hfi1_devdata *dd = rcd->dd;
 	u32 max_entries, egrtop, alloced_bytes = 0;
-	gfp_t gfp_flags;
 	u16 order, idx = 0;
 	int ret = 0;
 	u16 round_mtu = roundup_pow_of_two(hfi1_max_mtu);
 
-	/*
-	 * GFP_USER, but without GFP_FS, so buffer cache can be
-	 * coalesced (we hope); otherwise, even at order 4,
-	 * heavy filesystem activity makes these fail, and we can
-	 * use compound pages.
-	 */
-	gfp_flags = __GFP_RECLAIM | __GFP_IO | __GFP_COMP;
-
 	/*
 	 * The minimum size of the eager buffers is a groups of MTU-sized
 	 * buffers.
@@ -1969,7 +1953,7 @@ int hfi1_setup_eagerbufs(struct hfi1_ctxtdata *rcd)
 			dma_alloc_coherent(&dd->pcidev->dev,
 					   rcd->egrbufs.rcvtid_size,
 					   &rcd->egrbufs.buffers[idx].dma,
-					   gfp_flags);
+					   GFP_KERNEL);
 		if (rcd->egrbufs.buffers[idx].addr) {
 			rcd->egrbufs.buffers[idx].len =
 				rcd->egrbufs.rcvtid_size;
-- 
2.20.1


^ permalink raw reply related

* [PATCH 09/16] cnic: stop passing bogus gfp flags arguments to dma_alloc_coherent
From: Christoph Hellwig @ 2019-06-14 13:47 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Sean Paul, David Airlie,
	Daniel Vetter, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
	Ian Abbott, H Hartley Sweeten
  Cc: Intel Linux Wireless, moderated list:ARM PORT, dri-devel,
	intel-gfx, linux-rdma, linux-media, netdev, linux-wireless,
	linux-s390, devel, linux-mm, iommu, linux-kernel
In-Reply-To: <20190614134726.3827-1-hch@lst.de>

dma_alloc_coherent is not just the page allocator.  The only valid
arguments to pass are either GFP_ATOMIC or GFP_ATOMIC with possible
modifiers of __GFP_NORETRY or __GFP_NOWARN.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/net/ethernet/broadcom/cnic.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/cnic.c b/drivers/net/ethernet/broadcom/cnic.c
index 57dc3cbff36e..bd1c993680e5 100644
--- a/drivers/net/ethernet/broadcom/cnic.c
+++ b/drivers/net/ethernet/broadcom/cnic.c
@@ -1028,7 +1028,7 @@ static int __cnic_alloc_uio_rings(struct cnic_uio_dev *udev, int pages)
 	udev->l2_ring_size = pages * CNIC_PAGE_SIZE;
 	udev->l2_ring = dma_alloc_coherent(&udev->pdev->dev, udev->l2_ring_size,
 					   &udev->l2_ring_map,
-					   GFP_KERNEL | __GFP_COMP);
+					   GFP_KERNEL);
 	if (!udev->l2_ring)
 		return -ENOMEM;
 
@@ -1036,7 +1036,7 @@ static int __cnic_alloc_uio_rings(struct cnic_uio_dev *udev, int pages)
 	udev->l2_buf_size = CNIC_PAGE_ALIGN(udev->l2_buf_size);
 	udev->l2_buf = dma_alloc_coherent(&udev->pdev->dev, udev->l2_buf_size,
 					  &udev->l2_buf_map,
-					  GFP_KERNEL | __GFP_COMP);
+					  GFP_KERNEL);
 	if (!udev->l2_buf) {
 		__cnic_free_uio_rings(udev);
 		return -ENOMEM;
-- 
2.20.1


^ permalink raw reply related

* [PATCH 05/16] drm: don't mark pages returned from drm_pci_alloc reserved
From: Christoph Hellwig @ 2019-06-14 13:47 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Sean Paul, David Airlie,
	Daniel Vetter, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
	Ian Abbott, H Hartley Sweeten
  Cc: Intel Linux Wireless, moderated list:ARM PORT, dri-devel,
	intel-gfx, linux-rdma, linux-media, netdev, linux-wireless,
	linux-s390, devel, linux-mm, iommu, linux-kernel
In-Reply-To: <20190614134726.3827-1-hch@lst.de>

We are not allowed to call virt_to_page on pages returned from
dma_alloc_coherent, as in many cases the virtual address returned
is aactually a kernel direct mapping.  Also there generally is no
need to mark dma memory as reserved.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/gpu/drm/drm_bufs.c | 16 +---------------
 1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c
index 7418872d87c6..b640437ce90f 100644
--- a/drivers/gpu/drm/drm_bufs.c
+++ b/drivers/gpu/drm/drm_bufs.c
@@ -77,13 +77,6 @@ drm_dma_handle_t *drm_pci_alloc(struct drm_device * dev, size_t size, size_t ali
 		return NULL;
 	}
 
-	/* XXX - Is virt_to_page() legal for consistent mem? */
-	/* Reserve */
-	for (addr = (unsigned long)dmah->vaddr, sz = size;
-	     sz > 0; addr += PAGE_SIZE, sz -= PAGE_SIZE) {
-		SetPageReserved(virt_to_page((void *)addr));
-	}
-
 	return dmah;
 }
 
@@ -97,16 +90,9 @@ void __drm_legacy_pci_free(struct drm_device * dev, drm_dma_handle_t * dmah)
 	unsigned long addr;
 	size_t sz;
 
-	if (dmah->vaddr) {
-		/* XXX - Is virt_to_page() legal for consistent mem? */
-		/* Unreserve */
-		for (addr = (unsigned long)dmah->vaddr, sz = dmah->size;
-		     sz > 0; addr += PAGE_SIZE, sz -= PAGE_SIZE) {
-			ClearPageReserved(virt_to_page((void *)addr));
-		}
+	if (dmah->vaddr)
 		dma_free_coherent(&dev->pdev->dev, dmah->size, dmah->vaddr,
 				  dmah->busaddr);
-	}
 }
 
 /**
-- 
2.20.1


^ permalink raw reply related

* [PATCH 04/16] drm: move drm_pci_{alloc,free} to drm_legacy
From: Christoph Hellwig @ 2019-06-14 13:47 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Sean Paul, David Airlie,
	Daniel Vetter, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
	Ian Abbott, H Hartley Sweeten
  Cc: Intel Linux Wireless, moderated list:ARM PORT, dri-devel,
	intel-gfx, linux-rdma, linux-media, netdev, linux-wireless,
	linux-s390, devel, linux-mm, iommu, linux-kernel
In-Reply-To: <20190614134726.3827-1-hch@lst.de>

These functions are rather broken in that they try to pass __GFP_COMP
to dma_alloc_coherent, call virt_to_page on the return value and
mess with PageReserved.  And not actually used by any modern driver.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/gpu/drm/drm_bufs.c | 85 ++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_pci.c  | 89 --------------------------------------
 2 files changed, 85 insertions(+), 89 deletions(-)

diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c
index bfc419ed9d6c..7418872d87c6 100644
--- a/drivers/gpu/drm/drm_bufs.c
+++ b/drivers/gpu/drm/drm_bufs.c
@@ -38,6 +38,91 @@
 
 #include <linux/nospec.h>
 
+/**
+ * drm_pci_alloc - Allocate a PCI consistent memory block, for DMA.
+ * @dev: DRM device
+ * @size: size of block to allocate
+ * @align: alignment of block
+ *
+ * FIXME: This is a needless abstraction of the Linux dma-api and should be
+ * removed.
+ *
+ * Return: A handle to the allocated memory block on success or NULL on
+ * failure.
+ */
+drm_dma_handle_t *drm_pci_alloc(struct drm_device * dev, size_t size, size_t align)
+{
+	drm_dma_handle_t *dmah;
+	unsigned long addr;
+	size_t sz;
+
+	/* pci_alloc_consistent only guarantees alignment to the smallest
+	 * PAGE_SIZE order which is greater than or equal to the requested size.
+	 * Return NULL here for now to make sure nobody tries for larger alignment
+	 */
+	if (align > size)
+		return NULL;
+
+	dmah = kmalloc(sizeof(drm_dma_handle_t), GFP_KERNEL);
+	if (!dmah)
+		return NULL;
+
+	dmah->size = size;
+	dmah->vaddr = dma_alloc_coherent(&dev->pdev->dev, size,
+					 &dmah->busaddr,
+					 GFP_KERNEL | __GFP_COMP);
+
+	if (dmah->vaddr == NULL) {
+		kfree(dmah);
+		return NULL;
+	}
+
+	/* XXX - Is virt_to_page() legal for consistent mem? */
+	/* Reserve */
+	for (addr = (unsigned long)dmah->vaddr, sz = size;
+	     sz > 0; addr += PAGE_SIZE, sz -= PAGE_SIZE) {
+		SetPageReserved(virt_to_page((void *)addr));
+	}
+
+	return dmah;
+}
+
+/*
+ * Free a PCI consistent memory block without freeing its descriptor.
+ *
+ * This function is for internal use in the Linux-specific DRM core code.
+ */
+void __drm_legacy_pci_free(struct drm_device * dev, drm_dma_handle_t * dmah)
+{
+	unsigned long addr;
+	size_t sz;
+
+	if (dmah->vaddr) {
+		/* XXX - Is virt_to_page() legal for consistent mem? */
+		/* Unreserve */
+		for (addr = (unsigned long)dmah->vaddr, sz = dmah->size;
+		     sz > 0; addr += PAGE_SIZE, sz -= PAGE_SIZE) {
+			ClearPageReserved(virt_to_page((void *)addr));
+		}
+		dma_free_coherent(&dev->pdev->dev, dmah->size, dmah->vaddr,
+				  dmah->busaddr);
+	}
+}
+
+/**
+ * drm_pci_free - Free a PCI consistent memory block
+ * @dev: DRM device
+ * @dmah: handle to memory block
+ *
+ * FIXME: This is a needless abstraction of the Linux dma-api and should be
+ * removed.
+ */
+void drm_pci_free(struct drm_device * dev, drm_dma_handle_t * dmah)
+{
+	__drm_legacy_pci_free(dev, dmah);
+	kfree(dmah);
+}
+
 static struct drm_map_list *drm_find_matching_map(struct drm_device *dev,
 						  struct drm_local_map *map)
 {
diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
index 693748ad8b88..77a215f2a8e4 100644
--- a/drivers/gpu/drm/drm_pci.c
+++ b/drivers/gpu/drm/drm_pci.c
@@ -31,95 +31,6 @@
 #include "drm_internal.h"
 #include "drm_legacy.h"
 
-/**
- * drm_pci_alloc - Allocate a PCI consistent memory block, for DMA.
- * @dev: DRM device
- * @size: size of block to allocate
- * @align: alignment of block
- *
- * FIXME: This is a needless abstraction of the Linux dma-api and should be
- * removed.
- *
- * Return: A handle to the allocated memory block on success or NULL on
- * failure.
- */
-drm_dma_handle_t *drm_pci_alloc(struct drm_device * dev, size_t size, size_t align)
-{
-	drm_dma_handle_t *dmah;
-	unsigned long addr;
-	size_t sz;
-
-	/* pci_alloc_consistent only guarantees alignment to the smallest
-	 * PAGE_SIZE order which is greater than or equal to the requested size.
-	 * Return NULL here for now to make sure nobody tries for larger alignment
-	 */
-	if (align > size)
-		return NULL;
-
-	dmah = kmalloc(sizeof(drm_dma_handle_t), GFP_KERNEL);
-	if (!dmah)
-		return NULL;
-
-	dmah->size = size;
-	dmah->vaddr = dma_alloc_coherent(&dev->pdev->dev, size,
-					 &dmah->busaddr,
-					 GFP_KERNEL | __GFP_COMP);
-
-	if (dmah->vaddr == NULL) {
-		kfree(dmah);
-		return NULL;
-	}
-
-	/* XXX - Is virt_to_page() legal for consistent mem? */
-	/* Reserve */
-	for (addr = (unsigned long)dmah->vaddr, sz = size;
-	     sz > 0; addr += PAGE_SIZE, sz -= PAGE_SIZE) {
-		SetPageReserved(virt_to_page((void *)addr));
-	}
-
-	return dmah;
-}
-
-EXPORT_SYMBOL(drm_pci_alloc);
-
-/*
- * Free a PCI consistent memory block without freeing its descriptor.
- *
- * This function is for internal use in the Linux-specific DRM core code.
- */
-void __drm_legacy_pci_free(struct drm_device * dev, drm_dma_handle_t * dmah)
-{
-	unsigned long addr;
-	size_t sz;
-
-	if (dmah->vaddr) {
-		/* XXX - Is virt_to_page() legal for consistent mem? */
-		/* Unreserve */
-		for (addr = (unsigned long)dmah->vaddr, sz = dmah->size;
-		     sz > 0; addr += PAGE_SIZE, sz -= PAGE_SIZE) {
-			ClearPageReserved(virt_to_page((void *)addr));
-		}
-		dma_free_coherent(&dev->pdev->dev, dmah->size, dmah->vaddr,
-				  dmah->busaddr);
-	}
-}
-
-/**
- * drm_pci_free - Free a PCI consistent memory block
- * @dev: DRM device
- * @dmah: handle to memory block
- *
- * FIXME: This is a needless abstraction of the Linux dma-api and should be
- * removed.
- */
-void drm_pci_free(struct drm_device * dev, drm_dma_handle_t * dmah)
-{
-	__drm_legacy_pci_free(dev, dmah);
-	kfree(dmah);
-}
-
-EXPORT_SYMBOL(drm_pci_free);
-
 #ifdef CONFIG_PCI
 
 static int drm_get_pci_domain(struct drm_device *dev)
-- 
2.20.1


^ permalink raw reply related

* [PATCH 03/16] drm/i915: stop using drm_pci_alloc
From: Christoph Hellwig @ 2019-06-14 13:47 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Sean Paul, David Airlie,
	Daniel Vetter, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
	Ian Abbott, H Hartley Sweeten
  Cc: Intel Linux Wireless, moderated list:ARM PORT, dri-devel,
	intel-gfx, linux-rdma, linux-media, netdev, linux-wireless,
	linux-s390, devel, linux-mm, iommu, linux-kernel
In-Reply-To: <20190614134726.3827-1-hch@lst.de>

Remove usage of the legacy drm PCI DMA wrappers, and with that the
incorrect usage cocktail of __GFP_COMP, virt_to_page on DMA allocation
and SetPageReserved.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/gpu/drm/i915/i915_gem.c        | 30 +++++++++++++-------------
 drivers/gpu/drm/i915/i915_gem_object.h |  3 ++-
 drivers/gpu/drm/i915/intel_display.c   |  2 +-
 3 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index ad01c92aaf74..8f2053c91aff 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -228,7 +228,6 @@ i915_gem_get_aperture_ioctl(struct drm_device *dev, void *data,
 static int i915_gem_object_get_pages_phys(struct drm_i915_gem_object *obj)
 {
 	struct address_space *mapping = obj->base.filp->f_mapping;
-	drm_dma_handle_t *phys;
 	struct sg_table *st;
 	struct scatterlist *sg;
 	char *vaddr;
@@ -242,13 +241,13 @@ static int i915_gem_object_get_pages_phys(struct drm_i915_gem_object *obj)
 	 * to handle all possible callers, and given typical object sizes,
 	 * the alignment of the buddy allocation will naturally match.
 	 */
-	phys = drm_pci_alloc(obj->base.dev,
-			     roundup_pow_of_two(obj->base.size),
-			     roundup_pow_of_two(obj->base.size));
-	if (!phys)
+	obj->phys_vaddr = dma_alloc_coherent(&obj->base.dev->pdev->dev,
+			roundup_pow_of_two(obj->base.size),
+			&obj->phys_handle, GFP_KERNEL);
+	if (!obj->phys_vaddr)
 		return -ENOMEM;
 
-	vaddr = phys->vaddr;
+	vaddr = obj->phys_vaddr;
 	for (i = 0; i < obj->base.size / PAGE_SIZE; i++) {
 		struct page *page;
 		char *src;
@@ -286,18 +285,17 @@ static int i915_gem_object_get_pages_phys(struct drm_i915_gem_object *obj)
 	sg->offset = 0;
 	sg->length = obj->base.size;
 
-	sg_dma_address(sg) = phys->busaddr;
+	sg_dma_address(sg) = obj->phys_handle;
 	sg_dma_len(sg) = obj->base.size;
 
-	obj->phys_handle = phys;
-
 	__i915_gem_object_set_pages(obj, st, sg->length);
 
 	return 0;
 
 err_phys:
-	drm_pci_free(obj->base.dev, phys);
-
+	dma_free_coherent(&obj->base.dev->pdev->dev,
+			roundup_pow_of_two(obj->base.size), obj->phys_vaddr,
+			obj->phys_handle);
 	return err;
 }
 
@@ -335,7 +333,7 @@ i915_gem_object_put_pages_phys(struct drm_i915_gem_object *obj,
 
 	if (obj->mm.dirty) {
 		struct address_space *mapping = obj->base.filp->f_mapping;
-		char *vaddr = obj->phys_handle->vaddr;
+		char *vaddr = obj->phys_vaddr;
 		int i;
 
 		for (i = 0; i < obj->base.size / PAGE_SIZE; i++) {
@@ -363,7 +361,9 @@ i915_gem_object_put_pages_phys(struct drm_i915_gem_object *obj,
 	sg_free_table(pages);
 	kfree(pages);
 
-	drm_pci_free(obj->base.dev, obj->phys_handle);
+	dma_free_coherent(&obj->base.dev->pdev->dev,
+			roundup_pow_of_two(obj->base.size), obj->phys_vaddr,
+			obj->phys_handle);
 }
 
 static void
@@ -603,7 +603,7 @@ i915_gem_phys_pwrite(struct drm_i915_gem_object *obj,
 		     struct drm_i915_gem_pwrite *args,
 		     struct drm_file *file)
 {
-	void *vaddr = obj->phys_handle->vaddr + args->offset;
+	void *vaddr = obj->phys_vaddr + args->offset;
 	char __user *user_data = u64_to_user_ptr(args->data_ptr);
 
 	/* We manually control the domain here and pretend that it
@@ -1431,7 +1431,7 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
 		ret = i915_gem_gtt_pwrite_fast(obj, args);
 
 	if (ret == -EFAULT || ret == -ENOSPC) {
-		if (obj->phys_handle)
+		if (obj->phys_vaddr)
 			ret = i915_gem_phys_pwrite(obj, args, file);
 		else
 			ret = i915_gem_shmem_pwrite(obj, args);
diff --git a/drivers/gpu/drm/i915/i915_gem_object.h b/drivers/gpu/drm/i915/i915_gem_object.h
index ca93a40c0c87..14bd2d61d0f6 100644
--- a/drivers/gpu/drm/i915/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/i915_gem_object.h
@@ -290,7 +290,8 @@ struct drm_i915_gem_object {
 	};
 
 	/** for phys allocated objects */
-	struct drm_dma_handle *phys_handle;
+	dma_addr_t phys_handle;
+	void *phys_vaddr;
 
 	struct reservation_object __builtin_resv;
 };
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5098228f1302..4f8b368ac4e2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10066,7 +10066,7 @@ static u32 intel_cursor_base(const struct intel_plane_state *plane_state)
 	u32 base;
 
 	if (INTEL_INFO(dev_priv)->display.cursor_needs_physical)
-		base = obj->phys_handle->busaddr;
+		base = obj->phys_handle;
 	else
 		base = intel_plane_ggtt_offset(plane_state);
 
-- 
2.20.1


^ permalink raw reply related

* [PATCH 01/16] media: videobuf-dma-contig: use dma_mmap_coherent
From: Christoph Hellwig @ 2019-06-14 13:47 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Sean Paul, David Airlie,
	Daniel Vetter, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
	Ian Abbott, H Hartley Sweeten
  Cc: Intel Linux Wireless, moderated list:ARM PORT, dri-devel,
	intel-gfx, linux-rdma, linux-media, netdev, linux-wireless,
	linux-s390, devel, linux-mm, iommu, linux-kernel
In-Reply-To: <20190614134726.3827-1-hch@lst.de>

dma_alloc_coherent does not return a physical address, but a DMA
address, which might be remapped or have an offset.  Passing this
DMA address to vm_iomap_memory is completely bogus.  Use the proper
dma_mmap_coherent helper instead, and stop passing __GFP_COMP
to dma_alloc_coherent, as the memory management inside the DMA
allocator is hidden from the callers.

Fixes: a8f3c203e19b ("[media] videobuf-dma-contig: add cache support")
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/media/v4l2-core/videobuf-dma-contig.c | 23 +++++++------------
 1 file changed, 8 insertions(+), 15 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf-dma-contig.c b/drivers/media/v4l2-core/videobuf-dma-contig.c
index e1bf50df4c70..a5942ea38f1f 100644
--- a/drivers/media/v4l2-core/videobuf-dma-contig.c
+++ b/drivers/media/v4l2-core/videobuf-dma-contig.c
@@ -39,11 +39,11 @@ struct videobuf_dma_contig_memory {
 
 static int __videobuf_dc_alloc(struct device *dev,
 			       struct videobuf_dma_contig_memory *mem,
-			       unsigned long size, gfp_t flags)
+			       unsigned long size)
 {
 	mem->size = size;
-	mem->vaddr = dma_alloc_coherent(dev, mem->size,
-					&mem->dma_handle, flags);
+	mem->vaddr = dma_alloc_coherent(dev, mem->size, &mem->dma_handle,
+			GFP_KERNEL);
 
 	if (!mem->vaddr) {
 		dev_err(dev, "memory alloc size %ld failed\n", mem->size);
@@ -260,8 +260,7 @@ static int __videobuf_iolock(struct videobuf_queue *q,
 			return videobuf_dma_contig_user_get(mem, vb);
 
 		/* allocate memory for the read() method */
-		if (__videobuf_dc_alloc(q->dev, mem, PAGE_ALIGN(vb->size),
-					GFP_KERNEL))
+		if (__videobuf_dc_alloc(q->dev, mem, PAGE_ALIGN(vb->size)))
 			return -ENOMEM;
 		break;
 	case V4L2_MEMORY_OVERLAY:
@@ -280,7 +279,6 @@ static int __videobuf_mmap_mapper(struct videobuf_queue *q,
 	struct videobuf_dma_contig_memory *mem;
 	struct videobuf_mapping *map;
 	int retval;
-	unsigned long size;
 
 	dev_dbg(q->dev, "%s\n", __func__);
 
@@ -298,23 +296,18 @@ static int __videobuf_mmap_mapper(struct videobuf_queue *q,
 	BUG_ON(!mem);
 	MAGIC_CHECK(mem->magic, MAGIC_DC_MEM);
 
-	if (__videobuf_dc_alloc(q->dev, mem, PAGE_ALIGN(buf->bsize),
-				GFP_KERNEL | __GFP_COMP))
+	if (__videobuf_dc_alloc(q->dev, mem, PAGE_ALIGN(buf->bsize)))
 		goto error;
 
-	/* Try to remap memory */
-	size = vma->vm_end - vma->vm_start;
-	vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
-
 	/* the "vm_pgoff" is just used in v4l2 to find the
 	 * corresponding buffer data structure which is allocated
 	 * earlier and it does not mean the offset from the physical
 	 * buffer start address as usual. So set it to 0 to pass
-	 * the sanity check in vm_iomap_memory().
+	 * the sanity check in dma_mmap_coherent().
 	 */
 	vma->vm_pgoff = 0;
-
-	retval = vm_iomap_memory(vma, mem->dma_handle, size);
+	retval = dma_mmap_coherent(q->dev, vma, mem->vaddr, mem->dma_handle,
+			vma->vm_end - vma->vm_start);
 	if (retval) {
 		dev_err(q->dev, "mmap: remap failed with error %d. ",
 			retval);
-- 
2.20.1


^ permalink raw reply related

* pull-request: mac80211 2019-06-14
From: Johannes Berg @ 2019-06-14 13:50 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-wireless

Hi Dave,

Here's a round of fixes for the current tree, things are all over
and the only really important thing is the TDLS and MFP fix, both
of which allow a security bypass in MFP.

Please pull and let me know if there's any problem.

Thanks,
johannes



The following changes since commit 63863ee8e2f6f6ae47be3dff4af2f2806f5ca2dd:

  Merge tag 'gcc-plugins-v5.2-rc1' of ssh://gitolite.kernel.org/pub/scm/linux/kernel/git/kees/linux (2019-05-13 16:01:52 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211.git tags/mac80211-for-davem-2019-06-14

for you to fetch changes up to b65842025335711e2a0259feb4dbadb0c9ffb6d9:

  cfg80211: report measurement start TSF correctly (2019-06-14 15:46:33 +0200)

----------------------------------------------------------------
Various fixes, all over:
 * a few memory leaks
 * fixes for management frame protection security
   and A2/A3 confusion (affecting TDLS as well)
 * build fix for certificates
 * etc.

----------------------------------------------------------------
Andy Strohman (1):
      nl80211: fix station_info pertid memory leak

Avraham Stern (1):
      cfg80211: report measurement start TSF correctly

Eric Biggers (1):
      cfg80211: fix memory leak of wiphy device name

Gustavo A. R. Silva (1):
      mac80211_hwsim: mark expected switch fall-through

Johannes Berg (2):
      nl80211: fill all policy .type entries
      mac80211: drop robust management frames from unknown TA

John Crispin (1):
      mac80211: fix rate reporting inside cfg80211_calculate_bitrate_he()

Jouni Malinen (1):
      mac80211: Do not use stack memory with scatterlist for GMAC

Luca Coelho (1):
      cfg80211: use BIT_ULL in cfg80211_parse_mbssid_data()

Manikanta Pubbisetty (1):
      {nl,mac}80211: allow 4addr AP operation on crypto controlled devices

Maxim Mikityanskiy (1):
      wireless: Skip directory when generating certificates

Mordechay Goodstein (1):
      cfg80211: util: fix bit count off by one

Naftali Goldstein (1):
      mac80211: do not start any work during reconfigure flow

Pradeep Kumar Chitrapu (1):
      mac80211: free peer keys before vif down in mesh

Thomas Pedersen (1):
      mac80211: mesh: fix RCU warning

Yibo Zhao (1):
      mac80211: only warn once on chanctx_conf being NULL

Yu Wang (1):
      mac80211: handle deauthentication/disassociation from TDLS peer

YueHaibing (1):
      mac80211: remove set but not used variable 'old'

 drivers/net/wireless/mac80211_hwsim.c |  1 +
 include/net/cfg80211.h                |  3 +-
 net/mac80211/ieee80211_i.h            | 12 ++++-
 net/mac80211/key.c                    |  2 -
 net/mac80211/mesh.c                   |  6 ++-
 net/mac80211/mlme.c                   | 12 ++++-
 net/mac80211/rx.c                     |  2 +
 net/mac80211/tdls.c                   | 23 ++++++++
 net/mac80211/util.c                   |  8 ++-
 net/mac80211/wpa.c                    |  7 ++-
 net/wireless/Makefile                 |  1 +
 net/wireless/core.c                   |  8 ++-
 net/wireless/nl80211.c                | 99 ++++++++++++++++++++++++++---------
 net/wireless/pmsr.c                   |  4 +-
 net/wireless/scan.c                   |  4 +-
 net/wireless/util.c                   |  4 +-
 16 files changed, 156 insertions(+), 40 deletions(-)


^ permalink raw reply

* Re: [PATCH v2] Revert "cfg80211: fix processing world regdomain when non modular"
From: Hodaszi, Robert @ 2019-06-14 13:58 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless@vger.kernel.org
In-Reply-To: <ebab80c3f632f792373bfcace252c7a1bf65ce89.camel@sipsolutions.net>


------------------------------------------------------------------------
*From:* Johannes Berg <johannes@sipsolutions.net>
*Sent:* Friday, June 14, 2019 3:30PM
*To:* Hodaszi, Robert <Robert.Hodaszi@digi.com>
*Cc:* Linux-wireless <linux-wireless@vger.kernel.org>
*Subject:* Re: [PATCH v2] Revert "cfg80211: fix processing world 
regdomain when non modular"

> On Fri, 2019-06-14 at 13:16 +0000, Hodaszi, Robert wrote:
>> This reverts commit 96cce12ff6e0bc9d9fcb2235e08b7fc150f96fd2.
>>
>> Re-triggering a reg_process_hint with the last request on all events,
>> can make the regulatory domain fail in case of multiple WiFi modules. On
>> slower boards (espacially with mdev), enumeration of the WiFi modules
>> can end up in an intersected regulatory domain, and user cannot set it
>> with 'iw reg set' anymore.
>>
>> This is happening, because:
>> - 1st module enumerates, queues up a regulatory request
>> - request gets processed by __reg_process_hint_driver():
>>    - checks if previous was set by CORE -> yes
>>      - checks if regulator domain changed -> yes, from '00' to e.g. 'US'
>>        -> sends request to the 'crda'
>> - 2nd module enumerates, queues up a regulator request (which triggers
>> the reg_todo() work)
>> - reg_todo() -> reg_process_pending_hints() sees, that the last request
>> is not processed yet, so it tries to process it again.
>> __reg_process_hint driver() will run again, and:
>>    - checks if the last request's initiator was the core -> no, it was
>> the driver (1st WiFi module)
>>    - checks, if the previous initiator was the driver -> yes
>>      - checks if the regulator domain changed -> yes, it was '00' (set by
>> core, and crda call did not return yet), and should be changed to 'US'
>>
>> ------> __reg_process_hint_driver calls an intersect
>>
>> Besides, the reg_process_hint call with the last request is meaningless
>> since the crda call has a timeout work. If that timeout expires, the
>> first module's request will lost.
> It's pointless to resend when I still have the original patch pending,
> at least without any changes.
>
> That said, I looked at this today and I'm not sure how the code doesn't
> now have the original issue again?
>
> johannes
>

I didn't just resend that. I just realized, accidentally I forgot to fix 
the debug message printing function, that define doesn't exist anymore. 
Sorry for the confusion!

Under "original issue", you mean the issue, which commit 
96cce12ff6e0bc9d9fcb2235e08b7fc150f96fd2 (cfg80211: fix processing world 
regdomain when non modular) supposed to fix? That still won't work, but 
that didn't work neither before I reverted the patch, because crda call 
timeout will just drop the last packet. Also, as it re-processed the 
last request, not just resent it, it caused undesired states. Like when 
I used 2 WiFi modules with US regulatory domains, after enumeration, my 
global regulator domain was set to "Country 98".

To fix my issue, why I reverted the patch, and also fix the issue the 
reverted commit supposed to fix, I could imagine something like this. 
But I'm not sure, it doesn't have any side effect:

diff --git a/linux/net/wireless/reg.c b/linux/net/wireless/reg.c
index 6fdb01b20b..13d564558d 100644
--- a/linux/net/wireless/reg.c
+++ b/linux/net/wireless/reg.c
@@ -2798,7 +2798,8 @@ static void reg_process_pending_hints(void)

diff --git a/linux/net/wireless/reg.c b/linux/net/wireless/reg.c
index 6fdb01b20b..13d564558d 100644
--- a/linux/net/wireless/reg.c
+++ b/linux/net/wireless/reg.c
@@ -2798,7 +2798,8 @@ static void reg_process_pending_hints(void)

         /* When last_request->processed becomes true this will be 
rescheduled */
         if (lr && !lr->processed) {
-               reg_process_hint(lr);
+               if (!reg_query_database(lr))
+                       reg_free_request(lr);
                 return;
         }

@@ -3175,6 +3176,7 @@ static void restore_regulatory_settings(bool 
reset_user, bool cached)
         struct reg_beacon *reg_beacon, *btmp;
         LIST_HEAD(tmp_reg_req_list);
         struct cfg80211_registered_device *rdev;
+       struct regulatory_request *lr;

         ASSERT_RTNL();

@@ -3190,6 +3192,13 @@ static void restore_regulatory_settings(bool 
reset_user, bool cached)
         }
         spin_unlock(&reg_indoor_lock);

+       /* If last request is pending, save it, will resubmit it */
+       lr = get_last_request();
+       if (lr && !lr->processed)
+               rcu_assign_pointer(last_request, NULL);
+       else
+               lr = NULL;
+
         reset_regdomains(true, &world_regdom);
         restore_alpha2(alpha2, reset_user);

@@ -3267,6 +3276,9 @@ static void restore_regulatory_settings(bool 
reset_user, bool cached)
         list_splice_tail_init(&tmp_reg_req_list, &reg_requests_list);
         spin_unlock(&reg_requests_lock);

+       if (lr != NULL)
+               rcu_assign_pointer(last_request, lr);
+
         pr_debug("Kicking the queue\n");

         schedule_work(&reg_work);


Best regards,
Robert Hodaszi

^ permalink raw reply related

* Re: [PATCH v2] Revert "cfg80211: fix processing world regdomain when non modular"
From: Johannes Berg @ 2019-06-14 14:01 UTC (permalink / raw)
  To: Hodaszi, Robert; +Cc: linux-wireless@vger.kernel.org
In-Reply-To: <32951d52-3f9d-aaee-fa07-75585c03edba@digi.com>

On Fri, 2019-06-14 at 13:58 +0000, Hodaszi, Robert wrote:
> 
> I didn't just resend that. I just realized, accidentally I forgot to fix 
> the debug message printing function, that define doesn't exist anymore. 
> Sorry for the confusion!

Oops. I looked too superficially then and didn't even see the
difference, sorry.

I guess that's why Kalle always says you should have a patch changelog
:-)

> Under "original issue", you mean the issue, which commit 
> 96cce12ff6e0bc9d9fcb2235e08b7fc150f96fd2 (cfg80211: fix processing world 
> regdomain when non modular) supposed to fix? 

Yes.

> That still won't work, but 
> that didn't work neither before I reverted the patch, because crda call 
> timeout will just drop the last packet. Also, as it re-processed the 
> last request, not just resent it, it caused undesired states. Like when 
> I used 2 WiFi modules with US regulatory domains, after enumeration, my 
> global regulator domain was set to "Country 98".
> 
> To fix my issue, why I reverted the patch, and also fix the issue the 
> reverted commit supposed to fix, I could imagine something like this. 
> But I'm not sure, it doesn't have any side effect:

[snip]

Ok, thanks. I guess I'll have to look at this in more detail.

You don't happen to have a way to reproduce either issue with a hwsim
test case?

johannes


^ permalink raw reply

* Re: [PATCH 12/16] staging/comedi: mark as broken
From: Greg KH @ 2019-06-14 14:02 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Maarten Lankhorst, Maxime Ripard, Sean Paul, David Airlie,
	Daniel Vetter, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
	Ian Abbott, H Hartley Sweeten, devel, linux-s390,
	Intel Linux Wireless, linux-rdma, netdev, intel-gfx,
	linux-wireless, linux-kernel, dri-devel, linux-mm, iommu,
	moderated list:ARM PORT, linux-media
In-Reply-To: <20190614134726.3827-13-hch@lst.de>

On Fri, Jun 14, 2019 at 03:47:22PM +0200, Christoph Hellwig wrote:
> comedi_buf.c abuse the DMA API in gravely broken ways, as it assumes it
> can call virt_to_page on the result, and the just remap it as uncached
> using vmap.  Disable the driver until this API abuse has been fixed.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/staging/comedi/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/staging/comedi/Kconfig b/drivers/staging/comedi/Kconfig
> index 049b659fa6ad..e7c021d76cfa 100644
> --- a/drivers/staging/comedi/Kconfig
> +++ b/drivers/staging/comedi/Kconfig
> @@ -1,6 +1,7 @@
>  # SPDX-License-Identifier: GPL-2.0
>  config COMEDI
>  	tristate "Data acquisition support (comedi)"
> +	depends on BROKEN

Um, that's a huge sledgehammer.

Perhaps a hint as to how we can fix this up?  This is the first time
I've heard of the comedi code not handling dma properly.

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH v2] Revert "cfg80211: fix processing world regdomain when non modular"
From: Hodaszi, Robert @ 2019-06-14 14:06 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless@vger.kernel.org
In-Reply-To: <6a9c7642a2fcca60658036c605438ff2ac982bd0.camel@sipsolutions.net>


*From:* Johannes Berg <johannes@sipsolutions.net>
*Sent:* Friday, June 14, 2019 4:01PM
*To:* Hodaszi, Robert <Robert.Hodaszi@digi.com>
*Cc:* Linux-wireless <linux-wireless@vger.kernel.org>
*Subject:* Re: [PATCH v2] Revert "cfg80211: fix processing world 
regdomain when non modular"

> On Fri, 2019-06-14 at 13:58 +0000, Hodaszi, Robert wrote:
>> I didn't just resend that. I just realized, accidentally I forgot to fix
>> the debug message printing function, that define doesn't exist anymore.
>> Sorry for the confusion!
> Oops. I looked too superficially then and didn't even see the
> difference, sorry.
>
> I guess that's why Kalle always says you should have a patch changelog
> :-)
Shame on me, I was able to make a bug in a one-line change. :)
>
>> Under "original issue", you mean the issue, which commit
>> 96cce12ff6e0bc9d9fcb2235e08b7fc150f96fd2 (cfg80211: fix processing world
>> regdomain when non modular) supposed to fix?
> Yes.
>
>> That still won't work, but
>> that didn't work neither before I reverted the patch, because crda call
>> timeout will just drop the last packet. Also, as it re-processed the
>> last request, not just resent it, it caused undesired states. Like when
>> I used 2 WiFi modules with US regulatory domains, after enumeration, my
>> global regulator domain was set to "Country 98".
>>
>> To fix my issue, why I reverted the patch, and also fix the issue the
>> reverted commit supposed to fix, I could imagine something like this.
>> But I'm not sure, it doesn't have any side effect:
> [snip]
>
> Ok, thanks. I guess I'll have to look at this in more detail.
>
> You don't happen to have a way to reproduce either issue with a hwsim
> test case?
>
> johannes
>
To tell the truth, I never tried hwsim. But it's pretty trivial to repro 
it. You just need 2 WiFi modules, and put e.g. a "sleep 1" into the udev 
or mdev script, before it would call the "crda". That should trigger the 
issue immediately. After that change, I just did an "rmmod ath10k_pci; 
modprobe ath10k_pci", and bumm, "iw reg get" will should "Country 98".

Robert

^ permalink raw reply

* Re: [PATCH v3 0/3] mac80211/ath11k: HE mesh support
From: Johannes Berg @ 2019-06-14 14:10 UTC (permalink / raw)
  To: Sven Eckelmann, linux-wireless; +Cc: ath11k
In-Reply-To: <20190612193510.29489-1-sven@narfation.org>

Hi Sven,

Two comments:

1) It seems to me that patches 1/2 really should be in opposite order,
   since you can't really claim HE mesh support in hwsim when you don't
   have it in mac80211?
   Or maybe I misunderstand?

2) This series breaks the wpas_mesh_max_peering wpa_supplicant/hwsim
   test, and I'm not sure why. Perhaps some length calculations are bad?

johannes


^ permalink raw reply

* Re: [PATCH v3 2/3] mac80211: implement HE support for mesh
From: Johannes Berg @ 2019-06-14 14:11 UTC (permalink / raw)
  To: Sven Eckelmann, linux-wireless; +Cc: ath11k, Sven Eckelmann
In-Reply-To: <20190612193510.29489-3-sven@narfation.org>

On Wed, 2019-06-12 at 21:35 +0200, Sven Eckelmann wrote:
> 
> +u8 ieee80211_ie_len_he_cap(struct ieee80211_sub_if_data *sdata)
> +{
> +	const struct ieee80211_sta_he_cap *he_cap;
> +	struct ieee80211_supported_band *sband;
> +	u8 ie_len;
> +	u8 n;
> +
> +	sband = ieee80211_get_sband(sdata);
> +	if (!sband)
> +		return 0;
> +
> +	he_cap = ieee80211_get_he_mesh_cap(sband);
> +	if (!he_cap)
> +		return 0;
> +
> +	n = ieee80211_he_mcs_nss_size(&he_cap->he_cap_elem);
> +	ie_len = 2 + 1 +
> +		 sizeof(he_cap->he_cap_elem) + n +
> +		 ieee80211_he_ppe_size(he_cap->ppe_thres[0],
> +				       he_cap->he_cap_elem.phy_cap_info);
> +
> +	return ie_len;

There's no value in the "ie_len" variable here.

johannes


^ permalink raw reply

* Re: [PATCH v3 2/3] mac80211: implement HE support for mesh
From: Sven Eckelmann @ 2019-06-14 14:13 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, ath11k
In-Reply-To: <8d0d11ca39b6216b24cf4e9e3a1522072db5c0d2.camel@sipsolutions.net>

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

On Friday, 14 June 2019 16:11:15 CEST Johannes Berg wrote:
> > +       ie_len = 2 + 1 +
> > +                sizeof(he_cap->he_cap_elem) + n +
> > +                ieee80211_he_ppe_size(he_cap->ppe_thres[0],
> > +                                      he_cap->he_cap_elem.phy_cap_info);
> > +
> > +       return ie_len;
> 
> There's no value in the "ie_len" variable here.

Sorry, my parser just broke. Why is there no value in ie_len? Was just 
assigned a line before. Or are you talking about something else?

Kind regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH v3 2/3] mac80211: implement HE support for mesh
From: Johannes Berg @ 2019-06-14 14:14 UTC (permalink / raw)
  To: Sven Eckelmann; +Cc: linux-wireless, ath11k
In-Reply-To: <2194147.kqlnQF7k4C@bentobox>

On Fri, 2019-06-14 at 16:13 +0200, Sven Eckelmann wrote:
> On Friday, 14 June 2019 16:11:15 CEST Johannes Berg wrote:
> > > +       ie_len = 2 + 1 +
> > > +                sizeof(he_cap->he_cap_elem) + n +
> > > +                ieee80211_he_ppe_size(he_cap->ppe_thres[0],
> > > +                                      he_cap->he_cap_elem.phy_cap_info);
> > > +
> > > +       return ie_len;
> > 
> > There's no value in the "ie_len" variable here.
> 
> Sorry, my parser just broke. Why is there no value in ie_len? Was just 
> assigned a line before. Or are you talking about something else?

Heh, sorry. I mean, there's no value in having the variable, you could
just
	return 2 + 1 + ...

johannes


^ permalink raw reply

* RE: [PATCH 16/16] dma-mapping: use exact allocation in dma_alloc_contiguous
From: David Laight @ 2019-06-14 14:15 UTC (permalink / raw)
  To: 'Christoph Hellwig', Maarten Lankhorst, Maxime Ripard,
	Sean Paul, David Airlie, Daniel Vetter, Jani Nikula,
	Joonas Lahtinen, Rodrigo Vivi, Ian Abbott, H Hartley Sweeten
  Cc: Intel Linux Wireless, moderated list:ARM PORT,
	dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org,
	linux-rdma@vger.kernel.org, linux-media@vger.kernel.org,
	netdev@vger.kernel.org, linux-wireless@vger.kernel.org,
	linux-s390@vger.kernel.org, devel@driverdev.osuosl.org,
	linux-mm@kvack.org, iommu@lists.linux-foundation.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <20190614134726.3827-17-hch@lst.de>

From: Christoph Hellwig
> Sent: 14 June 2019 14:47
> 
> Many architectures (e.g. arm, m68 and sh) have always used exact
> allocation in their dma coherent allocator, which avoids a lot of
> memory waste especially for larger allocations.  Lift this behavior
> into the generic allocator so that dma-direct and the generic IOMMU
> code benefit from this behavior as well.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  include/linux/dma-contiguous.h |  8 +++++---
>  kernel/dma/contiguous.c        | 17 +++++++++++------
>  2 files changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/include/linux/dma-contiguous.h b/include/linux/dma-contiguous.h
> index c05d4e661489..2e542e314acf 100644
> --- a/include/linux/dma-contiguous.h
> +++ b/include/linux/dma-contiguous.h
> @@ -161,15 +161,17 @@ static inline struct page *dma_alloc_contiguous(struct device *dev, size_t size,
>  		gfp_t gfp)
>  {
>  	int node = dev ? dev_to_node(dev) : NUMA_NO_NODE;
> -	size_t align = get_order(PAGE_ALIGN(size));
> +	void *cpu_addr = alloc_pages_exact_node(node, size, gfp);
> 
> -	return alloc_pages_node(node, gfp, align);
> +	if (!cpu_addr)
> +		return NULL;
> +	return virt_to_page(p);
>  }

Does this still guarantee that requests for 16k will not cross a 16k boundary?
It looks like you are losing the alignment parameter.

There may be drivers and hardware that also require 12k allocates
to not cross 16k boundaries (etc).

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


^ permalink raw reply

* pull-request: mac80211-next 2019-06-14
From: Johannes Berg @ 2019-06-14 14:16 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-wireless

Hi Dave,

And ... here's a -next pull request. Nothing really major here,
see more details below.

Please pull and let me know if there's any problem.

Thanks,
johannes



The following changes since commit cec4f328c929f72ad634e8f385b589bd6eac80e5:

  enetc: fix le32/le16 degrading to integer warnings (2019-05-27 10:12:08 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git tags/mac80211-next-for-davem-2019-06-14

for you to fetch changes up to ddb754aa31813fd17d8374eba881827e6e2c85c6:

  mac80211: notify offchannel expire on mgmt_tx (2019-06-14 16:08:28 +0200)

----------------------------------------------------------------
Many changes all over:
 * HE (802.11ax) work continues
 * WPA3 offloads
 * work on extended key ID handling continues
 * fixes to honour AP supported rates with auth/assoc frames
 * nl80211 netlink policy improvements to fix some issues
   with strict validation on new commands with old attrs

----------------------------------------------------------------
Alexander Wetzel (1):
      mac80211: AMPDU handling for Extended Key ID

Chaitanya Tata (1):
      cfg80211: Handle bss expiry during connection

Chung-Hsien Hsu (3):
      nl80211: add NL80211_ATTR_IFINDEX to port authorized event
      nl80211: add WPA3 definition for SAE authentication
      nl80211: add support for SAE authentication offload

Greg Kroah-Hartman (1):
      mac80211: no need to check return value of debugfs_create functions

Ilan Peer (2):
      cfg80211: Add a function to iterate all BSS entries
      ieee80211: Add a missing extended capability flag definition

James Prestwood (2):
      nl80211: send event when CMD_FRAME duration expires
      mac80211: notify offchannel expire on mgmt_tx

Johannes Berg (6):
      nl80211: fill all policy .type entries
      nl80211: require and validate vendor command policy
      mac80211: call rate_control_send_low() internally
      mac80211: use STA info in rate_control_send_low()
      mac80211: fill low rate even for HAS_RATE_CONTROL
      mac80211: extend __rate_control_send_low warning

John Crispin (3):
      mac80211: add ieee80211_get_he_iftype_cap() helper
      mac80211: dynamically enable the TWT requester support on STA interfaces
      mac80211: allow turning TWT responder support on and off via netlink

 .../driver-api/80211/mac80211-advanced.rst         |   3 -
 drivers/net/wireless/intel/iwlegacy/3945-rs.c      |   3 -
 drivers/net/wireless/intel/iwlegacy/4965-rs.c      |   4 -
 drivers/net/wireless/intel/iwlwifi/dvm/rs.c        |   4 -
 drivers/net/wireless/intel/iwlwifi/mvm/rs.c        |   4 -
 drivers/net/wireless/mac80211_hwsim.c              |   2 +
 drivers/net/wireless/realtek/rtlwifi/rc.c          |   3 -
 include/linux/ieee80211.h                          |   8 ++
 include/net/cfg80211.h                             |  82 +++++++++--
 include/net/mac80211.h                             |  32 ++---
 include/net/netlink.h                              |   9 ++
 include/uapi/linux/nl80211.h                       |  24 ++++
 net/mac80211/cfg.c                                 |   8 +-
 net/mac80211/debugfs.c                             |   1 +
 net/mac80211/debugfs_key.c                         |   3 -
 net/mac80211/debugfs_netdev.c                      |  10 +-
 net/mac80211/debugfs_sta.c                         |   2 -
 net/mac80211/key.c                                 | 100 +++++++------
 net/mac80211/mlme.c                                |  25 +++-
 net/mac80211/offchannel.c                          |   4 +
 net/mac80211/rate.c                                |  27 ++--
 net/mac80211/rc80211_minstrel.c                    |   4 -
 net/mac80211/rc80211_minstrel_ht.c                 |   3 -
 net/mac80211/sta_info.c                            |  43 +++++-
 net/wireless/core.c                                |  13 ++
 net/wireless/core.h                                |   4 +
 net/wireless/nl80211.c                             | 155 +++++++++++++++++----
 net/wireless/scan.c                                |  33 ++++-
 net/wireless/sme.c                                 |  32 ++++-
 net/wireless/trace.h                               |  18 +++
 30 files changed, 495 insertions(+), 168 deletions(-)


^ permalink raw reply

* Re: [mac80211-next:master 17/20] drivers/net/wireless/intel/iwlwifi/dvm/rs.c:3317:3: error: 'const struct rate_control_ops' has no member named 'remove_sta_debugfs'; did you mean 'add_sta_debugfs'?
From: Greg Kroah-Hartman @ 2019-06-14 14:33 UTC (permalink / raw)
  To: Johannes Berg; +Cc: kbuild test robot, kbuild-all, linux-wireless
In-Reply-To: <201906142122.HSTAYprn%lkp@intel.com>

On Fri, Jun 14, 2019 at 09:17:28PM +0800, kbuild test robot wrote:
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git master
> head:   499169fb3a5a27b8322cdd559a09e79e390c211b
> commit: d3f7e94bc458dd1b20bef591ee2b82e2ae631858 [17/20] mac80211: remove unused and unneeded remove_sta_debugfs callback
> config: xtensa-allyesconfig (attached as .config)
> compiler: xtensa-linux-gcc (GCC) 7.4.0
> reproduce:
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         git checkout d3f7e94bc458dd1b20bef591ee2b82e2ae631858
>         # save the attached .config to linux build tree
>         GCC_VERSION=7.4.0 make.cross ARCH=xtensa 
> 
> If you fix the issue, kindly add following tag
> Reported-by: kbuild test robot <lkp@intel.com>
> 
> All errors (new ones prefixed by >>):
> 
> >> drivers/net/wireless/intel/iwlwifi/dvm/rs.c:3317:3: error: 'const struct rate_control_ops' has no member named 'remove_sta_debugfs'; did you mean 'add_sta_debugfs'?
>      .remove_sta_debugfs = rs_remove_debugfs,
>       ^~~~~~~~~~~~~~~~~~
>       add_sta_debugfs

Did you apply my "[PATCH 3/5] iwlwifi: dvm: no need to check return
value of debugfs_create function" patch also to this tree?  The 5th
patch in the series depended on it :(

thanks,

greg k-h


> >> drivers/net/wireless/intel/iwlwifi/dvm/rs.c:3317:24: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
>      .remove_sta_debugfs = rs_remove_debugfs,
>                            ^~~~~~~~~~~~~~~~~
>    drivers/net/wireless/intel/iwlwifi/dvm/rs.c:3317:24: note: (near initialization for 'rs_ops.get_expected_throughput')
>    cc1: some warnings being treated as errors
> --
> >> drivers/net/wireless/intel/iwlwifi/mvm/rs.c:4138:3: error: 'const struct rate_control_ops' has no member named 'remove_sta_debugfs'; did you mean 'add_sta_debugfs'?
>      .remove_sta_debugfs = rs_remove_sta_debugfs,
>       ^~~~~~~~~~~~~~~~~~
>       add_sta_debugfs
> >> drivers/net/wireless/intel/iwlwifi/mvm/rs.c:4138:24: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
>      .remove_sta_debugfs = rs_remove_sta_debugfs,
>                            ^~~~~~~~~~~~~~~~~~~~~
>    drivers/net/wireless/intel/iwlwifi/mvm/rs.c:4138:24: note: (near initialization for 'rs_mvm_ops_drv.get_expected_throughput')
>    cc1: some warnings being treated as errors
> --
> >> drivers/net//wireless/intel/iwlegacy/4965-rs.c:2815:3: error: 'const struct rate_control_ops' has no member named 'remove_sta_debugfs'; did you mean 'add_sta_debugfs'?
>      .remove_sta_debugfs = il4965_rs_remove_debugfs,
>       ^~~~~~~~~~~~~~~~~~
>       add_sta_debugfs
> >> drivers/net//wireless/intel/iwlegacy/4965-rs.c:2815:24: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
>      .remove_sta_debugfs = il4965_rs_remove_debugfs,
>                            ^~~~~~~~~~~~~~~~~~~~~~~~
>    drivers/net//wireless/intel/iwlegacy/4965-rs.c:2815:24: note: (near initialization for 'rs_4965_ops.get_expected_throughput')
>    cc1: some warnings being treated as errors
> 
> vim +3317 drivers/net/wireless/intel/iwlwifi/dvm/rs.c
> 
> 631ad703b drivers/net/wireless/iwlwifi/dvm/rs.c      Johannes Berg   2014-01-20  3305  
> 631ad703b drivers/net/wireless/iwlwifi/dvm/rs.c      Johannes Berg   2014-01-20  3306  static const struct rate_control_ops rs_ops = {
> b481de9ca drivers/net/wireless/iwlwifi/iwl-4965-rs.c Zhu Yi          2007-09-25  3307  	.name = RS_NAME,
> b481de9ca drivers/net/wireless/iwlwifi/iwl-4965-rs.c Zhu Yi          2007-09-25  3308  	.tx_status = rs_tx_status,
> b481de9ca drivers/net/wireless/iwlwifi/iwl-4965-rs.c Zhu Yi          2007-09-25  3309  	.get_rate = rs_get_rate,
> fe6b23dd3 drivers/net/wireless/iwlwifi/iwl-agn-rs.c  Reinette Chatre 2010-02-22  3310  	.rate_init = rs_rate_init_stub,
> b481de9ca drivers/net/wireless/iwlwifi/iwl-4965-rs.c Zhu Yi          2007-09-25  3311  	.alloc = rs_alloc,
> b481de9ca drivers/net/wireless/iwlwifi/iwl-4965-rs.c Zhu Yi          2007-09-25  3312  	.free = rs_free,
> b481de9ca drivers/net/wireless/iwlwifi/iwl-4965-rs.c Zhu Yi          2007-09-25  3313  	.alloc_sta = rs_alloc_sta,
> b481de9ca drivers/net/wireless/iwlwifi/iwl-4965-rs.c Zhu Yi          2007-09-25  3314  	.free_sta = rs_free_sta,
> 93dc646ad drivers/net/wireless/iwlwifi/iwl-4965-rs.c Zhu Yi          2007-09-27  3315  #ifdef CONFIG_MAC80211_DEBUGFS
> 93dc646ad drivers/net/wireless/iwlwifi/iwl-4965-rs.c Zhu Yi          2007-09-27  3316  	.add_sta_debugfs = rs_add_debugfs,
> 93dc646ad drivers/net/wireless/iwlwifi/iwl-4965-rs.c Zhu Yi          2007-09-27 @3317  	.remove_sta_debugfs = rs_remove_debugfs,
> 93dc646ad drivers/net/wireless/iwlwifi/iwl-4965-rs.c Zhu Yi          2007-09-27  3318  #endif
> b481de9ca drivers/net/wireless/iwlwifi/iwl-4965-rs.c Zhu Yi          2007-09-25  3319  };
> b481de9ca drivers/net/wireless/iwlwifi/iwl-4965-rs.c Zhu Yi          2007-09-25  3320  
> 
> :::::: The code at line 3317 was first introduced by commit
> :::::: 93dc646adb94127ca1c2e74275a85265ec57b9af [PATCH] iwlwifi: add debugfs framework to rate scale
> 
> :::::: TO: Zhu Yi <yi.zhu@intel.com>
> :::::: CC: David S. Miller <davem@sunset.davemloft.net>
> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation



^ permalink raw reply

* Re: [PATCH-v3 1/2] wireless: Support assoc-at-ms timer in sta-info
From: Ben Greear @ 2019-06-14 14:46 UTC (permalink / raw)
  To: Johannes Berg, linux-wireless
In-Reply-To: <21fa668485f4eb0a8056aac1797854f267d5f1e0.camel@sipsolutions.net>

On 6/14/19 6:38 AM, Johannes Berg wrote:
> On Mon, 2019-04-15 at 10:21 -0700, greearb@candelatech.com wrote:
>> From: Ben Greear <greearb@candelatech.com>
>>
>> Report time stamp of when sta became associated.
> 
> I guess that makes sense.
> 
>> Signed-off-by: Ben Greear <greearb@candelatech.com>
>> ---
>>   include/net/cfg80211.h       | 2 ++
>>   include/uapi/linux/nl80211.h | 2 ++
>>   net/wireless/nl80211.c       | 1 +
>>   3 files changed, 5 insertions(+)
>>
>> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
>> index f49eb1464b7a..a3ad78b9d9f4 100644
>> --- a/include/net/cfg80211.h
>> +++ b/include/net/cfg80211.h
>> @@ -1268,6 +1268,7 @@ struct cfg80211_tid_stats {
>>    *	indicate the relevant values in this struct for them
>>    * @connected_time: time(in secs) since a station is last connected
>>    * @inactive_time: time since last station activity (tx/rx) in milliseconds
>> + * @assoc_at_ms: time in ms of the last association
> 
> I think the "_at_ms" doesn't really make sense. "time in ms" also
> doesn't make sense as documentation. I think you need to say what should
> be assigned here.
> 
> Also, you're actually using ktime_get_real() for this, which again
> doesn't make much sense. For exposing an absolute time, I think we're
> better off with CLOCK_BOOTTIME like we use elsewhere:

The point of my patch was to allow 'iw' to return a more precise time
that the station has been associated, so I am not sure that BOOTIME is
a good thing to use for that?

Here are the pertinent parts of my iw patches:


diff --git a/station.c b/station.c
index 25cbbc3..e7738cc 100644
--- a/station.c
+++ b/station.c
@@ -314,6 +314,12 @@ static int print_sta_handler(struct nl_msg *msg, void *arg)
                 [NL80211_STA_INFO_ACK_SIGNAL_AVG] = { .type = NLA_U8 },
         };
         char *chain;
+       struct timeval now;
+       unsigned long long now_ms;
+
+       gettimeofday(&now, NULL);
+       now_ms = now.tv_sec * 1000;
+       now_ms += (now.tv_usec / 1000);

         nla_parse(tb, NL80211_ATTR_MAX, genlmsg_attrdata(gnlh, 0),
                   genlmsg_attrlen(gnlh, 0), NULL);
@@ -557,8 +563,11 @@ static int print_sta_handler(struct nl_msg *msg, void *arg)
         if (sinfo[NL80211_STA_INFO_CONNECTED_TIME])
                 printf("\n\tconnected time:\t%u seconds",
                         nla_get_u32(sinfo[NL80211_STA_INFO_CONNECTED_TIME]));
+       if (sinfo[NL80211_STA_INFO_ASSOC_AT_MS])
+               printf("\n\tassociated at:\t%llu ms",
+                        (unsigned long long)nla_get_u64(sinfo[NL80211_STA_INFO_ASSOC_AT_MS]));

-       printf("\n");
+       printf("\n\tcurrent time:\t%llu ms\n", now_ms);
         return NL_SKIP;
  }


Thanks,
Ben

> 
>   * @boottime_ns: CLOCK_BOOTTIME timestamp the frame was received at, this is
>   *      needed only for beacons and probe responses that update the scan cache.
> 
> 
>> + * @NL80211_STA_INFO_ASSOC_AT_MS: Timestamp of last association
> 
> _ASSOC_TIMESTAMP or something would make more sense too as the attribute
> name, and clearly the documentation needs to spell out the semantics
> here too.
> 
> johannes
> 


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com


^ permalink raw reply related


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