[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20131106124811.GA20404@gondor.apana.org.au>
Date: Wed, 6 Nov 2013 20:48:11 +0800
From: Herbert Xu <herbert@...dor.apana.org.au>
To: Mathias Krause <mathias.krause@...unet.com>
Cc: "David S. Miller" <davem@...emloft.net>,
Steffen Klassert <steffen.klassert@...unet.com>,
Dmitry Tarnyagin <dmitry.tarnyagin@...kless.no>,
netdev@...r.kernel.org
Subject: Re: [PATCH net-next 3/3] net: allow to leave the buffer fragmented
in skb_cow_data()
On Wed, Nov 06, 2013 at 01:42:03PM +0100, Mathias Krause wrote:
>
> Well, skb_cow_data() will only copy, i.e. call __pskb_pull_tail(), in
> case the skb is either cloned or fragmented. As you already said it
> won't be cloned in your case. Does it contain fragments, i.e. is
> skb_shinfo(skb)->nr_frags != 0? If not, we won't copy with the current
> code either.
Whenever we say page it means nr_frags != 0. So currently as
long as we have pages in our skb we will copy. With your patch
we will no longer copy in the case where we have pages but the
skb isn't cloned. In fact that is the whole point of your patch.
> Can you please explain why this would be needed? I still don't get the
> reasoning behind "pages are considered not writable at the moment even
> if they are anonymous".
As I said you don't know where the page in the skb came from. It
may point to read-only memory or memory that's shared with another
task that isn't expecting things to change underneath it.
It may well turn out to most if not all cases of pages are safe to
be written to if skb_cloned == 0. However, we'd need to do a full
audit of every source of page frags to be sure. For example, you'd
need to look at net drivers and splice.
Cheers,
--
Email: Herbert Xu <herbert@...dor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
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
Powered by blists - more mailing lists