From: Dave Hansen <haveblue@us.ibm.com>
To: davem@redhat.com
Cc: netdev@oss.sgi.com, Dave Hansen <haveblue@us.ibm.com>
Subject: [patch] af_packet: use void* for virtual addresses
Date: Fri, 27 Aug 2004 15:22:23 -0700 [thread overview]
Message-ID: <E1C0p77-0004Ik-00@kernel.beaverton.ibm.com> (raw)
I've been auditing code, cleaning up warning where code passes
"unsigned long"s to functions and macros that really take pointers.
Here's some explanation as to why I think these types were coded up
this way originally:
http://marc.theaimsgroup.com/?l=linux-mm&m=109155379124628&w=2
The attached patch make packet_opt->pg_vec a pointer to an array of
char*'s instead of a pointer to an array of "unsigned longs" that
stored virtual addresses. It also creates the inline function
pg_vec_endpage(), which hides the virt_to_page() call along with a
little address arithmetic.
One slight oddity in the current code is this:
pg_vec = kmalloc(req->tp_block_nr*sizeof(unsigned long*), GFP_KERNEL);
Since the previous code was storing virtual addresses in unsigned
longs, it was actually trying to allocate an array of *them*, not
of pointers to them. Not that it matters in the end, but I _think_
that type is technically incorrect.
I replaced it with this line:
pg_vec = kmalloc(req->tp_block_nr*sizeof(char *), GFP_KERNEL);
because I actually want an array of pointers.
This shouldn't have any functional effect on the code. I've been
running with it for a few weeks with no problems.
The alternative to reworking the types is to simply cast the argument
to a void* before calling virt_to_page().
Patched against 2.6.9-rc1-mm1 (but should apply to Linus's tree)
Signed-off-by: Dave Hansen <haveblue@us.ibm.com>
---
memhotplug-dave/net/packet/af_packet.c | 32 +++++++++++++++++++-------------
1 files changed, 19 insertions(+), 13 deletions(-)
diff -puN net/packet/af_packet.c~A0-af_packet_to_voidstar net/packet/af_packet.c
--- memhotplug/net/packet/af_packet.c~A0-af_packet_to_voidstar 2004-08-27 14:06:54.000000000 -0700
+++ memhotplug-dave/net/packet/af_packet.c 2004-08-27 14:06:54.000000000 -0700
@@ -66,6 +66,7 @@
#include <asm/uaccess.h>
#include <asm/ioctls.h>
#include <asm/page.h>
+#include <asm/io.h>
#include <linux/proc_fs.h>
#include <linux/seq_file.h>
#include <linux/poll.h>
@@ -173,7 +174,7 @@ struct packet_opt
{
struct tpacket_stats stats;
#ifdef CONFIG_PACKET_MMAP
- unsigned long *pg_vec;
+ char * *pg_vec;
unsigned int head;
unsigned int frames_per_block;
unsigned int frame_size;
@@ -198,15 +199,15 @@ struct packet_opt
#ifdef CONFIG_PACKET_MMAP
-static inline unsigned long packet_lookup_frame(struct packet_opt *po, unsigned int position)
+static inline char *packet_lookup_frame(struct packet_opt *po, unsigned int position)
{
unsigned int pg_vec_pos, frame_offset;
- unsigned long frame;
+ char *frame;
pg_vec_pos = position / po->frames_per_block;
frame_offset = position % po->frames_per_block;
- frame = (unsigned long) (po->pg_vec[pg_vec_pos] + (frame_offset * po->frame_size));
+ frame = po->pg_vec[pg_vec_pos] + (frame_offset * po->frame_size);
return frame;
}
@@ -1549,7 +1550,12 @@ static struct vm_operations_struct packe
.close =packet_mm_close,
};
-static void free_pg_vec(unsigned long *pg_vec, unsigned order, unsigned len)
+static inline struct page *pg_vec_endpage(char *one_pg_vec, unsigned int order)
+{
+ return virt_to_page(one_pg_vec + (PAGE_SIZE << order) - 1);
+}
+
+static void free_pg_vec(char **pg_vec, unsigned order, unsigned len)
{
int i;
@@ -1557,10 +1563,10 @@ static void free_pg_vec(unsigned long *p
if (pg_vec[i]) {
struct page *page, *pend;
- pend = virt_to_page(pg_vec[i] + (PAGE_SIZE << order) - 1);
+ pend = pg_vec_endpage(pg_vec[i], order);
for (page = virt_to_page(pg_vec[i]); page <= pend; page++)
ClearPageReserved(page);
- free_pages(pg_vec[i], order);
+ free_pages((unsigned long)pg_vec[i], order);
}
}
kfree(pg_vec);
@@ -1569,7 +1575,7 @@ static void free_pg_vec(unsigned long *p
static int packet_set_ring(struct sock *sk, struct tpacket_req *req, int closing)
{
- unsigned long *pg_vec = NULL;
+ char **pg_vec = NULL;
struct packet_opt *po = pkt_sk(sk);
int was_running, num, order = 0;
int err = 0;
@@ -1604,18 +1610,18 @@ static int packet_set_ring(struct sock *
err = -ENOMEM;
- pg_vec = kmalloc(req->tp_block_nr*sizeof(unsigned long*), GFP_KERNEL);
+ pg_vec = kmalloc(req->tp_block_nr*sizeof(char *), GFP_KERNEL);
if (pg_vec == NULL)
goto out;
- memset(pg_vec, 0, req->tp_block_nr*sizeof(unsigned long*));
+ memset(pg_vec, 0, req->tp_block_nr*sizeof(char **));
for (i=0; i<req->tp_block_nr; i++) {
struct page *page, *pend;
- pg_vec[i] = __get_free_pages(GFP_KERNEL, order);
+ pg_vec[i] = (char *)__get_free_pages(GFP_KERNEL, order);
if (!pg_vec[i])
goto out_free_pgvec;
- pend = virt_to_page(pg_vec[i] + (PAGE_SIZE << order) - 1);
+ pend = pg_vec_endpage(pg_vec[i], order);
for (page = virt_to_page(pg_vec[i]); page <= pend; page++)
SetPageReserved(page);
}
@@ -1623,7 +1629,7 @@ static int packet_set_ring(struct sock *
l = 0;
for (i=0; i<req->tp_block_nr; i++) {
- unsigned long ptr = pg_vec[i];
+ char *ptr = pg_vec[i];
struct tpacket_hdr *header;
int k;
_
next reply other threads:[~2004-08-27 22:22 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2004-08-27 22:22 Dave Hansen [this message]
2004-08-27 23:51 ` [patch] af_packet: use void* for virtual addresses David S. Miller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=E1C0p77-0004Ik-00@kernel.beaverton.ibm.com \
--to=haveblue@us.ibm.com \
--cc=davem@redhat.com \
--cc=netdev@oss.sgi.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.