aoe: miscellaneous fixes follow-up recent patch submissions

The commit f3b8e07af7744cbb, "aoe: commands in retransmit queue use new destination on failure", omits the copying of the coarse-grained time when an AoE command was sent during the failover from one destination MAC address on the AoE target to another.

The coarse-grained timing is only used when the system time changes or an unlikely length of time has passed since the sending of the AoE command. Users will not be impacted unless their system clock is very inaccurate or something unusual (e.g., 10 GbE link reset) happens during the period when the aoe driver is handling the failure of a port on the AoE target. Being effected will mean that an AoE target could be considered "down" too eagerly.

---
 drivers/block/aoe/aoecmd.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

index d9bc6ff..d609c47 100644
@@ -581,6 +581,7 @@ reassign_frame(struct list_head *pos)
 	nf->waited = 0;
 	nf->waited_total = f->waited_total;
 	nf->sent = f->sent;
+	nf->sent_jiffs = f->sent_jiffs;
 	f->skb = skb;
 	aoe_freetframe(f);
 	f->t->nout--;

1.7.1

This patch series applies to today's linux-next/akpm, commit d3faae60d84f586ff8937b77c8476bca1b5f8ec6.

Ed L. Cashin (8):
aoe: copy fallback timing information on destination failover
aoe: remove vestigial request queue allocation
aoe: increase default cap on outstanding AoE commands in the network
aoe: cleanup: correct comment for aoetgt nout
aoe: remove call to request handler from I/O completion
aoe: make error messages more specific in static minor allocation
aoe: initialize sysminor to avoid compiler warning
aoe: return real minor number for static minors

 drivers/block/aoe/aoe.h    |    2 +-
 drivers/block/aoe/aoeblk.c |   17 ++++-------------
 drivers/block/aoe/aoecmd.c |    5 ++---
 drivers/block/aoe/aoedev.c |   35 ++++++++++++++++++++++-------------
 4 files changed, 29 insertions(+), 30 deletions(-)

Before the aoe driver was an I/O request handler, it was a make_request-style block driver. Even so, there was a problem where sysfs expected a request queue to exist, so one was provided in commit 7135a71b19be1fa, "aoe: allocate unused request_queue for sysfs".

During the transition to the request-handler style, a patch was merged that was based on a driver without the noop queue, and the noop queue remained in place after the patch was merged, even though a new functional queue was introduced by the patch, allocated through blk_init_queue.

The user impact is a memory leak proportional to the number of AoE targets discovered. This patch removes the memory leak and cleans up vestiges of the old do-nothing queue from the aoeblk_gdalloc function.

---
 drivers/block/aoe/aoeblk.c |   17 ++++-------------
 1 files changed, 4 insertions(+), 13 deletions(-)

index 7ba0fcf..57ac72c 100644
@@ -278,18 +278,12 @@ aoeblk_gdalloc(void *vp)
 	if (q == NULL) {
 		pr_err("aoe: cannot allocate block queue for %ld.%d\n",
 			d->aoemajor, d->aoeminor);
-		mempool_destroy(mp);
-		goto err_disk;
+		goto err_mempool;
 	}
 
-	d->blkq = blk_alloc_queue(GFP_KERNEL);
-	if (!d->blkq)
-		goto err_mempool;
-	d->blkq->backing_dev_info.name = "aoe";
-	if (bdi_init(&d->blkq->backing_dev_info))
-		goto err_blkq;
 	spin_lock_irqsave(&d->lock, flags);
-	blk_queue_max_hw_sectors(d->blkq, BLK_DEF_MAX_SECTORS);
+	blk_queue_max_hw_sectors(q, BLK_DEF_MAX_SECTORS);
+	q->backing_dev_info.name = "aoe";
 	q->backing_dev_info.ra_pages = READ_AHEAD / PAGE_CACHE_SIZE;
 	d->bufpool = mp;
 	d->blkq = gd->queue = q;

@@ -314,11 +308,8 @@ aoeblk_gdalloc(void *vp)
 	aoedisk_add_sysfs(d);
 	return;
 
-err_blkq:
-	blk_cleanup_queue(d->blkq);
-	d->blkq = NULL;
 err_mempool:
-	mempool_destroy(d->bufpool);
+	mempool_destroy(mp);
 err_disk:
 	put_disk(gd);
 err:

1.7.1

A misplaced comment was attached to the nout member of the aoetgt. This change corrects the comment.

---
 drivers/block/aoe/aoe.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

