[PATCH Utopic SRU] powerpc/perf/hv-24x7: Simplify catalog_read()

[Chris J Arges]

On 10/13/2014 04:14 AM, tim.gardner at canonical.com wrote:
> From: "sukadev at linux.vnet.ibm.com" <sukadev at linux.vnet.ibm.com>
> 
> BugLink: http://bugs.launchpad.net/bugs/1380432
> 
> catalog_read() implements the read interface for the sysfs file
> 
> 	/sys/bus/event_source/devices/hv_24x7/interface/catalog
> 
> It essentially takes a buffer, an offset and count as parameters
> to the read() call.  It makes a hypervisor call to read a specific
> page from the catalog and copy the required bytes into the given
> buffer. Each call to catalog_read() returns at most one 4K page.
> 
> Given these requirements, we should be able to simplify the
> catalog_read().
> 
> Signed-off-by: Sukadev Bhattiprolu <sukadev at linux.vnet.ibm.com>
> Signed-off-by: Michael Ellerman <mpe at ellerman.id.au>
> (cherry picked from commit 56f12bee55d740dc47eed0ca9d5c72cffdffd6cf)
> Signed-off-by: Tim Gardner <tim.gardner at canonical.com>

I've tried verifying a build with this patch on top of our current 3.16
Utopic tree and the above sysfs directory was still not found. Are there
additional steps required to enable the hv_24x7 device? I have been
testing on a POWER8 machine.

Thanks,
--chris j arges

> ---
>  arch/powerpc/perf/hv-24x7.c | 101 ++++++--------------------------------------
>  1 file changed, 14 insertions(+), 87 deletions(-)
> 
> diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
> index e0766b8..9f284db 100644
> --- a/arch/powerpc/perf/hv-24x7.c
> +++ b/arch/powerpc/perf/hv-24x7.c
> @@ -75,86 +75,6 @@ static struct attribute_group format_group = {
>  
>  static struct kmem_cache *hv_page_cache;
>  
> -/*
> - * read_offset_data - copy data from one buffer to another while treating the
> - *                    source buffer as a small view on the total avaliable
> - *                    source data.
> - *
> - * @dest: buffer to copy into
> - * @dest_len: length of @dest in bytes
> - * @requested_offset: the offset within the source data we want. Must be > 0
> - * @src: buffer to copy data from
> - * @src_len: length of @src in bytes
> - * @source_offset: the offset in the sorce data that (src,src_len) refers to.
> - *                 Must be > 0
> - *
> - * returns the number of bytes copied.
> - *
> - * The following ascii art shows the various buffer possitioning we need to
> - * handle, assigns some arbitrary varibles to points on the buffer, and then
> - * shows how we fiddle with those values to get things we care about (copy
> - * start in src and copy len)
> - *
> - * s = @src buffer
> - * d = @dest buffer
> - * '.' areas in d are written to.
> - *
> - *                       u
> - *   x         w	 v  z
> - * d           |.........|
> - * s |----------------------|
> - *
> - *                      u
> - *   x         w	z     v
> - * d           |........------|
> - * s |------------------|
> - *
> - *   x         w        u,z,v
> - * d           |........|
> - * s |------------------|
> - *
> - *   x,w                u,v,z
> - * d |..................|
> - * s |------------------|
> - *
> - *   x        u
> - *   w        v		z
> - * d |........|
> - * s |------------------|
> - *
> - *   x      z   w      v
> - * d            |------|
> - * s |------|
> - *
> - * x = source_offset
> - * w = requested_offset
> - * z = source_offset + src_len
> - * v = requested_offset + dest_len
> - *
> - * w_offset_in_s = w - x = requested_offset - source_offset
> - * z_offset_in_s = z - x = src_len
> - * v_offset_in_s = v - x = request_offset + dest_len - src_len
> - */
> -static ssize_t read_offset_data(void *dest, size_t dest_len,
> -				loff_t requested_offset, void *src,
> -				size_t src_len, loff_t source_offset)
> -{
> -	size_t w_offset_in_s = requested_offset - source_offset;
> -	size_t z_offset_in_s = src_len;
> -	size_t v_offset_in_s = requested_offset + dest_len - src_len;
> -	size_t u_offset_in_s = min(z_offset_in_s, v_offset_in_s);
> -	size_t copy_len = u_offset_in_s - w_offset_in_s;
> -
> -	if (requested_offset < 0 || source_offset < 0)
> -		return -EINVAL;
> -
> -	if (z_offset_in_s <= w_offset_in_s)
> -		return 0;
> -
> -	memcpy(dest, src + w_offset_in_s, copy_len);
> -	return copy_len;
> -}
> -
>  static unsigned long h_get_24x7_catalog_page_(unsigned long phys_4096,
>  					      unsigned long version,
>  					      unsigned long index)
> @@ -183,8 +103,10 @@ static ssize_t catalog_read(struct file *filp, struct kobject *kobj,
>  {
>  	unsigned long hret;
>  	ssize_t ret = 0;
> -	size_t catalog_len = 0, catalog_page_len = 0, page_count = 0;
> +	size_t catalog_len = 0, catalog_page_len = 0;
>  	loff_t page_offset = 0;
> +	loff_t offset_in_page;
> +	size_t copy_len;
>  	uint64_t catalog_version_num = 0;
>  	void *page = kmem_cache_alloc(hv_page_cache, GFP_USER);
>  	struct hv_24x7_catalog_page_0 *page_0 = page;
> @@ -202,7 +124,7 @@ static ssize_t catalog_read(struct file *filp, struct kobject *kobj,
>  	catalog_len = catalog_page_len * 4096;
>  
>  	page_offset = offset / 4096;
> -	page_count  = count  / 4096;
> +	offset_in_page = offset % 4096;
>  
>  	if (page_offset >= catalog_page_len)
>  		goto e_free;
> @@ -216,8 +138,13 @@ static ssize_t catalog_read(struct file *filp, struct kobject *kobj,
>  		}
>  	}
>  
> -	ret = read_offset_data(buf, count, offset,
> -				page, 4096, page_offset * 4096);
> +	copy_len = 4096 - offset_in_page;
> +	if (copy_len > count)
> +		copy_len = count;
> +
> +	memcpy(buf, page+offset_in_page, copy_len);
> +	ret = copy_len;
> +
>  e_free:
>  	if (hret)
>  		pr_err("h_get_24x7_catalog_page(ver=%lld, page=%lld) failed:"
> @@ -225,9 +152,9 @@ e_free:
>  		       catalog_version_num, page_offset, hret);
>  	kfree(page);
>  
> -	pr_devel("catalog_read: offset=%lld(%lld) count=%zu(%zu) catalog_len=%zu(%zu) => %zd\n",
> -			offset, page_offset, count, page_count, catalog_len,
> -			catalog_page_len, ret);
> +	pr_devel("catalog_read: offset=%lld(%lld) count=%zu "
> +			"catalog_len=%zu(%zu) => %zd\n", offset, page_offset,
> +			count, catalog_len, catalog_page_len, ret);
>  
>  	return ret;
>  }
>