Linux RDMA and InfiniBand development
 help / color / mirror / Atom feed
* [PATCH v3 0/11] Fix race conditions related to stopping block layer queues
From: Bart Van Assche @ 2016-10-18 21:48 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Christoph Hellwig, James Bottomley, Martin K. Petersen,
	Mike Snitzer, Doug Ledford, Keith Busch, Ming Lin,
	Laurence Oberman, linux-block@vger.kernel.org,
	linux-scsi@vger.kernel.org, linux-rdma@vger.kernel.org,
	linux-nvme@lists.infradead.org

Hello Jens,

Multiple block drivers need the functionality to stop a request queue 
and to wait until all ongoing request_fn() / queue_rq() calls have 
finished without waiting until all outstanding requests have finished. 
Hence this patch series that introduces the blk_mq_quiesce_queue() and 
blk_mq_resume_queue() functions. The dm-mq, SRP and NVMe patches in this 
patch series are three examples of where these functions are useful. 
These patches have been tested on top of kernel v4.9-rc1. The following 
tests have been run to verify this patch series:
- My own srp-test suite that stress-tests SRP on top of dm-multipath.
- Mike's mptest suite that stress-tests dm-multipath.
- fio on top of the NVMeOF host driver that was connected to the NVMeOF
   target driver on the same host.
- Laurence verified the previous version (v2) of this patch series by
   running it through the Red Hat SRP test suite. Laurence also ran some
   NVMe tests (thanks Laurence!).

The changes compared to the second version of this patch series are:
- Changed the order of the patches in this patch series.
- Added several new patches: a patch that avoids that .queue_rq() gets
   invoked from the direct submission path if a queue has been stopped
   and also a patch that introduces the helper function
   blk_mq_hctx_stopped().
- blk_mq_quiesce_queue() has been reworked (thanks to Ming Lin and Sagi
   for their feedback).
- A bool 'kick' argument has been added to blk_mq_requeue_request().
- As proposed by Christoph, the code that waits for queuecommand() has
   been moved from the SRP transport driver to the SCSI core.

Changes between v2 and v1:
- Dropped the non-blk-mq changes from this patch series.
- Added support for harware queues with BLK_MQ_F_BLOCKING set.
- Added a call stack to the description of the dm race fix patch.
- Dropped the non-scsi-mq changes from the SRP patch.
- Added a patch that introduces blk_mq_queue_stopped() in the dm driver.

The individual patches in this series are:

0001-blk-mq-Do-not-invoke-.queue_rq-for-a-stopped-queue.patch
0002-blk-mq-Introduce-blk_mq_hctx_stopped.patch
0003-blk-mq-Introduce-blk_mq_queue_stopped.patch
0004-blk-mq-Introduce-blk_mq_quiesce_queue.patch
0005-blk-mq-Add-a-kick_requeue_list-argument-to-blk_mq_re.patch
0006-dm-Use-BLK_MQ_S_STOPPED-instead-of-QUEUE_FLAG_STOPPE.patch
0007-dm-Fix-a-race-condition-related-to-stopping-and-star.patch
0008-SRP-transport-Move-queuecommand-wait-code-to-SCSI-co.patch
0009-SRP-transport-scsi-mq-Wait-for-.queue_rq-if-necessar.patch
0010-nvme-Use-BLK_MQ_S_STOPPED-instead-of-QUEUE_FLAG_STOP.patch
0011-nvme-Fix-a-race-condition.patch

Thanks,

Bart.

^ permalink raw reply

* [PATCH v3 01/11] blk-mq: Do not invoke .queue_rq() for a stopped queue
From: Bart Van Assche @ 2016-10-18 21:48 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Christoph Hellwig, James Bottomley, Martin K. Petersen,
	Mike Snitzer, Doug Ledford, Keith Busch, Ming Lin,
	Laurence Oberman, linux-block@vger.kernel.org,
	linux-scsi@vger.kernel.org, linux-rdma@vger.kernel.org,
	linux-nvme@lists.infradead.org
In-Reply-To: <b39eb0e7-1007-eb63-8e7f-9a7f08508379@sandisk.com>

The meaning of the BLK_MQ_S_STOPPED flag is "do not call
.queue_rq()". Hence modify blk_mq_make_request() such that requests
are queued instead of issued if a queue has been stopped.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.com>
Cc: Sagi Grimberg <sagi@grimberg.me>
Cc: Johannes Thumshirn <jthumshirn@suse.de>
Cc: <stable@vger.kernel.org>
---
 block/blk-mq.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index ddc2eed..b5dcafb 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1332,9 +1332,9 @@ static blk_qc_t blk_mq_make_request(struct request_queue *q, struct bio *bio)
 		blk_mq_put_ctx(data.ctx);
 		if (!old_rq)
 			goto done;
-		if (!blk_mq_direct_issue_request(old_rq, &cookie))
-			goto done;
-		blk_mq_insert_request(old_rq, false, true, true);
+		if (test_bit(BLK_MQ_S_STOPPED, &data.hctx->state) ||
+		    blk_mq_direct_issue_request(old_rq, &cookie) != 0)
+			blk_mq_insert_request(old_rq, false, true, true);
 		goto done;
 	}
 
-- 
2.10.1


^ permalink raw reply related

* [PATCH v3 02/11] blk-mq: Introduce blk_mq_hctx_stopped()
From: Bart Van Assche @ 2016-10-18 21:49 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Christoph Hellwig, James Bottomley, Martin K. Petersen,
	Mike Snitzer, Doug Ledford, Keith Busch, Ming Lin,
	Laurence Oberman, linux-block@vger.kernel.org,
	linux-scsi@vger.kernel.org, linux-rdma@vger.kernel.org,
	linux-nvme@lists.infradead.org
In-Reply-To: <b39eb0e7-1007-eb63-8e7f-9a7f08508379@sandisk.com>

Multiple functions test the BLK_MQ_S_STOPPED bit so introduce
a helper function that performs this test.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.com>
Cc: Sagi Grimberg <sagi@grimberg.me>
Cc: Johannes Thumshirn <jthumshirn@suse.de>
---
 block/blk-mq.c         | 12 ++++++------
 drivers/md/dm-rq.c     |  2 +-
 include/linux/blk-mq.h |  5 +++++
 3 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index b5dcafb..b52b3a6 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -787,7 +787,7 @@ static void __blk_mq_run_hw_queue(struct blk_mq_hw_ctx *hctx)
 	struct list_head *dptr;
 	int queued;
 
