[PATCH 0/8] aoe: miscellaneous fixes follow-up recent patch submissions

[PATCH 0/8] aoe: miscellaneous fixes follow-up recent patch submissions

[Ed Cashin, Ed Cashin]

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(-)

[PATCH 1/8] aoe: copy fallback timing information on destination failover

[Ed Cashin, Ed Cashin]

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.

Signed-off-by: Ed Cashin <ecashin@coraid.com>
---
 drivers/block/aoe/aoecmd.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/block/aoe/aoecmd.c b/drivers/block/aoe/aoecmd.c
index d9bc6ff..d609c47 100644
--- a/drivers/block/aoe/aoecmd.c
+++ b/drivers/block/aoe/aoecmd.c
@@ -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

[PATCH 2/8] aoe: remove vestigial request queue allocation

[Ed Cashin, Ed Cashin]

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.

Signed-off-by: Ed Cashin <ecashin@coraid.com>
---
 drivers/block/aoe/aoeblk.c |   17 ++++-------------
 1 files changed, 4 insertions(+), 13 deletions(-)

diff --git a/drivers/block/aoe/aoeblk.c b/drivers/block/aoe/aoeblk.c
index 7ba0fcf..57ac72c 100644
--- a/drivers/block/aoe/aoeblk.c
+++ b/drivers/block/aoe/aoeblk.c
@@ -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

[PATCH 3/8] aoe: increase default cap on outstanding AoE commands in the network

[Ed Cashin, Ed Cashin]

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.

Signed-off-by: Ed Cashin <ecashin@coraid.com>
---
 drivers/block/aoe/aoecmd.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/block/aoe/aoecmd.c b/drivers/block/aoe/aoecmd.c
index d609c47..53b9869 100644
--- a/drivers/block/aoe/aoecmd.c
+++ b/drivers/block/aoe/aoecmd.c
@@ -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

[PATCH 4/8] aoe: cleanup: correct comment for aoetgt nout

[Ed Cashin, Ed Cashin]

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

Signed-off-by: Ed Cashin <ecashin@coraid.com>
---
 drivers/block/aoe/aoe.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/block/aoe/aoe.h b/drivers/block/aoe/aoe.h
index 9655ce3..bfd765c 100644
--- a/drivers/block/aoe/aoe.h
+++ b/drivers/block/aoe/aoe.h
@@ -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

[PATCH 5/8] aoe: remove call to request handler from I/O completion

[Ed Cashin, Ed Cashin]

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.

Signed-off-by: Ed Cashin <ecashin@coraid.com>
---
 drivers/block/aoe/aoecmd.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/drivers/block/aoe/aoecmd.c b/drivers/block/aoe/aoecmd.c
index 53b9869..391dd8e 100644
--- a/drivers/block/aoe/aoecmd.c
+++ b/drivers/block/aoe/aoecmd.c
@@ -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

[PATCH 6/8] aoe: make error messages more specific in static minor allocation

[Ed Cashin, Ed Cashin]

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.

Signed-off-by: Ed Cashin <ecashin@coraid.com>
---
 drivers/block/aoe/aoedev.c |   31 ++++++++++++++++++++-----------
 1 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/drivers/block/aoe/aoedev.c b/drivers/block/aoe/aoedev.c
index 91f7c99..88ccd6d 100644
--- a/drivers/block/aoe/aoedev.c
+++ b/drivers/block/aoe/aoedev.c
@@ -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

[PATCH 7/8] aoe: initialize sysminor to avoid compiler warning

[Ed Cashin, Ed Cashin]

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.

Signed-off-by: Ed Cashin <ecashin@coraid.com>
---
 drivers/block/aoe/aoedev.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/block/aoe/aoedev.c b/drivers/block/aoe/aoedev.c
index 88ccd6d..80b3d3e 100644
--- a/drivers/block/aoe/aoedev.c
+++ b/drivers/block/aoe/aoedev.c
@@ -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

[PATCH 8/8] aoe: return real minor number for static minors

[Ed Cashin, Ed Cashin]

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.

Signed-off-by: Ed Cashin <ecashin@coraid.com>
---
 drivers/block/aoe/aoedev.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/block/aoe/aoedev.c b/drivers/block/aoe/aoedev.c
index 80b3d3e..aaaea66 100644
--- a/drivers/block/aoe/aoedev.c
+++ b/drivers/block/aoe/aoedev.c
@@ -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

Re: [PATCH 0/8] aoe: miscellaneous fixes follow-up recent patch submissions

[Andrew Morton]

On Wed, 21 Nov 2012 19:52:41 -0500
Ed Cashin <ecashin@coraid.com> wrote:

> 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

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

Re: [PATCH 0/8] aoe: miscellaneous fixes follow-up recent patch submissions

[Ed Cashin]

On Nov 28, 2012, at 6:42 PM, Andrew Morton wrote:

> On Wed, 21 Nov 2012 19:52:41 -0500
> Ed Cashin <ecashin@coraid.com> wrote:
> 
>> 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
> 
> 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
  ecashin@coraid.com