+ */
> +ssize_t cxl_afu_read_err_buffer(struct cxl_afu *afu, char *buf,
> + loff_t off, size_t count)
> +{
> + loff_t aligned_off;
> + size_t aligned_count;
> + const void __iomem *ebuf = afu->afu_desc_mmio + afu->eb_offset;
> +
> + if (!afu->eb_len || count == 0 || off < 0)
if eb_len == 0 we don't even create the sysfs file. So is this check
needed?
> + return 0;
> +
> + /* calculate aligned read window */
> + count = min((size_t)(afu->eb_len - off), count);
What if count ends up being negative because off > afu->eb_len??
> + aligned_off = round_down(off, 8);
> + aligned_count = round_up(off + count, 8) - aligned_off;
I kinda preferred the start end variables, and length was just end -
start.
I though it was more readable. IMHO
How about:
aligned_start = round_down(off, 8);
aligned_end = round_up(off + count, 8);
aligned_length = aligned_end - aligned_start;
> +
> + /* fast path */
> + if ((aligned_off == off) && (aligned_count == count)) {
> + /* no need to use the bounce buffer */
> + _memcpy_fromio(buf, ebuf + off, count);
I would drop this, as the other code path should work fine.
Premature optimisation.....
> +
> + } else {
> + /* use bounce buffer for copy */
> + void *tbuf = (void *)__get_free_page(GFP_TEMPORARY);
> +
> + if (!tbuf)
> + return -ENOMEM;
> +
> + /* max we can copy in one read is PAGE_SIZE */
> + aligned_count = min(aligned_count, PAGE_SIZE);
> + _memcpy_fromio(tbuf, ebuf + aligned_off, aligned_count);
> +
> + count = min(count, aligned_count);
This doesn't seem right. count will equal PAGE_SIZE if it's too big but
it has to be smaller by (off & 7) in this case.
How about this?
#define MAX_COPY_SIZE PAGE_SIZE
...
void *bbuf;
/* Bounds check count with err buf length */
count = min((size_t)(afu->eb_len - off), count);
if ((off < 0) || (count < 0))
return 0;
/* Create aligned bounce buffer to copy into */
aligned_start = round_down(off, 8);
aligned_end = round_up(off + count, 8);
aligned_length = aligned_end - aligned_start;
if (aligned_length > MAX_COPY_SIZE) {
aligned_length = MAX_COPY_SIZE;
count = MAX_COPY_SIZE - (off & 0x7);
}
bbuf = (void *)__get_free_page(GFP_TEMPORARY);
if (!bbuf)
return -ENOMEM;
/* Use _memcpy_fromio() so the reads are aligned */
_memcpy_fromio(bbuf, ebuf + aligned_start, aligned_length);
memcpy(buf, bbuf + (off & 0x7), count);
free_page(bbuf);
> + memcpy(buf, tbuf + (off & 0x7), count);
> +
> + free_page((unsigned long)tbuf);
> + }
> +
> + return count;
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
