Use new return type vm_fault_t for fault handler. For
now, this is just documenting that the function returns
a VM_FAULT value rather than an errno. Once all instances
are converted, vm_fault_t will become a distinct type.
Ref-> commit 1c8f422059ae ("mm: change return type to vm_fault_t")
There is an existing bug when vm_insert_pfn() can return
ENOMEM which was ignored and VM_FAULT_NOPAGE returned as
default. The new inline vmf_insert_pfn() has removed
this inefficiency by returning correct vm_fault_ type.
Signed-off-by: Souptick Joarder <[email protected]>
---
drivers/misc/ocxl/context.c | 22 +++++++++++-----------
drivers/misc/ocxl/sysfs.c | 5 ++---
2 files changed, 13 insertions(+), 14 deletions(-)
diff --git a/drivers/misc/ocxl/context.c b/drivers/misc/ocxl/context.c
index 909e880..98daf91 100644
--- a/drivers/misc/ocxl/context.c
+++ b/drivers/misc/ocxl/context.c
@@ -83,7 +83,7 @@ int ocxl_context_attach(struct ocxl_context *ctx, u64 amr)
return rc;
}
-static int map_afu_irq(struct vm_area_struct *vma, unsigned long address,
+static vm_fault_t map_afu_irq(struct vm_area_struct *vma, unsigned long address,
u64 offset, struct ocxl_context *ctx)
{
u64 trigger_addr;
@@ -92,15 +92,15 @@ static int map_afu_irq(struct vm_area_struct *vma, unsigned long address,
if (!trigger_addr)
return VM_FAULT_SIGBUS;
- vm_insert_pfn(vma, address, trigger_addr >> PAGE_SHIFT);
- return VM_FAULT_NOPAGE;
+ return vmf_insert_pfn(vma, address, trigger_addr >> PAGE_SHIFT);
}
-static int map_pp_mmio(struct vm_area_struct *vma, unsigned long address,
+static vm_fault_t map_pp_mmio(struct vm_area_struct *vma, unsigned long address,
u64 offset, struct ocxl_context *ctx)
{
u64 pp_mmio_addr;
int pasid_off;
+ vm_fault_t ret;
if (offset >= ctx->afu->config.pp_mmio_stride)
return VM_FAULT_SIGBUS;
@@ -118,27 +118,27 @@ static int map_pp_mmio(struct vm_area_struct *vma, unsigned long address,
pasid_off * ctx->afu->config.pp_mmio_stride +
offset;
- vm_insert_pfn(vma, address, pp_mmio_addr >> PAGE_SHIFT);
+ ret = vmf_insert_pfn(vma, address, pp_mmio_addr >> PAGE_SHIFT);
mutex_unlock(&ctx->status_mutex);
- return VM_FAULT_NOPAGE;
+ return ret;
}
-static int ocxl_mmap_fault(struct vm_fault *vmf)
+static vm_fault_t ocxl_mmap_fault(struct vm_fault *vmf)
{
struct vm_area_struct *vma = vmf->vma;
struct ocxl_context *ctx = vma->vm_file->private_data;
u64 offset;
- int rc;
+ vm_fault_t ret;
offset = vmf->pgoff << PAGE_SHIFT;
pr_debug("%s: pasid %d address 0x%lx offset 0x%llx\n", __func__,
ctx->pasid, vmf->address, offset);
if (offset < ctx->afu->irq_base_offset)
- rc = map_pp_mmio(vma, vmf->address, offset, ctx);
+ ret = map_pp_mmio(vma, vmf->address, offset, ctx);
else
- rc = map_afu_irq(vma, vmf->address, offset, ctx);
- return rc;
+ ret = map_afu_irq(vma, vmf->address, offset, ctx);
+ return ret;
}
static const struct vm_operations_struct ocxl_vmops = {
diff --git a/drivers/misc/ocxl/sysfs.c b/drivers/misc/ocxl/sysfs.c
index d9753a1..0ab1fd1 100644
--- a/drivers/misc/ocxl/sysfs.c
+++ b/drivers/misc/ocxl/sysfs.c
@@ -64,7 +64,7 @@ static ssize_t global_mmio_read(struct file *filp, struct kobject *kobj,
return count;
}
-static int global_mmio_fault(struct vm_fault *vmf)
+static vm_fault_t global_mmio_fault(struct vm_fault *vmf)
{
struct vm_area_struct *vma = vmf->vma;
struct ocxl_afu *afu = vma->vm_private_data;
@@ -75,8 +75,7 @@ static int global_mmio_fault(struct vm_fault *vmf)
offset = vmf->pgoff;
offset += (afu->global_mmio_start >> PAGE_SHIFT);
- vm_insert_pfn(vma, vmf->address, offset);
- return VM_FAULT_NOPAGE;
+ return vmf_insert_pfn(vma, vmf->address, offset);
}
static const struct vm_operations_struct global_mmio_vmops = {
--
1.9.1
On 12/06/18 06:29, Souptick Joarder wrote:
> Use new return type vm_fault_t for fault handler. For
> now, this is just documenting that the function returns
> a VM_FAULT value rather than an errno. Once all instances
> are converted, vm_fault_t will become a distinct type.
>
> Ref-> commit 1c8f422059ae ("mm: change return type to vm_fault_t")
>
> There is an existing bug when vm_insert_pfn() can return
> ENOMEM which was ignored and VM_FAULT_NOPAGE returned as
> default. The new inline vmf_insert_pfn() has removed
> this inefficiency by returning correct vm_fault_ type.
>
> Signed-off-by: Souptick Joarder <[email protected]>
Looks okay to me
Acked-by: Andrew Donnellan <[email protected]>
> ---
> drivers/misc/ocxl/context.c | 22 +++++++++++-----------
> drivers/misc/ocxl/sysfs.c | 5 ++---
> 2 files changed, 13 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/misc/ocxl/context.c b/drivers/misc/ocxl/context.c
> index 909e880..98daf91 100644
> --- a/drivers/misc/ocxl/context.c
> +++ b/drivers/misc/ocxl/context.c
> @@ -83,7 +83,7 @@ int ocxl_context_attach(struct ocxl_context *ctx, u64 amr)
> return rc;
> }
>
> -static int map_afu_irq(struct vm_area_struct *vma, unsigned long address,
> +static vm_fault_t map_afu_irq(struct vm_area_struct *vma, unsigned long address,
> u64 offset, struct ocxl_context *ctx)
> {
> u64 trigger_addr;
> @@ -92,15 +92,15 @@ static int map_afu_irq(struct vm_area_struct *vma, unsigned long address,
> if (!trigger_addr)
> return VM_FAULT_SIGBUS;
>
> - vm_insert_pfn(vma, address, trigger_addr >> PAGE_SHIFT);
> - return VM_FAULT_NOPAGE;
> + return vmf_insert_pfn(vma, address, trigger_addr >> PAGE_SHIFT);
> }
>
> -static int map_pp_mmio(struct vm_area_struct *vma, unsigned long address,
> +static vm_fault_t map_pp_mmio(struct vm_area_struct *vma, unsigned long address,
> u64 offset, struct ocxl_context *ctx)
> {
> u64 pp_mmio_addr;
> int pasid_off;
> + vm_fault_t ret;
>
> if (offset >= ctx->afu->config.pp_mmio_stride)
> return VM_FAULT_SIGBUS;
> @@ -118,27 +118,27 @@ static int map_pp_mmio(struct vm_area_struct *vma, unsigned long address,
> pasid_off * ctx->afu->config.pp_mmio_stride +
> offset;
>
> - vm_insert_pfn(vma, address, pp_mmio_addr >> PAGE_SHIFT);
> + ret = vmf_insert_pfn(vma, address, pp_mmio_addr >> PAGE_SHIFT);
> mutex_unlock(&ctx->status_mutex);
> - return VM_FAULT_NOPAGE;
> + return ret;
> }
>
> -static int ocxl_mmap_fault(struct vm_fault *vmf)
> +static vm_fault_t ocxl_mmap_fault(struct vm_fault *vmf)
> {
> struct vm_area_struct *vma = vmf->vma;
> struct ocxl_context *ctx = vma->vm_file->private_data;
> u64 offset;
> - int rc;
> + vm_fault_t ret;
>
> offset = vmf->pgoff << PAGE_SHIFT;
> pr_debug("%s: pasid %d address 0x%lx offset 0x%llx\n", __func__,
> ctx->pasid, vmf->address, offset);
>
> if (offset < ctx->afu->irq_base_offset)
> - rc = map_pp_mmio(vma, vmf->address, offset, ctx);
> + ret = map_pp_mmio(vma, vmf->address, offset, ctx);
> else
> - rc = map_afu_irq(vma, vmf->address, offset, ctx);
> - return rc;
> + ret = map_afu_irq(vma, vmf->address, offset, ctx);
> + return ret;
> }
>
> static const struct vm_operations_struct ocxl_vmops = {
> diff --git a/drivers/misc/ocxl/sysfs.c b/drivers/misc/ocxl/sysfs.c
> index d9753a1..0ab1fd1 100644
> --- a/drivers/misc/ocxl/sysfs.c
> +++ b/drivers/misc/ocxl/sysfs.c
> @@ -64,7 +64,7 @@ static ssize_t global_mmio_read(struct file *filp, struct kobject *kobj,
> return count;
> }
>
> -static int global_mmio_fault(struct vm_fault *vmf)
> +static vm_fault_t global_mmio_fault(struct vm_fault *vmf)
> {
> struct vm_area_struct *vma = vmf->vma;
> struct ocxl_afu *afu = vma->vm_private_data;
> @@ -75,8 +75,7 @@ static int global_mmio_fault(struct vm_fault *vmf)
>
> offset = vmf->pgoff;
> offset += (afu->global_mmio_start >> PAGE_SHIFT);
> - vm_insert_pfn(vma, vmf->address, offset);
> - return VM_FAULT_NOPAGE;
> + return vmf_insert_pfn(vma, vmf->address, offset);
> }
>
> static const struct vm_operations_struct global_mmio_vmops = {
>
--
Andrew Donnellan OzLabs, ADL Canberra
[email protected] IBM Australia Limited
Le 11/06/2018 à 22:29, Souptick Joarder a écrit :
> Use new return type vm_fault_t for fault handler. For
> now, this is just documenting that the function returns
> a VM_FAULT value rather than an errno. Once all instances
> are converted, vm_fault_t will become a distinct type.
>
> Ref-> commit 1c8f422059ae ("mm: change return type to vm_fault_t")
>
> There is an existing bug when vm_insert_pfn() can return
> ENOMEM which was ignored and VM_FAULT_NOPAGE returned as
> default. The new inline vmf_insert_pfn() has removed
> this inefficiency by returning correct vm_fault_ type.
>
> Signed-off-by: Souptick Joarder <[email protected]>
> ---
Thanks!
Tested and
Acked-by: Frederic Barrat <[email protected]>
> drivers/misc/ocxl/context.c | 22 +++++++++++-----------
> drivers/misc/ocxl/sysfs.c | 5 ++---
> 2 files changed, 13 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/misc/ocxl/context.c b/drivers/misc/ocxl/context.c
> index 909e880..98daf91 100644
> --- a/drivers/misc/ocxl/context.c
> +++ b/drivers/misc/ocxl/context.c
> @@ -83,7 +83,7 @@ int ocxl_context_attach(struct ocxl_context *ctx, u64 amr)
> return rc;
> }
>
> -static int map_afu_irq(struct vm_area_struct *vma, unsigned long address,
> +static vm_fault_t map_afu_irq(struct vm_area_struct *vma, unsigned long address,
> u64 offset, struct ocxl_context *ctx)
> {
> u64 trigger_addr;
> @@ -92,15 +92,15 @@ static int map_afu_irq(struct vm_area_struct *vma, unsigned long address,
> if (!trigger_addr)
> return VM_FAULT_SIGBUS;
>
> - vm_insert_pfn(vma, address, trigger_addr >> PAGE_SHIFT);
> - return VM_FAULT_NOPAGE;
> + return vmf_insert_pfn(vma, address, trigger_addr >> PAGE_SHIFT);
> }
>
> -static int map_pp_mmio(struct vm_area_struct *vma, unsigned long address,
> +static vm_fault_t map_pp_mmio(struct vm_area_struct *vma, unsigned long address,
> u64 offset, struct ocxl_context *ctx)
> {
> u64 pp_mmio_addr;
> int pasid_off;
> + vm_fault_t ret;
>
> if (offset >= ctx->afu->config.pp_mmio_stride)
> return VM_FAULT_SIGBUS;
> @@ -118,27 +118,27 @@ static int map_pp_mmio(struct vm_area_struct *vma, unsigned long address,
> pasid_off * ctx->afu->config.pp_mmio_stride +
> offset;
>
> - vm_insert_pfn(vma, address, pp_mmio_addr >> PAGE_SHIFT);
> + ret = vmf_insert_pfn(vma, address, pp_mmio_addr >> PAGE_SHIFT);
> mutex_unlock(&ctx->status_mutex);
> - return VM_FAULT_NOPAGE;
> + return ret;
> }
>
> -static int ocxl_mmap_fault(struct vm_fault *vmf)
> +static vm_fault_t ocxl_mmap_fault(struct vm_fault *vmf)
> {
> struct vm_area_struct *vma = vmf->vma;
> struct ocxl_context *ctx = vma->vm_file->private_data;
> u64 offset;
> - int rc;
> + vm_fault_t ret;
>
> offset = vmf->pgoff << PAGE_SHIFT;
> pr_debug("%s: pasid %d address 0x%lx offset 0x%llx\n", __func__,
> ctx->pasid, vmf->address, offset);
>
> if (offset < ctx->afu->irq_base_offset)
> - rc = map_pp_mmio(vma, vmf->address, offset, ctx);
> + ret = map_pp_mmio(vma, vmf->address, offset, ctx);
> else
> - rc = map_afu_irq(vma, vmf->address, offset, ctx);
> - return rc;
> + ret = map_afu_irq(vma, vmf->address, offset, ctx);
> + return ret;
> }
>
> static const struct vm_operations_struct ocxl_vmops = {
> diff --git a/drivers/misc/ocxl/sysfs.c b/drivers/misc/ocxl/sysfs.c
> index d9753a1..0ab1fd1 100644
> --- a/drivers/misc/ocxl/sysfs.c
> +++ b/drivers/misc/ocxl/sysfs.c
> @@ -64,7 +64,7 @@ static ssize_t global_mmio_read(struct file *filp, struct kobject *kobj,
> return count;
> }
>
> -static int global_mmio_fault(struct vm_fault *vmf)
> +static vm_fault_t global_mmio_fault(struct vm_fault *vmf)
> {
> struct vm_area_struct *vma = vmf->vma;
> struct ocxl_afu *afu = vma->vm_private_data;
> @@ -75,8 +75,7 @@ static int global_mmio_fault(struct vm_fault *vmf)
>
> offset = vmf->pgoff;
> offset += (afu->global_mmio_start >> PAGE_SHIFT);
> - vm_insert_pfn(vma, vmf->address, offset);
> - return VM_FAULT_NOPAGE;
> + return vmf_insert_pfn(vma, vmf->address, offset);
> }
>
> static const struct vm_operations_struct global_mmio_vmops = {
>
On Thu, Jun 14, 2018 at 9:36 PM, Frederic Barrat <[email protected]> wrote:
>
>
> Le 11/06/2018 à 22:29, Souptick Joarder a écrit :
>>
>> Use new return type vm_fault_t for fault handler. For
>> now, this is just documenting that the function returns
>> a VM_FAULT value rather than an errno. Once all instances
>> are converted, vm_fault_t will become a distinct type.
>>
>> Ref-> commit 1c8f422059ae ("mm: change return type to vm_fault_t")
>>
>> There is an existing bug when vm_insert_pfn() can return
>> ENOMEM which was ignored and VM_FAULT_NOPAGE returned as
>> default. The new inline vmf_insert_pfn() has removed
>> this inefficiency by returning correct vm_fault_ type.
>>
>> Signed-off-by: Souptick Joarder <[email protected]>
>> ---
>
>
> Thanks!
>
> Tested and
> Acked-by: Frederic Barrat <[email protected]>
>
>
Frederic, is this patch queued for 4.19 ?
Souptick Joarder <[email protected]> writes:
> On Thu, Jun 14, 2018 at 9:36 PM, Frederic Barrat <[email protected]> wrote:
>> Le 11/06/2018 à 22:29, Souptick Joarder a écrit :
>>>
>>> Use new return type vm_fault_t for fault handler. For
>>> now, this is just documenting that the function returns
>>> a VM_FAULT value rather than an errno. Once all instances
>>> are converted, vm_fault_t will become a distinct type.
>>>
>>> Ref-> commit 1c8f422059ae ("mm: change return type to vm_fault_t")
>>>
>>> There is an existing bug when vm_insert_pfn() can return
>>> ENOMEM which was ignored and VM_FAULT_NOPAGE returned as
>>> default. The new inline vmf_insert_pfn() has removed
>>> this inefficiency by returning correct vm_fault_ type.
>>>
>>> Signed-off-by: Souptick Joarder <[email protected]>
>>> ---
>>
>>
>> Thanks!
>>
>> Tested and
>> Acked-by: Frederic Barrat <[email protected]>
>
> Frederic, is this patch queued for 4.19 ?
Not yet, you're a bit early. 4.18-rc1 only came out the other day :)
Your patches are tracked here:
http://patchwork.ozlabs.org/project/linuxppc-dev/list/?submitter=67924
They should appear in 4.19 unless something conflicts with them.
cheers
On Mon, 2018-06-11 at 20:29:04 UTC, Souptick Joarder wrote:
> Use new return type vm_fault_t for fault handler. For
> now, this is just documenting that the function returns
> a VM_FAULT value rather than an errno. Once all instances
> are converted, vm_fault_t will become a distinct type.
>
> Ref-> commit 1c8f422059ae ("mm: change return type to vm_fault_t")
>
> There is an existing bug when vm_insert_pfn() can return
> ENOMEM which was ignored and VM_FAULT_NOPAGE returned as
> default. The new inline vmf_insert_pfn() has removed
> this inefficiency by returning correct vm_fault_ type.
>
> Signed-off-by: Souptick Joarder <[email protected]>
> Acked-by: Andrew Donnellan <[email protected]>
> Acked-by: Frederic Barrat <[email protected]>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/a545cf032d11437ed86e62f00d4991
cheers
On Wed, Jul 11, 2018 at 6:54 PM, Michael Ellerman
<[email protected]> wrote:
> On Mon, 2018-06-11 at 20:29:04 UTC, Souptick Joarder wrote:
>> Use new return type vm_fault_t for fault handler. For
>> now, this is just documenting that the function returns
>> a VM_FAULT value rather than an errno. Once all instances
>> are converted, vm_fault_t will become a distinct type.
>>
>> Ref-> commit 1c8f422059ae ("mm: change return type to vm_fault_t")
>>
>> There is an existing bug when vm_insert_pfn() can return
>> ENOMEM which was ignored and VM_FAULT_NOPAGE returned as
>> default. The new inline vmf_insert_pfn() has removed
>> this inefficiency by returning correct vm_fault_ type.
>>
>> Signed-off-by: Souptick Joarder <[email protected]>
>> Acked-by: Andrew Donnellan <[email protected]>
>> Acked-by: Frederic Barrat <[email protected]>
>
> Applied to powerpc next, thanks.
>
> https://git.kernel.org/powerpc/c/a545cf032d11437ed86e62f00d4991
>
> cheers
Thanks :)