copy: Replace rte_memcpy with regular memcpy #155
Conversation
Eliminate rte_memcpy dependency by replacing it with regular memcpy. This may impact performance, but the only use of rte_memcpy was in the malloc bdev which is for testing only. Change-Id: I3e8592cb08262272518ec3d29ea165b4e8f48a5c
|
Ranjit,
This change proposes to replace the rte_memcpy() API in SPDK with a call to memcpy() rather than implementing a new spdk_memcpy() DPDK abstraction.
I believe you expressed an opinion about this in one of my previous pull requests. Please review pull request #155 <#155> provide your comments.
Also, I’d like to get approval from more than just the Intel engineers for these changes.
Can we agree upon a criteria for approval with these changes? I think there are many other individuals and companies in the SPDK community who have a stake in these changes and I’d like to see some approvals from others in the community as well.
John Meneghini
john.a.meneghini@gmail.com
… On May 3, 2017, at 4:04 PM, Daniel Verkamp ***@***.***> wrote:
@dverkamp-intel approved this pull request.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub <#155 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AMVL3dr0xbJi1RtJxKbmhzESrWwpXHqqks5r2N3OgaJpZM4NPyJA>.
|
There was a problem hiding this comment.
I believe this will change further. ie. The standard assert.h, stdio.h and errno.h files would move to spdk/stdinc.h (change in pull described here:
#157 (comment)
)
So, anyone wanting to use their own memcpy implementation should be able to via a custom stdinc.h.
Approving this change.
|
OK. That’s what I wanted to know. Thanks for your review and approval Ranjit.
John Meneghini
john.a.meneghini@gmail.com
… On May 5, 2017, at 10:52 AM, Ranjit Noronha ***@***.***> wrote:
@ranjitnoronha approved this pull request.
I believe this will change further. ie. The standard assert.h, stdio.h and errno.h files would move to spdk/stdinc.h (change in pull described here:
#157 (comment) <#157 (comment)>
)
So, anyone wanting to use their own memcpy implementation should be able to via a custom stdinc.h.
Approving this change.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub <#155 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AMVL3cx37BBqB4r5jhXDvlexmRAH2dL5ks5r2ze_gaJpZM4NPyJA>.
|
| @@ -112,7 +110,8 @@ mem_copy_submit(void *cb_arg, struct spdk_io_channel *ch, void *dst, void *src, | |||
| { | |||
| struct spdk_copy_task *copy_req; | |||
|
|
|||
| rte_memcpy(dst, src, (size_t)nbytes); | |||
| memcpy(dst, src, (size_t)nbytes); | |||
There was a problem hiding this comment.
I was hoping to abstract rte_memcpy because there are other high performance implementations of memcpy that each platform may want to implement. rte_memcpy is the perfect example of a high performance memcpy routine.
There was a problem hiding this comment.
In SPDK itself we don't actually use mempcy in any situation where it is performance sensitive*. This was actually the only large copy of variable size remotely close to the I/O path, and this module is only used for testing. Instead, we use zero-copy strategies throughout.
Now, users of SPDK may need to copy data around and are free to use their own high performance memcpy implementations. Since those calls to mempcy are from user code, they don't need SPDK to create a wrapper - they can just call their version directly.
*There are a couple of places where we do a memcpy, but they're small copies of sizes known at compilation time, so the compiler replaces them with the appropriate instructions.
|
This was merged as commit 1ad673a |
Eliminate rte_memcpy dependency by replacing it with
regular memcpy. This may impact performance, but the only
use of rte_memcpy was in the malloc bdev which is for
testing only.
Change-Id: I3e8592cb08262272518ec3d29ea165b4e8f48a5c