* Re: [PATCH 2.6.11] aoe [1/12]: remove too-low cap on minor number
[not found] <87mztbi79d.fsf@coraid.com>
@ 2005-03-17 23:46 ` Greg KH
2005-03-18 20:13 ` Ed L Cashin
` (13 more replies)
0 siblings, 14 replies; 30+ messages in thread
From: Greg KH @ 2005-03-17 23:46 UTC (permalink / raw)
To: Ed L. Cashin; +Cc: linux-kernel
I've applied 11 of these 12 patches (the one from Randy was already
included) to my trees.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 2.6.11] aoe [1/12]: remove too-low cap on minor number
2005-03-17 23:46 ` [PATCH 2.6.11] aoe [1/12]: remove too-low cap on minor number Greg KH
@ 2005-03-18 20:13 ` Ed L Cashin
2005-03-18 20:14 ` [PATCH 2.6.11] aoe [4/12]: handle distros that have a udev rules file instead of dir Ed L. Cashin
` (12 subsequent siblings)
13 siblings, 0 replies; 30+ messages in thread
From: Ed L Cashin @ 2005-03-18 20:13 UTC (permalink / raw)
To: linux-kernel; +Cc: Greg K-H
Greg KH <greg@kroah.com> writes:
> I've applied 11 of these 12 patches (the one from Randy was already
> included) to my trees.
Those haven't gone to the lkml yet. I'll post them through gmane.
(The original postings never made it to the list, and I can't get in
touch with the lkml admins, but gmane should work.)
--
Ed L Cashin <ecashin@coraid.com>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 2.6.11] aoe [4/12]: handle distros that have a udev rules file instead of dir
2005-03-17 23:46 ` [PATCH 2.6.11] aoe [1/12]: remove too-low cap on minor number Greg KH
2005-03-18 20:13 ` Ed L Cashin
@ 2005-03-18 20:14 ` Ed L. Cashin
2005-03-24 14:59 ` [PATCH 2.6.11] aoe [1/12]: remove too-low cap on minor number ecashin
` (11 subsequent siblings)
13 siblings, 0 replies; 30+ messages in thread
From: Ed L. Cashin @ 2005-03-18 20:14 UTC (permalink / raw)
To: linux-kernel; +Cc: Greg K-H
handle distros that have a udev rules file instead of dir
Signed-off-by: Ed L. Cashin <ecashin@coraid.com>
diff -uprN a/Documentation/aoe/udev-install.sh b/Documentation/aoe/udev-install.sh
--- a/Documentation/aoe/udev-install.sh 2005-03-10 11:59:55.000000000 -0500
+++ b/Documentation/aoe/udev-install.sh 2005-03-10 12:19:18.000000000 -0500
@@ -23,4 +23,8 @@ fi
# /etc/udev/rules.d
#
rules_d="`sed -n '/^udev_rules=/{ s!udev_rules=!!; s!\"!!g; p; }' $conf`"
-test "$rules_d" && sh -xc "cp `dirname $0`/udev.txt $rules_d/60-aoe.rules"
+if test -z "$rules_d" || test ! -d "$rules_d"; then
+ echo "$me Error: cannot find udev rules directory" 1>&2
+ exit 1
+fi
+sh -xc "cp `dirname $0`/udev.txt $rules_d/60-aoe.rules"
--
Ed L. Cashin <ecashin@coraid.com>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 2.6.11] aoe [1/12]: remove too-low cap on minor number
2005-03-17 23:46 ` [PATCH 2.6.11] aoe [1/12]: remove too-low cap on minor number Greg KH
2005-03-18 20:13 ` Ed L Cashin
2005-03-18 20:14 ` [PATCH 2.6.11] aoe [4/12]: handle distros that have a udev rules file instead of dir Ed L. Cashin
@ 2005-03-24 14:59 ` ecashin
2005-03-24 15:09 ` [PATCH 2.6.11] aoe [2/12]: allow multiple aoe devices with same MAC addr ecashin
` (10 subsequent siblings)
13 siblings, 0 replies; 30+ messages in thread
From: ecashin @ 2005-03-24 14:59 UTC (permalink / raw)
To: linux-kernel
remove too-low cap on minor number
Signed-off-by: Ed L. Cashin <ecashin@coraid.com>
diff -uprN a/drivers/block/aoe/aoe.h b/drivers/block/aoe/aoe.h
--- a/drivers/block/aoe/aoe.h 2005-03-10 11:59:55.000000000 -0500
+++ b/drivers/block/aoe/aoe.h 2005-03-10 12:19:04.000000000 -0500
@@ -2,9 +2,14 @@
#define VERSION "5"
#define AOE_MAJOR 152
#define DEVICE_NAME "aoe"
+
+/* set AOE_PARTITIONS to 1 to use whole-disks only
+ * default is 16, which is 15 partitions plus the whole disk
+ */
#ifndef AOE_PARTITIONS
#define AOE_PARTITIONS 16
#endif
+
#define SYSMINOR(aoemajor, aoeminor) ((aoemajor) * 10 + (aoeminor))
#define AOEMAJOR(sysminor) ((sysminor) / 10)
#define AOEMINOR(sysminor) ((sysminor) % 10)
diff -uprN a/drivers/block/aoe/aoecmd.c b/drivers/block/aoe/aoecmd.c
--- a/drivers/block/aoe/aoecmd.c 2005-03-10 11:59:55.000000000 -0500
+++ b/drivers/block/aoe/aoecmd.c 2005-03-10 12:19:04.000000000 -0500
@@ -577,7 +577,7 @@ aoecmd_cfg_rsp(struct sk_buff *skb)
struct aoe_cfghdr *ch;
ulong flags, bufcnt, sysminor, aoemajor;
struct sk_buff *sl;
- enum { MAXFRAMES = 8, MAXSYSMINOR = 255 };
+ enum { MAXFRAMES = 8 };
h = (struct aoe_hdr *) skb->mac.raw;
ch = (struct aoe_cfghdr *) (h+1);
@@ -594,9 +594,10 @@ aoecmd_cfg_rsp(struct sk_buff *skb)
}
sysminor = SYSMINOR(aoemajor, h->minor);
- if (sysminor > MAXSYSMINOR) {
- printk(KERN_INFO "aoe: aoecmd_cfg_rsp: sysminor %ld too "
- "large\n", sysminor);
+ if (sysminor * AOE_PARTITIONS + AOE_PARTITIONS > MINORMASK) {
+ printk(KERN_INFO
+ "aoe: e%ld.%d: minor number too large\n",
+ aoemajor, (int) h->minor);
return;
}
--
Ed L. Cashin <ecashin@coraid.com>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 2.6.11] aoe [2/12]: allow multiple aoe devices with same MAC addr
2005-03-17 23:46 ` [PATCH 2.6.11] aoe [1/12]: remove too-low cap on minor number Greg KH
` (2 preceding siblings ...)
2005-03-24 14:59 ` [PATCH 2.6.11] aoe [1/12]: remove too-low cap on minor number ecashin
@ 2005-03-24 15:09 ` ecashin
2005-03-24 15:13 ` [PATCH 2.6.11] aoe [3/12]: update driver version to 6 ecashin
` (9 subsequent siblings)
13 siblings, 0 replies; 30+ messages in thread
From: ecashin @ 2005-03-24 15:09 UTC (permalink / raw)
To: linux-kernel
allow multiple aoe devices with same MAC addr
Signed-off-by: Ed L. Cashin <ecashin@coraid.com>
diff -uprN a/drivers/block/aoe/aoe.h b/drivers/block/aoe/aoe.h
--- a/drivers/block/aoe/aoe.h 2005-03-10 12:19:04.000000000 -0500
+++ b/drivers/block/aoe/aoe.h 2005-03-10 12:19:11.000000000 -0500
@@ -156,7 +156,7 @@ void aoecmd_cfg_rsp(struct sk_buff *);
int aoedev_init(void);
void aoedev_exit(void);
-struct aoedev *aoedev_bymac(unsigned char *);
+struct aoedev *aoedev_by_aoeaddr(int maj, int min);
void aoedev_downdev(struct aoedev *d);
struct aoedev *aoedev_set(ulong, unsigned char *, struct net_device *, ulong);
int aoedev_busy(void);
diff -uprN a/drivers/block/aoe/aoecmd.c b/drivers/block/aoe/aoecmd.c
--- a/drivers/block/aoe/aoecmd.c 2005-03-10 12:19:04.000000000 -0500
+++ b/drivers/block/aoe/aoecmd.c 2005-03-10 12:19:11.000000000 -0500
@@ -380,14 +380,15 @@ aoecmd_ata_rsp(struct sk_buff *skb)
register long n;
ulong flags;
char ebuf[128];
-
+ u16 aoemajor;
+
hin = (struct aoe_hdr *) skb->mac.raw;
- d = aoedev_bymac(hin->src);
+ aoemajor = __be16_to_cpu(*((u16 *) hin->major));
+ d = aoedev_by_aoeaddr(aoemajor, hin->minor);
if (d == NULL) {
snprintf(ebuf, sizeof ebuf, "aoecmd_ata_rsp: ata response "
"for unknown device %d.%d\n",
- __be16_to_cpu(*((u16 *) hin->major)),
- hin->minor);
+ aoemajor, hin->minor);
aoechr_error(ebuf);
return;
}
diff -uprN a/drivers/block/aoe/aoedev.c b/drivers/block/aoe/aoedev.c
--- a/drivers/block/aoe/aoedev.c 2005-03-07 17:37:14.000000000 -0500
+++ b/drivers/block/aoe/aoedev.c 2005-03-10 12:19:11.000000000 -0500
@@ -13,7 +13,7 @@ static struct aoedev *devlist;
static spinlock_t devlist_lock;
struct aoedev *
-aoedev_bymac(unsigned char *macaddr)
+aoedev_by_aoeaddr(int maj, int min)
{
struct aoedev *d;
ulong flags;
@@ -21,7 +21,7 @@ aoedev_bymac(unsigned char *macaddr)
spin_lock_irqsave(&devlist_lock, flags);
for (d=devlist; d; d=d->next)
- if (!memcmp(d->addr, macaddr, 6))
+ if (d->aoemajor == maj && d->aoeminor == min)
break;
spin_unlock_irqrestore(&devlist_lock, flags);
@@ -125,7 +125,6 @@ aoedev_set(ulong sysminor, unsigned char
d->ifp = ifp;
if (d->sysminor != sysminor
- || memcmp(d->addr, addr, sizeof d->addr)
|| (d->flags & DEVFL_UP) == 0) {
aoedev_downdev(d); /* flushes outstanding frames */
memcpy(d->addr, addr, sizeof d->addr);
--
Ed L. Cashin <ecashin@coraid.com>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 2.6.11] aoe [3/12]: update driver version to 6
2005-03-17 23:46 ` [PATCH 2.6.11] aoe [1/12]: remove too-low cap on minor number Greg KH
` (3 preceding siblings ...)
2005-03-24 15:09 ` [PATCH 2.6.11] aoe [2/12]: allow multiple aoe devices with same MAC addr ecashin
@ 2005-03-24 15:13 ` ecashin
2005-03-24 15:15 ` [PATCH 2.6.11] aoe [4/12]: handle distros that have a udev rules file instead of dir ecashin
` (8 subsequent siblings)
13 siblings, 0 replies; 30+ messages in thread
From: ecashin @ 2005-03-24 15:13 UTC (permalink / raw)
To: linux-kernel
update driver version to 6
Signed-off-by: Ed L. Cashin <ecashin@coraid.com>
diff -uprN a/drivers/block/aoe/aoe.h b/drivers/block/aoe/aoe.h
--- a/drivers/block/aoe/aoe.h 2005-03-10 12:19:11.000000000 -0500
+++ b/drivers/block/aoe/aoe.h 2005-03-10 12:19:14.000000000 -0500
@@ -1,5 +1,5 @@
/* Copyright (c) 2004 Coraid, Inc. See COPYING for GPL terms. */
-#define VERSION "5"
+#define VERSION "6"
#define AOE_MAJOR 152
#define DEVICE_NAME "aoe"
--
Ed L. Cashin <ecashin@coraid.com>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 2.6.11] aoe [4/12]: handle distros that have a udev rules file instead of dir
2005-03-17 23:46 ` [PATCH 2.6.11] aoe [1/12]: remove too-low cap on minor number Greg KH
` (4 preceding siblings ...)
2005-03-24 15:13 ` [PATCH 2.6.11] aoe [3/12]: update driver version to 6 ecashin
@ 2005-03-24 15:15 ` ecashin
2005-03-24 15:17 ` [PATCH 2.6.11] aoe [5/12]: don't try to free null bufpool ecashin
` (7 subsequent siblings)
13 siblings, 0 replies; 30+ messages in thread
From: ecashin @ 2005-03-24 15:15 UTC (permalink / raw)
To: linux-kernel
handle distros that have a udev rules file instead of dir
Signed-off-by: Ed L. Cashin <ecashin@coraid.com>
diff -uprN a/Documentation/aoe/udev-install.sh b/Documentation/aoe/udev-install.sh
--- a/Documentation/aoe/udev-install.sh 2005-03-10 11:59:55.000000000 -0500
+++ b/Documentation/aoe/udev-install.sh 2005-03-10 12:19:18.000000000 -0500
@@ -23,4 +23,8 @@ fi
# /etc/udev/rules.d
#
rules_d="`sed -n '/^udev_rules=/{ s!udev_rules=!!; s!\"!!g; p; }' $conf`"
-test "$rules_d" && sh -xc "cp `dirname $0`/udev.txt $rules_d/60-aoe.rules"
+if test -z "$rules_d" || test ! -d "$rules_d"; then
+ echo "$me Error: cannot find udev rules directory" 1>&2
+ exit 1
+fi
+sh -xc "cp `dirname $0`/udev.txt $rules_d/60-aoe.rules"
--
Ed L. Cashin <ecashin@coraid.com>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 2.6.11] aoe [5/12]: don't try to free null bufpool
2005-03-17 23:46 ` [PATCH 2.6.11] aoe [1/12]: remove too-low cap on minor number Greg KH
` (5 preceding siblings ...)
2005-03-24 15:15 ` [PATCH 2.6.11] aoe [4/12]: handle distros that have a udev rules file instead of dir ecashin
@ 2005-03-24 15:17 ` ecashin
2005-03-24 15:58 ` Arjan van de Ven
2005-03-24 15:19 ` [PATCH 2.6.11] aoe [6/12]: Alexey Dobriyan sparse cleanup ecashin
` (6 subsequent siblings)
13 siblings, 1 reply; 30+ messages in thread
From: ecashin @ 2005-03-24 15:17 UTC (permalink / raw)
To: linux-kernel
don't try to free null bufpool
Signed-off-by: Ed L. Cashin <ecashin@coraid.com>
diff -uprN a/drivers/block/aoe/aoedev.c b/drivers/block/aoe/aoedev.c
--- a/drivers/block/aoe/aoedev.c 2005-03-10 12:19:11.000000000 -0500
+++ b/drivers/block/aoe/aoedev.c 2005-03-10 12:19:25.000000000 -0500
@@ -146,7 +146,8 @@ aoedev_freedev(struct aoedev *d)
put_disk(d->gd);
}
kfree(d->frames);
- mempool_destroy(d->bufpool);
+ if (d->bufpool)
+ mempool_destroy(d->bufpool);
kfree(d);
}
--
Ed L. Cashin <ecashin@coraid.com>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 2.6.11] aoe [6/12]: Alexey Dobriyan sparse cleanup
2005-03-17 23:46 ` [PATCH 2.6.11] aoe [1/12]: remove too-low cap on minor number Greg KH
` (6 preceding siblings ...)
2005-03-24 15:17 ` [PATCH 2.6.11] aoe [5/12]: don't try to free null bufpool ecashin
@ 2005-03-24 15:19 ` ecashin
2005-03-24 15:21 ` [PATCH 2.6.11] aoe [7/12]: support configuration of AOE_PARTITIONS from Kconfig ecashin
` (5 subsequent siblings)
13 siblings, 0 replies; 30+ messages in thread
From: ecashin @ 2005-03-24 15:19 UTC (permalink / raw)
To: linux-kernel
Alexey Dobriyan sparse cleanup
Signed-off-by: Alexey Dobriyan <adobriyan@mail.ru>
Signed-off-by: Ed L. Cashin <ecashin@coraid.com>
diff -uprN a/drivers/block/aoe/aoe.h b/drivers/block/aoe/aoe.h
--- a/drivers/block/aoe/aoe.h 2005-03-10 12:19:14.000000000 -0500
+++ b/drivers/block/aoe/aoe.h 2005-03-10 12:19:27.000000000 -0500
@@ -39,13 +39,13 @@ enum {
struct aoe_hdr {
unsigned char dst[6];
unsigned char src[6];
- unsigned char type[2];
+ __be16 type;
unsigned char verfl;
unsigned char err;
- unsigned char major[2];
+ __be16 major;
unsigned char minor;
unsigned char cmd;
- unsigned char tag[4];
+ __be32 tag;
};
struct aoe_atahdr {
@@ -63,8 +63,8 @@ struct aoe_atahdr {
};
struct aoe_cfghdr {
- unsigned char bufcnt[2];
- unsigned char fwver[2];
+ __be16 bufcnt;
+ __be16 fwver;
unsigned char res;
unsigned char aoeccmd;
unsigned char cslen[2];
diff -uprN a/drivers/block/aoe/aoecmd.c b/drivers/block/aoe/aoecmd.c
--- a/drivers/block/aoe/aoecmd.c 2005-03-10 12:19:11.000000000 -0500
+++ b/drivers/block/aoe/aoecmd.c 2005-03-10 12:19:27.000000000 -0500
@@ -90,19 +90,16 @@ newtag(struct aoedev *d)
static int
aoehdr_atainit(struct aoedev *d, struct aoe_hdr *h)
{
- u16 type = __constant_cpu_to_be16(ETH_P_AOE);
- u16 aoemajor = __cpu_to_be16(d->aoemajor);
u32 host_tag = newtag(d);
- u32 tag = __cpu_to_be32(host_tag);
memcpy(h->src, d->ifp->dev_addr, sizeof h->src);
memcpy(h->dst, d->addr, sizeof h->dst);
- memcpy(h->type, &type, sizeof type);
+ h->type = __constant_cpu_to_be16(ETH_P_AOE);
h->verfl = AOE_HVER;
- memcpy(h->major, &aoemajor, sizeof aoemajor);
+ h->major = cpu_to_be16(d->aoemajor);
h->minor = d->aoeminor;
h->cmd = AOECMD_ATA;
- memcpy(h->tag, &tag, sizeof tag);
+ h->tag = cpu_to_be32(host_tag);
return host_tag;
}
@@ -215,7 +212,6 @@ rexmit(struct aoedev *d, struct frame *f
struct aoe_hdr *h;
char buf[128];
u32 n;
- u32 net_tag;
n = newtag(d);
@@ -227,8 +223,7 @@ rexmit(struct aoedev *d, struct frame *f
h = (struct aoe_hdr *) f->data;
f->tag = n;
- net_tag = __cpu_to_be32(n);
- memcpy(h->tag, &net_tag, sizeof net_tag);
+ h->tag = cpu_to_be32(n);
skb = skb_prepare(d, f);
if (skb) {
@@ -308,16 +303,16 @@ ataid_complete(struct aoedev *d, unsigne
u16 n;
/* word 83: command set supported */
- n = __le16_to_cpu(*((u16 *) &id[83<<1]));
+ n = le16_to_cpup((__le16 *) &id[83<<1]);
/* word 86: command set/feature enabled */
- n |= __le16_to_cpu(*((u16 *) &id[86<<1]));
+ n |= le16_to_cpup((__le16 *) &id[86<<1]);
if (n & (1<<10)) { /* bit 10: LBA 48 */
d->flags |= DEVFL_EXT;
/* word 100: number lba48 sectors */
- ssize = __le64_to_cpu(*((u64 *) &id[100<<1]));
+ ssize = le64_to_cpup((__le64 *) &id[100<<1]);
/* set as in ide-disk.c:init_idedisk_capacity */
d->geo.cylinders = ssize;
@@ -328,12 +323,12 @@ ataid_complete(struct aoedev *d, unsigne
d->flags &= ~DEVFL_EXT;
/* number lba28 sectors */
- ssize = __le32_to_cpu(*((u32 *) &id[60<<1]));
+ ssize = le32_to_cpup((__le32 *) &id[60<<1]);
/* NOTE: obsolete in ATA 6 */
- d->geo.cylinders = __le16_to_cpu(*((u16 *) &id[54<<1]));
- d->geo.heads = __le16_to_cpu(*((u16 *) &id[55<<1]));
- d->geo.sectors = __le16_to_cpu(*((u16 *) &id[56<<1]));
+ d->geo.cylinders = le16_to_cpup((__le16 *) &id[54<<1]);
+ d->geo.heads = le16_to_cpup((__le16 *) &id[55<<1]);
+ d->geo.sectors = le16_to_cpup((__le16 *) &id[56<<1]);
}
d->ssize = ssize;
d->geo.start = 0;
@@ -383,7 +378,7 @@ aoecmd_ata_rsp(struct sk_buff *skb)
u16 aoemajor;
hin = (struct aoe_hdr *) skb->mac.raw;
- aoemajor = __be16_to_cpu(*((u16 *) hin->major));
+ aoemajor = be16_to_cpu(hin->major);
d = aoedev_by_aoeaddr(aoemajor, hin->minor);
if (d == NULL) {
snprintf(ebuf, sizeof ebuf, "aoecmd_ata_rsp: ata response "
@@ -395,15 +390,15 @@ aoecmd_ata_rsp(struct sk_buff *skb)
spin_lock_irqsave(&d->lock, flags);
- f = getframe(d, __be32_to_cpu(*((u32 *) hin->tag)));
+ f = getframe(d, be32_to_cpu(hin->tag));
if (f == NULL) {
spin_unlock_irqrestore(&d->lock, flags);
snprintf(ebuf, sizeof ebuf,
"%15s e%d.%d tag=%08x@%08lx\n",
"unexpected rsp",
- __be16_to_cpu(*((u16 *) hin->major)),
+ be16_to_cpu(hin->major),
hin->minor,
- __be32_to_cpu(*((u32 *) hin->tag)),
+ be32_to_cpu(hin->tag),
jiffies);
aoechr_error(ebuf);
return;
@@ -453,7 +448,7 @@ aoecmd_ata_rsp(struct sk_buff *skb)
printk(KERN_INFO "aoe: aoecmd_ata_rsp: unrecognized "
"outbound ata command %2.2Xh for %d.%d\n",
ahout->cmdstat,
- __be16_to_cpu(*((u16 *) hin->major)),
+ be16_to_cpu(hin->major),
hin->minor);
}
}
@@ -487,8 +482,6 @@ aoecmd_cfg(ushort aoemajor, unsigned cha
struct aoe_cfghdr *ch;
struct sk_buff *skb, *sl;
struct net_device *ifp;
- u16 aoe_type = __constant_cpu_to_be16(ETH_P_AOE);
- u16 net_aoemajor = __cpu_to_be16(aoemajor);
sl = NULL;
@@ -508,9 +501,9 @@ aoecmd_cfg(ushort aoemajor, unsigned cha
memset(h->dst, 0xff, sizeof h->dst);
memcpy(h->src, ifp->dev_addr, sizeof h->src);
- memcpy(h->type, &aoe_type, sizeof aoe_type);
+ h->type = __constant_cpu_to_be16(ETH_P_AOE);
h->verfl = AOE_HVER;
- memcpy(h->major, &net_aoemajor, sizeof net_aoemajor);
+ h->major = cpu_to_be16(aoemajor);
h->minor = aoeminor;
h->cmd = AOECMD_CFG;
@@ -576,7 +569,8 @@ aoecmd_cfg_rsp(struct sk_buff *skb)
struct aoedev *d;
struct aoe_hdr *h;
struct aoe_cfghdr *ch;
- ulong flags, bufcnt, sysminor, aoemajor;
+ ulong flags, sysminor, aoemajor;
+ u16 bufcnt;
struct sk_buff *sl;
enum { MAXFRAMES = 8 };
@@ -587,7 +581,7 @@ aoecmd_cfg_rsp(struct sk_buff *skb)
* Enough people have their dip switches set backwards to
* warrant a loud message for this special case.
*/
- aoemajor = __be16_to_cpu(*((u16 *) h->major));
+ aoemajor = be16_to_cpu(h->major);
if (aoemajor == 0xfff) {
printk(KERN_CRIT "aoe: aoecmd_cfg_rsp: Warning: shelf "
"address is all ones. Check shelf dip switches\n");
@@ -602,7 +596,7 @@ aoecmd_cfg_rsp(struct sk_buff *skb)
return;
}
- bufcnt = __be16_to_cpu(*((u16 *) ch->bufcnt));
+ bufcnt = be16_to_cpu(ch->bufcnt);
if (bufcnt > MAXFRAMES) /* keep it reasonable */
bufcnt = MAXFRAMES;
@@ -619,7 +613,7 @@ aoecmd_cfg_rsp(struct sk_buff *skb)
return;
}
- d->fw_ver = __be16_to_cpu(*((u16 *) ch->fwver));
+ d->fw_ver = be16_to_cpu(ch->fwver);
/* we get here only if the device is new */
sl = aoecmd_ata_id(d);
diff -uprN a/drivers/block/aoe/aoenet.c b/drivers/block/aoe/aoenet.c
--- a/drivers/block/aoe/aoenet.c 2005-03-07 17:37:14.000000000 -0500
+++ b/drivers/block/aoe/aoenet.c 2005-03-10 12:19:27.000000000 -0500
@@ -69,7 +69,7 @@ set_aoe_iflist(const char __user *user_s
u64
mac_addr(char addr[6])
{
- u64 n = 0;
+ __be64 n = 0;
char *p = (char *) &n;
memcpy(p + 2, addr, 6); /* (sizeof addr != 6) */
@@ -108,7 +108,7 @@ static int
aoenet_rcv(struct sk_buff *skb, struct net_device *ifp, struct packet_type *pt)
{
struct aoe_hdr *h;
- ulong n;
+ u32 n;
skb = skb_check(skb);
if (!skb)
@@ -121,7 +121,7 @@ aoenet_rcv(struct sk_buff *skb, struct n
skb_push(skb, ETH_HLEN); /* (1) */
h = (struct aoe_hdr *) skb->mac.raw;
- n = __be32_to_cpu(*((u32 *) h->tag));
+ n = be32_to_cpu(h->tag);
if ((h->verfl & AOEFL_RSP) == 0 || (n & 1<<31))
goto exit;
@@ -132,7 +132,7 @@ aoenet_rcv(struct sk_buff *skb, struct n
if (net_ratelimit())
printk(KERN_ERR "aoe: aoenet_rcv: error packet from %d.%d; "
"ecode=%d '%s'\n",
- __be16_to_cpu(*((u16 *) h->major)), h->minor,
+ be16_to_cpu(h->major), h->minor,
h->err, aoe_errlist[n]);
goto exit;
}
--
Ed L. Cashin <ecashin@coraid.com>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 2.6.11] aoe [7/12]: support configuration of AOE_PARTITIONS from Kconfig
2005-03-17 23:46 ` [PATCH 2.6.11] aoe [1/12]: remove too-low cap on minor number Greg KH
` (7 preceding siblings ...)
2005-03-24 15:19 ` [PATCH 2.6.11] aoe [6/12]: Alexey Dobriyan sparse cleanup ecashin
@ 2005-03-24 15:21 ` ecashin
2005-03-28 17:07 ` Christoph Hellwig
2005-03-24 15:23 ` [PATCH 2.6.11] aoe [8/12]: document env var for specifying number of partitions per dev ecashin
` (4 subsequent siblings)
13 siblings, 1 reply; 30+ messages in thread
From: ecashin @ 2005-03-24 15:21 UTC (permalink / raw)
To: linux-kernel
support configuration of AOE_PARTITIONS from Kconfig
Signed-off-by: Ed L. Cashin <ecashin@coraid.com>
diff -uprN a/drivers/block/Kconfig b/drivers/block/Kconfig
--- a/drivers/block/Kconfig 2005-03-07 17:37:58.000000000 -0500
+++ b/drivers/block/Kconfig 2005-03-10 12:19:54.000000000 -0500
@@ -506,4 +506,19 @@ config ATA_OVER_ETH
This driver provides Support for ATA over Ethernet block
devices like the Coraid EtherDrive (R) Storage Blade.
+config AOE_PARTITIONS
+ int "Partitions per AoE device" if ATA_OVER_ETH
+ default "16"
+ help
+ The default is to support 16 partitions per aoe device. Some
+ systems lack good support for devices with large minor
+ numbers.
+
+ Such systems will be able to use more aoe disks when
+ AOE_PARTITIONS is set to one, but you won't be able to
+ partition the disks, and you must make sure your device
+ nodes are created to work with the value you select.
+
+ If unsure, use 16.
+
endmenu
diff -uprN a/drivers/block/aoe/aoe.h b/drivers/block/aoe/aoe.h
--- a/drivers/block/aoe/aoe.h 2005-03-10 12:19:27.000000000 -0500
+++ b/drivers/block/aoe/aoe.h 2005-03-10 12:19:54.000000000 -0500
@@ -6,8 +6,9 @@
/* set AOE_PARTITIONS to 1 to use whole-disks only
* default is 16, which is 15 partitions plus the whole disk
*/
-#ifndef AOE_PARTITIONS
-#define AOE_PARTITIONS 16
+#define AOE_PARTITIONS CONFIG_AOE_PARTITIONS
+#if AOE_PARTITIONS < 1
+#error AOE_PARTITIONS less than one
#endif
#define SYSMINOR(aoemajor, aoeminor) ((aoemajor) * 10 + (aoeminor))
--
Ed L. Cashin <ecashin@coraid.com>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 2.6.11] aoe [8/12]: document env var for specifying number of partitions per dev
2005-03-17 23:46 ` [PATCH 2.6.11] aoe [1/12]: remove too-low cap on minor number Greg KH
` (8 preceding siblings ...)
2005-03-24 15:21 ` [PATCH 2.6.11] aoe [7/12]: support configuration of AOE_PARTITIONS from Kconfig ecashin
@ 2005-03-24 15:23 ` ecashin
2005-03-24 15:25 ` [PATCH 2.6.11] aoe [9/12]: add note about the need for deadlock-free sk_buff allocation ecashin
` (3 subsequent siblings)
13 siblings, 0 replies; 30+ messages in thread
From: ecashin @ 2005-03-24 15:23 UTC (permalink / raw)
To: linux-kernel
document env var for specifying number of partitions per dev
Signed-off-by: Ed L. Cashin <ecashin@coraid.com>
diff -uprN a/Documentation/aoe/mkdevs.sh b/Documentation/aoe/mkdevs.sh
--- a/Documentation/aoe/mkdevs.sh 2005-03-07 17:37:14.000000000 -0500
+++ b/Documentation/aoe/mkdevs.sh 2005-03-10 12:19:56.000000000 -0500
@@ -5,6 +5,7 @@ n_partitions=${n_partitions:-16}
if test "$#" != "1"; then
echo "Usage: sh `basename $0` {dir}" 1>&2
+ echo " n_partitions=16 sh `basename $0` {dir}" 1>&2
exit 1
fi
dir=$1
diff -uprN a/Documentation/aoe/mkshelf.sh b/Documentation/aoe/mkshelf.sh
--- a/Documentation/aoe/mkshelf.sh 2005-03-07 17:37:14.000000000 -0500
+++ b/Documentation/aoe/mkshelf.sh 2005-03-10 12:19:56.000000000 -0500
@@ -2,6 +2,7 @@
if test "$#" != "2"; then
echo "Usage: sh `basename $0` {dir} {shelfaddress}" 1>&2
+ echo " n_partitions=16 sh `basename $0` {dir} {shelfaddress}" 1>&2
exit 1
fi
n_partitions=${n_partitions:-16}
--
Ed L. Cashin <ecashin@coraid.com>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 2.6.11] aoe [9/12]: add note about the need for deadlock-free sk_buff allocation
2005-03-17 23:46 ` [PATCH 2.6.11] aoe [1/12]: remove too-low cap on minor number Greg KH
` (9 preceding siblings ...)
2005-03-24 15:23 ` [PATCH 2.6.11] aoe [8/12]: document env var for specifying number of partitions per dev ecashin
@ 2005-03-24 15:25 ` ecashin
2005-03-24 15:27 ` [PATCH 2.6.11] aoe [10/12]: Randy Dunlap: avoid warnings on sparc64 ecashin
` (2 subsequent siblings)
13 siblings, 0 replies; 30+ messages in thread
From: ecashin @ 2005-03-24 15:25 UTC (permalink / raw)
To: linux-kernel
add note about the need for deadlock-free sk_buff allocation
Signed-off-by: Ed L. Cashin <ecashin@coraid.com>
diff -uprN a/Documentation/aoe/todo.txt b/Documentation/aoe/todo.txt
--- a/Documentation/aoe/todo.txt 1969-12-31 19:00:00.000000000 -0500
+++ b/Documentation/aoe/todo.txt 2005-03-10 12:19:57.000000000 -0500
@@ -0,0 +1,14 @@
+There is a potential for deadlock when allocating a struct sk_buff for
+data that needs to be written out to aoe storage. If the data is
+being written from a dirty page in order to free that page, and if
+there are no other pages available, then deadlock may occur when a
+free page is needed for the sk_buff allocation. This situation has
+not been observed, but it would be nice to eliminate any potential for
+deadlock under memory pressure.
+
+Because ATA over Ethernet is not fragmented by the kernel's IP code,
+the destructore member of the struct sk_buff is available to the aoe
+driver. By using a mempool for allocating all but the first few
+sk_buffs, and by registering a destructor, we should be able to
+efficiently allocate sk_buffs without introducing any potential for
+deadlock.
--
Ed L. Cashin <ecashin@coraid.com>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 2.6.11] aoe [10/12]: Randy Dunlap: avoid warnings on sparc64
2005-03-17 23:46 ` [PATCH 2.6.11] aoe [1/12]: remove too-low cap on minor number Greg KH
` (10 preceding siblings ...)
2005-03-24 15:25 ` [PATCH 2.6.11] aoe [9/12]: add note about the need for deadlock-free sk_buff allocation ecashin
@ 2005-03-24 15:27 ` ecashin
2005-03-24 15:30 ` [PATCH 2.6.11] aoe [11/12]: add support for disk statistics ecashin
2005-03-24 15:32 ` [PATCH 2.6.11] aoe [12/12]: send outgoing packets in order ecashin
13 siblings, 0 replies; 30+ messages in thread
From: ecashin @ 2005-03-24 15:27 UTC (permalink / raw)
To: linux-kernel
Andrew Morton has already merged this patch.
Randy Dunlap: avoid warnings on sparc64
Signed-off-by: Randy Dunlap <rddunlap@osdl.org>
Signed-off-by: Ed L. Cashin <ecashin@coraid.com>
diff -uprN a/drivers/block/aoe/aoeblk.c b/drivers/block/aoe/aoeblk.c
--- a/drivers/block/aoe/aoeblk.c 2005-03-07 17:37:14.000000000 -0500
+++ b/drivers/block/aoe/aoeblk.c 2005-03-10 12:20:00.000000000 -0500
@@ -28,7 +28,8 @@ static ssize_t aoedisk_show_mac(struct g
{
struct aoedev *d = disk->private_data;
- return snprintf(page, PAGE_SIZE, "%012llx\n", mac_addr(d->addr));
+ return snprintf(page, PAGE_SIZE, "%012llx\n",
+ (unsigned long long)mac_addr(d->addr));
}
static ssize_t aoedisk_show_netif(struct gendisk * disk, char *page)
{
@@ -241,7 +242,8 @@ aoeblk_gdalloc(void *vp)
aoedisk_add_sysfs(d);
printk(KERN_INFO "aoe: %012llx e%lu.%lu v%04x has %llu "
- "sectors\n", mac_addr(d->addr), d->aoemajor, d->aoeminor,
+ "sectors\n", (unsigned long long)mac_addr(d->addr),
+ d->aoemajor, d->aoeminor,
d->fw_ver, (long long)d->ssize);
}
--
Ed L. Cashin <ecashin@coraid.com>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 2.6.11] aoe [11/12]: add support for disk statistics
2005-03-17 23:46 ` [PATCH 2.6.11] aoe [1/12]: remove too-low cap on minor number Greg KH
` (11 preceding siblings ...)
2005-03-24 15:27 ` [PATCH 2.6.11] aoe [10/12]: Randy Dunlap: avoid warnings on sparc64 ecashin
@ 2005-03-24 15:30 ` ecashin
2005-03-24 15:32 ` [PATCH 2.6.11] aoe [12/12]: send outgoing packets in order ecashin
13 siblings, 0 replies; 30+ messages in thread
From: ecashin @ 2005-03-24 15:30 UTC (permalink / raw)
To: linux-kernel
add support for disk statistics
Signed-off-by: Ed L. Cashin <ecashin@coraid.com>
diff -uprN a/drivers/block/aoe/aoe.h b/drivers/block/aoe/aoe.h
--- a/drivers/block/aoe/aoe.h 2005-03-10 12:19:54.000000000 -0500
+++ b/drivers/block/aoe/aoe.h 2005-03-10 12:20:02.000000000 -0500
@@ -91,6 +91,7 @@ enum {
struct buf {
struct list_head bufs;
+ ulong start_time; /* for disk stats */
ulong flags;
ulong nframesout;
char *bufaddr;
diff -uprN a/drivers/block/aoe/aoeblk.c b/drivers/block/aoe/aoeblk.c
--- a/drivers/block/aoe/aoeblk.c 2005-03-10 12:20:00.000000000 -0500
+++ b/drivers/block/aoe/aoeblk.c 2005-03-10 12:20:02.000000000 -0500
@@ -125,6 +125,7 @@ aoeblk_make_request(request_queue_t *q,
}
memset(buf, 0, sizeof(*buf));
INIT_LIST_HEAD(&buf->bufs);
+ buf->start_time = jiffies;
buf->bio = bio;
buf->resid = bio->bi_size;
buf->sector = bio->bi_sector;
diff -uprN a/drivers/block/aoe/aoecmd.c b/drivers/block/aoe/aoecmd.c
--- a/drivers/block/aoe/aoecmd.c 2005-03-10 12:19:27.000000000 -0500
+++ b/drivers/block/aoe/aoecmd.c 2005-03-10 12:20:02.000000000 -0500
@@ -456,6 +456,20 @@ aoecmd_ata_rsp(struct sk_buff *skb)
if (buf) {
buf->nframesout -= 1;
if (buf->nframesout == 0 && buf->resid == 0) {
+ unsigned long duration = jiffies - buf->start_time;
+ unsigned long n_sect = buf->bio->bi_size >> 9;
+ struct gendisk *disk = d->gd;
+
+ if (bio_data_dir(buf->bio) == WRITE) {
+ disk_stat_inc(disk, writes);
+ disk_stat_add(disk, write_ticks, duration);
+ disk_stat_add(disk, write_sectors, n_sect);
+ } else {
+ disk_stat_inc(disk, reads);
+ disk_stat_add(disk, read_ticks, duration);
+ disk_stat_add(disk, read_sectors, n_sect);
+ }
+ disk_stat_add(disk, io_ticks, duration);
n = (buf->flags & BUFFL_FAIL) ? -EIO : 0;
bio_endio(buf->bio, buf->bio->bi_size, n);
mempool_free(buf, d->bufpool);
--
Ed L. Cashin <ecashin@coraid.com>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 2.6.11] aoe [12/12]: send outgoing packets in order
2005-03-17 23:46 ` [PATCH 2.6.11] aoe [1/12]: remove too-low cap on minor number Greg KH
` (12 preceding siblings ...)
2005-03-24 15:30 ` [PATCH 2.6.11] aoe [11/12]: add support for disk statistics ecashin
@ 2005-03-24 15:32 ` ecashin
13 siblings, 0 replies; 30+ messages in thread
From: ecashin @ 2005-03-24 15:32 UTC (permalink / raw)
To: linux-kernel
I can't use list.h, since sk_buff doesn't have a list_head but instead
has two struct sk_buff pointers, and I want to avoid any extra memory
allocation.
send outgoing packets in order
Signed-off-by: Ed L. Cashin <ecashin@coraid.com>
diff -uprN a/drivers/block/aoe/aoe.h b/drivers/block/aoe/aoe.h
--- a/drivers/block/aoe/aoe.h 2005-03-10 12:20:02.000000000 -0500
+++ b/drivers/block/aoe/aoe.h 2005-03-10 12:20:04.000000000 -0500
@@ -132,7 +132,8 @@ struct aoedev {
struct timer_list timer;
spinlock_t lock;
struct net_device *ifp; /* interface ed is attached to */
- struct sk_buff *skblist;/* packets needing to be sent */
+ struct sk_buff *sendq_hd; /* packets needing to be sent, list head */
+ struct sk_buff *sendq_tl;
mempool_t *bufpool; /* for deadlock-free Buf allocation */
struct list_head bufq; /* queue of bios to work on */
struct buf *inprocess; /* the one we're currently working on */
diff -uprN a/drivers/block/aoe/aoeblk.c b/drivers/block/aoe/aoeblk.c
--- a/drivers/block/aoe/aoeblk.c 2005-03-10 12:20:02.000000000 -0500
+++ b/drivers/block/aoe/aoeblk.c 2005-03-10 12:20:04.000000000 -0500
@@ -147,8 +147,8 @@ aoeblk_make_request(request_queue_t *q,
list_add_tail(&buf->bufs, &d->bufq);
aoecmd_work(d);
- sl = d->skblist;
- d->skblist = NULL;
+ sl = d->sendq_hd;
+ d->sendq_hd = d->sendq_tl = NULL;
spin_unlock_irqrestore(&d->lock, flags);
diff -uprN a/drivers/block/aoe/aoecmd.c b/drivers/block/aoe/aoecmd.c
--- a/drivers/block/aoe/aoecmd.c 2005-03-10 12:20:02.000000000 -0500
+++ b/drivers/block/aoe/aoecmd.c 2005-03-10 12:20:04.000000000 -0500
@@ -178,8 +178,12 @@ aoecmd_ata_rw(struct aoedev *d, struct f
skb = skb_prepare(d, f);
if (skb) {
- skb->next = d->skblist;
- d->skblist = skb;
+ skb->next = NULL;
+ if (d->sendq_hd)
+ d->sendq_tl->next = skb;
+ else
+ d->sendq_hd = skb;
+ d->sendq_tl = skb;
}
}
@@ -227,8 +231,12 @@ rexmit(struct aoedev *d, struct frame *f
skb = skb_prepare(d, f);
if (skb) {
- skb->next = d->skblist;
- d->skblist = skb;
+ skb->next = NULL;
+ if (d->sendq_hd)
+ d->sendq_tl->next = skb;
+ else
+ d->sendq_hd = skb;
+ d->sendq_tl = skb;
}
}
@@ -280,8 +288,8 @@ tdie: spin_unlock_irqrestore(&d->lock,
}
}
- sl = d->skblist;
- d->skblist = NULL;
+ sl = d->sendq_hd;
+ d->sendq_hd = d->sendq_tl = NULL;
if (sl) {
n = d->rttavg <<= 1;
if (n > MAXTIMER)
@@ -481,8 +489,8 @@ aoecmd_ata_rsp(struct sk_buff *skb)
aoecmd_work(d);
- sl = d->skblist;
- d->skblist = NULL;
+ sl = d->sendq_hd;
+ d->sendq_hd = d->sendq_tl = NULL;
spin_unlock_irqrestore(&d->lock, flags);
@@ -531,7 +539,7 @@ aoecmd_cfg(ushort aoemajor, unsigned cha
/*
* Since we only call this in one place (and it only prepares one frame)
- * we just return the skb. Usually we'd chain it up to the d->skblist.
+ * we just return the skb. Usually we'd chain it up to the aoedev sendq.
*/
static struct sk_buff *
aoecmd_ata_id(struct aoedev *d)
--
Ed L. Cashin <ecashin@coraid.com>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 2.6.11] aoe [5/12]: don't try to free null bufpool
2005-03-24 15:17 ` [PATCH 2.6.11] aoe [5/12]: don't try to free null bufpool ecashin
@ 2005-03-24 15:58 ` Arjan van de Ven
2005-03-24 17:04 ` ecashin
0 siblings, 1 reply; 30+ messages in thread
From: Arjan van de Ven @ 2005-03-24 15:58 UTC (permalink / raw)
To: ecashin; +Cc: linux-kernel
On Thu, 2005-03-24 at 07:17 -0800, ecashin@noserose.net wrote:
> don't try to free null bufpool
in linux there is a "rule" that all memory free routines are supposed to
also accept NULL as argument, so I think this patch is not needed (and
even wrong)
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 2.6.11] aoe [5/12]: don't try to free null bufpool
2005-03-24 15:58 ` Arjan van de Ven
@ 2005-03-24 17:04 ` ecashin
2005-03-24 17:17 ` Arjan van de Ven
0 siblings, 1 reply; 30+ messages in thread
From: ecashin @ 2005-03-24 17:04 UTC (permalink / raw)
To: linux-kernel; +Cc: Arjan van de Ven, Greg K-H
Arjan van de Ven <arjan@infradead.org> writes:
> On Thu, 2005-03-24 at 07:17 -0800, ecashin@noserose.net wrote:
>> don't try to free null bufpool
>
> in linux there is a "rule" that all memory free routines are supposed to
> also accept NULL as argument, so I think this patch is not needed (and
> even wrong)
>
Hmm. The mm/mempool.c:mempool_destroy function immediately
dereferences the pointer passed to it:
void mempool_destroy(mempool_t *pool)
{
if (pool->curr_nr != pool->min_nr)
BUG(); /* There were outstanding elements */
free_pool(pool);
}
... so I'm not sure mempool_destroy fits the rule. Are you suggesting
that the patch should instead modify mempool_destroy?
--
Ed L Cashin <ecashin@coraid.com>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 2.6.11] aoe [5/12]: don't try to free null bufpool
2005-03-24 17:04 ` ecashin
@ 2005-03-24 17:17 ` Arjan van de Ven
2005-03-25 14:37 ` Jesper Juhl
2005-03-29 12:31 ` Jens Axboe
0 siblings, 2 replies; 30+ messages in thread
From: Arjan van de Ven @ 2005-03-24 17:17 UTC (permalink / raw)
To: ecashin; +Cc: linux-kernel, Greg K-H, axboe
On Thu, 2005-03-24 at 09:04 -0800, ecashin@noserose.net wrote:
> Arjan van de Ven <arjan@infradead.org> writes:
>
> > On Thu, 2005-03-24 at 07:17 -0800, ecashin@noserose.net wrote:
> >> don't try to free null bufpool
> >
> > in linux there is a "rule" that all memory free routines are supposed to
> > also accept NULL as argument, so I think this patch is not needed (and
> > even wrong)
> >
>
> Hmm. The mm/mempool.c:mempool_destroy function immediately
> dereferences the pointer passed to it:
>
> void mempool_destroy(mempool_t *pool)
> {
> if (pool->curr_nr != pool->min_nr)
> BUG(); /* There were outstanding elements */
> free_pool(pool);
> }
>
> ... so I'm not sure mempool_destroy fits the rule. Are you suggesting
> that the patch should instead modify mempool_destroy?
hmm perhaps... Jens?
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 2.6.11] aoe [5/12]: don't try to free null bufpool
2005-03-24 17:17 ` Arjan van de Ven
@ 2005-03-25 14:37 ` Jesper Juhl
2005-03-29 12:31 ` Jens Axboe
1 sibling, 0 replies; 30+ messages in thread
From: Jesper Juhl @ 2005-03-25 14:37 UTC (permalink / raw)
To: Arjan van de Ven; +Cc: ecashin, linux-kernel, Greg K-H, axboe
On Thu, 24 Mar 2005, Arjan van de Ven wrote:
> On Thu, 2005-03-24 at 09:04 -0800, ecashin@noserose.net wrote:
> > Arjan van de Ven <arjan@infradead.org> writes:
> >
> > > On Thu, 2005-03-24 at 07:17 -0800, ecashin@noserose.net wrote:
> > >> don't try to free null bufpool
> > >
> > > in linux there is a "rule" that all memory free routines are supposed to
> > > also accept NULL as argument, so I think this patch is not needed (and
> > > even wrong)
> > >
> >
> > Hmm. The mm/mempool.c:mempool_destroy function immediately
> > dereferences the pointer passed to it:
> >
> > void mempool_destroy(mempool_t *pool)
> > {
> > if (pool->curr_nr != pool->min_nr)
> > BUG(); /* There were outstanding elements */
> > free_pool(pool);
> > }
> >
> > ... so I'm not sure mempool_destroy fits the rule. Are you suggesting
> > that the patch should instead modify mempool_destroy?
>
> hmm perhaps... Jens?
>
Having mempool_destroy() be the one that checks seems safer, then callers
won't forget to check - easier to just check in one place.
If that's what you want, then here's a patch. If this is acceptable I can
create another one that removes the (then pointless) NULL checks from all
callers - let me know if that's wanted.
Signed-off-by: Jesper Juhl <juhl-lkml@dif.dk>
--- linux-2.6.12-rc1-mm3-orig/mm/mempool.c 2005-03-21 23:12:43.000000000 +0100
+++ linux-2.6.12-rc1-mm3/mm/mempool.c 2005-03-25 15:34:04.000000000 +0100
@@ -176,6 +176,8 @@ EXPORT_SYMBOL(mempool_resize);
*/
void mempool_destroy(mempool_t *pool)
{
+ if (!pool)
+ return;
if (pool->curr_nr != pool->min_nr)
BUG(); /* There were outstanding elements */
free_pool(pool);
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 2.6.11] aoe [7/12]: support configuration of AOE_PARTITIONS from Kconfig
2005-03-24 15:21 ` [PATCH 2.6.11] aoe [7/12]: support configuration of AOE_PARTITIONS from Kconfig ecashin
@ 2005-03-28 17:07 ` Christoph Hellwig
2005-03-29 16:06 ` Ed L Cashin
0 siblings, 1 reply; 30+ messages in thread
From: Christoph Hellwig @ 2005-03-28 17:07 UTC (permalink / raw)
To: ecashin; +Cc: linux-kernel
On Thu, Mar 24, 2005 at 07:21:28AM -0800, ecashin@noserose.net wrote:
>
> support configuration of AOE_PARTITIONS from Kconfig
>
> Signed-off-by: Ed L. Cashin <ecashin@coraid.com>
>
> diff -uprN a/drivers/block/Kconfig b/drivers/block/Kconfig
> --- a/drivers/block/Kconfig 2005-03-07 17:37:58.000000000 -0500
> +++ b/drivers/block/Kconfig 2005-03-10 12:19:54.000000000 -0500
> @@ -506,4 +506,19 @@ config ATA_OVER_ETH
> This driver provides Support for ATA over Ethernet block
> devices like the Coraid EtherDrive (R) Storage Blade.
>
> +config AOE_PARTITIONS
> + int "Partitions per AoE device" if ATA_OVER_ETH
> + default "16"
> + help
> + The default is to support 16 partitions per aoe device. Some
> + systems lack good support for devices with large minor
> + numbers.
> +
> + Such systems will be able to use more aoe disks when
> + AOE_PARTITIONS is set to one, but you won't be able to
> + partition the disks, and you must make sure your device
> + nodes are created to work with the value you select.
> +
> + If unsure, use 16.
> +
NACK. this changes devices nodes based on a compile-time option. Just
tell people to update their userland to a 2.6-copatible version.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 2.6.11] aoe [5/12]: don't try to free null bufpool
2005-03-24 17:17 ` Arjan van de Ven
2005-03-25 14:37 ` Jesper Juhl
@ 2005-03-29 12:31 ` Jens Axboe
1 sibling, 0 replies; 30+ messages in thread
From: Jens Axboe @ 2005-03-29 12:31 UTC (permalink / raw)
To: Arjan van de Ven; +Cc: ecashin, linux-kernel, Greg K-H
On Thu, Mar 24 2005, Arjan van de Ven wrote:
> On Thu, 2005-03-24 at 09:04 -0800, ecashin@noserose.net wrote:
> > Arjan van de Ven <arjan@infradead.org> writes:
> >
> > > On Thu, 2005-03-24 at 07:17 -0800, ecashin@noserose.net wrote:
> > >> don't try to free null bufpool
> > >
> > > in linux there is a "rule" that all memory free routines are supposed to
> > > also accept NULL as argument, so I think this patch is not needed (and
> > > even wrong)
> > >
> >
> > Hmm. The mm/mempool.c:mempool_destroy function immediately
> > dereferences the pointer passed to it:
> >
> > void mempool_destroy(mempool_t *pool)
> > {
> > if (pool->curr_nr != pool->min_nr)
> > BUG(); /* There were outstanding elements */
> > free_pool(pool);
> > }
> >
> > ... so I'm not sure mempool_destroy fits the rule. Are you suggesting
> > that the patch should instead modify mempool_destroy?
>
> hmm perhaps... Jens?
Not really my call, but I agree we should make mempool_destroy()
resilient against !pool to follow the path of least surprise.
--
Jens Axboe
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 2.6.11] aoe [7/12]: support configuration of AOE_PARTITIONS from Kconfig
2005-03-28 17:07 ` Christoph Hellwig
@ 2005-03-29 16:06 ` Ed L Cashin
2005-03-29 16:25 ` Christoph Hellwig
0 siblings, 1 reply; 30+ messages in thread
From: Ed L Cashin @ 2005-03-29 16:06 UTC (permalink / raw)
To: linux-kernel; +Cc: Christoph Hellwig, Greg K-H
Christoph Hellwig <hch@infradead.org> writes:
> On Thu, Mar 24, 2005 at 07:21:28AM -0800, ecashin@noserose.net wrote:
>>
>> support configuration of AOE_PARTITIONS from Kconfig
>>
>> Signed-off-by: Ed L. Cashin <ecashin@coraid.com>
>>
>> diff -uprN a/drivers/block/Kconfig b/drivers/block/Kconfig
>> --- a/drivers/block/Kconfig 2005-03-07 17:37:58.000000000 -0500
>> +++ b/drivers/block/Kconfig 2005-03-10 12:19:54.000000000 -0500
>> @@ -506,4 +506,19 @@ config ATA_OVER_ETH
>> This driver provides Support for ATA over Ethernet block
>> devices like the Coraid EtherDrive (R) Storage Blade.
>>
>> +config AOE_PARTITIONS
>> + int "Partitions per AoE device" if ATA_OVER_ETH
>> + default "16"
>> + help
>> + The default is to support 16 partitions per aoe device. Some
>> + systems lack good support for devices with large minor
>> + numbers.
>> +
>> + Such systems will be able to use more aoe disks when
>> + AOE_PARTITIONS is set to one, but you won't be able to
>> + partition the disks, and you must make sure your device
>> + nodes are created to work with the value you select.
>> +
>> + If unsure, use 16.
>> +
>
> NACK. this changes devices nodes based on a compile-time option.
I'm not sure I follow. This configuration option sets the number of
partitions per device in the driver. It doesn't create device nodes.
If the user has udev, then the device nodes are created correctly (on
Fedora Core 3), so that if the driver is configured with 1 partition
per device, the minor numbers for the disks are low.
The folks I've talked to who aren't using udev but want one partition
per device already know that they have to re-create their device
files.
> Just tell people to update their userland to a 2.6-copatible
> version.
Even if the glibc, coreutils, etc., get it right, some programs try to
parse the device node bits themselves and fail to find all the minor
number bits.
Making this configurable makes it possible for a debian sarge user or
a Slackware 10 user to run a 2.6.11 kernel and use up to 256 disks.
Even a Fedora Core 3 user has an mdadm that balks at minor numbers
like 1120. By using one partition per device, I can use FC3's mdadm
and have everything work.
The AoE users started doing this themselves, but this configuration
option allows non-C-programmers to do the same. It's helpful during
the transitional period and should be removed when the userland
software that people are actually running has caught up.
--
Ed L Cashin <ecashin@coraid.com>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 2.6.11] aoe [7/12]: support configuration of AOE_PARTITIONS from Kconfig
2005-03-29 16:06 ` Ed L Cashin
@ 2005-03-29 16:25 ` Christoph Hellwig
2005-03-29 16:48 ` Ed L Cashin
0 siblings, 1 reply; 30+ messages in thread
From: Christoph Hellwig @ 2005-03-29 16:25 UTC (permalink / raw)
To: Ed L Cashin; +Cc: linux-kernel, Greg K-H
On Tue, Mar 29, 2005 at 11:06:16AM -0500, Ed L Cashin wrote:
> >
> > NACK. this changes devices nodes based on a compile-time option.
>
> I'm not sure I follow. This configuration option sets the number of
> partitions per device in the driver. It doesn't create device nodes.
>
> If the user has udev, then the device nodes are created correctly (on
> Fedora Core 3), so that if the driver is configured with 1 partition
> per device, the minor numbers for the disks are low.
>
> The folks I've talked to who aren't using udev but want one partition
> per device already know that they have to re-create their device
> files.
It changes a kernel ABI, so people that have different config options
set can't use the same userland. It's a really big no-go.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 2.6.11] aoe [7/12]: support configuration of AOE_PARTITIONS from Kconfig
2005-03-29 16:25 ` Christoph Hellwig
@ 2005-03-29 16:48 ` Ed L Cashin
2005-03-29 16:57 ` Christoph Hellwig
0 siblings, 1 reply; 30+ messages in thread
From: Ed L Cashin @ 2005-03-29 16:48 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-kernel, Greg K-H
Christoph Hellwig <hch@infradead.org> writes:
> On Tue, Mar 29, 2005 at 11:06:16AM -0500, Ed L Cashin wrote:
>> >
>> > NACK. this changes devices nodes based on a compile-time option.
>>
>> I'm not sure I follow. This configuration option sets the number of
>> partitions per device in the driver. It doesn't create device nodes.
>>
>> If the user has udev, then the device nodes are created correctly (on
>> Fedora Core 3), so that if the driver is configured with 1 partition
>> per device, the minor numbers for the disks are low.
>>
>> The folks I've talked to who aren't using udev but want one partition
>> per device already know that they have to re-create their device
>> files.
>
> It changes a kernel ABI, so people that have different config options
> set can't use the same userland. It's a really big no-go.
I don't know if it matters now that we have udev. When udev manages
the device nodes it all just works, so there's practically not much of
an issue. The UUIDs in Software RAID and LVM make this even less of
an issue. Besides, if they aren't using udev and are using a custom
kernel *and* using this configure option, then they're changing the
ABI on purpose for a practical benefit.
If you're saying that it's bad in principal, then that's another
story. If that's what you mean, then it's a Linux policy issue, and
to follow convention I'd think that we'd need another major number.
That would be like the partitionable md devices, etc.
To me, the latter seems like the uglier way to go, and it would be
more permanent, while the configure option seems ugly but convenient
for this transitional stage, and could go away when high minor numbers
are well supported on the systems currently in use.
--
Ed L Cashin <ecashin@coraid.com>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 2.6.11] aoe [7/12]: support configuration of AOE_PARTITIONS from Kconfig
2005-03-29 16:48 ` Ed L Cashin
@ 2005-03-29 16:57 ` Christoph Hellwig
2005-04-07 18:28 ` Ed L Cashin
0 siblings, 1 reply; 30+ messages in thread
From: Christoph Hellwig @ 2005-03-29 16:57 UTC (permalink / raw)
To: Ed L Cashin; +Cc: Christoph Hellwig, linux-kernel, Greg K-H
On Tue, Mar 29, 2005 at 11:48:48AM -0500, Ed L Cashin wrote:
> I don't know if it matters now that we have udev. When udev manages
> the device nodes it all just works,
But most peopel still don't use udev.
> If you're saying that it's bad in principal, then that's another
> story. If that's what you mean, then it's a Linux policy issue, and
> to follow convention I'd think that we'd need another major number.
> That would be like the partitionable md devices, etc.
Yes, it's a policy issue. We don't do this weird config option anywhere
else.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 2.6.11] aoe [7/12]: support configuration of AOE_PARTITIONS from Kconfig
2005-03-29 16:57 ` Christoph Hellwig
@ 2005-04-07 18:28 ` Ed L Cashin
2005-04-07 18:49 ` Greg KH
0 siblings, 1 reply; 30+ messages in thread
From: Ed L Cashin @ 2005-04-07 18:28 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Greg K-H, linux-kernel
Christoph Hellwig <hch@infradead.org> writes:
> On Tue, Mar 29, 2005 at 11:48:48AM -0500, Ed L Cashin wrote:
>> I don't know if it matters now that we have udev. When udev manages
>> the device nodes it all just works,
>
> But most peopel still don't use udev.
>
>> If you're saying that it's bad in principal, then that's another
>> story. If that's what you mean, then it's a Linux policy issue, and
>> to follow convention I'd think that we'd need another major number.
>> That would be like the partitionable md devices, etc.
>
> Yes, it's a policy issue. We don't do this weird config option anywhere
> else.
A couple support calls later, I think I've come around to your point
of view. This patch isn't needed and may cause confusion.
Few aoe users really use partitions on their aoe disks, so I can make
the aoe driver have one minor number per disk as the default to avoid
the most common problems people encounter.
Then, aoe users who really need to partition their network disks can
use the partitionable md driver to "wrap" the aoe disk, like this:
mdadm -B -l linear --force -n 1 --auto=mdp /dev/md_p0 /dev/etherd/e7.0
fdisk /dev/md_p0
--
Ed L Cashin <ecashin@coraid.com>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 2.6.11] aoe [7/12]: support configuration of AOE_PARTITIONS from Kconfig
2005-04-07 18:28 ` Ed L Cashin
@ 2005-04-07 18:49 ` Greg KH
2005-04-07 18:56 ` Ed L Cashin
0 siblings, 1 reply; 30+ messages in thread
From: Greg KH @ 2005-04-07 18:49 UTC (permalink / raw)
To: Ed L Cashin; +Cc: Christoph Hellwig, linux-kernel
On Thu, Apr 07, 2005 at 02:28:59PM -0400, Ed L Cashin wrote:
> Christoph Hellwig <hch@infradead.org> writes:
>
> > On Tue, Mar 29, 2005 at 11:48:48AM -0500, Ed L Cashin wrote:
> >> I don't know if it matters now that we have udev. When udev manages
> >> the device nodes it all just works,
> >
> > But most peopel still don't use udev.
> >
> >> If you're saying that it's bad in principal, then that's another
> >> story. If that's what you mean, then it's a Linux policy issue, and
> >> to follow convention I'd think that we'd need another major number.
> >> That would be like the partitionable md devices, etc.
> >
> > Yes, it's a policy issue. We don't do this weird config option anywhere
> > else.
>
> A couple support calls later, I think I've come around to your point
> of view. This patch isn't needed and may cause confusion.
>
> Few aoe users really use partitions on their aoe disks, so I can make
> the aoe driver have one minor number per disk as the default to avoid
> the most common problems people encounter.
>
> Then, aoe users who really need to partition their network disks can
> use the partitionable md driver to "wrap" the aoe disk, like this:
>
> mdadm -B -l linear --force -n 1 --auto=mdp /dev/md_p0 /dev/etherd/e7.0
> fdisk /dev/md_p0
So, which one of the aoe patches listed at:
http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/patches/driver/
do you want me to drop? This one:
http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/patches/driver/aoe-AOE_PARTITIONS.patch
?
Or some other one too?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 2.6.11] aoe [7/12]: support configuration of AOE_PARTITIONS from Kconfig
2005-04-07 18:49 ` Greg KH
@ 2005-04-07 18:56 ` Ed L Cashin
2005-04-07 23:08 ` Greg KH
0 siblings, 1 reply; 30+ messages in thread
From: Ed L Cashin @ 2005-04-07 18:56 UTC (permalink / raw)
To: Greg KH; +Cc: Christoph Hellwig, linux-kernel
Greg KH <greg@kroah.com> writes:
...
> So, which one of the aoe patches listed at:
> http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/patches/driver/
> do you want me to drop? This one:
> http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/patches/driver/aoe-AOE_PARTITIONS.patch
> ?
> Or some other one too?
Just aoe-AOE_PARTITIONS.patch, the seventh of the twelve, should be
dropped.
Then later I'll send a batch of patches that will include a change to
make aoe disks non-partitionable by default.
--
Ed L Cashin <ecashin@coraid.com>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 2.6.11] aoe [7/12]: support configuration of AOE_PARTITIONS from Kconfig
2005-04-07 18:56 ` Ed L Cashin
@ 2005-04-07 23:08 ` Greg KH
2005-04-08 13:54 ` Ed L Cashin
0 siblings, 1 reply; 30+ messages in thread
From: Greg KH @ 2005-04-07 23:08 UTC (permalink / raw)
To: Ed L Cashin; +Cc: Christoph Hellwig, linux-kernel
On Thu, Apr 07, 2005 at 02:56:39PM -0400, Ed L Cashin wrote:
> Greg KH <greg@kroah.com> writes:
>
> ...
> > So, which one of the aoe patches listed at:
> > http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/patches/driver/
> > do you want me to drop? This one:
> > http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/patches/driver/aoe-AOE_PARTITIONS.patch
> > ?
> > Or some other one too?
>
> Just aoe-AOE_PARTITIONS.patch, the seventh of the twelve, should be
> dropped.
Ok, dropped.
> Then later I'll send a batch of patches that will include a change to
> make aoe disks non-partitionable by default.
That's fine. Mind if I forward the other aoe patches in that directory
to Linus soon?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 2.6.11] aoe [7/12]: support configuration of AOE_PARTITIONS from Kconfig
2005-04-07 23:08 ` Greg KH
@ 2005-04-08 13:54 ` Ed L Cashin
0 siblings, 0 replies; 30+ messages in thread
From: Ed L Cashin @ 2005-04-08 13:54 UTC (permalink / raw)
To: Greg KH; +Cc: linux-kernel
Greg KH <greg@kroah.com> writes:
> On Thu, Apr 07, 2005 at 02:56:39PM -0400, Ed L Cashin wrote:
...
>> Just aoe-AOE_PARTITIONS.patch, the seventh of the twelve, should be
>> dropped.
>
> Ok, dropped.
>
>> Then later I'll send a batch of patches that will include a change to
>> make aoe disks non-partitionable by default.
>
> That's fine. Mind if I forward the other aoe patches in that directory
> to Linus soon?
Go ahead. I have a new batch of patches to send, but it looks like I
might not get to it for a few days.
--
Ed L Cashin <ecashin@coraid.com>
^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2005-04-08 14:00 UTC | newest]
Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <87mztbi79d.fsf@coraid.com>
2005-03-17 23:46 ` [PATCH 2.6.11] aoe [1/12]: remove too-low cap on minor number Greg KH
2005-03-18 20:13 ` Ed L Cashin
2005-03-18 20:14 ` [PATCH 2.6.11] aoe [4/12]: handle distros that have a udev rules file instead of dir Ed L. Cashin
2005-03-24 14:59 ` [PATCH 2.6.11] aoe [1/12]: remove too-low cap on minor number ecashin
2005-03-24 15:09 ` [PATCH 2.6.11] aoe [2/12]: allow multiple aoe devices with same MAC addr ecashin
2005-03-24 15:13 ` [PATCH 2.6.11] aoe [3/12]: update driver version to 6 ecashin
2005-03-24 15:15 ` [PATCH 2.6.11] aoe [4/12]: handle distros that have a udev rules file instead of dir ecashin
2005-03-24 15:17 ` [PATCH 2.6.11] aoe [5/12]: don't try to free null bufpool ecashin
2005-03-24 15:58 ` Arjan van de Ven
2005-03-24 17:04 ` ecashin
2005-03-24 17:17 ` Arjan van de Ven
2005-03-25 14:37 ` Jesper Juhl
2005-03-29 12:31 ` Jens Axboe
2005-03-24 15:19 ` [PATCH 2.6.11] aoe [6/12]: Alexey Dobriyan sparse cleanup ecashin
2005-03-24 15:21 ` [PATCH 2.6.11] aoe [7/12]: support configuration of AOE_PARTITIONS from Kconfig ecashin
2005-03-28 17:07 ` Christoph Hellwig
2005-03-29 16:06 ` Ed L Cashin
2005-03-29 16:25 ` Christoph Hellwig
2005-03-29 16:48 ` Ed L Cashin
2005-03-29 16:57 ` Christoph Hellwig
2005-04-07 18:28 ` Ed L Cashin
2005-04-07 18:49 ` Greg KH
2005-04-07 18:56 ` Ed L Cashin
2005-04-07 23:08 ` Greg KH
2005-04-08 13:54 ` Ed L Cashin
2005-03-24 15:23 ` [PATCH 2.6.11] aoe [8/12]: document env var for specifying number of partitions per dev ecashin
2005-03-24 15:25 ` [PATCH 2.6.11] aoe [9/12]: add note about the need for deadlock-free sk_buff allocation ecashin
2005-03-24 15:27 ` [PATCH 2.6.11] aoe [10/12]: Randy Dunlap: avoid warnings on sparc64 ecashin
2005-03-24 15:30 ` [PATCH 2.6.11] aoe [11/12]: add support for disk statistics ecashin
2005-03-24 15:32 ` [PATCH 2.6.11] aoe [12/12]: send outgoing packets in order ecashin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.