From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dave Hansen Subject: [patch] af_packet: use void* for virtual addresses Date: Fri, 27 Aug 2004 15:22:23 -0700 Sender: netdev-bounce@oss.sgi.com Message-ID: Cc: netdev@oss.sgi.com, Dave Hansen Return-path: To: davem@redhat.com Errors-to: netdev-bounce@oss.sgi.com List-Id: netdev.vger.kernel.org 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 --- 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 #include #include +#include #include #include #include @@ -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; itp_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; itp_block_nr; i++) { - unsigned long ptr = pg_vec[i]; + char *ptr = pg_vec[i]; struct tpacket_hdr *header; int k; _