netdev - Re: [PATCH 1/2] af_packet: use vmalloc_to_page() instead for the addresss returned by vmalloc()

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20101201204457.GA8393@hmsreliant.think-freely.org>
Date:	Wed, 1 Dec 2010 15:44:57 -0500
From:	Neil Horman <nhorman@...driver.com>
To:	Eric Dumazet <eric.dumazet@...il.com>
Cc:	Changli Gao <xiaosuo@...il.com>,
	"David S. Miller" <davem@...emloft.net>,
	Jiri Pirko <jpirko@...hat.com>, netdev@...r.kernel.org
Subject: Re: [PATCH 1/2] af_packet: use vmalloc_to_page() instead for the
 addresss returned by vmalloc()

On Wed, Dec 01, 2010 at 04:13:16PM +0100, Eric Dumazet wrote:
> Le mercredi 01 décembre 2010 à 22:16 +0800, Changli Gao a écrit :
> 
> > Even then, tpacket_fill_skb() is called for every skb, and
> > pgv_to_page() is used in it. We have to optimize pgv_to_page().
> 
> With the __pure trick I gave, pgv_to_page() is _not_ called for the
> typical use case of af_packet : packet sniffing.
> 
> Compiler is able to remove the call completely, since
> 
> static inline void flush_dcache_page(struct page *page) { }
> 
> The only remaining pgv_to_page() call is the one done in mmap packet
> send, since we have to do :
> 
> page = pgv_to_page(data);
> get_page(page);
> 
> I personally dont use this path, its known to be buggy...
> 
> Optimize if you want, but make all this
> ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE conditional.
> 
> Its not needed to maintain an array of 'struct page *' if its not needed
> at all.
> 
+1 to this approach, its much cleaner than maintaing an array of struct pages to
do fast mappings.  I just spent a few hours putting a test patch together to do
such a mapping, and it requires either doing a lengthly search of the ring
buffer, or passing around the struct pgv that the potentially vmalloced address
came from, which fans out quite alot and gets tricky in several places (the tx
paths skb desctructor for instance).  At any rate, letting the compiler
eliminate code using __pure here seems much more efficient
Neil

> # vi +2448 block/blk-core.c
> 
> #if ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE
> /**
>  * rq_flush_dcache_pages - Helper function to flush all pages in a request
>  * @rq: the request to be flushed
>  *
>  * Description:
>  *     Flush all pages in @rq.
>  */
> void rq_flush_dcache_pages(struct request *rq)
> {
>         struct req_iterator iter;
>         struct bio_vec *bvec;
> 
>         rq_for_each_segment(bvec, rq, iter)
>                 flush_dcache_page(bvec->bv_page);
> }
> EXPORT_SYMBOL_GPL(rq_flush_dcache_pages);
> #endif
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html