network splice receive

From 592c46ea813c31c0d6b28bf543ce2f5dd884a75e Mon Sep 17 00:00:00 2001

From 10d906a9a5a16a022d5067bee3963a0e3a03ae0c Mon Sep 17 00:00:00 2001

Seems to work reasonably well for me, sometimes I do see small ranges inside the output file that are not correct, but I haven't been able to reproduce today. I think it has to do with page reuse, hence the NET_COPY_SPLICE ifdef that you can enable to just plain copy the data instead of referencing it.

Quoted 5 lines · Jens Axboe

Seems to work reasonably well for me, sometimes I do see small ranges inside the output file that are not correct, but I haven't been able to reproduce today. I think it has to do with page reuse, hence the NET_COPY_SPLICE ifdef that you can enable to just plain copy the data instead of referencing it.

I managed to reproduce. It's segments of 68-80 bytes beyond corrupt in the middle of the out, and there might be 1-3 of such occurences in the 30mb file I tested with. The first 16 bytes of the corruption are always the same:

0000 1800 4ff3 937f e000 6381 7275 0008

Perhaps that hex pattern rings a bell with someone intimate with the networking. The remaining wrong bytes don't seem to have anything in common.

Quoted 6 lines

0000 1800 4ff3 937f e000 6381 7275 0008

Perhaps that hex pattern rings a bell with someone intimate with the networking. The remaining wrong bytes don't seem to have anything in common.

Ok, the source mac address is 00:18:F3:4F:7F:93 and the destination is 00:E0:81:63:75:72 which are the middle 12 bytes of the 16.

Hope that helps someone clue me in as to which network part is reusing the data. Do I need to 'pin' the sk_buff until the pipe data has been consumed?

Quoted 7 lines · Jens Axboe

Quoted 6 lines

0000 1800 4ff3 937f e000 6381 7275 0008

Perhaps that hex pattern rings a bell with someone intimate with the networking. The remaining wrong bytes don't seem to have anything in common.

Quoted 3 lines

Ok, the source mac address is 00:18:F3:4F:7F:93 and the destination is 00:E0:81:63:75:72 which are the middle 12 bytes of the 16.

It appears you may have endianness issues and perhaps a 16 bit mis-offset. the 0008 at the end looks like a swapped 0x800 which is the ethernet type for IPV4.

Quoted 3 lines

Hope that helps someone clue me in as to which network part is reusing the data. Do I need to 'pin' the sk_buff until the pipe data has been consumed?

I would worry about the driver level first - likely thats where your corruption is.

Hi,

Here's an implementation of tcp network splice receive support. It's originally based on the patch set that Intel posted some time ago, but has been (close to) 100% reworked.

Now, I'm not a networking guru by any stretch of the imagination, so I'd like some input on the direction of the main patch. Is the approach feasible? Glaring errors? Missing bits?

Here's an implementation of tcp network splice receive support. It's originally based on the patch set that Intel posted some time ago, but has been (close to) 100% reworked.

Now, I'm not a networking guru by any stretch of the imagination, so I'd like some input on the direction of the main patch. Is the approach feasible? Glaring errors? Missing bits?

net/core/skbuff.c +8 -0

