Re: [v2,2/4] Simplify catalog_read()

[Michael Ellerman]

On Wed, 2014-24-09 at 19:24:39 UTC, sukadev@xxxxxxxxxxxxxxxxxx wrote:
> 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().
> 
> diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
> index 2f2215c..9427ef7 100644
> --- a/arch/powerpc/perf/hv-24x7.c
> +++ b/arch/powerpc/perf/hv-24x7.c
> @@ -185,6 +105,8 @@ static ssize_t catalog_read(struct file *filp, struct kobject *kobj,
>  	ssize_t ret = 0;
>  	size_t catalog_len = 0, catalog_page_len = 0, page_count = 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;
> @@ -203,6 +125,7 @@ static ssize_t catalog_read(struct file *filp, struct kobject *kobj,
>  
>  	page_offset = offset / 4096;
>  	page_count  = count  / 4096;

I don't see where page_count is used.

> +	offset_in_page = count % 4096;

Shouldn't offset_in_page be based on offset ?

cheers
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/