aboutsummaryrefslogtreecommitdiffstats
path: root/drivers
diff options
context:
space:
mode:
authorEd L. Cashin <ecashin@coraid.com>2008-02-08 07:20:05 -0500
committerLinus Torvalds <torvalds@woody.linux-foundation.org>2008-02-08 12:22:32 -0500
commit9bb237b6a670fa7a6af3adc65231b1f6fda44510 (patch)
tree6e30e57f3d79e28e8be92f8685037998c16d42da /drivers
parent262bf54144ebcb78cd0d057d2705dc5fb7bba7ac (diff)
aoe: dynamically allocate a capped number of skbs when necessary
What this Patch Does Even before this recent series of 12 patches to 2.6.22-rc4, the aoe driver was reusing a small set of skbs that were allocated once and were only used for outbound AoE commands. The network layer cannot be allowed to put_page on the data that is still associated with a bio we haven't returned to the block layer, so the aoe driver (even before the patch under discussion) is still the owner of skbs that have been handed to the network layer for transmission. We need to keep track of these skbs so that we can free them, but by tracking them, we can also easily re-use them. The new patch was a response to the behavior of certain network drivers. We cannot reuse an skb that the network driver still has in its transmit ring. Network drivers can defer transmit ring cleanup and then use the state in the skb to determine how many data segments to clean up in its transmit ring. The tg3 driver is one driver that behaves in this way. When the network driver defers cleanup of its transmit ring, the aoe driver can find itself in a situation where it would like to send an AoE command, and the AoE target is ready for more work, but the network driver still has all of the pre-allocated skbs. In that case, the new patch just calls alloc_skb, as you'd expect. We don't want to get carried away, though. We try not to do excessive allocation in the write path, so we cap the number of skbs we dynamically allocate. Probably calling it a "dynamic pool" is misleading. We were already trying to use a small fixed-size set of pre-allocated skbs before this patch, and this patch just provides a little headroom (with a ceiling, though) to accomodate network drivers that hang onto skbs, by allocating when needed. The d->skbpool_hd list of allocated skbs is necessary so that we can free them later. We didn't notice the need for this headroom until AoE targets got fast enough. Alternatives If the network layer never did a put_page on the pages in the bio's we get from the block layer, then it would be possible for us to hand skbs to the network layer and forget about them, allowing the network layer to free skbs itself (and thereby calling our own skb->destructor callback function if we needed that). In that case we could get rid of the pre-allocated skbs and also the d->skbpool_hd, instead just calling alloc_skb every time we wanted to transmit a packet. The slab allocator would effectively maintain the list of skbs. Besides a loss of CPU cache locality, the main concern with that approach the danger that it would increase the likelihood of deadlock when VM is trying to free pages by writing dirty data from the page cache through the aoe driver out to persistent storage on an AoE device. Right now we have a situation where we have pre-allocation that corresponds to how much we use, which seems ideal. Of course, there's still the separate issue of receiving the packets that tell us that a write has successfully completed on the AoE target. When memory is low and VM is using AoE to flush dirty data to free up pages, it would be perfect if there were a way for us to register a fast callback that could recognize write command completion responses. But I don't think the current problems with the receive side of the situation are a justification for exacerbating the problem on the transmit side. Signed-off-by: Ed L. Cashin <ecashin@coraid.com> Cc: Greg KH <greg@kroah.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Diffstat (limited to 'drivers')
-rw-r--r--drivers/block/aoe/aoe.h5
-rw-r--r--drivers/block/aoe/aoecmd.c117
-rw-r--r--drivers/block/aoe/aoedev.c52
3 files changed, 133 insertions, 41 deletions
diff --git a/drivers/block/aoe/aoe.h b/drivers/block/aoe/aoe.h
index 2248ab226576..67ef4d755e64 100644
--- a/drivers/block/aoe/aoe.h
+++ b/drivers/block/aoe/aoe.h
@@ -89,6 +89,7 @@ enum {
89 MIN_BUFS = 16, 89 MIN_BUFS = 16,
90 NTARGETS = 8, 90 NTARGETS = 8,
91 NAOEIFS = 8, 91 NAOEIFS = 8,
92 NSKBPOOLMAX = 128,
92 93
93 TIMERTICK = HZ / 10, 94 TIMERTICK = HZ / 10,
94 MINTIMER = HZ >> 2, 95 MINTIMER = HZ >> 2,
@@ -138,6 +139,7 @@ struct aoetgt {
138 u16 useme; 139 u16 useme;
139 ulong lastwadj; /* last window adjustment */ 140 ulong lastwadj; /* last window adjustment */
140 int wpkts, rpkts; 141 int wpkts, rpkts;
142 int dataref;
141}; 143};
142 144
143struct aoedev { 145struct aoedev {
@@ -159,6 +161,9 @@ struct aoedev {
159 spinlock_t lock; 161 spinlock_t lock;
160 struct sk_buff *sendq_hd; /* packets needing to be sent, list head */ 162 struct sk_buff *sendq_hd; /* packets needing to be sent, list head */
161 struct sk_buff *sendq_tl; 163 struct sk_buff *sendq_tl;
164 struct sk_buff *skbpool_hd;
165 struct sk_buff *skbpool_tl;
166 int nskbpool;
162 mempool_t *bufpool; /* for deadlock-free Buf allocation */ 167 mempool_t *bufpool; /* for deadlock-free Buf allocation */
163 struct list_head bufq; /* queue of bios to work on */ 168 struct list_head bufq; /* queue of bios to work on */
164 struct buf *inprocess; /* the one we're currently working on */ 169 struct buf *inprocess; /* the one we're currently working on */
diff --git a/drivers/block/aoe/aoecmd.c b/drivers/block/aoe/aoecmd.c
index 1be5150bcd3b..b49e06ef121e 100644
--- a/drivers/block/aoe/aoecmd.c
+++ b/drivers/block/aoe/aoecmd.c
@@ -106,45 +106,104 @@ ifrotate(struct aoetgt *t)
106 } 106 }
107} 107}
108 108
109static void
110skb_pool_put(struct aoedev *d, struct sk_buff *skb)
111{
112 if (!d->skbpool_hd)
113 d->skbpool_hd = skb;
114 else
115 d->skbpool_tl->next = skb;
116 d->skbpool_tl = skb;
117}
118
119static struct sk_buff *
120skb_pool_get(struct aoedev *d)
121{
122 struct sk_buff *skb;
123
124 skb = d->skbpool_hd;
125 if (skb && atomic_read(&skb_shinfo(skb)->dataref) == 1) {
126 d->skbpool_hd = skb->next;
127 skb->next = NULL;
128 return skb;
129 }
130 if (d->nskbpool < NSKBPOOLMAX
131 && (skb = new_skb(ETH_ZLEN))) {
132 d->nskbpool++;
133 return skb;
134 }
135 return NULL;
136}
137
138/* freeframe is where we do our load balancing so it's a little hairy. */
109static struct frame * 139static struct frame *
110freeframe(struct aoedev *d) 140freeframe(struct aoedev *d)
111{ 141{
112 struct frame *f, *e; 142 struct frame *f, *e, *rf;
113 struct aoetgt **t; 143 struct aoetgt **t;
114 ulong n; 144 struct sk_buff *skb;
115 145
116 if (d->targets[0] == NULL) { /* shouldn't happen, but I'm paranoid */ 146 if (d->targets[0] == NULL) { /* shouldn't happen, but I'm paranoid */
117 printk(KERN_ERR "aoe: NULL TARGETS!\n"); 147 printk(KERN_ERR "aoe: NULL TARGETS!\n");
118 return NULL; 148 return NULL;
119 } 149 }
120 t = d->targets; 150 t = d->tgt;
121 do { 151 t++;
122 if (t != d->htgt 152 if (t >= &d->targets[NTARGETS] || !*t)
123 && (*t)->ifp->nd 153 t = d->targets;
124 && (*t)->nout < (*t)->maxout) { 154 for (;;) {
125 n = (*t)->nframes; 155 if ((*t)->nout < (*t)->maxout
156 && t != d->htgt
157 && (*t)->ifp->nd) {
158 rf = NULL;
126 f = (*t)->frames; 159 f = (*t)->frames;
127 e = f + n; 160 e = f + (*t)->nframes;
128 for (; f < e; f++) { 161 for (; f < e; f++) {
129 if (f->tag != FREETAG) 162 if (f->tag != FREETAG)
130 continue; 163 continue;
131 if (atomic_read(&skb_shinfo(f->skb)->dataref) 164 skb = f->skb;
165 if (!skb
166 && !(f->skb = skb = new_skb(ETH_ZLEN)))
167 continue;
168 if (atomic_read(&skb_shinfo(skb)->dataref)
132 != 1) { 169 != 1) {
133 n--; 170 if (!rf)
171 rf = f;
134 continue; 172 continue;
135 } 173 }
136 skb_shinfo(f->skb)->nr_frags = 0; 174gotone: skb_shinfo(skb)->nr_frags = skb->data_len = 0;
137 f->skb->data_len = 0; 175 skb_trim(skb, 0);
138 skb_trim(f->skb, 0);
139 d->tgt = t; 176 d->tgt = t;
140 ifrotate(*t); 177 ifrotate(*t);
141 return f; 178 return f;
142 } 179 }
143 if (n == 0) /* slow polling network card */ 180 /* Work can be done, but the network layer is
181 holding our precious packets. Try to grab
182 one from the pool. */
183 f = rf;
184 if (f == NULL) { /* more paranoia */
185 printk(KERN_ERR
186 "aoe: freeframe: %s.\n",
187 "unexpected null rf");
188 d->flags |= DEVFL_KICKME;
189 return NULL;
190 }
191 skb = skb_pool_get(d);
192 if (skb) {
193 skb_pool_put(d, f->skb);
194 f->skb = skb;
195 goto gotone;
196 }
197 (*t)->dataref++;
198 if ((*t)->nout == 0)
144 d->flags |= DEVFL_KICKME; 199 d->flags |= DEVFL_KICKME;
145 } 200 }
201 if (t == d->tgt) /* we've looped and found nada */
202 break;
146 t++; 203 t++;
147 } while (t < &d->targets[NTARGETS] && *t); 204 if (t >= &d->targets[NTARGETS] || !*t)
205 t = d->targets;
206 }
148 return NULL; 207 return NULL;
149} 208}
150 209
@@ -894,33 +953,23 @@ addtgt(struct aoedev *d, char *addr, ulong nframes)
894 return NULL; 953 return NULL;
895 954
896 t = kcalloc(1, sizeof *t, GFP_ATOMIC); 955 t = kcalloc(1, sizeof *t, GFP_ATOMIC);
956 if (!t)
957 return NULL;
897 f = kcalloc(nframes, sizeof *f, GFP_ATOMIC); 958 f = kcalloc(nframes, sizeof *f, GFP_ATOMIC);
898 if (!t || !f) 959 if (!f) {
899 goto bail; 960 kfree(t);
961 return NULL;
962 }
963
900 t->nframes = nframes; 964 t->nframes = nframes;
901 t->frames = f; 965 t->frames = f;
902 e = f + nframes; 966 e = f + nframes;
903 for (; f < e; f++) { 967 for (; f < e; f++)
904 f->tag = FREETAG; 968 f->tag = FREETAG;
905 f->skb = new_skb(ETH_ZLEN);
906 if (!f->skb)
907 break;
908 }
909 if (f != e) {
910 while (f > t->frames) {
911 f--;
912 dev_kfree_skb(f->skb);
913 }
914 goto bail;
915 }
916 memcpy(t->addr, addr, sizeof t->addr); 969 memcpy(t->addr, addr, sizeof t->addr);
917 t->ifp = t->ifs; 970 t->ifp = t->ifs;
918 t->maxout = t->nframes; 971 t->maxout = t->nframes;
919 return *tt = t; 972 return *tt = t;
920bail:
921 kfree(t);
922 kfree(f);
923 return NULL;
924} 973}
925 974
926void 975void
diff --git a/drivers/block/aoe/aoedev.c b/drivers/block/aoe/aoedev.c
index e26f6f4a28a2..839a964906ce 100644
--- a/drivers/block/aoe/aoedev.c
+++ b/drivers/block/aoe/aoedev.c
@@ -7,11 +7,13 @@
7#include <linux/hdreg.h> 7#include <linux/hdreg.h>
8#include <linux/blkdev.h> 8#include <linux/blkdev.h>
9#include <linux/netdevice.h> 9#include <linux/netdevice.h>
10#include <linux/delay.h>
10#include "aoe.h" 11#include "aoe.h"
11 12
12static void dummy_timer(ulong); 13static void dummy_timer(ulong);
13static void aoedev_freedev(struct aoedev *); 14static void aoedev_freedev(struct aoedev *);
14static void freetgt(struct aoetgt *t); 15static void freetgt(struct aoedev *d, struct aoetgt *t);
16static void skbpoolfree(struct aoedev *d);
15 17
16static struct aoedev *devlist; 18static struct aoedev *devlist;
17static spinlock_t devlist_lock; 19static spinlock_t devlist_lock;
@@ -125,9 +127,10 @@ aoedev_freedev(struct aoedev *d)
125 t = d->targets; 127 t = d->targets;
126 e = t + NTARGETS; 128 e = t + NTARGETS;
127 for (; t < e && *t; t++) 129 for (; t < e && *t; t++)
128 freetgt(*t); 130 freetgt(d, *t);
129 if (d->bufpool) 131 if (d->bufpool)
130 mempool_destroy(d->bufpool); 132 mempool_destroy(d->bufpool);
133 skbpoolfree(d);
131 kfree(d); 134 kfree(d);
132} 135}
133 136
@@ -176,6 +179,43 @@ aoedev_flush(const char __user *str, size_t cnt)
176 return 0; 179 return 0;
177} 180}
178 181
182/* I'm not really sure that this is a realistic problem, but if the
183network driver goes gonzo let's just leak memory after complaining. */
184static void
185skbfree(struct sk_buff *skb)
186{
187 enum { Sms = 100, Tms = 3*1000};
188 int i = Tms / Sms;
189
190 if (skb == NULL)
191 return;
192 while (atomic_read(&skb_shinfo(skb)->dataref) != 1 && i-- > 0)
193 msleep(Sms);
194 if (i <= 0) {
195 printk(KERN_ERR
196 "aoe: %s holds ref: %s\n",
197 skb->dev ? skb->dev->name : "netif",
198 "cannot free skb -- memory leaked.");
199 return;
200 }
201 skb_shinfo(skb)->nr_frags = skb->data_len = 0;
202 skb_trim(skb, 0);
203 dev_kfree_skb(skb);
204}
205
206static void
207skbpoolfree(struct aoedev *d)
208{
209 struct sk_buff *skb;
210
211 while ((skb = d->skbpool_hd)) {
212 d->skbpool_hd = skb->next;
213 skb->next = NULL;
214 skbfree(skb);
215 }
216 d->skbpool_tl = NULL;
217}
218
179/* find it or malloc it */ 219/* find it or malloc it */
180struct aoedev * 220struct aoedev *
181aoedev_by_sysminor_m(ulong sysminor) 221aoedev_by_sysminor_m(ulong sysminor)
@@ -215,16 +255,14 @@ aoedev_by_sysminor_m(ulong sysminor)
215} 255}
216 256
217static void 257static void
218freetgt(struct aoetgt *t) 258freetgt(struct aoedev *d, struct aoetgt *t)
219{ 259{
220 struct frame *f, *e; 260 struct frame *f, *e;
221 261
222 f = t->frames; 262 f = t->frames;
223 e = f + t->nframes; 263 e = f + t->nframes;
224 for (; f < e; f++) { 264 for (; f < e; f++)
225 skb_shinfo(f->skb)->nr_frags = 0; 265 skbfree(f->skb);
226 dev_kfree_skb(f->skb);
227 }
228 kfree(t->frames); 266 kfree(t->frames);
229 kfree(t); 267 kfree(t);
230} 268}