+static inline int spd_fill_page(struct splice_pipe_desc *spd, struct page *page,
+				unsigned int len, unsigned int offset)
+{
+	struct page *p;
+
+	if (unlikely(spd->nr_pages == PIPE_BUFFERS))
+		return 1;
+

  [  263.936418] RIP  [<ffffffff8038c60c>] skb_splice_bits+0xac/0x1c9
  [  263.942516]  RSP <ffff81003c79fc88>

This a vm_bug_on in get_page().

Quoted 12 lines

net/core/skbuff.c +12 -0

+static inline int spd_fill_page(struct splice_pipe_desc *spd, struct page *page,
+				unsigned int len, unsigned int offset)
+{
+	struct page *p;
+
+	if (unlikely(spd->nr_pages == PIPE_BUFFERS))
+		return 1;
+
+#ifdef NET_COPY_SPLICE
+	p = alloc_pages(GFP_KERNEL, 0);
+	if (!p)
+		return 1;

net/core/skbuff.c fault: · line 1167

Quoted 6 lines

net/core/skbuff.c +6 -0

+
+	memcpy(page_address(p) + offset, page_address(page) + offset, len);
+#else
+	p = page;
+	get_page(p);
+#endif

Some pages have zero reference counter here.

Quoted 7 lines · Jens Axboe

Here's an implementation of tcp network splice receive support. It's originally based on the patch set that Intel posted some time ago, but has been (close to) 100% reworked.

Now, I'm not a networking guru by any stretch of the imagination, so I'd like some input on the direction of the main patch. Is the approach feasible? Glaring errors? Missing bits?

263.709262] ------------[ cut here ]------------

  [  263.713932] kernel BUG at include/linux/mm.h:285!
  [  263.718678] invalid opcode: 0000 [1] PREEMPT SMP 
  [  263.723561] CPU 0 
  [  263.725665] Modules linked in: button loop snd_intel8x0
  snd_ac97_codec ac97_bus snd_pcm snd_timer snd soundcore psmouse
  snd_page_alloc k8temp i2c_nforcen
  [  263.755666] Pid: 2709, comm: splice-fromnet Not tainted
  2.6.22-rc4-splice #2
  [  263.762759] RIP: 0010:[<ffffffff8038c60c>]  [<ffffffff8038c60c>]
  skb_splice_bits+0xac/0x1c9
  [  263.771212] RSP: 0018:ffff81003c79fc88  EFLAGS: 00010246
  [  263.776564] RAX: 0000000000000000 RBX: 00000000000005a8 RCX:
  ffff81003ff04778
  [  263.783743] RDX: ffff81003ff04778 RSI: 0000000000000ab2 RDI:
  000000000003d52d
  [  263.790925] RBP: ffff81003c79fdd8 R08: 0000000000000000 R09:
  ffff81003d927b78
  [  263.798104] R10: ffffffff803b0181 R11: ffff81003c79fde8 R12:
  ffff81003d52d000
  [  263.805284] R13: 000000000000054e R14: ffff81003d927b78 R15:
  ffff81003bbc6ea0
  [  263.812463] FS:  00002ac4089a86d0(0000) GS:ffffffff804fb000(0000)
  knlGS:0000000000000000
  [  263.820611] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
  [  263.826396] CR2: 00002ac4088320e0 CR3: 000000003c987000 CR4:
  00000000000006e0
  [  263.833578] Process splice-fromnet (pid: 2709, threadinfo
  ffff81003c79e000, task ffff81003755c380)
  [  263.842591] Stack:  ffff81003ff04720 0000000000000000
  ffff81003755c380 0000000000000046
  [  263.850897]  00000000000000c6 0000000000000046 ffff81003b0428b8
  ffff81003d0b5b10
  [  263.858543]  00000000000000c6 ffff81003d0b5b10 ffff81003b0428b8
  ffff81003d0b5b10
  [  263.865957] Call Trace:
  [  263.868683]  [<ffffffff803dc630>] _read_unlock_irq+0x31/0x4e
  [  263.874393]  [<ffffffff803afb54>] tcp_splice_data_recv+0x20/0x22
  [  263.880447]  [<ffffffff803afa2b>] tcp_read_sock+0xa2/0x1ab
  [  263.885983]  [<ffffffff803afb34>] tcp_splice_data_recv+0x0/0x22
  [  263.891951]  [<ffffffff803b01c1>] tcp_splice_read+0xae/0x1a3
  [  263.897655]  [<ffffffff8038920f>] sock_def_readable+0x0/0x6f
  [  263.903366]  [<ffffffff80384a65>] sock_splice_read+0x15/0x17
  [  263.909072]  [<ffffffff8029e773>] do_splice_to+0x76/0x88
  [  263.914432]  [<ffffffff8029fcc8>] sys_splice+0x1a8/0x232
  [  263.919795]  [<ffffffff802097ce>] system_call+0x7e/0x83
  [  263.925067] 
  [  263.926606] 
  [  263.926607] Code: 0f 0b eb fe 44 89 e6 81 e6 ff 0f 00 00 90 ff 42
  08 48 63 55 
  [  263.936418] RIP  [<ffffffff8038c60c>] skb_splice_bits+0xac/0x1c9
  [  263.942516]  RSP <ffff81003c79fc88>

This a vm_bug_on in get_page().

Quoted 8 lines

net/core/skbuff.c +8 -0

+static inline int spd_fill_page(struct splice_pipe_desc *spd, struct page *page,
+				unsigned int len, unsigned int offset)
+{
+	struct page *p;
+
+	if (unlikely(spd->nr_pages == PIPE_BUFFERS))
+		return 1;
+

net/core/skbuff.c fault: · line 1167

Quoted 10 lines

net/core/skbuff.c +10 -0

+#ifdef NET_COPY_SPLICE
+	p = alloc_pages(GFP_KERNEL, 0);
+	if (!p)
+		return 1;
+
+	memcpy(page_address(p) + offset, page_address(page) + offset, len);
+#else
+	p = page;
+	get_page(p);
+#endif

Some pages have zero reference counter here.

Is commented NET_COPY_SPLICE part from old implementation? It will be always slower than existing approach due to allocation overhead.

Quoted 1 line

Some pages have zero reference counter here.

But it's somewhat annoying to get pages with zero reference counts there, I wonder how that happens. I guess if the skb->data originated from kmalloc() then we don't really know. The main intent there was just to ensure the page wasn't going away, but clearly it's not good enough to ensure that reuse isn't taking place.

Signature · 1 line

Jens Axboe

Quoted 7 lines · Jens Axboe

Quoted 1 line

Some pages have zero reference counter here.

But it's somewhat annoying to get pages with zero reference counts there, I wonder how that happens. I guess if the skb->data originated from kmalloc() then we don't really know. The main intent there was just to ensure the page wasn't going away, but clearly it's not good enough to ensure that reuse isn't taking place.

What bout checking if page belongs to kmalloc cache (or any other cache via priviate pointers) and do not perform any kind of reference counting on them? I will play with this a bit later today.

Quoted 3 lines

What bout checking if page belongs to kmalloc cache (or any other cache via priviate pointers) and do not perform any kind of reference counting on them? I will play with this a bit later today.

That might work, but sounds a little dirty... But there's probably no way around. Be sure to look at the #splice-net branch if you are playing with this, I've updated it a number of times and fixed some bugs in there. Notably it now gets the offset right, and handles fragments and fraglist as well.

Signature · 1 line

Jens Axboe

Quoted 9 lines · Jens Axboe

Quoted 3 lines

What bout checking if page belongs to kmalloc cache (or any other cache via priviate pointers) and do not perform any kind of reference counting on them? I will play with this a bit later today.

That might work, but sounds a little dirty... But there's probably no way around. Be sure to look at the #splice-net branch if you are playing with this, I've updated it a number of times and fixed some bugs in there. Notably it now gets the offset right, and handles fragments and fraglist as well.

I've pulled splice-net, which indeed fixed some issues, but referencing slab pages is still is not allowed. There are at least two problems (although they are related):
1. if we do not increment reference counter for slab pages, they eventually get refilled and slab exploses after it understood that its pages are in use (or user dies when page is moved out of his control in slab).
2. get/put page does not work with slab pages, and simple increment/decrement of the reference counters is not allowed too.

Both problems have the same root - slab does not allow anyone to manipulate page's members. That should be broken/changed to allow splice to put its hands into network using the fastest way.
I will think about it.

Perhaps it's possible to solve this at a different level - can we hang on to the skb until the pipe buffer has been consumed, and prevent reuse that way? Then we don't have to care what backing the skb has, as long as it (and its data) isn't being reused until we drop the reference to it in sock_pipe_buf_release().

From: Jens Axboe <[email protected]>
Date: Fri, 8 Jun 2007 09:48:24 +0200

Quoted 5 lines

Perhaps it's possible to solve this at a different level - can we hang on to the skb until the pipe buffer has been consumed, and prevent reuse that way? Then we don't have to care what backing the skb has, as long as it (and its data) isn't being reused until we drop the reference to it in sock_pipe_buf_release().

Depending upon whether the pipe buffer consumption is bounded of not, this will jam up the TCP sender because the SKB data allocation is charged against the socket send buffer allocation.

Quoted 9 lines · David Miller

From: Jens Axboe <[email protected]>
Date: Fri, 8 Jun 2007 09:48:24 +0200

Quoted 5 lines

Perhaps it's possible to solve this at a different level - can we hang on to the skb until the pipe buffer has been consumed, and prevent reuse that way? Then we don't have to care what backing the skb has, as long as it (and its data) isn't being reused until we drop the reference to it in sock_pipe_buf_release().

Quoted 3 lines

Depending upon whether the pipe buffer consumption is bounded of not, this will jam up the TCP sender because the SKB data allocation is charged against the socket send buffer allocation.

Forgive my network ignorance, but is that a problem? Since you bring it up, I guess so :-)

We can grow the pipe, should we have to. So instead of blocking waiting on reader consumption, we can extend the size of the pipe and keep going.

Signature · 1 line

Jens Axboe

Quoted 14 lines · Jens Axboe

Quoted 12 lines · David Miller

From: Jens Axboe <[email protected]>
Date: Fri, 8 Jun 2007 09:48:24 +0200

Quoted 5 lines

Perhaps it's possible to solve this at a different level - can we hang on to the skb until the pipe buffer has been consumed, and prevent reuse that way? Then we don't have to care what backing the skb has, as long as it (and its data) isn't being reused until we drop the reference to it in sock_pipe_buf_release().

Depending upon whether the pipe buffer consumption is bounded of not, this will jam up the TCP sender because the SKB data allocation is charged against the socket send buffer allocation.

Quoted 2 lines

Forgive my network ignorance, but is that a problem? Since you bring it up, I guess so :-)

