On Mon 28 Jan 00:13 PST 2019, Ankit Jain wrote:
> This change adds mmap functionality to rmtfs_mem driver.
> Userspace application can map the address and use this
> mapped address directly as buffer for read/write call to disk.
> and avoid the read/write call to the shared path to copy the
> buffer to userspace application.
>
Cool, I like this.
> Change-Id: Id64936a302b1a08dbce62774c90f9eff7420a9ad
Please drop the Gerrit tag.
> Signed-off-by: Ankit Jain <jankit@xxxxxxxxxxxxxx>
> ---
> drivers/soc/qcom/rmtfs_mem.c | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> diff --git a/drivers/soc/qcom/rmtfs_mem.c b/drivers/soc/qcom/rmtfs_mem.c
> index 7200d76..cd634f6 100644
> --- a/drivers/soc/qcom/rmtfs_mem.c
> +++ b/drivers/soc/qcom/rmtfs_mem.c
> @@ -137,6 +137,30 @@ static int qcom_rmtfs_mem_release(struct inode *inode, struct file *filp)
> .name = "rmtfs",
> };
>
> +static int qcom_rmtfs_mem_mmap(struct file *filep, struct vm_area_struct *vma)
> +{
> + int result;
> + struct qcom_rmtfs_mem *rmtfs_mem = filep->private_data;
> +
> + if (vma->vm_end - vma->vm_start > rmtfs_mem->size) {
> + pr_err("vm_end[%lu] - vm_start[%lu] [%lu] > mem->size[%pa]\n",
> + vma->vm_end, vma->vm_start,
> + (vma->vm_end - vma->vm_start), &rmtfs_mem->size);
I don't think this is helpful to the general reader of the kernel log.
How about making it a debug print? So that people interested in
debugging their rmtfs related issues can dynamically enable it?
Also, rmtfs_mem has a dev with a suitable name, so use:
dev_dbg(&rmtfs_mem->dev, ...
> + return -EINVAL;
> + }
> +
> + vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);
> + result = remap_pfn_range(vma,
> + vma->vm_start,
> + rmtfs_mem->addr >> PAGE_SHIFT,
> + vma->vm_end - vma->vm_start,
> + vma->vm_page_prot);
> + if (result != 0)
> + pr_err("mmap Failed with errno %d\n", result);
This isn't very helpful either, the reader of the kernel log will be
able to know that some mmap failed, but it gives no clue on where to
start searching.
As result is returned to the caller, I suggest that you add the error
print there instead - so that you get the error message from the
component you're trying to debug.
> +
> + return result;
And by that you can just:
return remap_pfn_range(...
> +}
> +
Regards,
Bjorn