index 9655ce3..bfd765c 100644
@@ -133,7 +133,7 @@ struct aoetgt {
 	struct list_head ffree;			/* list of free frames */
 	struct aoeif ifs[NAOEIFS];
 	struct aoeif *ifp;	/* current aoeif in use */
-	ushort nout;		/* value of nout when skb was sent */
+	ushort nout;		/* number of AoE commands outstanding */
 	ushort maxout;		/* current value for max outstanding */
 	ushort next_cwnd;	/* incr maxout after decrementing to zero */
 	ushort ssthresh;	/* slow start threshold */

1.7.1

There is no need to call the request handler function in the I/O completion routine. The user impact of not doing it is a more "nice" aoe driver that is less susceptible to causing soft lockups.

---
 drivers/block/aoe/aoecmd.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

index 53b9869..391dd8e 100644
@@ -1139,8 +1139,6 @@ badrsp:
 	if (buf && --buf->nframesout == 0 && buf->resid == 0)
 		aoe_end_buf(d, buf);
 
-	aoecmd_work(d);
-
 	spin_unlock_irq(&d->lock);
 	aoedev_put(d);
 	dev_kfree_skb(skb);

1.7.1

For some special-purpose systems where udev isn't present, static allocation of minor numbers is desirable. This update distinguishes different failure scenarios, to help the user understand what went wrong.

---
 drivers/block/aoe/aoedev.c |   31 ++++++++++++++++++++-----------
 1 files changed, 20 insertions(+), 11 deletions(-)

index 91f7c99..88ccd6d 100644
@@ -69,25 +69,34 @@ minor_get_static(ulong *sysminor, ulong aoemaj, int aoemin)
 		NPERSHELF = 16,
 	};
 
+	if (aoemin >= NPERSHELF) {
+		pr_err("aoe: %s %d slots per shelf\n",
+			"static minor device numbers support only",
+			NPERSHELF);
+		error = -1;
+		goto out;
+	}
+
 	n = aoemaj * NPERSHELF + aoemin;
-	if (aoemin >= NPERSHELF || n >= N_DEVS) {
+	if (n >= N_DEVS) {
 		pr_err("aoe: %s with e%ld.%d\n",
 			"cannot use static minor device numbers",
 			aoemaj, aoemin);
 		error = -1;
-	} else {
-		spin_lock_irqsave(&used_minors_lock, flags);
-		if (test_bit(n, used_minors)) {
-			pr_err("aoe: %s %lu\n",
-				"existing device already has static minor number",
-				n);
-			error = -1;
-		} else
-			set_bit(n, used_minors);
-		spin_unlock_irqrestore(&used_minors_lock, flags);
+		goto out;
 	}
 
+	spin_lock_irqsave(&used_minors_lock, flags);
+	if (test_bit(n, used_minors)) {
+		pr_err("aoe: %s %lu\n",
+			"existing device already has static minor number",
+			n);
+		error = -1;
+	} else
+		set_bit(n, used_minors);
+	spin_unlock_irqrestore(&used_minors_lock, flags);
 	*sysminor = n;
+out:
 	return error;
 }
 

1.7.1

Because the minor_get and related functions use the return values for errors, the compiler doesn't know that sysminor will always either 1) be initialized in aoedev_by_aoeaddr by the call to minor_get, or 2) be unused as the "goto out" is executed.

This patch avoids the compiler warning.

---
 drivers/block/aoe/aoedev.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

index 88ccd6d..80b3d3e 100644
@@ -383,7 +383,7 @@ aoedev_by_aoeaddr(ulong maj, int min, int do_alloc)
 	struct aoedev *d;
 	int i;
 	ulong flags;
-	ulong sysminor;
+	ulong sysminor = 0;
 
 	spin_lock_irqsave(&devlist_lock, flags);
 

1.7.1

The value returned by the static minor device number number allocator is the real minor number, so it must be multiplied by the supported number of partitions per aoedev.

Without this fix the support for systems without udev is incomplete, and the few users of aoe on such systems will have surprising results when device nodes names do not match the AoE target.

---
 drivers/block/aoe/aoedev.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

index 80b3d3e..aaaea66 100644
@@ -95,7 +95,7 @@ minor_get_static(ulong *sysminor, ulong aoemaj, int aoemin)
 	} else
 		set_bit(n, used_minors);
 	spin_unlock_irqrestore(&used_minors_lock, flags);
-	*sysminor = n;
+	*sysminor = n * AOE_PARTITIONS;
 out:
 	return error;
 }

1.7.1

The aoe driver will never be waiting for more than aoe_maxout AoE commands from a given remote network port on an AoE target. Increasing the cap increases performance. Users can tighten the setting to reduce the amount of memory used for handling AoE traffic or the network bandwidth used for AoE.

---
 drivers/block/aoe/aoecmd.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

index d609c47..53b9869 100644
@@ -29,7 +29,7 @@ static int aoe_deadsecs = 60 * 3;
 module_param(aoe_deadsecs, int, 0644);
 MODULE_PARM_DESC(aoe_deadsecs, "After aoe_deadsecs seconds, give up and fail dev.");
 
-static int aoe_maxout = 16;
+static int aoe_maxout = 64;
 module_param(aoe_maxout, int, 0644);
 MODULE_PARM_DESC(aoe_maxout,
 	"Only aoe_maxout outstanding packets for every MAC on eX.Y.");

1.7.1

This series didn't contain your usual aoe-update-driver-internal-version-number-to-xx.patch

That's true, yes. This was a bunch of fixups to 64+. I suppose I could have called it "64+1" or something, but more patches are coming, so I wasn't sure it was worth it.

Thanks for mentioning it.

  Ed Cashin
  [email protected]