David means, that socket bufer allocation is limited, and delaying freeing can end up with exhausint that limit.

Quoted 3 lines

We can grow the pipe, should we have to. So instead of blocking waiting on reader consumption, we can extend the size of the pipe and keep going.

I have a code, which roughly works (but I will test it some more), which just introduces reference counters for slab pages, so that the would not be actually freed via page reclaim, but only after reference counters are dropped. That forced changes in mm/slab.c so likely it is unacceptible solution, but it is interesting as is.

OK, so a delayed empty of the pipe could end up causing packet drops elsewhere due to allocation exhaustion?

Quoted 9 lines

Quoted 3 lines

We can grow the pipe, should we have to. So instead of blocking waiting on reader consumption, we can extend the size of the pipe and keep going.

I have a code, which roughly works (but I will test it some more), which just introduces reference counters for slab pages, so that the would not be actually freed via page reclaim, but only after reference counters are dropped. That forced changes in mm/slab.c so likely it is unacceptible solution, but it is interesting as is.

Hmm, still seems like it's working around the problem. We essentially just need to ensure that the data doesn't get _reused_, not just freed. It doesn't help holding a reference to the page, if someone else just reuses it and fills it with other data before it has been consumed and released by the pipe buffer operations.

That's why I thought the skb referencing was the better idea, then we don't have to care about the backing of the skb either. Provided that preventing the free of the skb before the pipe buffer has been consumed guarantees that the contents aren't reused.

