From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH RFC 1/6] skbuff: support per-page destructors in copy_ubufs Date: Fri, 11 May 2012 11:58:12 +0100 Message-ID: <1336733892.23818.69.camel@zakaz.uk.xensource.com> References: <8a3235fbceef37758ef23169c4c152e8d1251d3b.1336397823.git.mst@redhat.com> <1336671977.14220.26.camel@zakaz.uk.xensource.com> <20120510184246.GE14647@redhat.com> <1336726800.23818.33.camel@zakaz.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Cc: David Miller , "netdev@vger.kernel.org" , "eric.dumazet@gmail.com" To: "Michael S. Tsirkin" Return-path: Received: from smtp.ctxuk.citrix.com ([62.200.22.115]:48617 "EHLO SMTP.EU.CITRIX.COM" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755474Ab2EKK6V (ORCPT ); Fri, 11 May 2012 06:58:21 -0400 In-Reply-To: <1336726800.23818.33.camel@zakaz.uk.xensource.com> Sender: netdev-owner@vger.kernel.org List-ID: On Fri, 2012-05-11 at 10:00 +0100, Ian Campbell wrote: > I'm seeing copy_ubufs called in my remote NFS test, which I don't > think I expected -- I'll investigate why this is happening today. It's tcp_transmit_skb which can (conditionally) call skb_clone (backtrace below) I suspect this means that the existing SKBTX_DEV_ZEROCOPY semantics are a superset of what we need to consider for the destructor case. I'm assuming here that the existing SKBTX_DEV_ZEROCOPY is copying aside exactly the right amount and isn't conservatively coying more often than necessary. shinfo->tx_flags are pretty scarce -- can we afford a new one for this usecase? Or perhaps this is actually a function of the callsite not the of individual skb and we want to have some concept of "deep" and "shallow" clones combined with SKBTX_DEV_ZEROCOPY to decide when to copy_ubufs or not? e.g. deep clone => always copy if SKBTX_DEV_ZEROCOPY and shallow clone => only copy if SKBTX_DEV_ZEROCOPY && destructor_arg!=NULL (neither copy if !SKBTX_DEV_ZEROCOPY). Oh, I suppose that reintroduces the copy_ubufs under a (shallow) cloned skb race if one of those skbs eventually finds itself in a situation where a skb_frag_orphan is required doesn't it. Hrm :-/ Will have to have a think... Ian. [ 109.680828] ------------[ cut here ]------------ [ 109.685440] WARNING: at /local/scratch/ianc/devel/kernels/linux/include/linux/skbuff.h:1732 skb_clone+0xe6/0xf0() [ 109.695678] Hardware name: [ 109.699162] ORPHANING [ 109.701434] Modules linked in: [ 109.704495] Pid: 10, comm: kworker/0:1 Tainted: G W 3.4.0-rc4-x86_64-native+ #186 [ 109.712830] Call Trace: [ 109.715278] [] warn_slowpath_common+0x7a/0xb0 [ 109.721273] [] warn_slowpath_fmt+0x41/0x50 [ 109.727007] [] ? tcp_transmit_skb+0x9a/0x8f0 [ 109.732914] [] skb_clone+0xe6/0xf0 [ 109.737957] [] tcp_transmit_skb+0x9a/0x8f0 [ 109.743694] [] tcp_write_xmit+0x1ea/0x9c0 [ 109.749343] [] tcp_push_one+0x2b/0x40 [ 109.754648] [] tcp_sendpage+0x64b/0x6d0 [ 109.760126] [] inet_sendpage+0x4d/0xf0 [ 109.765518] [] xs_sendpages+0x117/0x2a0 [ 109.770996] [] ? xprt_reserve+0x2d0/0x2d0 [ 109.776647] [] xs_tcp_send_request+0x58/0x110 [ 109.782644] [] xprt_transmit+0x6b/0x2d0 [ 109.788123] [] ? call_transmit_status+0xd0/0xd0 [ 109.794293] [] call_transmit+0x1d0/0x290 [ 109.799857] [] ? call_transmit_status+0xd0/0xd0 [ 109.806029] [] __rpc_execute+0x65/0x260 [ 109.811505] [] ? __rpc_execute+0x260/0x260 [ 109.817241] [] rpc_async_schedule+0x10/0x20 [ 109.823066] [] process_one_work+0x11f/0x460 [ 109.828895] [] worker_thread+0x173/0x3f0 [ 109.834459] [] ? manage_workers+0x210/0x210 [ 109.840283] [] kthread+0x96/0xa0 [ 109.845179] [] kernel_thread_helper+0x4/0x10 [ 109.851092] [] ? kthread_freezable_should_stop+0x70/0x70 [ 109.858053] [] ? gs_change+0xb/0xb [ 109.863087] ---[ end trace 3e3acdb7cc57c191 ]---