| Message ID | 20191105154951.546285-1-toke@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | net/fq_impl: Switch to kvmalloc() for memory allocation | expand |
On Tue, 2019-11-05 at 16:49 +0100, Toke Høiland-Jørgensen wrote: > The FQ implementation used by mac80211 allocates memory using kmalloc(), > which can fail; and Johannes reported that this actually happens in > practice. > > To avoid this, switch the allocation to kvmalloc() instead; this also > brings fq_impl in line with all the FQ qdiscs. > > Fixes: 557fc4a09803 ("fq: add fair queuing framework") > Reported-by: Johannes Berg <johannes@sipsolutions.net> > Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com> > --- > include/net/fq_impl.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/net/fq_impl.h b/include/net/fq_impl.h > index 107c0d700ed6..d3873db91c71 100644 > --- a/include/net/fq_impl.h > +++ b/include/net/fq_impl.h > @@ -313,7 +313,7 @@ static int fq_init(struct fq *fq, int flows_cnt) > fq->limit = 8192; > fq->memory_limit = 16 << 20; /* 16 MBytes */ > > - fq->flows = kcalloc(fq->flows_cnt, sizeof(fq->flows[0]), GFP_KERNEL); > + fq->flows = kvcalloc(fq->flows_cnt, sizeof(fq->flows[0]), GFP_KERNEL); You need a corresponding kvfree(), no? johannes
Johannes Berg <johannes@sipsolutions.net> writes: > On Tue, 2019-11-05 at 16:49 +0100, Toke Høiland-Jørgensen wrote: >> The FQ implementation used by mac80211 allocates memory using kmalloc(), >> which can fail; and Johannes reported that this actually happens in >> practice. >> >> To avoid this, switch the allocation to kvmalloc() instead; this also >> brings fq_impl in line with all the FQ qdiscs. >> >> Fixes: 557fc4a09803 ("fq: add fair queuing framework") >> Reported-by: Johannes Berg <johannes@sipsolutions.net> >> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com> >> --- >> include/net/fq_impl.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/include/net/fq_impl.h b/include/net/fq_impl.h >> index 107c0d700ed6..d3873db91c71 100644 >> --- a/include/net/fq_impl.h >> +++ b/include/net/fq_impl.h >> @@ -313,7 +313,7 @@ static int fq_init(struct fq *fq, int flows_cnt) >> fq->limit = 8192; >> fq->memory_limit = 16 << 20; /* 16 MBytes */ >> >> - fq->flows = kcalloc(fq->flows_cnt, sizeof(fq->flows[0]), GFP_KERNEL); >> + fq->flows = kvcalloc(fq->flows_cnt, sizeof(fq->flows[0]), GFP_KERNEL); > > You need a corresponding kvfree(), no? Oh, right, of course; silly me. Guess I was a bit too trigger-happy on the nice one-character patch ;) Will send a v2 -Toke
On Tue, 2019-11-05 at 16:56 +0100, Toke Høiland-Jørgensen wrote: > > Oh, right, of course; silly me. Guess I was a bit too trigger-happy on > the nice one-character patch ;) > I guess two characters is still pretty good ;) johannes
Johannes Berg <johannes@sipsolutions.net> writes: > On Tue, 2019-11-05 at 16:56 +0100, Toke Høiland-Jørgensen wrote: >> >> Oh, right, of course; silly me. Guess I was a bit too trigger-happy on >> the nice one-character patch ;) >> > > I guess two characters is still pretty good ;) Hehe, yeah, I can live with that ;) -Toke
diff --git a/include/net/fq_impl.h b/include/net/fq_impl.h index 107c0d700ed6..d3873db91c71 100644 --- a/include/net/fq_impl.h +++ b/include/net/fq_impl.h @@ -313,7 +313,7 @@ static int fq_init(struct fq *fq, int flows_cnt) fq->limit = 8192; fq->memory_limit = 16 << 20; /* 16 MBytes */ - fq->flows = kcalloc(fq->flows_cnt, sizeof(fq->flows[0]), GFP_KERNEL); + fq->flows = kvcalloc(fq->flows_cnt, sizeof(fq->flows[0]), GFP_KERNEL); if (!fq->flows) return -ENOMEM;
The FQ implementation used by mac80211 allocates memory using kmalloc(), which can fail; and Johannes reported that this actually happens in practice. To avoid this, switch the allocation to kvmalloc() instead; this also brings fq_impl in line with all the FQ qdiscs. Fixes: 557fc4a09803 ("fq: add fair queuing framework") Reported-by: Johannes Berg <johannes@sipsolutions.net> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com> --- include/net/fq_impl.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)