Signature · 1 line

Jens Axboe

Quoted 2 lines · Jens Axboe

OK, so a delayed empty of the pipe could end up causing packet drops elsewhere due to allocation exhaustion?

Yes.

Quoted 16 lines

Quoted 9 lines

Quoted 3 lines

We can grow the pipe, should we have to. So instead of blocking waiting on reader consumption, we can extend the size of the pipe and keep going.

I have a code, which roughly works (but I will test it some more), which just introduces reference counters for slab pages, so that the would not be actually freed via page reclaim, but only after reference counters are dropped. That forced changes in mm/slab.c so likely it is unacceptible solution, but it is interesting as is.

Hmm, still seems like it's working around the problem. We essentially just need to ensure that the data doesn't get _reused_, not just freed. It doesn't help holding a reference to the page, if someone else just reuses it and fills it with other data before it has been consumed and released by the pipe buffer operations.

Quoted 4 lines

That's why I thought the skb referencing was the better idea, then we don't have to care about the backing of the skb either. Provided that preventing the free of the skb before the pipe buffer has been consumed guarantees that the contents aren't reused.

It is not only better idea, it is the only correct one. Attached patch for interested reader, which does slab pages accounting, but it is broken. It does not fires up with kernel bug, but it fills output file with random garbage from reused and dirtied pages. And I do not know why, but received file is always smaller than file being sent (when file has resonable size like 10mb, with 4-40kb filesize things seems to be ok).

I've started skb referencing work, let's see where this will end up.

Here's a start, for the splice side at least of storing a buf-private entity with the ops.

Quoted 2 lines · Jens Axboe

Here's a start, for the splice side at least of storing a buf-private entity with the ops.

:) I tested the same implementation, but I put skb pointer into page->private. My approach is not correct, since the same page can hold several objects, so if there are several splicers, this will scream. I've tested your patch on top of splice-net branch, here is a result:

