[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCHv3 2/9] cutils: add a function to find non-zero c
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCHv3 2/9] cutils: add a function to find non-zero content in a buffer |
Date: |
Thu, 21 Mar 2013 12:12:32 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130311 Thunderbird/17.0.4 |
On 03/21/2013 09:57 AM, Peter Lieven wrote:
> this adds buffer_find_nonzero_offset() which is a SSE2/Altives
s/Altives/Altivec/
> optimized function that searches for non-zero content in a
> buffer.
>
> due to the optimizations used in the function there are restrictions
> on buffer address and search length. the function
> can_use_buffer_find_nonzero_content() can be used to check if
> the function can be used safely.
>
> Signed-off-by: Peter Lieven <address@hidden>
> ---
> include/qemu-common.h | 3 +++
> util/cutils.c | 50
> +++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 53 insertions(+)
> +inline bool can_use_buffer_find_nonzero_offset(const void *buf, size_t len);
> +inline size_t buffer_find_nonzero_offset(const void *buf, size_t len);
Ouch. It is okay to add a 'static inline' function, but then the
implementation must live in this header. Otherwise, the function must
not be inline, or you risk linker errors.
> +++ b/util/cutils.c
> @@ -143,6 +143,56 @@ int qemu_fdatasync(int fd)
> }
>
> /*
> + * Searches for an area with non-zero content in a buffer
> + *
> + * Attention! The len must be a multiple of 8 * sizeof(VECTYPE)
Should we call out BUFFER_FIND_NONZERO_OFFSET_UNROLL_FACTOR instead of a
magic number here? But I'm okay with leaving it as-is.
> + * and addr must be a multiple of sizeof(VECTYPE) due to
Trailing whitespace (here, and on several other lines). Please run your
series through scripts/checkpatch.pl before submitting v4.
> + * restriction of optimizations in this function.
> + *
> + * can_use_buffer_find_nonzero_offset() can be used to check
> + * these requirements.
> + *
> + * The return value is the offset of the non-zero area rounded
> + * down to 8 * sizeof(VECTYPE). If the buffer is all zero
Same comment on this use of '8'.
> + * the return value is equal to len.
> + */
> +
> +inline size_t buffer_find_nonzero_offset(const void *buf, size_t len)
s/inline// (or move it to a 'static inline' definition in the .h)
> +{
> + VECTYPE *p = (VECTYPE *)buf;
> + VECTYPE zero = ZERO_SPLAT;
> + size_t i;
> +
You copied the 'Attention! ...' message from buffer_is_zero, which
currently asserts that its condition is held. Therefore, consistency
would argue that you should assert your preconditions here, even if it
adds more to the code size. But this is something where a maintainer
might have a better opinion on whether to keep the code robust with an
assert(), or whether the faster operation without sanity checking is
more appropriate (in which case a followup to remove the assert from
buffer_is_zero would make sense).
> * Checks if a buffer is all zeroes
> *
> * Attention! The len must be a multiple of 4 * sizeof(long) due to
>
Cleaning up whitespace is trivial; but the incorrect use of 'inline'
requires a v4.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCHv3 0/9] buffer_is_zero / migration optimizations, Peter Lieven, 2013/03/21
- [Qemu-devel] [PATCHv3 1/9] move vector definitions to qemu-common.h, Peter Lieven, 2013/03/21
- [Qemu-devel] [PATCHv3 4/9] bitops: use vector algorithm to optimize find_next_bit(), Peter Lieven, 2013/03/21
- [Qemu-devel] [PATCHv3 8/9] migration: do not search dirty pages in bulk stage, Peter Lieven, 2013/03/21
- [Qemu-devel] [PATCHv3 7/9] migration: do not sent zero pages in bulk stage, Peter Lieven, 2013/03/21
- [Qemu-devel] [PATCHv3 2/9] cutils: add a function to find non-zero content in a buffer, Peter Lieven, 2013/03/21
- Re: [Qemu-devel] [PATCHv3 2/9] cutils: add a function to find non-zero content in a buffer,
Eric Blake <=
- [Qemu-devel] [PATCHv3 9/9] migration: use XBZRLE only after bulk stage, Peter Lieven, 2013/03/21
- [Qemu-devel] [PATCHv3 5/9] migration: search for zero instead of dup pages, Peter Lieven, 2013/03/21
- [Qemu-devel] [PATCHv3 6/9] migration: add an indicator for bulk state of ram migration, Peter Lieven, 2013/03/21
- [Qemu-devel] [PATCHv3 3/9] buffer_is_zero: use vector optimizations if possible, Peter Lieven, 2013/03/21