-	if (unlikely(test_bit(BLK_MQ_S_STOPPED, &hctx->state)))
+	if (unlikely(blk_mq_hctx_stopped(hctx)))
 		return;
 
 	WARN_ON(!cpumask_test_cpu(raw_smp_processor_id(), hctx->cpumask) &&
@@ -912,8 +912,8 @@ static int blk_mq_hctx_next_cpu(struct blk_mq_hw_ctx *hctx)
 
 void blk_mq_run_hw_queue(struct blk_mq_hw_ctx *hctx, bool async)
 {
-	if (unlikely(test_bit(BLK_MQ_S_STOPPED, &hctx->state) ||
-	    !blk_mq_hw_queue_mapped(hctx)))
+	if (unlikely(blk_mq_hctx_stopped(hctx) ||
+		     !blk_mq_hw_queue_mapped(hctx)))
 		return;
 
 	if (!async && !(hctx->flags & BLK_MQ_F_BLOCKING)) {
@@ -938,7 +938,7 @@ void blk_mq_run_hw_queues(struct request_queue *q, bool async)
 	queue_for_each_hw_ctx(q, hctx, i) {
 		if ((!blk_mq_hctx_has_pending(hctx) &&
 		    list_empty_careful(&hctx->dispatch)) ||
-		    test_bit(BLK_MQ_S_STOPPED, &hctx->state))
+		    blk_mq_hctx_stopped(hctx))
 			continue;
 
 		blk_mq_run_hw_queue(hctx, async);
@@ -988,7 +988,7 @@ void blk_mq_start_stopped_hw_queues(struct request_queue *q, bool async)
 	int i;
 
 	queue_for_each_hw_ctx(q, hctx, i) {
-		if (!test_bit(BLK_MQ_S_STOPPED, &hctx->state))
+		if (!blk_mq_hctx_stopped(hctx))
 			continue;
 
 		clear_bit(BLK_MQ_S_STOPPED, &hctx->state);
@@ -1332,7 +1332,7 @@ static blk_qc_t blk_mq_make_request(struct request_queue *q, struct bio *bio)
 		blk_mq_put_ctx(data.ctx);
 		if (!old_rq)
 			goto done;
-		if (test_bit(BLK_MQ_S_STOPPED, &data.hctx->state) ||
+		if (blk_mq_hctx_stopped(data.hctx) ||
 		    blk_mq_direct_issue_request(old_rq, &cookie) != 0)
 			blk_mq_insert_request(old_rq, false, true, true);
 		goto done;
diff --git a/drivers/md/dm-rq.c b/drivers/md/dm-rq.c
index dc75bea..76d1666 100644
--- a/drivers/md/dm-rq.c
+++ b/drivers/md/dm-rq.c
@@ -909,7 +909,7 @@ static int dm_mq_queue_rq(struct blk_mq_hw_ctx *hctx,
 	 * hctx that it really shouldn't.  The following check guards
 	 * against this rarity (albeit _not_ race-free).
 	 */
-	if (unlikely(test_bit(BLK_MQ_S_STOPPED, &hctx->state)))
+	if (unlikely(blk_mq_hctx_stopped(hctx)))
 		return BLK_MQ_RQ_QUEUE_BUSY;
 
 	if (ti->type->busy && ti->type->busy(ti))
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 535ab2e..bb000c3 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -239,6 +239,11 @@ int blk_mq_reinit_tagset(struct blk_mq_tag_set *set);
 
 void blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set, int nr_hw_queues);
 
+static inline bool blk_mq_hctx_stopped(struct blk_mq_hw_ctx *hctx)
+{
+	return test_bit(BLK_MQ_S_STOPPED, &hctx->state);
+}
+
 /*
  * Driver command data is immediately after the request. So subtract request
  * size to get back to the original request, add request size to get the PDU.
-- 
2.10.1


^ permalink raw reply related

* [PATCH v3 03/11] blk-mq: Introduce blk_mq_queue_stopped()
From: Bart Van Assche @ 2016-10-18 21:49 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Christoph Hellwig, James Bottomley, Martin K. Petersen,
	Mike Snitzer, Doug Ledford, Keith Busch, Ming Lin,
	Laurence Oberman, linux-block@vger.kernel.org,
	linux-scsi@vger.kernel.org, linux-rdma@vger.kernel.org,
	linux-nvme@lists.infradead.org
In-Reply-To: <b39eb0e7-1007-eb63-8e7f-9a7f08508379@sandisk.com>

The function blk_queue_stopped() allows to test whether or not a
traditional request queue has been stopped. Introduce a helper
function that allows block drivers to query easily whether or not
one or more hardware contexts of a blk-mq queue have been stopped.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-mq.c         | 20 ++++++++++++++++++++
 include/linux/blk-mq.h |  1 +
 2 files changed, 21 insertions(+)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index b52b3a6..4643fa8 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -946,6 +946,26 @@ void blk_mq_run_hw_queues(struct request_queue *q, bool async)
 }
 EXPORT_SYMBOL(blk_mq_run_hw_queues);
 
+/**
+ * blk_mq_queue_stopped() - check whether one or more hctxs have been stopped
+ * @q: request queue.
+ *
+ * The caller is responsible for serializing this function against
+ * blk_mq_{start,stop}_hw_queue().
+ */
+bool blk_mq_queue_stopped(struct request_queue *q)
+{
+	struct blk_mq_hw_ctx *hctx;
+	int i;
+
+	queue_for_each_hw_ctx(q, hctx, i)
+		if (blk_mq_hctx_stopped(hctx))
+			return true;
+
+	return false;
+}
+EXPORT_SYMBOL(blk_mq_queue_stopped);
+
 void blk_mq_stop_hw_queue(struct blk_mq_hw_ctx *hctx)
 {
 	cancel_work(&hctx->run_work);
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index bb000c3..523376a 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -223,6 +223,7 @@ void blk_mq_delay_kick_requeue_list(struct request_queue *q, unsigned long msecs
 void blk_mq_abort_requeue_list(struct request_queue *q);
 void blk_mq_complete_request(struct request *rq, int error);
 
+bool blk_mq_queue_stopped(struct request_queue *q);
 void blk_mq_stop_hw_queue(struct blk_mq_hw_ctx *hctx);
 void blk_mq_start_hw_queue(struct blk_mq_hw_ctx *hctx);
 void blk_mq_stop_hw_queues(struct request_queue *q);
-- 
2.10.1


^ permalink raw reply related

* [PATCH v3 04/11] blk-mq: Introduce blk_mq_quiesce_queue()
From: Bart Van Assche @ 2016-10-18 21:50 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Christoph Hellwig, James Bottomley, Martin K. Petersen,
	Mike Snitzer, Doug Ledford, Keith Busch, Ming Lin,
	Laurence Oberman, linux-block@vger.kernel.org,
	linux-scsi@vger.kernel.org, linux-rdma@vger.kernel.org,
	linux-nvme@lists.infradead.org
In-Reply-To: <b39eb0e7-1007-eb63-8e7f-9a7f08508379@sandisk.com>

blk_mq_quiesce_queue() waits until ongoing .queue_rq() invocations
have finished. This function does *not* wait until all outstanding
requests have finished (this means invocation of request.end_io()).

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Ming Lei <tom.leiming@gmail.com>
Cc: Hannes Reinecke <hare@suse.com>
Cc: Johannes Thumshirn <jthumshirn@suse.de>
---
 block/blk-mq.c         | 78 ++++++++++++++++++++++++++++++++++++++++++++------
 include/linux/blk-mq.h |  3 ++
 include/linux/blkdev.h |  1 +
 3 files changed, 73 insertions(+), 9 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 4643fa8..d41ed92 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -115,6 +115,30 @@ void blk_mq_unfreeze_queue(struct request_queue *q)
 }
 EXPORT_SYMBOL_GPL(blk_mq_unfreeze_queue);
 
+/**
+ * blk_mq_quiesce_queue() - wait until all ongoing queue_rq calls have finished
+ *
+ * Note: this function does not prevent that the struct request end_io()
+ * callback function is invoked. Additionally, it is not prevented that
+ * new queue_rq() calls occur unless the queue has been stopped first.
+ */
+void blk_mq_quiesce_queue(struct request_queue *q)
+{
+	struct blk_mq_hw_ctx *hctx;
+	unsigned int i;
+	bool rcu = false;
+
+	queue_for_each_hw_ctx(q, hctx, i) {
+		if (hctx->flags & BLK_MQ_F_BLOCKING)
+			synchronize_srcu(&hctx->queue_rq_srcu);
+		else
+			rcu = true;
+	}
+	if (rcu)
+		synchronize_rcu();
+}
+EXPORT_SYMBOL_GPL(blk_mq_quiesce_queue);
+
 void blk_mq_wake_waiters(struct request_queue *q)
 {
 	struct blk_mq_hw_ctx *hctx;
@@ -778,7 +802,7 @@ static inline unsigned int queued_to_index(unsigned int queued)
  * of IO. In particular, we'd like FIFO behaviour on handling existing
  * items on the hctx->dispatch list. Ignore that for now.
  */
-static void __blk_mq_run_hw_queue(struct blk_mq_hw_ctx *hctx)
+static void blk_mq_process_rq_list(struct blk_mq_hw_ctx *hctx)
 {
 	struct request_queue *q = hctx->queue;
 	struct request *rq;
@@ -790,9 +814,6 @@ static void __blk_mq_run_hw_queue(struct blk_mq_hw_ctx *hctx)
 	if (unlikely(blk_mq_hctx_stopped(hctx)))
 		return;
 
-	WARN_ON(!cpumask_test_cpu(raw_smp_processor_id(), hctx->cpumask) &&
-		cpu_online(hctx->next_cpu));
-
 	hctx->run++;
 
 	/*
@@ -883,6 +904,24 @@ static void __blk_mq_run_hw_queue(struct blk_mq_hw_ctx *hctx)
 	}
 }
 
+static void __blk_mq_run_hw_queue(struct blk_mq_hw_ctx *hctx)
+{
+	int srcu_idx;
+
+	WARN_ON(!cpumask_test_cpu(raw_smp_processor_id(), hctx->cpumask) &&
+		cpu_online(hctx->next_cpu));
+
+	if (!(hctx->flags & BLK_MQ_F_BLOCKING)) {
+		rcu_read_lock();
+		blk_mq_process_rq_list(hctx);
+		rcu_read_unlock();
+	} else {
+		srcu_idx = srcu_read_lock(&hctx->queue_rq_srcu);
+		blk_mq_process_rq_list(hctx);
+		srcu_read_unlock(&hctx->queue_rq_srcu, srcu_idx);
+	}
+}
+
 /*
  * It'd be great if the workqueue API had a way to pass
  * in a mask and had some smarts for more clever placement.
@@ -1278,6 +1317,14 @@ static int blk_mq_direct_issue_request(struct request *rq, blk_qc_t *cookie)
 	return -1;
 }
 
+static void blk_mq_try_issue_directly(struct blk_mq_hw_ctx *hctx,
+				      struct request *rq, blk_qc_t *cookie)
+{
+	if (blk_mq_hctx_stopped(hctx) ||
+	    blk_mq_direct_issue_request(rq, cookie) != 0)
+		blk_mq_insert_request(rq, false, true, true);
+}
+
 /*
  * Multiple hardware queue variant. This will not use per-process plugs,
  * but will attempt to bypass the hctx queueing if we can go straight to
@@ -1289,7 +1336,7 @@ static blk_qc_t blk_mq_make_request(struct request_queue *q, struct bio *bio)
 	const int is_flush_fua = bio->bi_opf & (REQ_PREFLUSH | REQ_FUA);
 	struct blk_map_ctx data;
 	struct request *rq;
-	unsigned int request_count = 0;
+	unsigned int request_count = 0, srcu_idx;
 	struct blk_plug *plug;
 	struct request *same_queue_rq = NULL;
 	blk_qc_t cookie;
@@ -1332,7 +1379,7 @@ static blk_qc_t blk_mq_make_request(struct request_queue *q, struct bio *bio)
 		blk_mq_bio_to_request(rq, bio);
 
 		/*
-		 * We do limited pluging. If the bio can be merged, do that.
+		 * We do limited plugging. If the bio can be merged, do that.
 		 * Otherwise the existing request in the plug list will be
 		 * issued. So the plug list will have one request at most
 		 */
@@ -1352,9 +1399,16 @@ static blk_qc_t blk_mq_make_request(struct request_queue *q, struct bio *bio)
 		blk_mq_put_ctx(data.ctx);
 		if (!old_rq)
 			goto done;
-		if (blk_mq_hctx_stopped(data.hctx) ||
-		    blk_mq_direct_issue_request(old_rq, &cookie) != 0)
-			blk_mq_insert_request(old_rq, false, true, true);
+
+		if (!(data.hctx->flags & BLK_MQ_F_BLOCKING)) {
+			rcu_read_lock();
+			blk_mq_try_issue_directly(data.hctx, old_rq, &cookie);
+			rcu_read_unlock();
+		} else {
+			srcu_idx = srcu_read_lock(&data.hctx->queue_rq_srcu);
+			blk_mq_try_issue_directly(data.hctx, old_rq, &cookie);
+			srcu_read_unlock(&data.hctx->queue_rq_srcu, srcu_idx);
+		}
 		goto done;
 	}
 
@@ -1633,6 +1687,9 @@ static void blk_mq_exit_hctx(struct request_queue *q,
 	if (set->ops->exit_hctx)
 		set->ops->exit_hctx(hctx, hctx_idx);
 
+	if (hctx->flags & BLK_MQ_F_BLOCKING)
+		cleanup_srcu_struct(&hctx->queue_rq_srcu);
+
 	blk_mq_remove_cpuhp(hctx);
 	blk_free_flush_queue(hctx->fq);
 	sbitmap_free(&hctx->ctx_map);
@@ -1713,6 +1770,9 @@ static int blk_mq_init_hctx(struct request_queue *q,
 				   flush_start_tag + hctx_idx, node))
 		goto free_fq;
 
+	if (hctx->flags & BLK_MQ_F_BLOCKING)
+		init_srcu_struct(&hctx->queue_rq_srcu);
+
 	return 0;
 
  free_fq:
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 523376a..02c3918 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -3,6 +3,7 @@
 
 #include <linux/blkdev.h>
 #include <linux/sbitmap.h>
+#include <linux/srcu.h>
 
 struct blk_mq_tags;
 struct blk_flush_queue;
@@ -35,6 +36,8 @@ struct blk_mq_hw_ctx {
 
 	struct blk_mq_tags	*tags;
 
+	struct srcu_struct	queue_rq_srcu;
+
 	unsigned long		queued;
 	unsigned long		run;
 #define BLK_MQ_MAX_DISPATCH_ORDER	7
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index c47c358..8259d87 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -824,6 +824,7 @@ extern void __blk_run_queue(struct request_queue *q);
 extern void __blk_run_queue_uncond(struct request_queue *q);
 extern void blk_run_queue(struct request_queue *);
 extern void blk_run_queue_async(struct request_queue *q);
+extern void blk_mq_quiesce_queue(struct request_queue *q);
 extern int blk_rq_map_user(struct request_queue *, struct request *,
 			   struct rq_map_data *, void __user *, unsigned long,
 			   gfp_t);
-- 
2.10.1


^ permalink raw reply related

* [PATCH v3 05/11] blk-mq: Add a kick_requeue_list argument to blk_mq_requeue_request()
From: Bart Van Assche @ 2016-10-18 21:51 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Christoph Hellwig, James Bottomley, Martin K. Petersen,
	Mike Snitzer, Doug Ledford, Keith Busch, Ming Lin,
	Laurence Oberman,
	linux-block-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <b39eb0e7-1007-eb63-8e7f-9a7f08508379-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>

Most blk_mq_requeue_request() and blk_mq_add_to_requeue_list() calls
are followed by kicking the requeue list. Hence add an argument to
these two functions that allows to kick the requeue list. This was
proposed by Christoph Hellwig.

Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
Cc: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
Cc: Hannes Reinecke <hare-IBi9RG/b67k@public.gmane.org>
Cc: Sagi Grimberg <sagi-NQWnxTmZq1alnMjI0IkVqw@public.gmane.org>
Cc: Johannes Thumshirn <jthumshirn-l3A5Bk7waGM@public.gmane.org>
---
 block/blk-flush.c            |  5 +----
 block/blk-mq.c               | 10 +++++++---
 drivers/block/xen-blkfront.c |  2 +-
 drivers/md/dm-rq.c           |  2 +-
 drivers/nvme/host/core.c     |  2 +-
 drivers/scsi/scsi_lib.c      |  4 +---
 include/linux/blk-mq.h       |  5 +++--
 7 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/block/blk-flush.c b/block/blk-flush.c
index 6a14b68..a834aed 100644
--- a/block/blk-flush.c
+++ b/block/blk-flush.c
@@ -134,10 +134,7 @@ static void blk_flush_restore_request(struct request *rq)
 static bool blk_flush_queue_rq(struct request *rq, bool add_front)
 {
 	if (rq->q->mq_ops) {
-		struct request_queue *q = rq->q;
-
-		blk_mq_add_to_requeue_list(rq, add_front);
-		blk_mq_kick_requeue_list(q);
+		blk_mq_add_to_requeue_list(rq, add_front, true);
 		return false;
 	} else {
 		if (add_front)
diff --git a/block/blk-mq.c b/block/blk-mq.c
index d41ed92..b0c8b44 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -491,12 +491,12 @@ static void __blk_mq_requeue_request(struct request *rq)
 	}
 }
 
-void blk_mq_requeue_request(struct request *rq)
+void blk_mq_requeue_request(struct request *rq, bool kick_requeue_list)
 {
 	__blk_mq_requeue_request(rq);
 
 	BUG_ON(blk_queued_rq(rq));
-	blk_mq_add_to_requeue_list(rq, true);
+	blk_mq_add_to_requeue_list(rq, true, kick_requeue_list);
 }
 EXPORT_SYMBOL(blk_mq_requeue_request);
 
@@ -534,7 +534,8 @@ static void blk_mq_requeue_work(struct work_struct *work)
 	blk_mq_start_hw_queues(q);
 }
 
-void blk_mq_add_to_requeue_list(struct request *rq, bool at_head)
+void blk_mq_add_to_requeue_list(struct request *rq, bool at_head,
+				bool kick_requeue_list)
 {
 	struct request_queue *q = rq->q;
 	unsigned long flags;
@@ -553,6 +554,9 @@ void blk_mq_add_to_requeue_list(struct request *rq, bool at_head)
 		list_add_tail(&rq->queuelist, &q->requeue_list);
 	}
 	spin_unlock_irqrestore(&q->requeue_lock, flags);
+
+	if (kick_requeue_list)
+		blk_mq_kick_requeue_list(q);
 }
 EXPORT_SYMBOL(blk_mq_add_to_requeue_list);
 
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 9908597..1ca702d 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -2043,7 +2043,7 @@ static int blkif_recover(struct blkfront_info *info)
 		/* Requeue pending requests (flush or discard) */
 		list_del_init(&req->queuelist);
 		BUG_ON(req->nr_phys_segments > segs);
-		blk_mq_requeue_request(req);
+		blk_mq_requeue_request(req, false);
 	}
 	blk_mq_kick_requeue_list(info->rq);
 
diff --git a/drivers/md/dm-rq.c b/drivers/md/dm-rq.c
index 76d1666..d5cec26 100644
--- a/drivers/md/dm-rq.c
+++ b/drivers/md/dm-rq.c
@@ -354,7 +354,7 @@ EXPORT_SYMBOL(dm_mq_kick_requeue_list);
 
 static void dm_mq_delay_requeue_request(struct request *rq, unsigned long msecs)
 {
-	blk_mq_requeue_request(rq);
+	blk_mq_requeue_request(rq, false);
 	__dm_mq_kick_requeue_list(rq->q, msecs);
 }
 
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 329381a..e4a6f2d 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -203,7 +203,7 @@ void nvme_requeue_req(struct request *req)
 {
 	unsigned long flags;
 
-	blk_mq_requeue_request(req);
+	blk_mq_requeue_request(req, false);
 	spin_lock_irqsave(req->q->queue_lock, flags);
 	if (!blk_queue_stopped(req->q))
 		blk_mq_kick_requeue_list(req->q);
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 2cca9cf..ab5b06f 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -86,10 +86,8 @@ scsi_set_blocked(struct scsi_cmnd *cmd, int reason)
 static void scsi_mq_requeue_cmd(struct scsi_cmnd *cmd)
 {
 	struct scsi_device *sdev = cmd->device;
-	struct request_queue *q = cmd->request->q;
 
-	blk_mq_requeue_request(cmd->request);
-	blk_mq_kick_requeue_list(q);
+	blk_mq_requeue_request(cmd->request, true);
 	put_device(&sdev->sdev_gendev);
 }
 
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 02c3918..1fcdc04 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -218,8 +218,9 @@ void blk_mq_start_request(struct request *rq);
 void blk_mq_end_request(struct request *rq, int error);
 void __blk_mq_end_request(struct request *rq, int error);
 
-void blk_mq_requeue_request(struct request *rq);
-void blk_mq_add_to_requeue_list(struct request *rq, bool at_head);
+void blk_mq_requeue_request(struct request *rq, bool kick_requeue_list);
+void blk_mq_add_to_requeue_list(struct request *rq, bool at_head,
+				bool kick_requeue_list);
 void blk_mq_cancel_requeue_work(struct request_queue *q);
 void blk_mq_kick_requeue_list(struct request_queue *q);
 void blk_mq_delay_kick_requeue_list(struct request_queue *q, unsigned long msecs);
-- 
2.10.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH v3 06/11] dm: Use BLK_MQ_S_STOPPED instead of QUEUE_FLAG_STOPPED in blk-mq code
From: Bart Van Assche @ 2016-10-18 21:51 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Christoph Hellwig, James Bottomley, Martin K. Petersen,
	Mike Snitzer, Doug Ledford, Keith Busch, Ming Lin,
	Laurence Oberman, linux-block@vger.kernel.org,
	linux-scsi@vger.kernel.org, linux-rdma@vger.kernel.org,
	linux-nvme@lists.infradead.org
In-Reply-To: <b39eb0e7-1007-eb63-8e7f-9a7f08508379@sandisk.com>

Instead of manipulating both QUEUE_FLAG_STOPPED and BLK_MQ_S_STOPPED
in the dm start and stop queue functions, only manipulate the latter
flag.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Mike Snitzer <snitzer@redhat.com>
---
 drivers/md/dm-rq.c | 18 ++----------------
 1 file changed, 2 insertions(+), 16 deletions(-)

diff --git a/drivers/md/dm-rq.c b/drivers/md/dm-rq.c
index d5cec26..9c34606 100644
--- a/drivers/md/dm-rq.c
+++ b/drivers/md/dm-rq.c
@@ -75,12 +75,6 @@ static void dm_old_start_queue(struct request_queue *q)
 
 static void dm_mq_start_queue(struct request_queue *q)
 {
-	unsigned long flags;
-
-	spin_lock_irqsave(q->queue_lock, flags);
-	queue_flag_clear(QUEUE_FLAG_STOPPED, q);
-	spin_unlock_irqrestore(q->queue_lock, flags);
-
 	blk_mq_start_stopped_hw_queues(q, true);
 	blk_mq_kick_requeue_list(q);
 }
@@ -105,16 +99,8 @@ static void dm_old_stop_queue(struct request_queue *q)
 
 static void dm_mq_stop_queue(struct request_queue *q)
 {
-	unsigned long flags;
-
-	spin_lock_irqsave(q->queue_lock, flags);
-	if (blk_queue_stopped(q)) {
-		spin_unlock_irqrestore(q->queue_lock, flags);
+	if (blk_mq_queue_stopped(q))
 		return;
-	}
-
-	queue_flag_set(QUEUE_FLAG_STOPPED, q);
-	spin_unlock_irqrestore(q->queue_lock, flags);
 
 	/* Avoid that requeuing could restart the queue. */
 	blk_mq_cancel_requeue_work(q);
@@ -341,7 +327,7 @@ static void __dm_mq_kick_requeue_list(struct request_queue *q, unsigned long mse
 	unsigned long flags;
 
 	spin_lock_irqsave(q->queue_lock, flags);
-	if (!blk_queue_stopped(q))
+	if (!blk_mq_queue_stopped(q))
 		blk_mq_delay_kick_requeue_list(q, msecs);
 	spin_unlock_irqrestore(q->queue_lock, flags);
 }
-- 
2.10.1


^ permalink raw reply related

* Re: [PATCHv12 0/3] rdmacg: IB/core: rdma controller support
From: Tejun Heo @ 2016-10-18 21:51 UTC (permalink / raw)
  To: Parav Pandit
  Cc: Leon Romanovsky, cgroups-u79uwXL29TY76Z2rM5mHXA, linux-rdma,
	Li Zefan, Johannes Weiner, Doug Ledford, Christoph Hellwig,
	Liran Liss, Hefty, Sean, Jason Gunthorpe, Haggai Eran,
	james.l.morris-QHcLZuEGTsvQT0dZR+AlfA, Or Gerlitz, Matan Barak
In-Reply-To: <CAG53R5UciPpa5d8BWyR-tks3LBrBwRCN2NyBbbm1e3EE-OWSYQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Hello,

On Wed, Oct 19, 2016 at 01:32:01AM +0530, Parav Pandit wrote:
> On Fri, Oct 14, 2016 at 4:44 AM, Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> > I think what you want is just a way to specify absolute limits using
> > percentages of what's available at the time of configuration -
> > e.g. being able to say "allow upto 30% of what's available in the
> > parent".
> 
> Yes. I am concerned about how to configure < 1% value to avoid
> floating point math in kernel as thats discouraged.
> Configuring in range of 1 to 100% for a given resource limits to only
> 100 or less cgroup instances which I think is not desired.

Heh, we can go for per-mil and use %0 as the suffix if absolutely
necessary but is this a real issue?

> > If so, the simplest way would be simply updating the
> > existing knobs to accept % inputs in addition to absolute values on
> > writes.
> 
> I was not sure to overload rdma.max file for accepting % inputs as
> thats not done in other cgroups. So I was thinking more of weights
> interface which avoids floating point problem and also allows much
> wider configuration range.

I think it's a lot more consistent to implement all absoulte limits
through max.  weight is for actual proportional control which this
isn't.  It's just a fancy way of specifying absolute limits.

Thanks.

-- 
tejun

^ permalink raw reply

* [PATCH v3 07/11] dm: Fix a race condition related to stopping and starting queues
From: Bart Van Assche @ 2016-10-18 21:52 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Christoph Hellwig, James Bottomley, Martin K. Petersen,
	Mike Snitzer, Doug Ledford, Keith Busch, Ming Lin,
	Laurence Oberman, linux-block@vger.kernel.org,
	linux-scsi@vger.kernel.org, linux-rdma@vger.kernel.org,
	linux-nvme@lists.infradead.org
In-Reply-To: <b39eb0e7-1007-eb63-8e7f-9a7f08508379@sandisk.com>

Ensure that all ongoing dm_mq_queue_rq() and dm_mq_requeue_request()
calls have stopped before setting the "queue stopped" flag. This
allows to remove the "queue stopped" test from dm_mq_queue_rq() and
dm_mq_requeue_request(). This patch fixes a race condition because
dm_mq_queue_rq() is called without holding the queue lock and hence
BLK_MQ_S_STOPPED can be set at any time while dm_mq_queue_rq() is
in progress. This patch prevents that the following hang occurs
sporadically when using dm-mq:

INFO: task systemd-udevd:10111 blocked for more than 480 seconds.
Call Trace:
 [<ffffffff8161f397>] schedule+0x37/0x90
 [<ffffffff816239ef>] schedule_timeout+0x27f/0x470
 [<ffffffff8161e76f>] io_schedule_timeout+0x9f/0x110
 [<ffffffff8161fb36>] bit_wait_io+0x16/0x60
 [<ffffffff8161f929>] __wait_on_bit_lock+0x49/0xa0
 [<ffffffff8114fe69>] __lock_page+0xb9/0xc0
 [<ffffffff81165d90>] truncate_inode_pages_range+0x3e0/0x760
 [<ffffffff81166120>] truncate_inode_pages+0x10/0x20
 [<ffffffff81212a20>] kill_bdev+0x30/0x40
 [<ffffffff81213d41>] __blkdev_put+0x71/0x360
 [<ffffffff81214079>] blkdev_put+0x49/0x170
 [<ffffffff812141c0>] blkdev_close+0x20/0x30
 [<ffffffff811d48e8>] __fput+0xe8/0x1f0
 [<ffffffff811d4a29>] ____fput+0x9/0x10
 [<ffffffff810842d3>] task_work_run+0x83/0xb0
 [<ffffffff8106606e>] do_exit+0x3ee/0xc40
 [<ffffffff8106694b>] do_group_exit+0x4b/0xc0
 [<ffffffff81073d9a>] get_signal+0x2ca/0x940
 [<ffffffff8101bf43>] do_signal+0x23/0x660
 [<ffffffff810022b3>] exit_to_usermode_loop+0x73/0xb0
 [<ffffffff81002cb0>] syscall_return_slowpath+0xb0/0xc0
 [<ffffffff81624e33>] entry_SYSCALL_64_fastpath+0xa6/0xa8

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
Cc: Mike Snitzer <snitzer@redhat.com>
---
 drivers/md/dm-rq.c | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/drivers/md/dm-rq.c b/drivers/md/dm-rq.c
index 9c34606..107ed19 100644
--- a/drivers/md/dm-rq.c
+++ b/drivers/md/dm-rq.c
@@ -105,6 +105,8 @@ static void dm_mq_stop_queue(struct request_queue *q)
 	/* Avoid that requeuing could restart the queue. */
 	blk_mq_cancel_requeue_work(q);
 	blk_mq_stop_hw_queues(q);
+	/* Wait until dm_mq_queue_rq() has finished. */
+	blk_mq_quiesce_queue(q);
 }
 
 void dm_stop_queue(struct request_queue *q)
@@ -887,17 +889,6 @@ static int dm_mq_queue_rq(struct blk_mq_hw_ctx *hctx,
 		dm_put_live_table(md, srcu_idx);
 	}
 
-	/*
-	 * On suspend dm_stop_queue() handles stopping the blk-mq
-	 * request_queue BUT: even though the hw_queues are marked
-	 * BLK_MQ_S_STOPPED at that point there is still a race that
-	 * is allowing block/blk-mq.c to call ->queue_rq against a
-	 * hctx that it really shouldn't.  The following check guards
-	 * against this rarity (albeit _not_ race-free).
-	 */
-	if (unlikely(blk_mq_hctx_stopped(hctx)))
-		return BLK_MQ_RQ_QUEUE_BUSY;
-
 	if (ti->type->busy && ti->type->busy(ti))
 		return BLK_MQ_RQ_QUEUE_BUSY;
 
-- 
2.10.1


^ permalink raw reply related

* [PATCH v3 08/11] SRP transport: Move queuecommand() wait code to SCSI core
From: Bart Van Assche @ 2016-10-18 21:52 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Christoph Hellwig, James Bottomley, Martin K. Petersen,
	Mike Snitzer, Doug Ledford, Keith Busch, Ming Lin,
	Laurence Oberman, linux-block@vger.kernel.org,
	linux-scsi@vger.kernel.org, linux-rdma@vger.kernel.org,
	linux-nvme@lists.infradead.org
In-Reply-To: <b39eb0e7-1007-eb63-8e7f-9a7f08508379@sandisk.com>

Additionally, add a comment about the queuecommand() call from
scsi_send_eh_cmnd().

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: James Bottomley <jejb@linux.vnet.ibm.com>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Sagi Grimberg <sagi@grimberg.me>
Cc: Doug Ledford <dledford@redhat.com>
---
 drivers/scsi/scsi_lib.c           | 40 +++++++++++++++++++++++++++++++++++++++
 drivers/scsi/scsi_transport_srp.c | 35 ++--------------------------------
 include/scsi/scsi_host.h          |  1 +
 3 files changed, 43 insertions(+), 33 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index ab5b06f..a5a1b5d 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2722,6 +2722,46 @@ void sdev_evt_send_simple(struct scsi_device *sdev,
 EXPORT_SYMBOL_GPL(sdev_evt_send_simple);
 
 /**
+ * scsi_request_fn_active() - number of kernel threads inside scsi_request_fn()
+ * @shost: SCSI host for which to count the number of scsi_request_fn() callers.
+ *
+ * To do: add support for scsi-mq in this function.
+ */
+static int scsi_request_fn_active(struct Scsi_Host *shost)
+{
+	struct scsi_device *sdev;
+	struct request_queue *q;
+	int request_fn_active = 0;
+
+	shost_for_each_device(sdev, shost) {
+		q = sdev->request_queue;
+
+		spin_lock_irq(q->queue_lock);
+		request_fn_active += q->request_fn_active;
+		spin_unlock_irq(q->queue_lock);
+	}
+
+	return request_fn_active;
+}
+
+/**
+ * scsi_wait_for_queuecommand() - wait for ongoing queuecommand() calls
+ *
+ * Wait until the ongoing shost->hostt->queuecommand() calls that are
+ * invoked from scsi_request_fn() have finished.
+ *
+ * To do: avoid that scsi_send_eh_cmnd() calls queuecommand() after
+ * scsi_internal_device_block() has blocked a SCSI device and remove and also
+ * remove the rport mutex lock and unlock calls from srp_queuecommand().
+ */
+void scsi_wait_for_queuecommand(struct Scsi_Host *shost)
+{
+	while (scsi_request_fn_active(shost))
+		msleep(20);
+}
+EXPORT_SYMBOL(scsi_wait_for_queuecommand);
+
+/**
  *	scsi_device_quiesce - Block user issued commands.
  *	@sdev:	scsi device to quiesce.
  *
diff --git a/drivers/scsi/scsi_transport_srp.c b/drivers/scsi/scsi_transport_srp.c
index e3cd3ec..8b190dc 100644
--- a/drivers/scsi/scsi_transport_srp.c
+++ b/drivers/scsi/scsi_transport_srp.c
@@ -24,7 +24,6 @@
 #include <linux/err.h>
 #include <linux/slab.h>
 #include <linux/string.h>
-#include <linux/delay.h>
 
 #include <scsi/scsi.h>
 #include <scsi/scsi_cmnd.h>
@@ -402,36 +401,6 @@ static void srp_reconnect_work(struct work_struct *work)
 	}
 }
 
-/**
- * scsi_request_fn_active() - number of kernel threads inside scsi_request_fn()
- * @shost: SCSI host for which to count the number of scsi_request_fn() callers.
- *
- * To do: add support for scsi-mq in this function.
- */
-static int scsi_request_fn_active(struct Scsi_Host *shost)
-{
-	struct scsi_device *sdev;
-	struct request_queue *q;
-	int request_fn_active = 0;
-
-	shost_for_each_device(sdev, shost) {
-		q = sdev->request_queue;
-
-		spin_lock_irq(q->queue_lock);
-		request_fn_active += q->request_fn_active;
-		spin_unlock_irq(q->queue_lock);
-	}
-
-	return request_fn_active;
-}
-
-/* Wait until ongoing shost->hostt->queuecommand() calls have finished. */
-static void srp_wait_for_queuecommand(struct Scsi_Host *shost)
-{
-	while (scsi_request_fn_active(shost))
-		msleep(20);
-}
-
 static void __rport_fail_io_fast(struct srp_rport *rport)
 {
 	struct Scsi_Host *shost = rport_to_shost(rport);
@@ -446,7 +415,7 @@ static void __rport_fail_io_fast(struct srp_rport *rport)
 	/* Involve the LLD if possible to terminate all I/O on the rport. */
 	i = to_srp_internal(shost->transportt);
 	if (i->f->terminate_rport_io) {
-		srp_wait_for_queuecommand(shost);
+		scsi_wait_for_queuecommand(shost);
 		i->f->terminate_rport_io(rport);
 	}
 }
@@ -576,7 +545,7 @@ int srp_reconnect_rport(struct srp_rport *rport)
 	if (res)
 		goto out;
 	scsi_target_block(&shost->shost_gendev);
-	srp_wait_for_queuecommand(shost);
+	scsi_wait_for_queuecommand(shost);
 	res = rport->state != SRP_RPORT_LOST ? i->f->reconnect(rport) : -ENODEV;
 	pr_debug("%s (state %d): transport.reconnect() returned %d\n",
 		 dev_name(&shost->shost_gendev), rport->state, res);
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index 7e4cd53..0e2c361 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -789,6 +789,7 @@ extern void scsi_remove_host(struct Scsi_Host *);
 extern struct Scsi_Host *scsi_host_get(struct Scsi_Host *);
 extern void scsi_host_put(struct Scsi_Host *t);
 extern struct Scsi_Host *scsi_host_lookup(unsigned short);
+extern void scsi_wait_for_queuecommand(struct Scsi_Host *shost);
 extern const char *scsi_host_state_name(enum scsi_host_state);
 extern void scsi_cmd_get_serial(struct Scsi_Host *, struct scsi_cmnd *);
 
-- 
2.10.1


^ permalink raw reply related

* [PATCH v3 09/11] SRP transport, scsi-mq: Wait for .queue_rq() if necessary
From: Bart Van Assche @ 2016-10-18 21:52 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Christoph Hellwig, James Bottomley, Martin K. Petersen,
	Mike Snitzer, Doug Ledford, Keith Busch, Ming Lin,
	Laurence Oberman, linux-block@vger.kernel.org,
	linux-scsi@vger.kernel.org, linux-rdma@vger.kernel.org,
	linux-nvme@lists.infradead.org
In-Reply-To: <b39eb0e7-1007-eb63-8e7f-9a7f08508379@sandisk.com>

Rename srp_wait_for_queuecommand() into scsi_wait_for_queuecommand().
Ensure that if scsi-mq is enabled that scsi_wait_for_queuecommand()
waits until ongoing shost->hostt->queuecommand() calls have finished.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: James Bottomley <jejb@linux.vnet.ibm.com>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Doug Ledford <dledford@redhat.com>
---
 drivers/scsi/scsi_lib.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index a5a1b5d..b7e9662 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2724,8 +2724,6 @@ EXPORT_SYMBOL_GPL(sdev_evt_send_simple);
 /**
  * scsi_request_fn_active() - number of kernel threads inside scsi_request_fn()
  * @shost: SCSI host for which to count the number of scsi_request_fn() callers.
- *
- * To do: add support for scsi-mq in this function.
  */
 static int scsi_request_fn_active(struct Scsi_Host *shost)
 {
@@ -2744,11 +2742,19 @@ static int scsi_request_fn_active(struct Scsi_Host *shost)
 	return request_fn_active;
 }
 
+static void scsi_mq_wait_for_queuecommand(struct Scsi_Host *shost)
+{
+	struct scsi_device *sdev;
+
+	shost_for_each_device(sdev, shost)
+		blk_mq_quiesce_queue(sdev->request_queue);
+}
+
 /**
  * scsi_wait_for_queuecommand() - wait for ongoing queuecommand() calls
  *
  * Wait until the ongoing shost->hostt->queuecommand() calls that are
- * invoked from scsi_request_fn() have finished.
+ * invoked from either scsi_request_fn() or scsi_queue_rq() have finished.
  *
  * To do: avoid that scsi_send_eh_cmnd() calls queuecommand() after
  * scsi_internal_device_block() has blocked a SCSI device and remove and also
@@ -2756,8 +2762,12 @@ static int scsi_request_fn_active(struct Scsi_Host *shost)
  */
 void scsi_wait_for_queuecommand(struct Scsi_Host *shost)
 {
-	while (scsi_request_fn_active(shost))
-		msleep(20);
+	if (shost->use_blk_mq) {
+		scsi_mq_wait_for_queuecommand(shost);
+	} else {
+		while (scsi_request_fn_active(shost))
+			msleep(20);
+	}
 }
 EXPORT_SYMBOL(scsi_wait_for_queuecommand);
 
-- 
2.10.1


^ permalink raw reply related

* [PATCH v3 10/11] nvme: Use BLK_MQ_S_STOPPED instead of QUEUE_FLAG_STOPPED in blk-mq code
From: Bart Van Assche @ 2016-10-18 21:53 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Christoph Hellwig, James Bottomley, Martin K. Petersen,
	Mike Snitzer, Doug Ledford, Keith Busch, Ming Lin,
	Laurence Oberman, linux-block@vger.kernel.org,
	linux-scsi@vger.kernel.org, linux-rdma@vger.kernel.org,
	linux-nvme@lists.infradead.org
In-Reply-To: <b39eb0e7-1007-eb63-8e7f-9a7f08508379@sandisk.com>

Make nvme_requeue_req() check BLK_MQ_S_STOPPED instead of
QUEUE_FLAG_STOPPED. Remove the QUEUE_FLAG_STOPPED manipulations
that became superfluous because of this change. This patch fixes
a race condition: using queue_flag_clear_unlocked() is not safe
if any other function that manipulates the queue flags can be
called concurrently, e.g. blk_cleanup_queue().

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Keith Busch <keith.busch@intel.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Sagi Grimberg <sagi@grimberg.me>
---
 drivers/nvme/host/core.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index e4a6f2d..18a265d 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -205,7 +205,7 @@ void nvme_requeue_req(struct request *req)
 
 	blk_mq_requeue_request(req, false);
 	spin_lock_irqsave(req->q->queue_lock, flags);
-	if (!blk_queue_stopped(req->q))
+	if (!blk_mq_queue_stopped(req->q))
 		blk_mq_kick_requeue_list(req->q);
 	spin_unlock_irqrestore(req->q->queue_lock, flags);
 }
@@ -2077,10 +2077,6 @@ void nvme_stop_queues(struct nvme_ctrl *ctrl)
 
 	mutex_lock(&ctrl->namespaces_mutex);
 	list_for_each_entry(ns, &ctrl->namespaces, list) {
-		spin_lock_irq(ns->queue->queue_lock);
-		queue_flag_set(QUEUE_FLAG_STOPPED, ns->queue);
-		spin_unlock_irq(ns->queue->queue_lock);
-
 		blk_mq_cancel_requeue_work(ns->queue);
 		blk_mq_stop_hw_queues(ns->queue);
 	}
@@ -2094,7 +2090,6 @@ void nvme_start_queues(struct nvme_ctrl *ctrl)
 
 	mutex_lock(&ctrl->namespaces_mutex);
 	list_for_each_entry(ns, &ctrl->namespaces, list) {
-		queue_flag_clear_unlocked(QUEUE_FLAG_STOPPED, ns->queue);
 		blk_mq_start_stopped_hw_queues(ns->queue, true);
 		blk_mq_kick_requeue_list(ns->queue);
 	}
-- 
2.10.1


^ permalink raw reply related

* [PATCH v3 11/11] nvme: Fix a race condition
From: Bart Van Assche @ 2016-10-18 21:53 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Christoph Hellwig, James Bottomley, Martin K. Petersen,
	Mike Snitzer, Doug Ledford, Keith Busch, Ming Lin,
	Laurence Oberman, linux-block@vger.kernel.org,
	linux-scsi@vger.kernel.org, linux-rdma@vger.kernel.org,
	linux-nvme@lists.infradead.org
In-Reply-To: <b39eb0e7-1007-eb63-8e7f-9a7f08508379@sandisk.com>

Avoid that nvme_queue_rq() is still running when nvme_stop_queues()
returns.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Keith Busch <keith.busch@intel.com>
Cc: Sagi Grimberg <sagi@grimberg.me>
Cc: Christoph Hellwig <hch@lst.de>
---
 drivers/nvme/host/core.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 18a265d..96f00c7 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -201,13 +201,7 @@ static struct nvme_ns *nvme_get_ns_from_disk(struct gendisk *disk)
 
 void nvme_requeue_req(struct request *req)
 {
-	unsigned long flags;
-
-	blk_mq_requeue_request(req, false);
-	spin_lock_irqsave(req->q->queue_lock, flags);
-	if (!blk_mq_queue_stopped(req->q))
-		blk_mq_kick_requeue_list(req->q);
-	spin_unlock_irqrestore(req->q->queue_lock, flags);
+	blk_mq_requeue_request(req, true);
 }
 EXPORT_SYMBOL_GPL(nvme_requeue_req);
 
@@ -2074,11 +2068,14 @@ EXPORT_SYMBOL_GPL(nvme_kill_queues);
 void nvme_stop_queues(struct nvme_ctrl *ctrl)
 {
 	struct nvme_ns *ns;
+	struct request_queue *q;
 
 	mutex_lock(&ctrl->namespaces_mutex);
 	list_for_each_entry(ns, &ctrl->namespaces, list) {
-		blk_mq_cancel_requeue_work(ns->queue);
-		blk_mq_stop_hw_queues(ns->queue);
+		q = ns->queue;
+		blk_mq_cancel_requeue_work(q);
+		blk_mq_stop_hw_queues(q);
+		blk_mq_quiesce_queue(q);
 	}
 	mutex_unlock(&ctrl->namespaces_mutex);
 }
-- 
2.10.1


^ permalink raw reply related

* Re: [PATCH v3 0/11] Fix race conditions related to stopping block layer queues
From: Bart Van Assche @ 2016-10-18 21:56 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Christoph Hellwig, James Bottomley, Martin K. Petersen,
	Mike Snitzer, Doug Ledford, Keith Busch, Ming Lin,
	Laurence Oberman,
	linux-block-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <b39eb0e7-1007-eb63-8e7f-9a7f08508379-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>

On 10/18/2016 02:48 PM, Bart Van Assche wrote:
> - blk_mq_quiesce_queue() has been reworked (thanks to Ming Lin and Sagi
>    for their feedback).

(replying to my own e-mail)

A correction: Ming Lei provided feedback on v2 of this patch series 
instead of Ming Lin.

Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: iscsi_trx going into D state
From: Robert LeBlanc @ 2016-10-18 22:13 UTC (permalink / raw)
  To: Nicholas A. Bellinger
  Cc: Zhu Lingshan, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1476774332.8490.43.camel-XoQW25Eq2zviZyQQd+hFbcojREIfoBdhmpATvIKMPHk@public.gmane.org>

Nicholas,

We patched this in and for the first time in many reboots, we didn't
have iSCSI going straight into D state. We have had to work on a
couple of other things, so we don't know if this is just a coincidence
or not. We will reboot back into the old kernel and back a few times
and do some more testing, but so far it has given us a little bit of
hope that we may be narrowing down on the root cause. We will report
back once we have some more info.

Thank you,
Robert LeBlanc
----------------
Robert LeBlanc
PGP Fingerprint 79A2 9CA4 6CC4 45DD A904  C70E E654 3BB2 FA62 B9F1


On Tue, Oct 18, 2016 at 1:05 AM, Nicholas A. Bellinger
<nab-IzHhD5pYlfBP7FQvKIMDCQ@public.gmane.org> wrote:
> Hello Robert, Zhu & Co,
>
> Thanks for your detailed bug report.  Comments inline below.
>
> On Mon, 2016-10-17 at 22:42 -0600, Robert LeBlanc wrote:
>> Sorry I forget that Android has an aversion to plain text emails.
>>
>> If we can provide any information to help, let us know. We are willing
>> to patch in more debug statements or whatever you think might help.
>> Today has been a difficult day. Thanks for looking into it, I tried
>> looking at it, but it is way over my head.
>>
>> ----------------
>> Robert LeBlanc
>> PGP Fingerprint 79A2 9CA4 6CC4 45DD A904  C70E E654 3BB2 FA62 B9F1
>>
>>
>> On Mon, Oct 17, 2016 at 9:06 PM, Zhu Lingshan <lszhu-IBi9RG/b67k@public.gmane.org> wrote:
>> > Hi Robert,
>> >
>> > I think the reason why you can not logout the targets is that iscsi_np in D
>> > status. I think the patches fixed something, but it seems to be more than
>> > one code path can trigger these similar issues. as you can see, there are
>> > several call stacks, I am still working on it. Actually in my environment I
>> > see there is another call stack not listed in your mail....
>> >
>> > Thanks,
>> > BR
>> > Zhu Lingshan
>> >
>> >
>> > On 10/18/2016 03:11 AM, Robert LeBlanc wrote:
>> >>
>> >> Sorry hit send too soon.
>> >>
>> >> In addition, on the client we see:
>> >> # ps -aux | grep D | grep kworker
>> >> root      5583  0.0  0.0      0     0 ?        D    11:55   0:03
>> >> [kworker/11:0]
>> >> root      7721  0.1  0.0      0     0 ?        D    12:00   0:04
>> >> [kworker/4:25]
>> >> root     10877  0.0  0.0      0     0 ?        D    09:27   0:00
>> >> [kworker/22:1]
>> >> root     11246  0.0  0.0      0     0 ?        D    10:28   0:00
>> >> [kworker/30:2]
>> >> root     14034  0.0  0.0      0     0 ?        D    12:20   0:02
>> >> [kworker/19:15]
>> >> root     14048  0.0  0.0      0     0 ?        D    12:20   0:00
>> >> [kworker/16:0]
>> >> root     15871  0.0  0.0      0     0 ?        D    12:25   0:00
>> >> [kworker/13:0]
>> >> root     17442  0.0  0.0      0     0 ?        D    12:28   0:00
>> >> [kworker/9:1]
>> >> root     17816  0.0  0.0      0     0 ?        D    12:30   0:00
>> >> [kworker/11:1]
>> >> root     18744  0.0  0.0      0     0 ?        D    12:32   0:00
>> >> [kworker/10:2]
>> >> root     19060  0.0  0.0      0     0 ?        D    12:32   0:00
>> >> [kworker/29:0]
>> >> root     21748  0.0  0.0      0     0 ?        D    12:40   0:00
>> >> [kworker/21:0]
>> >> root     21967  0.0  0.0      0     0 ?        D    12:40   0:00
>> >> [kworker/22:0]
>> >> root     21978  0.0  0.0      0     0 ?        D    12:40   0:00
>> >> [kworker/22:2]
>> >> root     22024  0.0  0.0      0     0 ?        D    12:40   0:00
>> >> [kworker/22:4]
>> >> root     22035  0.0  0.0      0     0 ?        D    12:40   0:00
>> >> [kworker/22:5]
>> >> root     22060  0.0  0.0      0     0 ?        D    12:40   0:00
>> >> [kworker/16:1]
>> >> root     22282  0.0  0.0      0     0 ?        D    12:41   0:00
>> >> [kworker/26:0]
>> >> root     22362  0.0  0.0      0     0 ?        D    12:42   0:00
>> >> [kworker/18:9]
>> >> root     22426  0.0  0.0      0     0 ?        D    12:42   0:00
>> >> [kworker/16:3]
>> >> root     23298  0.0  0.0      0     0 ?        D    12:43   0:00
>> >> [kworker/12:1]
>> >> root     23302  0.0  0.0      0     0 ?        D    12:43   0:00
>> >> [kworker/12:5]
>> >> root     24264  0.0  0.0      0     0 ?        D    12:46   0:00
>> >> [kworker/30:1]
>> >> root     24271  0.0  0.0      0     0 ?        D    12:46   0:00
>> >> [kworker/14:8]
>> >> root     24441  0.0  0.0      0     0 ?        D    12:47   0:00
>> >> [kworker/9:7]
>> >> root     24443  0.0  0.0      0     0 ?        D    12:47   0:00
>> >> [kworker/9:9]
>> >> root     25005  0.0  0.0      0     0 ?        D    12:48   0:00
>> >> [kworker/30:3]
>> >> root     25158  0.0  0.0      0     0 ?        D    12:49   0:00
>> >> [kworker/9:12]
>> >> root     26382  0.0  0.0      0     0 ?        D    12:52   0:00
>> >> [kworker/13:2]
>> >> root     26453  0.0  0.0      0     0 ?        D    12:52   0:00
>> >> [kworker/21:2]
>> >> root     26724  0.0  0.0      0     0 ?        D    12:53   0:00
>> >> [kworker/19:1]
>> >> root     28400  0.0  0.0      0     0 ?        D    05:20   0:00
>> >> [kworker/25:1]
>> >> root     29552  0.0  0.0      0     0 ?        D    11:40   0:00
>> >> [kworker/17:1]
>> >> root     29811  0.0  0.0      0     0 ?        D    11:40   0:00
>> >> [kworker/7:10]
>> >> root     31903  0.0  0.0      0     0 ?        D    11:43   0:00
>> >> [kworker/26:1]
>> >>
>> >> And all of the processes have this stack:
>> >> [<ffffffffa0727ed5>] iser_release_work+0x25/0x60 [ib_iser]
>> >> [<ffffffff8109633f>] process_one_work+0x14f/0x400
>> >> [<ffffffff81096bb4>] worker_thread+0x114/0x470
>> >> [<ffffffff8109c6f8>] kthread+0xd8/0xf0
>> >> [<ffffffff8172004f>] ret_from_fork+0x3f/0x70
>> >> [<ffffffffffffffff>] 0xffffffffffffffff
>> >>
>> >> We are not able to log out of the sessions in all cases. And have to
>> >> restart the box.
>> >>
>> >> iscsiadm -m session will show messages like:
>> >> iscsiadm: could not read session targetname: 5
>> >> iscsiadm: could not find session info for session100
>> >> iscsiadm: could not read session targetname: 5
>> >> iscsiadm: could not find session info for session101
>> >> iscsiadm: could not read session targetname: 5
>> >> iscsiadm: could not find session info for session103
>> >> ...
>> >>
>> >> I can't find any way to force iscsiadm to clean up these sessions
>> >> possibly due to tasks in D state.
>> >> ----------------
>> >> Robert LeBlanc
>> >> PGP Fingerprint 79A2 9CA4 6CC4 45DD A904  C70E E654 3BB2 FA62 B9F1
>> >>
>> >>
>> >> On Mon, Oct 17, 2016 at 10:32 AM, Robert LeBlanc <robert-4JaGZRWAfWbajFs6igw21g@public.gmane.org>
>> >> wrote:
>> >>>
>> >>> Some more info as we hit this this morning. We have volumes mirrored
>> >>> between two targets and we had one target on the kernel with the three
>> >>> patches mentioned in this thread [0][1][2] and the other was on a
>> >>> kernel without the patches. We decided that after a week and a half we
>> >>> wanted to get both targets on the same kernel so we rebooted the
>> >>> non-patched target. Within an hour we saw iSCSI in D state with the
>> >>> same stack trace so it seems that we are not hitting any of the
>> >>> WARN_ON lines. We are getting both iscsi_trx and iscsi_np both in D
>> >>> state, this time we have two iscsi_trx processes in D state. I don't
>> >>> know if stale sessions on the clients could be contributing to this
>> >>> issue (the target trying to close non-existent sessions??). This is on
>> >>> 4.4.23. Any more debug info we can throw at this problem to help?
>> >>>
>> >>> Thank you,
>> >>> Robert LeBlanc
>> >>>
>> >>> # ps aux | grep D | grep iscsi
>> >>> root     16525  0.0  0.0      0     0 ?        D    08:50   0:00
>> >>> [iscsi_np]
>> >>> root     16614  0.0  0.0      0     0 ?        D    08:50   0:00
>> >>> [iscsi_trx]
>> >>> root     16674  0.0  0.0      0     0 ?        D    08:50   0:00
>> >>> [iscsi_trx]
>> >>>
>> >>> # for i in 16525 16614 16674; do echo $i; cat /proc/$i/stack; done
>> >>> 16525
>> >>> [<ffffffff814f0d5f>] iscsit_stop_session+0x19f/0x1d0
>> >>> [<ffffffff814e2516>] iscsi_check_for_session_reinstatement+0x1e6/0x270
>> >>> [<ffffffff814e4ed0>] iscsi_target_check_for_existing_instances+0x30/0x40
>> >>> [<ffffffff814e5020>] iscsi_target_do_login+0x140/0x640
>> >>> [<ffffffff814e63bc>] iscsi_target_start_negotiation+0x1c/0xb0
>> >>> [<ffffffff814e410b>] iscsi_target_login_thread+0xa9b/0xfc0
>> >>> [<ffffffff8109c748>] kthread+0xd8/0xf0
>> >>> [<ffffffff8172018f>] ret_from_fork+0x3f/0x70
>> >>> [<ffffffffffffffff>] 0xffffffffffffffff
>> >>> 16614
>> >>> [<ffffffff814cca79>] target_wait_for_sess_cmds+0x49/0x1a0
>> >>> [<ffffffffa064692b>] isert_wait_conn+0x1ab/0x2f0 [ib_isert]
>> >>> [<ffffffff814f0ef2>] iscsit_close_connection+0x162/0x870
>> >>> [<ffffffff814df9bf>] iscsit_take_action_for_connection_exit+0x7f/0x100
>> >>> [<ffffffff814f00a0>] iscsi_target_rx_thread+0x5a0/0xe80
>> >>> [<ffffffff8109c748>] kthread+0xd8/0xf0
>> >>> [<ffffffff8172018f>] ret_from_fork+0x3f/0x70
>> >>> [<ffffffffffffffff>] 0xffffffffffffffff
>> >>> 16674
>> >>> [<ffffffff814cca79>] target_wait_for_sess_cmds+0x49/0x1a0
>> >>> [<ffffffffa064692b>] isert_wait_conn+0x1ab/0x2f0 [ib_isert]
>> >>> [<ffffffff814f0ef2>] iscsit_close_connection+0x162/0x870
>> >>> [<ffffffff814df9bf>] iscsit_take_action_for_connection_exit+0x7f/0x100
>> >>> [<ffffffff814f00a0>] iscsi_target_rx_thread+0x5a0/0xe80
>> >>> [<ffffffff8109c748>] kthread+0xd8/0xf0
>> >>> [<ffffffff8172018f>] ret_from_fork+0x3f/0x70
>> >>> [<ffffffffffffffff>] 0xffffffffffffffff
>> >>>
>> >>>
>> >>> [0] https://www.spinics.net/lists/target-devel/msg13463.html
>> >>> [1] http://marc.info/?l=linux-scsi&m=147282568910535&w=2
>> >>> [2] http://www.spinics.net/lists/linux-scsi/msg100221.html
>
> The call chain above is iscsi session reinstatement driven by
> open-iscsi/iser resulting in target-core to sleep indefinitely, waiting
> for outstanding target-core backend driver se_cmd I/O to complete in
> order to make forward progress.
>
> Note, there is a v4.1+ se_cmd->cmd_kref reference leak bug for
> TMR ABORT_TASK during simultaneous target back-end I/O completion
> timeouts here:
>
> http://www.spinics.net/lists/target-devel/msg13530.html
>
> If you are actively observing TMR ABORT_TASK preceding the hung task
> timeout warnings above with v4.4.y + v4.2.y iser-target exports, then
> it's likely the same bug.  Please apply the patch on your v4.x setup to
> verify.
>
> If no TMR ABORT_TASK timeouts + session reinstatements are occurring on
> your iser-target setup, then it is a separate bug.
>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH net-next 6/6] net: use core MTU range checking in misc drivers
From: Jarod Wilson @ 2016-10-19  2:33 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jarod Wilson, netdev, Stefan Richter, Faisal Latif, linux-rdma,
	Cliff Whickman, Robin Holt, Jes Sorensen, Marek Lindner,
	Simon Wunderlich, Antonio Quartulli
In-Reply-To: <20161019023333.15760-1-jarod@redhat.com>

firewire-net:
- set min/max_mtu
- remove fwnet_change_mtu

nes:
- set max_mtu
- clean up nes_netdev_change_mtu

xpnet:
- set min/max_mtu
- remove xpnet_dev_change_mtu

hippi:
- set min/max_mtu
- remove hippi_change_mtu

batman-adv:
- set max_mtu
- remove batadv_interface_change_mtu
- initialization is a little async, not 100% certain that max_mtu is set
  in the optimal place, don't have hardware to test with

rionet:
- set min/max_mtu
- remove rionet_change_mtu

slip:
- set min/max_mtu
- streamline sl_change_mtu

CC: netdev@vger.kernel.org
CC: Stefan Richter <stefanr@s5r6.in-berlin.de>
CC: Faisal Latif <faisal.latif@intel.com>
CC: linux-rdma@vger.kernel.org
CC: Cliff Whickman <cpw@sgi.com>
CC: Robin Holt <robinmholt@gmail.com>
CC: Jes Sorensen <jes@trained-monkey.org>
CC: Marek Lindner <mareklindner@neomailbox.ch>
CC: Simon Wunderlich <sw@simonwunderlich.de>
CC: Antonio Quartulli <a@unstable.cc>
Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
 drivers/firewire/net.c              | 12 ++----------
 drivers/infiniband/hw/nes/nes.c     |  1 -
 drivers/infiniband/hw/nes/nes.h     |  4 ++--
 drivers/infiniband/hw/nes/nes_nic.c | 10 +++-------
 drivers/misc/sgi-xp/xpnet.c         | 21 ++++-----------------
 drivers/net/hippi/rrunner.c         |  1 -
 drivers/net/rionet.c                | 15 +++------------
 drivers/net/slip/slip.c             | 11 +++++------
 include/linux/hippidevice.h         |  1 -
 net/802/hippi.c                     | 14 ++------------
 net/batman-adv/soft-interface.c     | 13 +------------
 11 files changed, 22 insertions(+), 81 deletions(-)

diff --git a/drivers/firewire/net.c b/drivers/firewire/net.c
index 309311b..b5f125c 100644
--- a/drivers/firewire/net.c
+++ b/drivers/firewire/net.c
@@ -1349,15 +1349,6 @@ static netdev_tx_t fwnet_tx(struct sk_buff *skb, struct net_device *net)
 	return NETDEV_TX_OK;
 }
 
-static int fwnet_change_mtu(struct net_device *net, int new_mtu)
-{
-	if (new_mtu < 68)
-		return -EINVAL;
-
-	net->mtu = new_mtu;
-	return 0;
-}
-
 static const struct ethtool_ops fwnet_ethtool_ops = {
 	.get_link	= ethtool_op_get_link,
 };
@@ -1366,7 +1357,6 @@ static const struct net_device_ops fwnet_netdev_ops = {
 	.ndo_open       = fwnet_open,
 	.ndo_stop	= fwnet_stop,
 	.ndo_start_xmit = fwnet_tx,
-	.ndo_change_mtu = fwnet_change_mtu,
 };
 
 static void fwnet_init_dev(struct net_device *net)
@@ -1481,6 +1471,8 @@ static int fwnet_probe(struct fw_unit *unit,
 	max_mtu = (1 << (card->max_receive + 1))
 		  - sizeof(struct rfc2734_header) - IEEE1394_GASP_HDR_SIZE;
 	net->mtu = min(1500U, max_mtu);
+	net->min_mtu = ETH_MIN_MTU;
+	net->max_mtu = net->mtu;
 
 	/* Set our hardware address while we're at it */
 	ha = (union fwnet_hwaddr *)net->dev_addr;
diff --git a/drivers/infiniband/hw/nes/nes.c b/drivers/infiniband/hw/nes/nes.c
index 35cbb17..2baa45a 100644
--- a/drivers/infiniband/hw/nes/nes.c
+++ b/drivers/infiniband/hw/nes/nes.c
@@ -65,7 +65,6 @@ MODULE_DESCRIPTION("NetEffect RNIC Low-level iWARP Driver");
 MODULE_LICENSE("Dual BSD/GPL");
 MODULE_VERSION(DRV_VERSION);
 
-int max_mtu = 9000;
 int interrupt_mod_interval = 0;
 
 /* Interoperability */
diff --git a/drivers/infiniband/hw/nes/nes.h b/drivers/infiniband/hw/nes/nes.h
index e7430c9..85acd08 100644
--- a/drivers/infiniband/hw/nes/nes.h
+++ b/drivers/infiniband/hw/nes/nes.h
@@ -83,6 +83,8 @@
 #define NES_FIRST_QPN           64
 #define NES_SW_CONTEXT_ALIGN    1024
 
+#define NES_MAX_MTU		9000
+
 #define NES_NIC_MAX_NICS        16
 #define NES_MAX_ARP_TABLE_SIZE  4096
 
@@ -169,8 +171,6 @@ do { \
 #include "nes_cm.h"
 #include "nes_mgt.h"
 
-extern int max_mtu;
-#define max_frame_len (max_mtu+ETH_HLEN)
 extern int interrupt_mod_interval;
 extern int nes_if_count;
 extern int mpa_version;
diff --git a/drivers/infiniband/hw/nes/nes_nic.c b/drivers/infiniband/hw/nes/nes_nic.c
index 2b27d13..7f8597d 100644
--- a/drivers/infiniband/hw/nes/nes_nic.c
+++ b/drivers/infiniband/hw/nes/nes_nic.c
@@ -981,20 +981,16 @@ static int nes_netdev_change_mtu(struct net_device *netdev, int new_mtu)
 {
 	struct nes_vnic	*nesvnic = netdev_priv(netdev);
 	struct nes_device *nesdev = nesvnic->nesdev;
-	int ret = 0;
 	u8 jumbomode = 0;
 	u32 nic_active;
 	u32 nic_active_bit;
 	u32 uc_all_active;
 	u32 mc_all_active;
 
-	if ((new_mtu < ETH_ZLEN) || (new_mtu > max_mtu))
-		return -EINVAL;
-
 	netdev->mtu = new_mtu;
 	nesvnic->max_frame_size	= new_mtu + VLAN_ETH_HLEN;
 
-	if (netdev->mtu	> 1500)	{
+	if (netdev->mtu	> ETH_DATA_LEN)	{
 		jumbomode=1;
 	}
 	nes_nic_init_timer_defaults(nesdev, jumbomode);
@@ -1020,7 +1016,7 @@ static int nes_netdev_change_mtu(struct net_device *netdev, int new_mtu)
 		nes_write_indexed(nesdev, NES_IDX_NIC_UNICAST_ALL, nic_active);
 	}
 
-	return ret;
+	return 0;
 }
 
 
@@ -1658,7 +1654,7 @@ struct net_device *nes_netdev_init(struct nes_device *nesdev,
 
 	netdev->watchdog_timeo = NES_TX_TIMEOUT;
 	netdev->irq = nesdev->pcidev->irq;
-	netdev->mtu = ETH_DATA_LEN;
+	netdev->max_mtu = NES_MAX_MTU;
 	netdev->hard_header_len = ETH_HLEN;
 	netdev->addr_len = ETH_ALEN;
 	netdev->type = ARPHRD_ETHER;
diff --git a/drivers/misc/sgi-xp/xpnet.c b/drivers/misc/sgi-xp/xpnet.c
index 557f978..0c26eaf 100644
--- a/drivers/misc/sgi-xp/xpnet.c
+++ b/drivers/misc/sgi-xp/xpnet.c
@@ -118,6 +118,8 @@ static DEFINE_SPINLOCK(xpnet_broadcast_lock);
  * now, the default is 64KB.
  */
 #define XPNET_MAX_MTU (0x800000UL - L1_CACHE_BYTES)
+/* 68 comes from min TCP+IP+MAC header */
+#define XPNET_MIN_MTU 68
 /* 32KB has been determined to be the ideal */
 #define XPNET_DEF_MTU (0x8000UL)
 
@@ -330,22 +332,6 @@ xpnet_dev_stop(struct net_device *dev)
 	return 0;
 }
 
-static int
-xpnet_dev_change_mtu(struct net_device *dev, int new_mtu)
-{
-	/* 68 comes from min TCP+IP+MAC header */
-	if ((new_mtu < 68) || (new_mtu > XPNET_MAX_MTU)) {
-		dev_err(xpnet, "ifconfig %s mtu %d failed; value must be "
-			"between 68 and %ld\n", dev->name, new_mtu,
-			XPNET_MAX_MTU);
-		return -EINVAL;
-	}
-
-	dev->mtu = new_mtu;
-	dev_dbg(xpnet, "ifconfig %s mtu set to %d\n", dev->name, new_mtu);
-	return 0;
-}
-
 /*
  * Notification that the other end has received the message and
  * DMA'd the skb information.  At this point, they are done with
@@ -519,7 +505,6 @@ static const struct net_device_ops xpnet_netdev_ops = {
 	.ndo_open		= xpnet_dev_open,
 	.ndo_stop		= xpnet_dev_stop,
 	.ndo_start_xmit		= xpnet_dev_hard_start_xmit,
-	.ndo_change_mtu		= xpnet_dev_change_mtu,
 	.ndo_tx_timeout		= xpnet_dev_tx_timeout,
 	.ndo_set_mac_address 	= eth_mac_addr,
 	.ndo_validate_addr	= eth_validate_addr,
@@ -555,6 +540,8 @@ xpnet_init(void)
 
 	xpnet_device->netdev_ops = &xpnet_netdev_ops;
 	xpnet_device->mtu = XPNET_DEF_MTU;
+	xpnet_device->min_mtu = XPNET_MIN_MTU;
+	xpnet_device->max_mtu = XPNET_MAX_MTU;
 
 	/*
 	 * Multicast assumes the LSB of the first octet is set for multicast
diff --git a/drivers/net/hippi/rrunner.c b/drivers/net/hippi/rrunner.c
index 95c0b45..f5a9728 100644
--- a/drivers/net/hippi/rrunner.c
+++ b/drivers/net/hippi/rrunner.c
@@ -68,7 +68,6 @@ static const struct net_device_ops rr_netdev_ops = {
 	.ndo_stop		= rr_close,
 	.ndo_do_ioctl		= rr_ioctl,
 	.ndo_start_xmit		= rr_start_xmit,
-	.ndo_change_mtu		= hippi_change_mtu,
 	.ndo_set_mac_address	= hippi_mac_addr,
 };
 
diff --git a/drivers/net/rionet.c b/drivers/net/rionet.c
index a31f461..300bb14 100644
--- a/drivers/net/rionet.c
+++ b/drivers/net/rionet.c
@@ -466,17 +466,6 @@ static void rionet_set_msglevel(struct net_device *ndev, u32 value)
 	rnet->msg_enable = value;
 }
 
-static int rionet_change_mtu(struct net_device *ndev, int new_mtu)
-{
-	if ((new_mtu < 68) || (new_mtu > RIONET_MAX_MTU)) {
-		printk(KERN_ERR "%s: Invalid MTU size %d\n",
-		       ndev->name, new_mtu);
-		return -EINVAL;
-	}
-	ndev->mtu = new_mtu;
-	return 0;
-}
-
 static const struct ethtool_ops rionet_ethtool_ops = {
 	.get_drvinfo = rionet_get_drvinfo,
 	.get_msglevel = rionet_get_msglevel,
@@ -488,7 +477,6 @@ static const struct net_device_ops rionet_netdev_ops = {
 	.ndo_open		= rionet_open,
 	.ndo_stop		= rionet_close,
 	.ndo_start_xmit		= rionet_start_xmit,
-	.ndo_change_mtu		= rionet_change_mtu,
 	.ndo_validate_addr	= eth_validate_addr,
 	.ndo_set_mac_address	= eth_mac_addr,
 };
@@ -525,6 +513,9 @@ static int rionet_setup_netdev(struct rio_mport *mport, struct net_device *ndev)
 
 	ndev->netdev_ops = &rionet_netdev_ops;
 	ndev->mtu = RIONET_MAX_MTU;
+	/* MTU range: 68 - 4082 */
+	ndev->min_mtu = ETH_MIN_MTU;
+	ndev->max_mtu = RIONET_MAX_MTU;
 	ndev->features = NETIF_F_LLTX;
 	SET_NETDEV_DEV(ndev, &mport->dev);
 	ndev->ethtool_ops = &rionet_ethtool_ops;
diff --git a/drivers/net/slip/slip.c b/drivers/net/slip/slip.c
index 9ed6d1c..7e933d8 100644
--- a/drivers/net/slip/slip.c
+++ b/drivers/net/slip/slip.c
@@ -561,12 +561,7 @@ static int sl_change_mtu(struct net_device *dev, int new_mtu)
 {
 	struct slip *sl = netdev_priv(dev);
 
-	if (new_mtu < 68 || new_mtu > 65534)
-		return -EINVAL;
-
-	if (new_mtu != dev->mtu)
-		return sl_realloc_bufs(sl, new_mtu);
-	return 0;
+	return sl_realloc_bufs(sl, new_mtu);
 }
 
 /* Netdevice get statistics request */
@@ -663,6 +658,10 @@ static void sl_setup(struct net_device *dev)
 	dev->addr_len		= 0;
 	dev->tx_queue_len	= 10;
 
+	/* MTU range: 68 - 65534 */
+	dev->min_mtu = 68;
+	dev->max_mtu = 65534;
+
 	/* New-style flags. */
 	dev->flags		= IFF_NOARP|IFF_POINTOPOINT|IFF_MULTICAST;
 }
diff --git a/include/linux/hippidevice.h b/include/linux/hippidevice.h
index 8ec23fb..402f99e 100644
--- a/include/linux/hippidevice.h
+++ b/include/linux/hippidevice.h
@@ -32,7 +32,6 @@ struct hippi_cb {
 };
 
 __be16 hippi_type_trans(struct sk_buff *skb, struct net_device *dev);
-int hippi_change_mtu(struct net_device *dev, int new_mtu);
 int hippi_mac_addr(struct net_device *dev, void *p);
 int hippi_neigh_setup_dev(struct net_device *dev, struct neigh_parms *p);
 struct net_device *alloc_hippi_dev(int sizeof_priv);
diff --git a/net/802/hippi.c b/net/802/hippi.c
index ade1a52..5e4427b 100644
--- a/net/802/hippi.c
+++ b/net/802/hippi.c
@@ -116,18 +116,6 @@ __be16 hippi_type_trans(struct sk_buff *skb, struct net_device *dev)
 
 EXPORT_SYMBOL(hippi_type_trans);
 
-int hippi_change_mtu(struct net_device *dev, int new_mtu)
-{
-	/*
-	 * HIPPI's got these nice large MTUs.
-	 */
-	if ((new_mtu < 68) || (new_mtu > 65280))
-		return -EINVAL;
-	dev->mtu = new_mtu;
-	return 0;
-}
-EXPORT_SYMBOL(hippi_change_mtu);
-
 /*
  * For HIPPI we will actually use the lower 4 bytes of the hardware
  * address as the I-FIELD rather than the actual hardware address.
@@ -174,6 +162,8 @@ static void hippi_setup(struct net_device *dev)
 	dev->type		= ARPHRD_HIPPI;
 	dev->hard_header_len 	= HIPPI_HLEN;
 	dev->mtu		= 65280;
+	dev->min_mtu		= 68;
+	dev->max_mtu		= 65280;
 	dev->addr_len		= HIPPI_ALEN;
 	dev->tx_queue_len	= 25 /* 5 */;
 	memset(dev->broadcast, 0xFF, HIPPI_ALEN);
diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c
index 49e16b6..112679d 100644
--- a/net/batman-adv/soft-interface.c
+++ b/net/batman-adv/soft-interface.c
@@ -158,17 +158,6 @@ static int batadv_interface_set_mac_addr(struct net_device *dev, void *p)
 	return 0;
 }
 
-static int batadv_interface_change_mtu(struct net_device *dev, int new_mtu)
-{
-	/* check ranges */
-	if ((new_mtu < 68) || (new_mtu > batadv_hardif_min_mtu(dev)))
-		return -EINVAL;
-
-	dev->mtu = new_mtu;
-
-	return 0;
-}
-
 /**
  * batadv_interface_set_rx_mode - set the rx mode of a device
  * @dev: registered network device to modify
@@ -920,7 +909,6 @@ static const struct net_device_ops batadv_netdev_ops = {
 	.ndo_vlan_rx_add_vid = batadv_interface_add_vid,
 	.ndo_vlan_rx_kill_vid = batadv_interface_kill_vid,
 	.ndo_set_mac_address = batadv_interface_set_mac_addr,
-	.ndo_change_mtu = batadv_interface_change_mtu,
 	.ndo_set_rx_mode = batadv_interface_set_rx_mode,
 	.ndo_start_xmit = batadv_interface_tx,
 	.ndo_validate_addr = eth_validate_addr,
@@ -987,6 +975,7 @@ struct net_device *batadv_softif_create(struct net *net, const char *name)
 	dev_net_set(soft_iface, net);
 
 	soft_iface->rtnl_link_ops = &batadv_link_ops;
+	soft_iface->max_mtu = batadv_hardif_min_mtu(soft_iface);
 
 	ret = register_netdevice(soft_iface);
 	if (ret < 0) {
-- 
2.10.0

^ permalink raw reply related

* some test question//Re: [For help] configure crossbar build tool in CMakelist.txt
From: oulijun @ 2016-10-19  2:53 UTC (permalink / raw)
  To: Jason Gunthorpe; +Cc: linux-rdma, Linuxarm
In-Reply-To: <20161018155004.GA24189-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>

Hi, Jason

Indeed, you are right! when use the cmd: CC= aarch64-linux-gnu-gcc cmake -GNinja ..

the libnl doesn't work

the print log as follows:

ubuntu@62fe1357a077:~/rdma-core/build$ ninja
[27/164] Linking C executable bin/iwpmd
FAILED: : && /opt/gcc-linaro-aarch64-linux-gnu-4.9-2014.09_linux/bin/aarch64-linux-gnu-gcc -std=gnu99 -Wall -Wextra -Wno-sign-compare -Wno-unused-parameter -Wmissing-prototypes -Wmissing-declarations -Wwrite-strings -Wformat=2 -Wshadow -Wno-missing-field-initializers -Wstrict-prototypes -Wold-style-definition -O2 -g -Wl,--as-needed -Wl,--no-undefined iwpmd/CMakeFiles/iwpmd.dir/iwarp_pm_common.c.o iwpmd/CMakeFiles/iwpmd.dir/iwarp_pm_helper.c.o iwpmd/CMakeFiles/iwpmd.dir/iwarp_pm_server.c.o -o bin/iwpmd ccan/libccan.a -lnl-route-3 -lnl-3 -lpthread && :
/opt/gcc-linaro-aarch64-linux-gnu-4.9-2014.09_linux/bin/../lib/gcc/aarch64-linux-gnu/4.9.2/../../../../aarch64-linux-gnu/bin/ld: cannot find -lnl-route-3
/opt/gcc-linaro-aarch64-linux-gnu-4.9-2014.09_linux/bin/../lib/gcc/aarch64-linux-gnu/4.9.2/../../../../aarch64-linux-gnu/bin/ld: cannot find -lnl-3
collect2: error: ld returned 1 exit status
[27/164] Building C object libibverbs/CMakeFiles/ibverbs.dir/cmd.c.o
ninja: build stopped: subcommand failed.


when use the cmd: CC=aarch64-linux-gnu-gcc cmake -GNinja -DENABLE_RESOLVE_NEIGH=0 ..

the provider/hns/CMakelist.txt can be called correctly and generate the library libhns-rdmav2.so(not be tested), but it will generate warning with mlx5

the print log as follows:

Building C object providers/mlx5/CMakeFiles/mlx5-rdmav2.dir/cq.c.o
../providers/mlx5/cq.c:414:13: warning: function declaration isn't a prototype [-Wstrict-prototypes]
static void mlx5_stall_poll_cq()
^
../providers/mlx5/cq.c: In function 'mlx5_stall_poll_cq':
../providers/mlx5/cq.c:414:13: warning: old-style function definition [-Wold-style-definition]

In summary, I have two questions:

1. if don't use libnl(use -DENABLE_RESOLVE_NEIGH=0), it will not confluent the function of library?

2. the rdma-core.git use the default build method by used build.sh, and i need modify it by added the building item or modify the README.md for hns, is the right approach?


Thanks

在 2016/10/18 23:50, Jason Gunthorpe 写道:
> On Tue, Oct 18, 2016 at 06:55:08PM +0800, oulijun wrote:
> 
>>   if I use crossbar build tool aarch64-linux-gnu-gcc for building
>>   the directory(provider/hns), what i should do it ?
> 
> You cannot cross compile only a part of the project, you must cross
> compile everything.
> 
> $ mkdir build; cd build
> $ CC=aarch64-linux-gnu-gcc cmake -GNinja
> $ ninja
> 
> I haven't extensively tested cross compiling, you may run into
> problems, in particular I know pkgconfig for libnl doesn't work
> reliably when cross compiling.
> 
> You may need to build with -DENABLE_RESOLVE_NEIGH=0 if you don't have
> a cross compiled libnl3 available.
> 
>> My modification currently according to the others as fllows:
>>
>> in the file : provider/hns/CMakelist.txt
>>
>> set(CMAKE_C_COMPILER /opt/gcc-linaro-aarch64-linux-gnu-4.9-2014.09_linux/bin/aarch64-linux-gnu-gcc)
> 
> This is not the right approach, the hns provider CMakelist should
> detect the compiler is not ARM64 and just do nothing.
> 
> Perhaps something like this:
> 
> CHECK_C_SOURCE_COMPILES("
> #ifndef __ARM64__
> #error Failed
> #endif
>  int main(int argc,const char *argv[]) { return 1; }"
>  HAVE_ARCH_ARM64)
> 
> if (HAVE_ARCH_ARM64)
>  [..]
> endif()
>  
> Jason
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> .
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 0/3] iopmem : A block device for PCIe memory
From: Dan Williams @ 2016-10-19  3:51 UTC (permalink / raw)
  To: Stephen Bates
  Cc: linux-kernel@vger.kernel.org, linux-nvdimm@lists.01.org,
	linux-rdma, linux-block, Linux MM, Ross Zwisler, Matthew Wilcox,
	jgunthorpe, haggaie, Christoph Hellwig, Jens Axboe,
	Jonathan Corbet, jim.macdonald, sbates, Logan Gunthorpe,
	David Woodhouse, Raj, Ashok
In-Reply-To: <1476826937-20665-1-git-send-email-sbates@raithlin.com>

[ adding Ashok and David for potential iommu comments ]

On Tue, Oct 18, 2016 at 2:42 PM, Stephen Bates <sbates@raithlin.com> wrote:
> This patch follows from an RFC we did earlier this year [1]. This
> patchset applies cleanly to v4.9-rc1.
>
> Updates since RFC
> -----------------
>   Rebased.
>   Included the iopmem driver in the submission.
>
> History
> -------
>
> There have been several attempts to upstream patchsets that enable
> DMAs between PCIe peers. These include Peer-Direct [2] and DMA-Buf
> style patches [3]. None have been successful to date. Haggai Eran
> gives a nice overview of the prior art in this space in his cover
> letter [3].
>
> Motivation and Use Cases
> ------------------------
>
> PCIe IO devices are getting faster. It is not uncommon now to find PCIe
> network and storage devices that can generate and consume several GB/s.
> Almost always these devices have either a high performance DMA engine, a
> number of exposed PCIe BARs or both.
>
> Until this patch, any high-performance transfer of information between
> two PICe devices has required the use of a staging buffer in system
> memory. With this patch the bandwidth to system memory is not compromised
> when high-throughput transfers occurs between PCIe devices. This means
> that more system memory bandwidth is available to the CPU cores for data
> processing and manipulation. In addition, in systems where the two PCIe
> devices reside behind a PCIe switch the datapath avoids the CPU
> entirely.

I agree with the motivation and the need for a solution, but I have
some questions about this implementation.

>
> Consumers
> ---------
>
> We provide a PCIe device driver in an accompanying patch that can be
> used to map any PCIe BAR into a DAX capable block device. For
> non-persistent BARs this simply serves as an alternative to using
> system memory bounce buffers. For persistent BARs this can serve as an
> additional storage device in the system.

Why block devices?  I wonder if iopmem was initially designed back
when we were considering enabling DAX for raw block devices.  However,
that support has since been ripped out / abandoned.  You currently
need a filesystem on top of a block-device to get DAX operation.
Putting xfs or ext4 on top of PCI-E memory mapped range seems awkward
if all you want is a way to map the bar for another PCI-E device in
the topology.

If you're only using the block-device as a entry-point to create
dax-mappings then a device-dax (drivers/dax/) character-device might
be a better fit.

>
> Testing and Performance
> -----------------------
>
> We have done a moderate about of testing of this patch on a QEMU
> environment and on real hardware. On real hardware we have observed
> peer-to-peer writes of up to 4GB/s and reads of up to 1.2 GB/s. In
> both cases these numbers are limitations of our consumer hardware. In
> addition, we have observed that the CPU DRAM bandwidth is not impacted
> when using IOPMEM which is not the case when a traditional path
> through system memory is taken.
>
> For more information on the testing and performance results see the
> GitHub site [4].
>
> Known Issues
> ------------
>
> 1. Address Translation. Suggestions have been made that in certain
> architectures and topologies the dma_addr_t passed to the DMA master
> in a peer-2-peer transfer will not correctly route to the IO memory
> intended. However in our testing to date we have not seen this to be
> an issue, even in systems with IOMMUs and PCIe switches. It is our
> understanding that an IOMMU only maps system memory and would not
> interfere with device memory regions. (It certainly has no opportunity
> to do so if the transfer gets routed through a switch).
>

There may still be platforms where peer-to-peer cycles are routed up
through the root bridge and then back down to target device, but we
can address that when / if it happens.  I wonder if we could (ab)use a
software-defined 'pasid' as the requester id for a peer-to-peer
mapping that needs address translation.

> 2. Memory Segment Spacing. This patch has the same limitations that
> ZONE_DEVICE does in that memory regions must be spaces at least
> SECTION_SIZE bytes part. On x86 this is 128MB and there are cases where
> BARs can be placed closer together than this. Thus ZONE_DEVICE would not
> be usable on neighboring BARs. For our purposes, this is not an issue as
> we'd only be looking at enabling a single BAR in a given PCIe device.
> More exotic use cases may have problems with this.

I'm working on patches for 4.10 to allow mixing multiple
devm_memremap_pages() allocations within the same physical section.
Hopefully this won't be a problem going forward.

> 3. Coherency Issues. When IOMEM is written from both the CPU and a PCIe
> peer there is potential for coherency issues and for writes to occur out
> of order. This is something that users of this feature need to be
> cognizant of. Though really, this isn't much different than the
> existing situation with things like RDMA: if userspace sets up an MR
> for remote use, they need to be careful about using that memory region
> themselves.

We might be able to mitigate this by indicating that the mapping is
busy for device-to-device transfers.  But you're right we could wait
to see how much of a problem this is in practice.

>
> 4. Architecture. Currently this patch is applicable only to x86_64
> architectures. The same is true for much of the code pertaining to
> PMEM and ZONE_DEVICE. It is hoped that the work will be extended to other
> ARCH over time.
>
> References
> ----------
> [1] https://patchwork.kernel.org/patch/8583221/
> [2] http://comments.gmane.org/gmane.linux.drivers.rdma/21849
> [3] http://www.spinics.net/lists/linux-rdma/msg38748.html
> [4] https://github.com/sbates130272/zone-device
>
> Logan Gunthorpe (1):
>   memremap.c : Add support for ZONE_DEVICE IO memory with struct pages.

I haven't yet grokked the motivation for this, but I'll go comment on
that separately.

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

^ permalink raw reply

* Re: some test question//Re: [For help] configure crossbar build tool in CMakelist.txt
From: Jason Gunthorpe @ 2016-10-19  4:11 UTC (permalink / raw)
  To: oulijun; +Cc: linux-rdma, Linuxarm
In-Reply-To: <5806E02E.7030400-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>

On Wed, Oct 19, 2016 at 10:53:34AM +0800, oulijun wrote:

> when use the cmd: CC=aarch64-linux-gnu-gcc cmake -GNinja -DENABLE_RESOLVE_NEIGH=0 ..
> 
> the provider/hns/CMakelist.txt can be called correctly and generate
> the library libhns-rdmav2.so(not be tested), but it will generate
> warning with mlx5
> 
> the print log as follows:
> 
> Building C object providers/mlx5/CMakeFiles/mlx5-rdmav2.dir/cq.c.o
> ../providers/mlx5/cq.c:414:13: warning: function declaration isn't a prototype [-Wstrict-prototypes]
> static void mlx5_stall_poll_cq()

Thanks, you may be the first person to compile on ARM64, so I'm glad
to hear that is your only warning, I will get it fixed.

> In summary, I have two questions:
> 
> 1. if don't use libnl(use -DENABLE_RESOLVE_NEIGH=0), it will not
> confluent the function of library?

Correct, RoCEE will not work entirely as expected if you try and run
the libibverbs from such a build.

> 2. the rdma-core.git use the default build method by used build.sh,
> and i need modify it by added the building item or modify the
> README.md for hns, is the right approach?

No.

If you native compile on an actual ARM64 machine you will not have
problems, simply follow the README.md directions to install the
required packages on the actual machine.

Your problem is cross compiling. Cross compiling is hard. You need to
make libnl3 available in your cross compiler's 'sysroot'. I can't
really give you generic good directions for that..

One option is to cross compile libnl and 'cross-install' it into the
compiler tree.

Another option is to download the libnl3 ARM64 packages from Debian and
then unpack and rename directories 'just so' to make it work.

But if you don't intend to run the build binaries (eg when you go to
test, you will build natively on ARM64), then don't worry about it.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: iscsi_trx going into D state
From: Nicholas A. Bellinger @ 2016-10-19  6:25 UTC (permalink / raw)
  To: Robert LeBlanc
  Cc: Zhu Lingshan, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CAANLjFqXt5r=c9F75vjeK=_zLa8zCS1priLuZo=A1ZSHKZ=1Bw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Tue, 2016-10-18 at 16:13 -0600, Robert LeBlanc wrote:
> Nicholas,
> 
> We patched this in and for the first time in many reboots, we didn't
> have iSCSI going straight into D state. We have had to work on a
> couple of other things, so we don't know if this is just a coincidence
> or not. We will reboot back into the old kernel and back a few times
> and do some more testing, but so far it has given us a little bit of
> hope that we may be narrowing down on the root cause. We will report
> back once we have some more info.
> 
> Thank you,
> Robert LeBlanc
> ----------------
> Robert LeBlanc
> PGP Fingerprint 79A2 9CA4 6CC4 45DD A904  C70E E654 3BB2 FA62 B9F1
> 

Hello Robert,

Thanks for the update.  Btw, if the original /var/log/messages
reproduction logs for iser-target are still handy, I'm happy to have
a look to confirm.  Feel free to send them along here, or off-list if
necessary.

For further reference, you can also enable Linux kernel crash dump
(LKCD) at build time using CONFIG_CRASH_DUMP=y, so it's possible to
manually generate a vmcore dumpfile of the running system via 'echo c
> /proc/sysrq-trigger', once the bug occurs.

http://cateee.net/lkddb/web-lkddb/CRASH_DUMP.html

Note in order to fully debug within this in a LKCD environment, it
requires the vmcore dump from /var/crash/, unstripped vmlinux,
target_core_mod, iscsi_target_mod and ib_isert modules matching the
specific particular x86_64 build setup of the running system.

Also, can you share a bit more about the details of your particular
iser-target + backend setup..?

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 04/10] mm: replace get_user_pages_locked() write/force parameters with gup_flags
From: Jan Kara @ 2016-10-19  7:32 UTC (permalink / raw)
  To: Lorenzo Stoakes
  Cc: Jan Kara, linux-mm, Linus Torvalds, Hugh Dickins, Dave Hansen,
	Rik van Riel, Mel Gorman, Andrew Morton, adi-buildroot-devel,
	ceph-devel, dri-devel, intel-gfx, kvm, linux-alpha,
	linux-arm-kernel, linux-cris-kernel, linux-fbdev, linux-fsdevel,
	linux-ia64, linux-kernel, linux-media, linux-mips, linux-rdma,
	linux-s390, linux-samsung-soc, linux-scsi, linux-security-module,
	linux-sh
In-Reply-To: <20161018135609.GA30025@lucifer>

On Tue 18-10-16 14:56:09, Lorenzo Stoakes wrote:
> On Tue, Oct 18, 2016 at 02:54:25PM +0200, Jan Kara wrote:
> > > @@ -1282,7 +1282,7 @@ long get_user_pages(unsigned long start, unsigned long nr_pages,
> > >  			    int write, int force, struct page **pages,
> > >  			    struct vm_area_struct **vmas);
> > >  long get_user_pages_locked(unsigned long start, unsigned long nr_pages,
> > > -		    int write, int force, struct page **pages, int *locked);
> > > +		    unsigned int gup_flags, struct page **pages, int *locked);
> >
> > Hum, the prototype is inconsistent with e.g. __get_user_pages_unlocked()
> > where gup_flags come after **pages argument. Actually it makes more sense
> > to have it before **pages so that input arguments come first and output
> > arguments second but I don't care that much. But it definitely should be
> > consistent...
> 
> It was difficult to decide quite how to arrange parameters as there was
> inconsitency with regards to parameter ordering already - for example
> __get_user_pages() places its flags argument before pages whereas, as you note,
> __get_user_pages_unlocked() puts them afterwards.
> 
> I ended up compromising by trying to match the existing ordering of the function
> as much as I could by replacing write, force pairs with gup_flags in the same
> location (with the exception of get_user_pages_unlocked() which I felt should
> match __get_user_pages_unlocked() in signature) or if there was already a
> gup_flags parameter as in the case of __get_user_pages_unlocked() I simply
> removed the write, force pair and left the flags as the last parameter.
> 
> I am happy to rearrange parameters as needed, however I am not sure if it'd be
> worthwhile for me to do so (I am keen to try to avoid adding too much noise here
> :)
> 
> If we were to rearrange parameters for consistency I'd suggest adjusting
> __get_user_pages_unlocked() to put gup_flags before pages and do the same with
> get_user_pages_unlocked(), let me know what you think.

Yeah, ok. If the inconsistency is already there, just leave it for now.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

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

^ permalink raw reply

* Re: [PATCH 04/10] mm: replace get_user_pages_locked() write/force parameters with gup_flags
From: Jan Kara @ 2016-10-19  7:33 UTC (permalink / raw)
  To: Lorenzo Stoakes
  Cc: linux-mm, Linus Torvalds, Jan Kara, Hugh Dickins, Dave Hansen,
	Rik van Riel, Mel Gorman, Andrew Morton, adi-buildroot-devel,
	ceph-devel, dri-devel, intel-gfx, kvm, linux-alpha,
	linux-arm-kernel, linux-cris-kernel, linux-fbdev, linux-fsdevel,
	linux-ia64, linux-kernel, linux-media, linux-mips, linux-rdma,
	linux-s390, linux-samsung-soc, linux-scsi, linux-security-module,
	linux-sh@
In-Reply-To: <20161013002020.3062-5-lstoakes@gmail.com>

On Thu 13-10-16 01:20:14, Lorenzo Stoakes wrote:
> This patch removes the write and force parameters from get_user_pages_locked()
> and replaces them with a gup_flags parameter to make the use of FOLL_FORCE
> explicit in callers as use of this flag can result in surprising behaviour (and
> hence bugs) within the mm subsystem.
> 
> Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>

After our discussion the patch looks good to me. You can add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

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

^ permalink raw reply

* Re: [PATCH 05/10] mm: replace get_vaddr_frames() write/force parameters with gup_flags
From: Jan Kara @ 2016-10-19  7:34 UTC (permalink / raw)
  To: Lorenzo Stoakes
  Cc: linux-mips, linux-fbdev, Jan Kara, kvm, linux-sh, Dave Hansen,
	dri-devel, linux-mm, netdev, sparclinux, linux-ia64, linux-s390,
	linux-samsung-soc, linux-scsi, linux-rdma, x86, Hugh Dickins,
	linux-media, Rik van Riel, intel-gfx, adi-buildroot-devel,
	ceph-devel, linux-arm-kernel, linux-cris-kernel, Linus Torvalds,
	linuxppc-dev, linux-kernel, linux-security-module, linux-alpha
In-Reply-To: <20161013002020.3062-6-lstoakes@gmail.com>

On Thu 13-10-16 01:20:15, Lorenzo Stoakes wrote:
> This patch removes the write and force parameters from get_vaddr_frames() and
> replaces them with a gup_flags parameter to make the use of FOLL_FORCE explicit
> in callers as use of this flag can result in surprising behaviour (and hence
> bugs) within the mm subsystem.
> 
> Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>

Looks good. You can add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  drivers/gpu/drm/exynos/exynos_drm_g2d.c    |  3 ++-
>  drivers/media/platform/omap/omap_vout.c    |  2 +-
>  drivers/media/v4l2-core/videobuf2-memops.c |  6 +++++-
>  include/linux/mm.h                         |  2 +-
>  mm/frame_vector.c                          | 13 ++-----------
>  5 files changed, 11 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_g2d.c b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> index aa92dec..fbd13fa 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> @@ -488,7 +488,8 @@ static dma_addr_t *g2d_userptr_get_dma_addr(struct drm_device *drm_dev,
>  		goto err_free;
>  	}
>  
> -	ret = get_vaddr_frames(start, npages, true, true, g2d_userptr->vec);
> +	ret = get_vaddr_frames(start, npages, FOLL_FORCE | FOLL_WRITE,
> +		g2d_userptr->vec);
>  	if (ret != npages) {
>  		DRM_ERROR("failed to get user pages from userptr.\n");
>  		if (ret < 0)
> diff --git a/drivers/media/platform/omap/omap_vout.c b/drivers/media/platform/omap/omap_vout.c
> index e668dde..a31b95c 100644
> --- a/drivers/media/platform/omap/omap_vout.c
> +++ b/drivers/media/platform/omap/omap_vout.c
> @@ -214,7 +214,7 @@ static int omap_vout_get_userptr(struct videobuf_buffer *vb, u32 virtp,
>  	if (!vec)
>  		return -ENOMEM;
>  
> -	ret = get_vaddr_frames(virtp, 1, true, false, vec);
> +	ret = get_vaddr_frames(virtp, 1, FOLL_WRITE, vec);
>  	if (ret != 1) {
>  		frame_vector_destroy(vec);
>  		return -EINVAL;
> diff --git a/drivers/media/v4l2-core/videobuf2-memops.c b/drivers/media/v4l2-core/videobuf2-memops.c
> index 3c3b517..1cd322e 100644
> --- a/drivers/media/v4l2-core/videobuf2-memops.c
> +++ b/drivers/media/v4l2-core/videobuf2-memops.c
> @@ -42,6 +42,10 @@ struct frame_vector *vb2_create_framevec(unsigned long start,
>  	unsigned long first, last;
>  	unsigned long nr;
>  	struct frame_vector *vec;
> +	unsigned int flags = FOLL_FORCE;
> +
> +	if (write)
> +		flags |= FOLL_WRITE;
>  
>  	first = start >> PAGE_SHIFT;
>  	last = (start + length - 1) >> PAGE_SHIFT;
> @@ -49,7 +53,7 @@ struct frame_vector *vb2_create_framevec(unsigned long start,
>  	vec = frame_vector_create(nr);
>  	if (!vec)
>  		return ERR_PTR(-ENOMEM);
> -	ret = get_vaddr_frames(start & PAGE_MASK, nr, write, true, vec);
> +	ret = get_vaddr_frames(start & PAGE_MASK, nr, flags, vec);
>  	if (ret < 0)
>  		goto out_destroy;
>  	/* We accept only complete set of PFNs */
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 27ab538..5ff084f6 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1305,7 +1305,7 @@ struct frame_vector {
>  struct frame_vector *frame_vector_create(unsigned int nr_frames);
>  void frame_vector_destroy(struct frame_vector *vec);
>  int get_vaddr_frames(unsigned long start, unsigned int nr_pfns,
> -		     bool write, bool force, struct frame_vector *vec);
> +		     unsigned int gup_flags, struct frame_vector *vec);
>  void put_vaddr_frames(struct frame_vector *vec);
>  int frame_vector_to_pages(struct frame_vector *vec);
>  void frame_vector_to_pfns(struct frame_vector *vec);
> diff --git a/mm/frame_vector.c b/mm/frame_vector.c
> index 81b6749..db77dcb 100644
> --- a/mm/frame_vector.c
> +++ b/mm/frame_vector.c
> @@ -11,10 +11,7 @@
>   * get_vaddr_frames() - map virtual addresses to pfns
>   * @start:	starting user address
>   * @nr_frames:	number of pages / pfns from start to map
> - * @write:	whether pages will be written to by the caller
> - * @force:	whether to force write access even if user mapping is
> - *		readonly. See description of the same argument of
> -		get_user_pages().
> + * @gup_flags:	flags modifying lookup behaviour
>   * @vec:	structure which receives pages / pfns of the addresses mapped.
>   *		It should have space for at least nr_frames entries.
>   *
> @@ -34,23 +31,17 @@
>   * This function takes care of grabbing mmap_sem as necessary.
>   */
>  int get_vaddr_frames(unsigned long start, unsigned int nr_frames,
> -		     bool write, bool force, struct frame_vector *vec)
> +		     unsigned int gup_flags, struct frame_vector *vec)
>  {
>  	struct mm_struct *mm = current->mm;
>  	struct vm_area_struct *vma;
>  	int ret = 0;
>  	int err;
>  	int locked;
> -	unsigned int gup_flags = 0;
>  
>  	if (nr_frames == 0)
>  		return 0;
>  
> -	if (write)
> -		gup_flags |= FOLL_WRITE;
> -	if (force)
> -		gup_flags |= FOLL_FORCE;
> -
>  	if (WARN_ON_ONCE(nr_frames > vec->nr_allocated))
>  		nr_frames = vec->nr_allocated;
>  
> -- 
> 2.10.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply

* Re: [PATCH 06/10] mm: replace get_user_pages() write/force parameters with gup_flags
From: Jan Kara @ 2016-10-19  7:44 UTC (permalink / raw)
  To: Lorenzo Stoakes
  Cc: linux-mips, linux-fbdev, Jan Kara, kvm, linux-sh, Dave Hansen,
	dri-devel, linux-mm, netdev, sparclinux, linux-ia64, linux-s390,
	linux-samsung-soc, linux-scsi, linux-rdma, x86, Hugh Dickins,
	linux-media, Rik van Riel, intel-gfx, adi-buildroot-devel,
	ceph-devel, linux-arm-kernel, linux-cris-kernel, Linus Torvalds,
	linuxppc-dev, linux-kernel, linux-security-module, linux-alpha
In-Reply-To: <20161013002020.3062-7-lstoakes@gmail.com>

On Thu 13-10-16 01:20:16, Lorenzo Stoakes wrote:
> This patch removes the write and force parameters from get_user_pages() and
> replaces them with a gup_flags parameter to make the use of FOLL_FORCE explicit
> in callers as use of this flag can result in surprising behaviour (and hence
> bugs) within the mm subsystem.
> 
> Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>

The patch looks good. You can add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  arch/cris/arch-v32/drivers/cryptocop.c                 |  4 +---
>  arch/ia64/kernel/err_inject.c                          |  2 +-
>  arch/x86/mm/mpx.c                                      |  5 ++---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c                |  7 +++++--
>  drivers/gpu/drm/radeon/radeon_ttm.c                    |  3 ++-
>  drivers/gpu/drm/via/via_dmablit.c                      |  4 ++--
>  drivers/infiniband/core/umem.c                         |  6 +++++-
>  drivers/infiniband/hw/mthca/mthca_memfree.c            |  2 +-
>  drivers/infiniband/hw/qib/qib_user_pages.c             |  3 ++-
>  drivers/infiniband/hw/usnic/usnic_uiom.c               |  5 ++++-
>  drivers/media/v4l2-core/videobuf-dma-sg.c              |  7 +++++--
>  drivers/misc/mic/scif/scif_rma.c                       |  3 +--
>  drivers/misc/sgi-gru/grufault.c                        |  2 +-
>  drivers/platform/goldfish/goldfish_pipe.c              |  3 ++-
>  drivers/rapidio/devices/rio_mport_cdev.c               |  3 ++-
>  .../vc04_services/interface/vchiq_arm/vchiq_2835_arm.c |  3 +--
>  .../vc04_services/interface/vchiq_arm/vchiq_arm.c      |  3 +--
>  drivers/virt/fsl_hypervisor.c                          |  4 ++--
>  include/linux/mm.h                                     |  2 +-
>  mm/gup.c                                               | 12 +++---------
>  mm/mempolicy.c                                         |  2 +-
>  mm/nommu.c                                             | 18 ++++--------------
>  22 files changed, 49 insertions(+), 54 deletions(-)
> 
> diff --git a/arch/cris/arch-v32/drivers/cryptocop.c b/arch/cris/arch-v32/drivers/cryptocop.c
> index b5698c8..099e170 100644
> --- a/arch/cris/arch-v32/drivers/cryptocop.c
> +++ b/arch/cris/arch-v32/drivers/cryptocop.c
> @@ -2722,7 +2722,6 @@ static int cryptocop_ioctl_process(struct inode *inode, struct file *filp, unsig
>  	err = get_user_pages((unsigned long int)(oper.indata + prev_ix),
>  			     noinpages,
>  			     0,  /* read access only for in data */
> -			     0, /* no force */
>  			     inpages,
>  			     NULL);
>  
> @@ -2736,8 +2735,7 @@ static int cryptocop_ioctl_process(struct inode *inode, struct file *filp, unsig
>  	if (oper.do_cipher){
>  		err = get_user_pages((unsigned long int)oper.cipher_outdata,
>  				     nooutpages,
> -				     1, /* write access for out data */
> -				     0, /* no force */
> +				     FOLL_WRITE, /* write access for out data */
>  				     outpages,
>  				     NULL);
>  		up_read(&current->mm->mmap_sem);
> diff --git a/arch/ia64/kernel/err_inject.c b/arch/ia64/kernel/err_inject.c
> index 09f8457..5ed0ea9 100644
> --- a/arch/ia64/kernel/err_inject.c
> +++ b/arch/ia64/kernel/err_inject.c
> @@ -142,7 +142,7 @@ store_virtual_to_phys(struct device *dev, struct device_attribute *attr,
>  	u64 virt_addr=simple_strtoull(buf, NULL, 16);
>  	int ret;
>  
> -	ret = get_user_pages(virt_addr, 1, VM_READ, 0, NULL, NULL);
> +	ret = get_user_pages(virt_addr, 1, FOLL_WRITE, NULL, NULL);
>  	if (ret<=0) {
>  #ifdef ERR_INJ_DEBUG
>  		printk("Virtual address %lx is not existing.\n",virt_addr);
> diff --git a/arch/x86/mm/mpx.c b/arch/x86/mm/mpx.c
> index 8047687..e4f8009 100644
> --- a/arch/x86/mm/mpx.c
> +++ b/arch/x86/mm/mpx.c
> @@ -544,10 +544,9 @@ static int mpx_resolve_fault(long __user *addr, int write)
>  {
>  	long gup_ret;
>  	int nr_pages = 1;
> -	int force = 0;
>  
> -	gup_ret = get_user_pages((unsigned long)addr, nr_pages, write,
> -			force, NULL, NULL);
> +	gup_ret = get_user_pages((unsigned long)addr, nr_pages,
> +			write ? FOLL_WRITE : 0,	NULL, NULL);
>  	/*
>  	 * get_user_pages() returns number of pages gotten.
>  	 * 0 means we failed to fault in and get anything,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 887483b..dcaf691 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -555,10 +555,13 @@ struct amdgpu_ttm_tt {
>  int amdgpu_ttm_tt_get_user_pages(struct ttm_tt *ttm, struct page **pages)
>  {
>  	struct amdgpu_ttm_tt *gtt = (void *)ttm;
> -	int write = !(gtt->userflags & AMDGPU_GEM_USERPTR_READONLY);
> +	unsigned int flags = 0;
>  	unsigned pinned = 0;
>  	int r;
>  
> +	if (!(gtt->userflags & AMDGPU_GEM_USERPTR_READONLY))
> +		flags |= FOLL_WRITE;
> +
>  	if (gtt->userflags & AMDGPU_GEM_USERPTR_ANONONLY) {
>  		/* check that we only use anonymous memory
>  		   to prevent problems with writeback */
> @@ -581,7 +584,7 @@ int amdgpu_ttm_tt_get_user_pages(struct ttm_tt *ttm, struct page **pages)
>  		list_add(&guptask.list, &gtt->guptasks);
>  		spin_unlock(&gtt->guptasklock);
>  
> -		r = get_user_pages(userptr, num_pages, write, 0, p, NULL);
> +		r = get_user_pages(userptr, num_pages, flags, p, NULL);
>  
>  		spin_lock(&gtt->guptasklock);
>  		list_del(&guptask.list);
> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
> index 4552682..3de5e6e 100644
> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> @@ -566,7 +566,8 @@ static int radeon_ttm_tt_pin_userptr(struct ttm_tt *ttm)
>  		uint64_t userptr = gtt->userptr + pinned * PAGE_SIZE;
>  		struct page **pages = ttm->pages + pinned;
>  
> -		r = get_user_pages(userptr, num_pages, write, 0, pages, NULL);
> +		r = get_user_pages(userptr, num_pages, write ? FOLL_WRITE : 0,
> +				   pages, NULL);
>  		if (r < 0)
>  			goto release_pages;
>  
> diff --git a/drivers/gpu/drm/via/via_dmablit.c b/drivers/gpu/drm/via/via_dmablit.c
> index 7e2a12c..1a3ad76 100644
> --- a/drivers/gpu/drm/via/via_dmablit.c
> +++ b/drivers/gpu/drm/via/via_dmablit.c
> @@ -241,8 +241,8 @@ via_lock_all_dma_pages(drm_via_sg_info_t *vsg,  drm_via_dmablit_t *xfer)
>  	down_read(&current->mm->mmap_sem);
>  	ret = get_user_pages((unsigned long)xfer->mem_addr,
>  			     vsg->num_pages,
> -			     (vsg->direction == DMA_FROM_DEVICE),
> -			     0, vsg->pages, NULL);
> +			     (vsg->direction == DMA_FROM_DEVICE) ? FOLL_WRITE : 0,
> +			     vsg->pages, NULL);
>  
>  	up_read(&current->mm->mmap_sem);
>  	if (ret != vsg->num_pages) {
> diff --git a/drivers/infiniband/core/umem.c b/drivers/infiniband/core/umem.c
> index c68746c..224ad27 100644
> --- a/drivers/infiniband/core/umem.c
> +++ b/drivers/infiniband/core/umem.c
> @@ -94,6 +94,7 @@ struct ib_umem *ib_umem_get(struct ib_ucontext *context, unsigned long addr,
>  	unsigned long dma_attrs = 0;
>  	struct scatterlist *sg, *sg_list_start;
>  	int need_release = 0;
> +	unsigned int gup_flags = FOLL_WRITE;
>  
>  	if (dmasync)
>  		dma_attrs |= DMA_ATTR_WRITE_BARRIER;
> @@ -183,6 +184,9 @@ struct ib_umem *ib_umem_get(struct ib_ucontext *context, unsigned long addr,
>  	if (ret)
>  		goto out;
>  
> +	if (!umem->writable)
> +		gup_flags |= FOLL_FORCE;
> +
>  	need_release = 1;
>  	sg_list_start = umem->sg_head.sgl;
>  
> @@ -190,7 +194,7 @@ struct ib_umem *ib_umem_get(struct ib_ucontext *context, unsigned long addr,
>  		ret = get_user_pages(cur_base,
>  				     min_t(unsigned long, npages,
>  					   PAGE_SIZE / sizeof (struct page *)),
> -				     1, !umem->writable, page_list, vma_list);
> +				     gup_flags, page_list, vma_list);
>  
>  		if (ret < 0)
>  			goto out;
> diff --git a/drivers/infiniband/hw/mthca/mthca_memfree.c b/drivers/infiniband/hw/mthca/mthca_memfree.c
> index 6c00d04..c6fe89d 100644
> --- a/drivers/infiniband/hw/mthca/mthca_memfree.c
> +++ b/drivers/infiniband/hw/mthca/mthca_memfree.c
> @@ -472,7 +472,7 @@ int mthca_map_user_db(struct mthca_dev *dev, struct mthca_uar *uar,
>  		goto out;
>  	}
>  
> -	ret = get_user_pages(uaddr & PAGE_MASK, 1, 1, 0, pages, NULL);
> +	ret = get_user_pages(uaddr & PAGE_MASK, 1, FOLL_WRITE, pages, NULL);
>  	if (ret < 0)
>  		goto out;
>  
> diff --git a/drivers/infiniband/hw/qib/qib_user_pages.c b/drivers/infiniband/hw/qib/qib_user_pages.c
> index 2d2b94f..75f0862 100644
> --- a/drivers/infiniband/hw/qib/qib_user_pages.c
> +++ b/drivers/infiniband/hw/qib/qib_user_pages.c
> @@ -67,7 +67,8 @@ static int __qib_get_user_pages(unsigned long start_page, size_t num_pages,
>  
>  	for (got = 0; got < num_pages; got += ret) {
>  		ret = get_user_pages(start_page + got * PAGE_SIZE,
> -				     num_pages - got, 1, 1,
> +				     num_pages - got,
> +				     FOLL_WRITE | FOLL_FORCE,
>  				     p + got, NULL);
>  		if (ret < 0)
>  			goto bail_release;
> diff --git a/drivers/infiniband/hw/usnic/usnic_uiom.c b/drivers/infiniband/hw/usnic/usnic_uiom.c
> index a0b6ebe..1ccee6e 100644
> --- a/drivers/infiniband/hw/usnic/usnic_uiom.c
> +++ b/drivers/infiniband/hw/usnic/usnic_uiom.c
> @@ -111,6 +111,7 @@ static int usnic_uiom_get_pages(unsigned long addr, size_t size, int writable,
>  	int i;
>  	int flags;
>  	dma_addr_t pa;
> +	unsigned int gup_flags;
>  
>  	if (!can_do_mlock())
>  		return -EPERM;
> @@ -135,6 +136,8 @@ static int usnic_uiom_get_pages(unsigned long addr, size_t size, int writable,
>  
>  	flags = IOMMU_READ | IOMMU_CACHE;
>  	flags |= (writable) ? IOMMU_WRITE : 0;
> +	gup_flags = FOLL_WRITE;
> +	gup_flags |= (writable) ? 0 : FOLL_FORCE;
>  	cur_base = addr & PAGE_MASK;
>  	ret = 0;
>  
> @@ -142,7 +145,7 @@ static int usnic_uiom_get_pages(unsigned long addr, size_t size, int writable,
>  		ret = get_user_pages(cur_base,
>  					min_t(unsigned long, npages,
>  					PAGE_SIZE / sizeof(struct page *)),
> -					1, !writable, page_list, NULL);
> +					gup_flags, page_list, NULL);
>  
>  		if (ret < 0)
>  			goto out;
> diff --git a/drivers/media/v4l2-core/videobuf-dma-sg.c b/drivers/media/v4l2-core/videobuf-dma-sg.c
> index f300f06..1db0af6 100644
> --- a/drivers/media/v4l2-core/videobuf-dma-sg.c
> +++ b/drivers/media/v4l2-core/videobuf-dma-sg.c
> @@ -156,6 +156,7 @@ static int videobuf_dma_init_user_locked(struct videobuf_dmabuf *dma,
>  {
>  	unsigned long first, last;
>  	int err, rw = 0;
> +	unsigned int flags = FOLL_FORCE;
>  
>  	dma->direction = direction;
>  	switch (dma->direction) {
> @@ -178,12 +179,14 @@ static int videobuf_dma_init_user_locked(struct videobuf_dmabuf *dma,
>  	if (NULL == dma->pages)
>  		return -ENOMEM;
>  
> +	if (rw == READ)
> +		flags |= FOLL_WRITE;
> +
>  	dprintk(1, "init user [0x%lx+0x%lx => %d pages]\n",
>  		data, size, dma->nr_pages);
>  
>  	err = get_user_pages(data & PAGE_MASK, dma->nr_pages,
> -			     rw == READ, 1, /* force */
> -			     dma->pages, NULL);
> +			     flags, dma->pages, NULL);
>  
>  	if (err != dma->nr_pages) {
>  		dma->nr_pages = (err >= 0) ? err : 0;
> diff --git a/drivers/misc/mic/scif/scif_rma.c b/drivers/misc/mic/scif/scif_rma.c
> index e0203b1..f806a44 100644
> --- a/drivers/misc/mic/scif/scif_rma.c
> +++ b/drivers/misc/mic/scif/scif_rma.c
> @@ -1396,8 +1396,7 @@ int __scif_pin_pages(void *addr, size_t len, int *out_prot,
>  		pinned_pages->nr_pages = get_user_pages(
>  				(u64)addr,
>  				nr_pages,
> -				!!(prot & SCIF_PROT_WRITE),
> -				0,
> +				(prot & SCIF_PROT_WRITE) ? FOLL_WRITE : 0,
>  				pinned_pages->pages,
>  				NULL);
>  		up_write(&mm->mmap_sem);
> diff --git a/drivers/misc/sgi-gru/grufault.c b/drivers/misc/sgi-gru/grufault.c
> index a2d97b9..6fb773d 100644
> --- a/drivers/misc/sgi-gru/grufault.c
> +++ b/drivers/misc/sgi-gru/grufault.c
> @@ -198,7 +198,7 @@ static int non_atomic_pte_lookup(struct vm_area_struct *vma,
>  #else
>  	*pageshift = PAGE_SHIFT;
>  #endif
> -	if (get_user_pages(vaddr, 1, write, 0, &page, NULL) <= 0)
> +	if (get_user_pages(vaddr, 1, write ? FOLL_WRITE : 0, &page, NULL) <= 0)
>  		return -EFAULT;
>  	*paddr = page_to_phys(page);
>  	put_page(page);
> diff --git a/drivers/platform/goldfish/goldfish_pipe.c b/drivers/platform/goldfish/goldfish_pipe.c
> index 07462d7..1aba2c7 100644
> --- a/drivers/platform/goldfish/goldfish_pipe.c
> +++ b/drivers/platform/goldfish/goldfish_pipe.c
> @@ -309,7 +309,8 @@ static ssize_t goldfish_pipe_read_write(struct file *filp, char __user *buffer,
>  		 * much memory to the process.
>  		 */
>  		down_read(&current->mm->mmap_sem);
> -		ret = get_user_pages(address, 1, !is_write, 0, &page, NULL);
> +		ret = get_user_pages(address, 1, is_write ? 0 : FOLL_WRITE,
> +				&page, NULL);
>  		up_read(&current->mm->mmap_sem);
>  		if (ret < 0)
>  			break;
> diff --git a/drivers/rapidio/devices/rio_mport_cdev.c b/drivers/rapidio/devices/rio_mport_cdev.c
> index 436dfe8..9013a58 100644
> --- a/drivers/rapidio/devices/rio_mport_cdev.c
> +++ b/drivers/rapidio/devices/rio_mport_cdev.c
> @@ -892,7 +892,8 @@ rio_dma_transfer(struct file *filp, u32 transfer_mode,
>  		down_read(&current->mm->mmap_sem);
>  		pinned = get_user_pages(
>  				(unsigned long)xfer->loc_addr & PAGE_MASK,
> -				nr_pages, dir == DMA_FROM_DEVICE, 0,
> +				nr_pages,
> +				dir == DMA_FROM_DEVICE ? FOLL_WRITE : 0,
>  				page_list, NULL);
>  		up_read(&current->mm->mmap_sem);
>  
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> index c29040f..1091b9f 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> @@ -423,8 +423,7 @@ create_pagelist(char __user *buf, size_t count, unsigned short type,
>  		actual_pages = get_user_pages(task, task->mm,
>  				          (unsigned long)buf & ~(PAGE_SIZE - 1),
>  					  num_pages,
> -					  (type == PAGELIST_READ) /*Write */ ,
> -					  0 /*Force */ ,
> +					  (type == PAGELIST_READ) ? FOLL_WRITE : 0,
>  					  pages,
>  					  NULL /*vmas */);
>  		up_read(&task->mm->mmap_sem);
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> index e11c0e0..7b6cd4d 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> @@ -1477,8 +1477,7 @@ dump_phys_mem(void *virt_addr, uint32_t num_bytes)
>  		current->mm,              /* mm */
>  		(unsigned long)virt_addr, /* start */
>  		num_pages,                /* len */
> -		0,                        /* write */
> -		0,                        /* force */
> +		0,                        /* gup_flags */
>  		pages,                    /* pages (array of page pointers) */
>  		NULL);                    /* vmas */
>  	up_read(&current->mm->mmap_sem);
> diff --git a/drivers/virt/fsl_hypervisor.c b/drivers/virt/fsl_hypervisor.c
> index 60bdad3..150ce2a 100644
> --- a/drivers/virt/fsl_hypervisor.c
> +++ b/drivers/virt/fsl_hypervisor.c
> @@ -245,8 +245,8 @@ static long ioctl_memcpy(struct fsl_hv_ioctl_memcpy __user *p)
>  	/* Get the physical addresses of the source buffer */
>  	down_read(&current->mm->mmap_sem);
>  	num_pinned = get_user_pages(param.local_vaddr - lb_offset,
> -		num_pages, (param.source == -1) ? READ : WRITE,
> -		0, pages, NULL);
> +		num_pages, (param.source == -1) ? 0 : FOLL_WRITE,
> +		pages, NULL);
>  	up_read(&current->mm->mmap_sem);
>  
>  	if (num_pinned != num_pages) {
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 5ff084f6..686a477 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1279,7 +1279,7 @@ long get_user_pages_remote(struct task_struct *tsk, struct mm_struct *mm,
>  			    int write, int force, struct page **pages,
>  			    struct vm_area_struct **vmas);
>  long get_user_pages(unsigned long start, unsigned long nr_pages,
> -			    int write, int force, struct page **pages,
> +			    unsigned int gup_flags, struct page **pages,
>  			    struct vm_area_struct **vmas);
>  long get_user_pages_locked(unsigned long start, unsigned long nr_pages,
>  		    unsigned int gup_flags, struct page **pages, int *locked);
> diff --git a/mm/gup.c b/mm/gup.c
> index 7a0d033..dc91303 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -977,18 +977,12 @@ EXPORT_SYMBOL(get_user_pages_remote);
>   * obviously don't pass FOLL_REMOTE in here.
>   */
>  long get_user_pages(unsigned long start, unsigned long nr_pages,
> -		int write, int force, struct page **pages,
> +		unsigned int gup_flags, struct page **pages,
>  		struct vm_area_struct **vmas)
>  {
> -	unsigned int flags = FOLL_TOUCH;
> -
> -	if (write)
> -		flags |= FOLL_WRITE;
> -	if (force)
> -		flags |= FOLL_FORCE;
> -
>  	return __get_user_pages_locked(current, current->mm, start, nr_pages,
> -				       pages, vmas, NULL, false, flags);
> +				       pages, vmas, NULL, false,
> +				       gup_flags | FOLL_TOUCH);
>  }
>  EXPORT_SYMBOL(get_user_pages);
>  
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index ad1c96a..0b859af 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -850,7 +850,7 @@ static int lookup_node(unsigned long addr)
>  	struct page *p;
>  	int err;
>  
> -	err = get_user_pages(addr & PAGE_MASK, 1, 0, 0, &p, NULL);
> +	err = get_user_pages(addr & PAGE_MASK, 1, 0, &p, NULL);
>  	if (err >= 0) {
>  		err = page_to_nid(p);
>  		put_page(p);
> diff --git a/mm/nommu.c b/mm/nommu.c
> index 842cfdd..70cb844 100644
> --- a/mm/nommu.c
> +++ b/mm/nommu.c
> @@ -160,18 +160,11 @@ long __get_user_pages(struct task_struct *tsk, struct mm_struct *mm,
>   * - don't permit access to VMAs that don't support it, such as I/O mappings
>   */
>  long get_user_pages(unsigned long start, unsigned long nr_pages,
> -		    int write, int force, struct page **pages,
> +		    unsigned int gup_flags, struct page **pages,
>  		    struct vm_area_struct **vmas)
>  {
> -	int flags = 0;
> -
> -	if (write)
> -		flags |= FOLL_WRITE;
> -	if (force)
> -		flags |= FOLL_FORCE;
> -
> -	return __get_user_pages(current, current->mm, start, nr_pages, flags,
> -				pages, vmas, NULL);
> +	return __get_user_pages(current, current->mm, start, nr_pages,
> +				gup_flags, pages, vmas, NULL);
>  }
>  EXPORT_SYMBOL(get_user_pages);
>  
> @@ -179,10 +172,7 @@ long get_user_pages_locked(unsigned long start, unsigned long nr_pages,
>  			    unsigned int gup_flags, struct page **pages,
>  			    int *locked)
>  {
> -	int write = gup_flags & FOLL_WRITE;
> -	int force = gup_flags & FOLL_FORCE;
> -
> -	return get_user_pages(start, nr_pages, write, force, pages, NULL);
> +	return get_user_pages(start, nr_pages, gup_flags, pages, NULL);
>  }
>  EXPORT_SYMBOL(get_user_pages_locked);
>  
> -- 
> 2.10.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply

* Re: [PATCH 07/10] mm: replace get_user_pages_remote() write/force parameters with gup_flags
From: Jan Kara @ 2016-10-19  7:47 UTC (permalink / raw)
  To: Lorenzo Stoakes
  Cc: linux-mips, linux-fbdev, Jan Kara, kvm, linux-sh, Dave Hansen,
	dri-devel, linux-mm, netdev, sparclinux, linux-ia64, linux-s390,
	linux-samsung-soc, linux-scsi, linux-rdma, x86, Hugh Dickins,
	linux-media, Rik van Riel, intel-gfx, adi-buildroot-devel,
	ceph-devel, linux-arm-kernel, linux-cris-kernel, Linus Torvalds,
	linuxppc-dev, linux-kernel, linux-security-module, linux-alpha
In-Reply-To: <20161013002020.3062-8-lstoakes@gmail.com>

On Thu 13-10-16 01:20:17, Lorenzo Stoakes wrote:
> This patch removes the write and force parameters from get_user_pages_remote()
> and replaces them with a gup_flags parameter to make the use of FOLL_FORCE
> explicit in callers as use of this flag can result in surprising behaviour (and
> hence bugs) within the mm subsystem.
> 
> Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>

Looks good. You can add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  drivers/gpu/drm/etnaviv/etnaviv_gem.c   |  7 +++++--
>  drivers/gpu/drm/i915/i915_gem_userptr.c |  6 +++++-
>  drivers/infiniband/core/umem_odp.c      |  7 +++++--
>  fs/exec.c                               |  9 +++++++--
>  include/linux/mm.h                      |  2 +-
>  kernel/events/uprobes.c                 |  6 ++++--
>  mm/gup.c                                | 22 +++++++---------------
>  mm/memory.c                             |  6 +++++-
>  security/tomoyo/domain.c                |  2 +-
>  9 files changed, 40 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> index 5ce3603..0370b84 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> @@ -748,19 +748,22 @@ static struct page **etnaviv_gem_userptr_do_get_pages(
>  	int ret = 0, pinned, npages = etnaviv_obj->base.size >> PAGE_SHIFT;
>  	struct page **pvec;
>  	uintptr_t ptr;
> +	unsigned int flags = 0;
>  
>  	pvec = drm_malloc_ab(npages, sizeof(struct page *));
>  	if (!pvec)
>  		return ERR_PTR(-ENOMEM);
>  
> +	if (!etnaviv_obj->userptr.ro)
> +		flags |= FOLL_WRITE;
> +
>  	pinned = 0;
>  	ptr = etnaviv_obj->userptr.ptr;
>  
>  	down_read(&mm->mmap_sem);
>  	while (pinned < npages) {
>  		ret = get_user_pages_remote(task, mm, ptr, npages - pinned,
> -					    !etnaviv_obj->userptr.ro, 0,
> -					    pvec + pinned, NULL);
> +					    flags, pvec + pinned, NULL);
>  		if (ret < 0)
>  			break;
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c
> index e537930..c6f780f 100644
> --- a/drivers/gpu/drm/i915/i915_gem_userptr.c
> +++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
> @@ -508,6 +508,10 @@ __i915_gem_userptr_get_pages_worker(struct work_struct *_work)
>  	pvec = drm_malloc_gfp(npages, sizeof(struct page *), GFP_TEMPORARY);
>  	if (pvec != NULL) {
>  		struct mm_struct *mm = obj->userptr.mm->mm;
> +		unsigned int flags = 0;
> +
> +		if (!obj->userptr.read_only)
> +			flags |= FOLL_WRITE;
>  
>  		ret = -EFAULT;
>  		if (atomic_inc_not_zero(&mm->mm_users)) {
> @@ -517,7 +521,7 @@ __i915_gem_userptr_get_pages_worker(struct work_struct *_work)
>  					(work->task, mm,
>  					 obj->userptr.ptr + pinned * PAGE_SIZE,
>  					 npages - pinned,
> -					 !obj->userptr.read_only, 0,
> +					 flags,
>  					 pvec + pinned, NULL);
>  				if (ret < 0)
>  					break;
> diff --git a/drivers/infiniband/core/umem_odp.c b/drivers/infiniband/core/umem_odp.c
> index 75077a0..1f0fe32 100644
> --- a/drivers/infiniband/core/umem_odp.c
> +++ b/drivers/infiniband/core/umem_odp.c
> @@ -527,6 +527,7 @@ int ib_umem_odp_map_dma_pages(struct ib_umem *umem, u64 user_virt, u64 bcnt,
>  	u64 off;
>  	int j, k, ret = 0, start_idx, npages = 0;
>  	u64 base_virt_addr;
> +	unsigned int flags = 0;
>  
>  	if (access_mask == 0)
>  		return -EINVAL;
> @@ -556,6 +557,9 @@ int ib_umem_odp_map_dma_pages(struct ib_umem *umem, u64 user_virt, u64 bcnt,
>  		goto out_put_task;
>  	}
>  
> +	if (access_mask & ODP_WRITE_ALLOWED_BIT)
> +		flags |= FOLL_WRITE;
> +
>  	start_idx = (user_virt - ib_umem_start(umem)) >> PAGE_SHIFT;
>  	k = start_idx;
>  
> @@ -574,8 +578,7 @@ int ib_umem_odp_map_dma_pages(struct ib_umem *umem, u64 user_virt, u64 bcnt,
>  		 */
>  		npages = get_user_pages_remote(owning_process, owning_mm,
>  				user_virt, gup_num_pages,
> -				access_mask & ODP_WRITE_ALLOWED_BIT,
> -				0, local_page_list, NULL);
> +				flags, local_page_list, NULL);
>  		up_read(&owning_mm->mmap_sem);
>  
>  		if (npages < 0)
> diff --git a/fs/exec.c b/fs/exec.c
> index 6fcfb3f..4e497b9 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -191,6 +191,7 @@ static struct page *get_arg_page(struct linux_binprm *bprm, unsigned long pos,
>  {
>  	struct page *page;
>  	int ret;
> +	unsigned int gup_flags = FOLL_FORCE;
>  
>  #ifdef CONFIG_STACK_GROWSUP
>  	if (write) {
> @@ -199,12 +200,16 @@ static struct page *get_arg_page(struct linux_binprm *bprm, unsigned long pos,
>  			return NULL;
>  	}
>  #endif
> +
> +	if (write)
> +		gup_flags |= FOLL_WRITE;
> +
>  	/*
>  	 * We are doing an exec().  'current' is the process
>  	 * doing the exec and bprm->mm is the new process's mm.
>  	 */
> -	ret = get_user_pages_remote(current, bprm->mm, pos, 1, write,
> -			1, &page, NULL);
> +	ret = get_user_pages_remote(current, bprm->mm, pos, 1, gup_flags,
> +			&page, NULL);
>  	if (ret <= 0)
>  		return NULL;
>  
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 686a477..2a481d3 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1276,7 +1276,7 @@ long __get_user_pages(struct task_struct *tsk, struct mm_struct *mm,
>  		      struct vm_area_struct **vmas, int *nonblocking);
>  long get_user_pages_remote(struct task_struct *tsk, struct mm_struct *mm,
>  			    unsigned long start, unsigned long nr_pages,
> -			    int write, int force, struct page **pages,
> +			    unsigned int gup_flags, struct page **pages,
>  			    struct vm_area_struct **vmas);
>  long get_user_pages(unsigned long start, unsigned long nr_pages,
>  			    unsigned int gup_flags, struct page **pages,
> diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
> index d4129bb..f9ec9ad 100644
> --- a/kernel/events/uprobes.c
> +++ b/kernel/events/uprobes.c
> @@ -300,7 +300,8 @@ int uprobe_write_opcode(struct mm_struct *mm, unsigned long vaddr,
>  
>  retry:
>  	/* Read the page with vaddr into memory */
> -	ret = get_user_pages_remote(NULL, mm, vaddr, 1, 0, 1, &old_page, &vma);
> +	ret = get_user_pages_remote(NULL, mm, vaddr, 1, FOLL_FORCE, &old_page,
> +			&vma);
>  	if (ret <= 0)
>  		return ret;
>  
> @@ -1710,7 +1711,8 @@ static int is_trap_at_addr(struct mm_struct *mm, unsigned long vaddr)
>  	 * but we treat this as a 'remote' access since it is
>  	 * essentially a kernel access to the memory.
>  	 */
> -	result = get_user_pages_remote(NULL, mm, vaddr, 1, 0, 1, &page, NULL);
> +	result = get_user_pages_remote(NULL, mm, vaddr, 1, FOLL_FORCE, &page,
> +			NULL);
>  	if (result < 0)
>  		return result;
>  
> diff --git a/mm/gup.c b/mm/gup.c
> index dc91303..0deecf3 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -905,9 +905,7 @@ EXPORT_SYMBOL(get_user_pages_unlocked);
>   * @mm:		mm_struct of target mm
>   * @start:	starting user address
>   * @nr_pages:	number of pages from start to pin
> - * @write:	whether pages will be written to by the caller
> - * @force:	whether to force access even when user mapping is currently
> - *		protected (but never forces write access to shared mapping).
> + * @gup_flags:	flags modifying lookup behaviour
>   * @pages:	array that receives pointers to the pages pinned.
>   *		Should be at least nr_pages long. Or NULL, if caller
>   *		only intends to ensure the pages are faulted in.
> @@ -936,9 +934,9 @@ EXPORT_SYMBOL(get_user_pages_unlocked);
>   * or similar operation cannot guarantee anything stronger anyway because
>   * locks can't be held over the syscall boundary.
>   *
> - * If write=0, the page must not be written to. If the page is written to,
> - * set_page_dirty (or set_page_dirty_lock, as appropriate) must be called
> - * after the page is finished with, and before put_page is called.
> + * If gup_flags & FOLL_WRITE == 0, the page must not be written to. If the page
> + * is written to, set_page_dirty (or set_page_dirty_lock, as appropriate) must
> + * be called after the page is finished with, and before put_page is called.
>   *
>   * get_user_pages is typically used for fewer-copy IO operations, to get a
>   * handle on the memory by some means other than accesses via the user virtual
> @@ -955,18 +953,12 @@ EXPORT_SYMBOL(get_user_pages_unlocked);
>   */
>  long get_user_pages_remote(struct task_struct *tsk, struct mm_struct *mm,
>  		unsigned long start, unsigned long nr_pages,
> -		int write, int force, struct page **pages,
> +		unsigned int gup_flags, struct page **pages,
>  		struct vm_area_struct **vmas)
>  {
> -	unsigned int flags = FOLL_TOUCH | FOLL_REMOTE;
> -
> -	if (write)
> -		flags |= FOLL_WRITE;
> -	if (force)
> -		flags |= FOLL_FORCE;
> -
>  	return __get_user_pages_locked(tsk, mm, start, nr_pages, pages, vmas,
> -				       NULL, false, flags);
> +				       NULL, false,
> +				       gup_flags | FOLL_TOUCH | FOLL_REMOTE);
>  }
>  EXPORT_SYMBOL(get_user_pages_remote);
>  
> diff --git a/mm/memory.c b/mm/memory.c
> index fc1987d..20a9adb 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -3873,6 +3873,10 @@ static int __access_remote_vm(struct task_struct *tsk, struct mm_struct *mm,
>  {
>  	struct vm_area_struct *vma;
>  	void *old_buf = buf;
> +	unsigned int flags = FOLL_FORCE;
> +
> +	if (write)
> +		flags |= FOLL_WRITE;
>  
>  	down_read(&mm->mmap_sem);
>  	/* ignore errors, just check how much was successfully transferred */
> @@ -3882,7 +3886,7 @@ static int __access_remote_vm(struct task_struct *tsk, struct mm_struct *mm,
>  		struct page *page = NULL;
>  
>  		ret = get_user_pages_remote(tsk, mm, addr, 1,
> -				write, 1, &page, &vma);
> +				flags, &page, &vma);
>  		if (ret <= 0) {
>  #ifndef CONFIG_HAVE_IOREMAP_PROT
>  			break;
> diff --git a/security/tomoyo/domain.c b/security/tomoyo/domain.c
> index ade7c6c..682b73a 100644
> --- a/security/tomoyo/domain.c
> +++ b/security/tomoyo/domain.c
> @@ -881,7 +881,7 @@ bool tomoyo_dump_page(struct linux_binprm *bprm, unsigned long pos,
>  	 * the execve().
>  	 */
>  	if (get_user_pages_remote(current, bprm->mm, pos, 1,
> -				0, 1, &page, NULL) <= 0)
> +				FOLL_FORCE, &page, NULL) <= 0)
>  		return false;
>  #else
>  	page = bprm->page[pos / PAGE_SIZE];
> -- 
> 2.10.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ 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