[   44.798853] Slab corruption: skbuff_head_cache start=ffff81003b726668, len=192
[   44.806148] Redzone: 0x9f911029d74e35b/0x9f911029d74e35b.
[   44.811598] Last user: [<ffffffff803699fd>](kfree_skbmem+0x7a/0x7e)
[   44.818012] 0b0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6a 6b 6b a5
[   44.824889] Prev obj: start=ffff81003b726590, len=192
[   44.829985] Redzone: 0xd84156c5635688c0/0xd84156c5635688c0.
[   44.835604] Last user: [<ffffffff8036a22c>](__alloc_skb+0x40/0x13f)
[   44.842010] 000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[   44.848896] 010: 20 58 7e 3b 00 81 ff ff 00 00 00 00 00 00 00 00
[   44.855772] Next obj: start=ffff81003b726740, len=192
[   44.860868] Redzone: 0x9f911029d74e35b/0x9f911029d74e35b.
[   44.866314] Last user: [<ffffffff803699fd>](kfree_skbmem+0x7a/0x7e)
[   44.872721] 000: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
[   44.879597] 010: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b

I will try some things for the nearest 30-60 minutes, and then will move to canoe trip until thuesday, so will not be able to work on this idea.

I will try some things for the nearest 30-60 minutes, and then will move to canoe trip until thuesday, so will not be able to work on this idea.

Quoted 2 lines · Evgeniy Polyakov

I will try some things for the nearest 30-60 minutes, and then will move to canoe trip until thuesday, so will not be able to work on this idea.

Ok, replacing in fs/splice.c every page_cache_release() with static void splice_page_release(struct page *p)

{
	if (!PageSlab(p))
		page_cache_release(p);
}

and putting cloned skb into private field instead of original on in spd_fill_page() ends up without kernel hung.

I'm not sure it is correct, that page can be released in fs/splice.c without calling any callback from network code, when network data is being processed.

Size of the received file is bigger than file sent, file contains repeated blocks of data sometimes. Cloned skb usage is likely too big overhead, although for receiving fast clone is unused in most cases, so there might be some gain.

Attached your patch with above changes.

Quoted 10 lines · Evgeniy Polyakov

Quoted 2 lines · Evgeniy Polyakov

I will try some things for the nearest 30-60 minutes, and then will move to canoe trip until thuesday, so will not be able to work on this idea.

Ok, replacing in fs/splice.c every page_cache_release() with static void splice_page_release(struct page *p)

{
	if (!PageSlab(p))
		page_cache_release(p);
}

Ehm, I don't see why that should be necessary. Except in splice_to_pipe(), I have considered that we need to pass in a release function if mapping fails at some point. But it's probably best to do that in the caller, since they have the knowledge of how to release the pages.

The rest of the PageSlab() tests are bogus.

Quoted 2 lines

and putting cloned skb into private field instead of original on in spd_fill_page() ends up without kernel hung.

Why? Seems pointless to allocate a clone just to hold on to the skb, a reference should be equally good. I would not be opposed to doing it this way, I just don't see what a clone buys us as compared to just holding that reference to the skb.

Quoted 3 lines

I'm not sure it is correct, that page can be released in fs/splice.c without calling any callback from network code, when network data is being processed.

Please explain!

Quoted 18 lines · Jens Axboe

Quoted 10 lines · Evgeniy Polyakov

Quoted 2 lines · Evgeniy Polyakov

I will try some things for the nearest 30-60 minutes, and then will move to canoe trip until thuesday, so will not be able to work on this idea.

Ok, replacing in fs/splice.c every page_cache_release() with static void splice_page_release(struct page *p)

{
	if (!PageSlab(p))
		page_cache_release(p);
}

Ehm, I don't see why that should be necessary. Except in splice_to_pipe(), I have considered that we need to pass in a release function if mapping fails at some point. But it's probably best to do that in the caller, since they have the knowledge of how to release the pages.

Quoted 1 line

The rest of the PageSlab() tests are bogus.

I had a crashdump, where page was released via splice_to_pipe() indeed, I did not investigate if it is possible to release provided page in other places. I think if in future there will other slab usage cases except networking receiving, that might be useful, but as is it is not needed.

Quoted 7 lines

Quoted 2 lines

and putting cloned skb into private field instead of original on in spd_fill_page() ends up without kernel hung.

Why? Seems pointless to allocate a clone just to hold on to the skb, a reference should be equally good. I would not be opposed to doing it this way, I just don't see what a clone buys us as compared to just holding that reference to the skb.

Receiving code does not expect shared skbs - too many fields are changed with assumptions that it is a private copy.

Quoted 5 lines

I had a crashdump, where page was released via splice_to_pipe() indeed, I did not investigate if it is possible to release provided page in other places. I think if in future there will other slab usage cases except networking receiving, that might be useful, but as is it is not needed.

Read the just posted code, it has moved way beyond this :-)

Quoted 10 lines

Quoted 7 lines

Quoted 2 lines

and putting cloned skb into private field instead of original on in spd_fill_page() ends up without kernel hung.

Why? Seems pointless to allocate a clone just to hold on to the skb, a reference should be equally good. I would not be opposed to doing it this way, I just don't see what a clone buys us as compared to just holding that reference to the skb.

Receiving code does not expect shared skbs - too many fields are changed with assumptions that it is a private copy.

Actually the main problem is that tcp_read_sock() unconditionally frees the skb, so it wouldn't help if we grabbed a reference to it. I've yet to receive an explanation of why it does so, seem awkward and violates the whole principle of reference counted objects. Davem??

So for now, skb_splice_bits() clones the incoming skb to avoid that. I'd hope we can get rid of that by fixing tcp_read_sock(), though.

Signature · 1 line

Jens Axboe

Quoted 11 lines

Quoted 10 lines

Quoted 7 lines

Quoted 2 lines

and putting cloned skb into private field instead of original on in spd_fill_page() ends up without kernel hung.

Why? Seems pointless to allocate a clone just to hold on to the skb, a reference should be equally good. I would not be opposed to doing it this way, I just don't see what a clone buys us as compared to just holding that reference to the skb.

Receiving code does not expect shared skbs - too many fields are changed with assumptions that it is a private copy.

Quoted 7 lines

Actually the main problem is that tcp_read_sock() unconditionally frees the skb, so it wouldn't help if we grabbed a reference to it. I've yet to receive an explanation of why it does so, seem awkward and violates the whole principle of reference counted objects. Davem??

So for now, skb_splice_bits() clones the incoming skb to avoid that. I'd hope we can get rid of that by fixing tcp_read_sock(), though.

It does that because it knows, that skb is not allowed to be shared there. Similar things are being done in udp for example - code changes internal mebers of skb, since it knows skb is not shared.

For example generic_make_request() is not allowed to change, say, bio->bi_sector or bi_destructor, since it does not own a block request, not matter what bi_cnt is. From another side, ->bi_destructor() can do whatever it wants with bio without any check for its reference counter.

Quoted 19 lines · Evgeniy Polyakov

Quoted 11 lines

Quoted 10 lines

Quoted 7 lines

Quoted 2 lines

and putting cloned skb into private field instead of original on in spd_fill_page() ends up without kernel hung.

Why? Seems pointless to allocate a clone just to hold on to the skb, a reference should be equally good. I would not be opposed to doing it this way, I just don't see what a clone buys us as compared to just holding that reference to the skb.

Receiving code does not expect shared skbs - too many fields are changed with assumptions that it is a private copy.

Quoted 7 lines

Actually the main problem is that tcp_read_sock() unconditionally frees the skb, so it wouldn't help if we grabbed a reference to it. I've yet to receive an explanation of why it does so, seem awkward and violates the whole principle of reference counted objects. Davem??

So for now, skb_splice_bits() clones the incoming skb to avoid that. I'd hope we can get rid of that by fixing tcp_read_sock(), though.

Quoted 8 lines

It does that because it knows, that skb is not allowed to be shared there. Similar things are being done in udp for example - code changes internal mebers of skb, since it knows skb is not shared.

For example generic_make_request() is not allowed to change, say, bio->bi_sector or bi_destructor, since it does not own a block request, not matter what bi_cnt is. From another side, ->bi_destructor() can do whatever it wants with bio without any check for its reference counter.

But generic_make_request() DOES change ->bi_sector, that's how partition remapping works :-). The destructor can of course do whatever it wants, by definition the bio is not referenced at that point (or it would not have been called). So while I think your analogy is quite poor, I do now follow the code (even if I think it's ugly). There's quite a big

difference between changing parts of the elements of a structure to just grabbing a reference to it. If the skb cannot be referenced, skb_get() should return NULL.

But that aside, I see the issue. I'll just stick to the clone, it works fine as-is (well almost, there's a leak there, but functionally it's ok!).

Signature · 1 line

Jens Axboe

Quoted 7 lines

difference between changing parts of the elements of a structure to just grabbing a reference to it. If the skb cannot be referenced, skb_get() should return NULL.

But that aside, I see the issue. I'll just stick to the clone, it works fine as-is (well almost, there's a leak there, but functionally it's ok!).

Btw, is it allowed to use splice from network with, say, nfs? Since RPC code uses sk_user_data as long as network splice.

Size of the received file is bigger than file sent, file contains repeated blocks of data sometimes. Cloned skb usage is likely too big overhead, although for receiving fast clone is unused in most cases, so there might be some gain.