[PATCH] arm/vdso: introduce vdso_mremap hook

[PATCH] arm/vdso: introduce vdso_mremap hook

[Dmitry Safonov]

  Add vdso_mremap hook which will fix context.vdso pointer after mremap()
on vDSO vma. This is needed for correct landing after syscall execution.
Primary goal of this is for CRIU on arm - we need to restore vDSO image
at the exactly same place where the vma was in dumped application. With
the help of this hook we'll move vDSO at the new position.
  The CRIU code handles situations like when vDSO of dumped application
was different from vDSO on restoring system. This usally happens when
some new symbols are being added to vDSO. In these situations CRIU
inserts jump trampolines from old vDSO blob to new vDSO on restore.
By that reason even if on restore vDSO blob lies on the same address as
blob in dumped application - we still need to move it if it differs.

  There was previously attempt to add this functionality for arm64 by
arch_mremap hook [1], while this patch introduces this with minimal
effort - the same way I've added it to x86:
commit b059a453b1cf ("x86/vdso: Add mremap hook to vm_special_mapping")

  At this moment, vdso restoring code is disabled for arm/arm64 arch
in CRIU [2], so C/R is only working for !CONFIG_VDSO kernels. This patch
is aimed to fix that.
  The same hook may be introduced for arm64 kernel, but at this moment
arm64 vdso code is actively reworked by Kevin, so we can do it on top.
  Separately, I've refactored arch_remap hook out from ppc64 [3].

[1]: https://marc.info/?i=1448455781-26660-1-git-send-email-cov@codeaurora.org
[2]: https://github.com/xemul/criu/blob/master/Makefile#L39
[3]: https://marc.info/?i=20161027170948.8279-1-dsafonov@virtuozzo.com

Cc: Kevin Brodsky <kevin.brodsky@arm.com>
Cc: Christopher Covington <cov@codeaurora.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Will Deacon <will.deacon@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-mm@kvack.org
Cc: Cyrill Gorcunov <gorcunov@openvz.org>
Cc: Pavel Emelyanov <xemul@virtuozzo.com>
Signed-off-by: Dmitry Safonov <dsafonov@virtuozzo.com>
---
 arch/arm/kernel/vdso.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/arch/arm/kernel/vdso.c b/arch/arm/kernel/vdso.c
index 53cf86cf2d1a..d1001f87c2f6 100644
--- a/arch/arm/kernel/vdso.c
+++ b/arch/arm/kernel/vdso.c
@@ -54,8 +54,11 @@ static const struct vm_special_mapping vdso_data_mapping = {
 	.pages = &vdso_data_page,
 };
 
+static int vdso_mremap(const struct vm_special_mapping *sm,
+		struct vm_area_struct *new_vma);
 static struct vm_special_mapping vdso_text_mapping __ro_after_init = {
 	.name = "[vdso]",
+	.mremap = vdso_mremap,
 };
 
 struct elfinfo {
@@ -254,6 +257,24 @@ void arm_install_vdso(struct mm_struct *mm, unsigned long addr)
 		mm->context.vdso = addr;
 }
 
+static int vdso_mremap(const struct vm_special_mapping *sm,
+		struct vm_area_struct *new_vma)
+{
+	unsigned long new_size = new_vma->vm_end - new_vma->vm_start;
+	unsigned long vdso_size = (vdso_total_pages - 1) << PAGE_SHIFT;
+
+	/* Disallow partial vDSO blob remap */
+	if (vdso_size != new_size)
+		return -EINVAL;
+
+	if (WARN_ON_ONCE(current->mm != new_vma->vm_mm))
+		return -EFAULT;
+
+	current->mm->context.vdso = new_vma->vm_start;
+
+	return 0;
+}
+
 static void vdso_write_begin(struct vdso_data *vdata)
 {
 	++vdso_data->seq_count;
-- 
2.10.2

Re: [PATCH] arm/vdso: introduce vdso_mremap hook

[Christopher Covington]

Hi Dmitry,

On 11/01/2016 01:22 PM, Dmitry Safonov wrote:
>   Add vdso_mremap hook which will fix context.vdso pointer after mremap()
> on vDSO vma. This is needed for correct landing after syscall execution.
> Primary goal of this is for CRIU on arm - we need to restore vDSO image
> at the exactly same place where the vma was in dumped application. With
> the help of this hook we'll move vDSO at the new position.
>   The CRIU code handles situations like when vDSO of dumped application
> was different from vDSO on restoring system. This usally happens when
> some new symbols are being added to vDSO. In these situations CRIU
> inserts jump trampolines from old vDSO blob to new vDSO on restore.
> By that reason even if on restore vDSO blob lies on the same address as
> blob in dumped application - we still need to move it if it differs.
> 
>   There was previously attempt to add this functionality for arm64 by
> arch_mremap hook [1], while this patch introduces this with minimal
> effort - the same way I've added it to x86:
> commit b059a453b1cf ("x86/vdso: Add mremap hook to vm_special_mapping")
> 
>   At this moment, vdso restoring code is disabled for arm/arm64 arch
> in CRIU [2], so C/R is only working for !CONFIG_VDSO kernels. This patch
> is aimed to fix that.
>   The same hook may be introduced for arm64 kernel, but at this moment
> arm64 vdso code is actively reworked by Kevin, so we can do it on top.
>   Separately, I've refactored arch_remap hook out from ppc64 [3].
> 
> [1]: https://marc.info/?i=1448455781-26660-1-git-send-email-cov@codeaurora.org
> [2]: https://github.com/xemul/criu/blob/master/Makefile#L39
> [3]: https://marc.info/?i=20161027170948.8279-1-dsafonov@virtuozzo.com
> 
> Cc: Kevin Brodsky <kevin.brodsky@arm.com>
> Cc: Christopher Covington <cov@codeaurora.org>
> Cc: Andy Lutomirski <luto@amacapital.net>
> Cc: Oleg Nesterov <oleg@redhat.com>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-mm@kvack.org
> Cc: Cyrill Gorcunov <gorcunov@openvz.org>
> Cc: Pavel Emelyanov <xemul@virtuozzo.com>
> Signed-off-by: Dmitry Safonov <dsafonov@virtuozzo.com>
> ---
>  arch/arm/kernel/vdso.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/arch/arm/kernel/vdso.c b/arch/arm/kernel/vdso.c
> index 53cf86cf2d1a..d1001f87c2f6 100644
> --- a/arch/arm/kernel/vdso.c
> +++ b/arch/arm/kernel/vdso.c
> @@ -54,8 +54,11 @@ static const struct vm_special_mapping vdso_data_mapping = {
>  	.pages = &vdso_data_page,
>  };
>  
> +static int vdso_mremap(const struct vm_special_mapping *sm,
> +		struct vm_area_struct *new_vma);
>  static struct vm_special_mapping vdso_text_mapping __ro_after_init = {
>  	.name = "[vdso]",
> +	.mremap = vdso_mremap,
>  };
>  
>  struct elfinfo {
> @@ -254,6 +257,24 @@ void arm_install_vdso(struct mm_struct *mm, unsigned long addr)
>  		mm->context.vdso = addr;
>  }
>  
> +static int vdso_mremap(const struct vm_special_mapping *sm,
> +		struct vm_area_struct *new_vma)
> +{
> +	unsigned long new_size = new_vma->vm_end - new_vma->vm_start;
> +	unsigned long vdso_size = (vdso_total_pages - 1) << PAGE_SHIFT;
> +
> +	/* Disallow partial vDSO blob remap */
> +	if (vdso_size != new_size)
> +		return -EINVAL;
> +
> +	if (WARN_ON_ONCE(current->mm != new_vma->vm_mm))
> +		return -EFAULT;
> +
> +	current->mm->context.vdso = new_vma->vm_start;
> +
> +	return 0;
> +}
> +
>  static void vdso_write_begin(struct vdso_data *vdata)
>  {
>  	++vdso_data->seq_count;
> 

What do you think about putting this code somewhere generic (not under
arch/*), so that powerpc and arm64 can reuse it once the cosmetic changes
to make them compatible are made? My thought was that it could be defined
underneath CONFIG_GENERIC_VDSO, which architectures could select as they
became compatible.

Thanks,
Cov

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code
Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH] arm/vdso: introduce vdso_mremap hook

[Dmitry Safonov]

On 11/07/2016 08:00 PM, Christopher Covington wrote:
> Hi Dmitry,
>
> On 11/01/2016 01:22 PM, Dmitry Safonov wrote:
>>   Add vdso_mremap hook which will fix context.vdso pointer after mremap()
>> on vDSO vma. This is needed for correct landing after syscall execution.
>> Primary goal of this is for CRIU on arm - we need to restore vDSO image
>> at the exactly same place where the vma was in dumped application. With
>> the help of this hook we'll move vDSO at the new position.
>>   The CRIU code handles situations like when vDSO of dumped application
>> was different from vDSO on restoring system. This usally happens when
>> some new symbols are being added to vDSO. In these situations CRIU
>> inserts jump trampolines from old vDSO blob to new vDSO on restore.
>> By that reason even if on restore vDSO blob lies on the same address as
>> blob in dumped application - we still need to move it if it differs.
>>
>>   There was previously attempt to add this functionality for arm64 by
>> arch_mremap hook [1], while this patch introduces this with minimal
>> effort - the same way I've added it to x86:
>> commit b059a453b1cf ("x86/vdso: Add mremap hook to vm_special_mapping")
>>
>>   At this moment, vdso restoring code is disabled for arm/arm64 arch
>> in CRIU [2], so C/R is only working for !CONFIG_VDSO kernels. This patch
>> is aimed to fix that.
>>   The same hook may be introduced for arm64 kernel, but at this moment
>> arm64 vdso code is actively reworked by Kevin, so we can do it on top.
>>   Separately, I've refactored arch_remap hook out from ppc64 [3].
>>
>> [1]: https://marc.info/?i=1448455781-26660-1-git-send-email-cov@codeaurora.org
>> [2]: https://github.com/xemul/criu/blob/master/Makefile#L39
>> [3]: https://marc.info/?i=20161027170948.8279-1-dsafonov@virtuozzo.com
>>
>> Cc: Kevin Brodsky <kevin.brodsky@arm.com>
>> Cc: Christopher Covington <cov@codeaurora.org>
>> Cc: Andy Lutomirski <luto@amacapital.net>
>> Cc: Oleg Nesterov <oleg@redhat.com>
>> Cc: Russell King <linux@armlinux.org.uk>
>> Cc: Will Deacon <will.deacon@arm.com>
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-mm@kvack.org
>> Cc: Cyrill Gorcunov <gorcunov@openvz.org>
>> Cc: Pavel Emelyanov <xemul@virtuozzo.com>
>> Signed-off-by: Dmitry Safonov <dsafonov@virtuozzo.com>
>> ---
>>  arch/arm/kernel/vdso.c | 21 +++++++++++++++++++++
>>  1 file changed, 21 insertions(+)
>>
>> diff --git a/arch/arm/kernel/vdso.c b/arch/arm/kernel/vdso.c
>> index 53cf86cf2d1a..d1001f87c2f6 100644
>> --- a/arch/arm/kernel/vdso.c
>> +++ b/arch/arm/kernel/vdso.c
>> @@ -54,8 +54,11 @@ static const struct vm_special_mapping vdso_data_mapping = {
>>  	.pages = &vdso_data_page,
>>  };
>>
>> +static int vdso_mremap(const struct vm_special_mapping *sm,
>> +		struct vm_area_struct *new_vma);
>>  static struct vm_special_mapping vdso_text_mapping __ro_after_init = {
>>  	.name = "[vdso]",
>> +	.mremap = vdso_mremap,
>>  };
>>
>>  struct elfinfo {
>> @@ -254,6 +257,24 @@ void arm_install_vdso(struct mm_struct *mm, unsigned long addr)
>>  		mm->context.vdso = addr;
>>  }
>>
>> +static int vdso_mremap(const struct vm_special_mapping *sm,
>> +		struct vm_area_struct *new_vma)
>> +{
>> +	unsigned long new_size = new_vma->vm_end - new_vma->vm_start;
>> +	unsigned long vdso_size = (vdso_total_pages - 1) << PAGE_SHIFT;
>> +
>> +	/* Disallow partial vDSO blob remap */
>> +	if (vdso_size != new_size)
>> +		return -EINVAL;
>> +
>> +	if (WARN_ON_ONCE(current->mm != new_vma->vm_mm))
>> +		return -EFAULT;
>> +
>> +	current->mm->context.vdso = new_vma->vm_start;
>> +
>> +	return 0;
>> +}
>> +
>>  static void vdso_write_begin(struct vdso_data *vdata)
>>  {
>>  	++vdso_data->seq_count;
>>
>
> What do you think about putting this code somewhere generic (not under
> arch/*), so that powerpc and arm64 can reuse it once the cosmetic changes
> to make them compatible are made? My thought was that it could be defined
> underneath CONFIG_GENERIC_VDSO, which architectures could select as they
> became compatible.

Hi Chistopher,

Well, I don't think we won something out of generalization of simple 
assignment for context.vdso pointer accross arches. And a need to rename
vdso over arches for saving one single line?
Also I don't like a bit this arch_mremap hook and need to nullify
vdso pointer.

But, anyway, I don't mind if your patches got applied instead - this
unability to move vdso prevent's to support vdso vma C/R, as you know ;)

-- 
              Dmitry

Re: [PATCH] arm/vdso: introduce vdso_mremap hook

[Christopher Covington]

On 11/07/2016 12:16 PM, Dmitry Safonov wrote:
> On 11/07/2016 08:00 PM, Christopher Covington wrote:
>> Hi Dmitry,
>>
>> On 11/01/2016 01:22 PM, Dmitry Safonov wrote:
>>>   Add vdso_mremap hook which will fix context.vdso pointer after mremap()
>>> on vDSO vma. This is needed for correct landing after syscall execution.
>>> Primary goal of this is for CRIU on arm - we need to restore vDSO image
>>> at the exactly same place where the vma was in dumped application. With
>>> the help of this hook we'll move vDSO at the new position.
>>>   The CRIU code handles situations like when vDSO of dumped application
>>> was different from vDSO on restoring system. This usally happens when
>>> some new symbols are being added to vDSO. In these situations CRIU
>>> inserts jump trampolines from old vDSO blob to new vDSO on restore.
>>> By that reason even if on restore vDSO blob lies on the same address as
>>> blob in dumped application - we still need to move it if it differs.
>>>
>>>   There was previously attempt to add this functionality for arm64 by
>>> arch_mremap hook [1], while this patch introduces this with minimal
>>> effort - the same way I've added it to x86:
>>> commit b059a453b1cf ("x86/vdso: Add mremap hook to vm_special_mapping")
>>>
>>>   At this moment, vdso restoring code is disabled for arm/arm64 arch
>>> in CRIU [2], so C/R is only working for !CONFIG_VDSO kernels. This patch
>>> is aimed to fix that.
>>>   The same hook may be introduced for arm64 kernel, but at this moment
>>> arm64 vdso code is actively reworked by Kevin, so we can do it on top.
>>>   Separately, I've refactored arch_remap hook out from ppc64 [3].
>>>
>>> [1]: https://marc.info/?i=1448455781-26660-1-git-send-email-cov@codeaurora.org
>>> [2]: https://github.com/xemul/criu/blob/master/Makefile#L39
>>> [3]: https://marc.info/?i=20161027170948.8279-1-dsafonov@virtuozzo.com
>>>
>>> Cc: Kevin Brodsky <kevin.brodsky@arm.com>
>>> Cc: Christopher Covington <cov@codeaurora.org>
>>> Cc: Andy Lutomirski <luto@amacapital.net>
>>> Cc: Oleg Nesterov <oleg@redhat.com>
>>> Cc: Russell King <linux@armlinux.org.uk>
>>> Cc: Will Deacon <will.deacon@arm.com>
>>> Cc: linux-arm-kernel@lists.infradead.org
>>> Cc: linux-mm@kvack.org
>>> Cc: Cyrill Gorcunov <gorcunov@openvz.org>
>>> Cc: Pavel Emelyanov <xemul@virtuozzo.com>
>>> Signed-off-by: Dmitry Safonov <dsafonov@virtuozzo.com>
>>> ---
>>>  arch/arm/kernel/vdso.c | 21 +++++++++++++++++++++
>>>  1 file changed, 21 insertions(+)
>>>
>>> diff --git a/arch/arm/kernel/vdso.c b/arch/arm/kernel/vdso.c
>>> index 53cf86cf2d1a..d1001f87c2f6 100644
>>> --- a/arch/arm/kernel/vdso.c
>>> +++ b/arch/arm/kernel/vdso.c
>>> @@ -54,8 +54,11 @@ static const struct vm_special_mapping vdso_data_mapping = {
>>>      .pages = &vdso_data_page,
>>>  };
>>>
>>> +static int vdso_mremap(const struct vm_special_mapping *sm,
>>> +        struct vm_area_struct *new_vma);
>>>  static struct vm_special_mapping vdso_text_mapping __ro_after_init = {
>>>      .name = "[vdso]",
>>> +    .mremap = vdso_mremap,
>>>  };
>>>
>>>  struct elfinfo {
>>> @@ -254,6 +257,24 @@ void arm_install_vdso(struct mm_struct *mm, unsigned long addr)
>>>          mm->context.vdso = addr;
>>>  }
>>>
>>> +static int vdso_mremap(const struct vm_special_mapping *sm,
>>> +        struct vm_area_struct *new_vma)
>>> +{
>>> +    unsigned long new_size = new_vma->vm_end - new_vma->vm_start;
>>> +    unsigned long vdso_size = (vdso_total_pages - 1) << PAGE_SHIFT;
>>> +
>>> +    /* Disallow partial vDSO blob remap */
>>> +    if (vdso_size != new_size)
>>> +        return -EINVAL;
>>> +
>>> +    if (WARN_ON_ONCE(current->mm != new_vma->vm_mm))
>>> +        return -EFAULT;
>>> +
>>> +    current->mm->context.vdso = new_vma->vm_start;
>>> +
>>> +    return 0;
>>> +}
>>> +
>>>  static void vdso_write_begin(struct vdso_data *vdata)
>>>  {
>>>      ++vdso_data->seq_count;
>>>
>>
>> What do you think about putting this code somewhere generic (not under
>> arch/*), so that powerpc and arm64 can reuse it once the cosmetic changes
>> to make them compatible are made? My thought was that it could be defined
>> underneath CONFIG_GENERIC_VDSO, which architectures could select as they
>> became compatible.
> 
> Hi Chistopher,
> 
> Well, I don't think we won something out of generalization of simple assignment for context.vdso pointer accross arches. And a need to rename
> vdso over arches for saving one single line?

I count 17 lines, which duplicated across 3 architectures becomes 51 lines.
Presumable in the future other architectures will want CRIU support as well.
Additionally, should fixes ever be required, fixing one implementation instead
of 3+ is preferred.

> Also I don't like a bit this arch_mremap hook and need to nullify
> vdso pointer.

I'm sorry for the confusion but I in no way meant to imply that the
arch_mremap hook should be carried forward. I fully  agree that the function
pointer in struct vm_special_mapping is the better way to go.

If you don't want to implement a version with vdso_mremap defined in a
generic location (using it from struct vm_special_mapping), do you mind if I
propose such a version?

Thanks,
Cov

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code
Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH] arm/vdso: introduce vdso_mremap hook

[Dmitry Safonov]

On 11/07/2016 09:08 PM, Christopher Covington wrote:
> On 11/07/2016 12:16 PM, Dmitry Safonov wrote:
>> On 11/07/2016 08:00 PM, Christopher Covington wrote:
>>> Hi Dmitry,
>>>
>>> On 11/01/2016 01:22 PM, Dmitry Safonov wrote:
>>>>   Add vdso_mremap hook which will fix context.vdso pointer after mremap()
>>>> on vDSO vma. This is needed for correct landing after syscall execution.
>>>> Primary goal of this is for CRIU on arm - we need to restore vDSO image
>>>> at the exactly same place where the vma was in dumped application. With
>>>> the help of this hook we'll move vDSO at the new position.
>>>>   The CRIU code handles situations like when vDSO of dumped application
>>>> was different from vDSO on restoring system. This usally happens when
>>>> some new symbols are being added to vDSO. In these situations CRIU
>>>> inserts jump trampolines from old vDSO blob to new vDSO on restore.
>>>> By that reason even if on restore vDSO blob lies on the same address as
>>>> blob in dumped application - we still need to move it if it differs.
>>>>
>>>>   There was previously attempt to add this functionality for arm64 by
>>>> arch_mremap hook [1], while this patch introduces this with minimal
>>>> effort - the same way I've added it to x86:
>>>> commit b059a453b1cf ("x86/vdso: Add mremap hook to vm_special_mapping")
>>>>
>>>>   At this moment, vdso restoring code is disabled for arm/arm64 arch
>>>> in CRIU [2], so C/R is only working for !CONFIG_VDSO kernels. This patch
>>>> is aimed to fix that.
>>>>   The same hook may be introduced for arm64 kernel, but at this moment
>>>> arm64 vdso code is actively reworked by Kevin, so we can do it on top.
>>>>   Separately, I've refactored arch_remap hook out from ppc64 [3].
>>>>
>>>> [1]: https://marc.info/?i=1448455781-26660-1-git-send-email-cov@codeaurora.org
>>>> [2]: https://github.com/xemul/criu/blob/master/Makefile#L39
>>>> [3]: https://marc.info/?i=20161027170948.8279-1-dsafonov@virtuozzo.com
>>>>
>>>> Cc: Kevin Brodsky <kevin.brodsky@arm.com>
>>>> Cc: Christopher Covington <cov@codeaurora.org>
>>>> Cc: Andy Lutomirski <luto@amacapital.net>
>>>> Cc: Oleg Nesterov <oleg@redhat.com>
>>>> Cc: Russell King <linux@armlinux.org.uk>
>>>> Cc: Will Deacon <will.deacon@arm.com>
>>>> Cc: linux-arm-kernel@lists.infradead.org
>>>> Cc: linux-mm@kvack.org
>>>> Cc: Cyrill Gorcunov <gorcunov@openvz.org>
>>>> Cc: Pavel Emelyanov <xemul@virtuozzo.com>
>>>> Signed-off-by: Dmitry Safonov <dsafonov@virtuozzo.com>
>>>> ---
>>>>  arch/arm/kernel/vdso.c | 21 +++++++++++++++++++++
>>>>  1 file changed, 21 insertions(+)
>>>>
>>>> diff --git a/arch/arm/kernel/vdso.c b/arch/arm/kernel/vdso.c
>>>> index 53cf86cf2d1a..d1001f87c2f6 100644
>>>> --- a/arch/arm/kernel/vdso.c
>>>> +++ b/arch/arm/kernel/vdso.c
>>>> @@ -54,8 +54,11 @@ static const struct vm_special_mapping vdso_data_mapping = {
>>>>      .pages = &vdso_data_page,
>>>>  };
>>>>
>>>> +static int vdso_mremap(const struct vm_special_mapping *sm,
>>>> +        struct vm_area_struct *new_vma);
>>>>  static struct vm_special_mapping vdso_text_mapping __ro_after_init = {
>>>>      .name = "[vdso]",
>>>> +    .mremap = vdso_mremap,
>>>>  };
>>>>
>>>>  struct elfinfo {
>>>> @@ -254,6 +257,24 @@ void arm_install_vdso(struct mm_struct *mm, unsigned long addr)
>>>>          mm->context.vdso = addr;
>>>>  }
>>>>
>>>> +static int vdso_mremap(const struct vm_special_mapping *sm,
>>>> +        struct vm_area_struct *new_vma)
>>>> +{
>>>> +    unsigned long new_size = new_vma->vm_end - new_vma->vm_start;
>>>> +    unsigned long vdso_size = (vdso_total_pages - 1) << PAGE_SHIFT;
>>>> +
>>>> +    /* Disallow partial vDSO blob remap */
>>>> +    if (vdso_size != new_size)
>>>> +        return -EINVAL;
>>>> +
>>>> +    if (WARN_ON_ONCE(current->mm != new_vma->vm_mm))
>>>> +        return -EFAULT;
>>>> +
>>>> +    current->mm->context.vdso = new_vma->vm_start;
>>>> +
>>>> +    return 0;
>>>> +}
>>>> +
>>>>  static void vdso_write_begin(struct vdso_data *vdata)
>>>>  {
>>>>      ++vdso_data->seq_count;
>>>>
>>>
>>> What do you think about putting this code somewhere generic (not under
>>> arch/*), so that powerpc and arm64 can reuse it once the cosmetic changes
>>> to make them compatible are made? My thought was that it could be defined
>>> underneath CONFIG_GENERIC_VDSO, which architectures could select as they
>>> became compatible.
>>
>> Hi Chistopher,
>>
>> Well, I don't think we won something out of generalization of simple assignment for context.vdso pointer accross arches. And a need to rename
>> vdso over arches for saving one single line?
>
> I count 17 lines, which duplicated across 3 architectures becomes 51 lines.
> Presumable in the future other architectures will want CRIU support as well.
> Additionally, should fixes ever be required, fixing one implementation instead
> of 3+ is preferred.
>
>> Also I don't like a bit this arch_mremap hook and need to nullify
>> vdso pointer.
>
> I'm sorry for the confusion but I in no way meant to imply that the
> arch_mremap hook should be carried forward. I fully  agree that the function
> pointer in struct vm_special_mapping is the better way to go.
>
> If you don't want to implement a version with vdso_mremap defined in a
> generic location (using it from struct vm_special_mapping), do you mind if I
> propose such a version?

Sure, do it, no objections.

-- 
              Dmitry

Re: [PATCH] arm/vdso: introduce vdso_mremap hook

[Russell King - ARM Linux]

On Tue, Nov 01, 2016 at 08:22:14PM +0300, Dmitry Safonov wrote:
> diff --git a/arch/arm/kernel/vdso.c b/arch/arm/kernel/vdso.c
> index 53cf86cf2d1a..d1001f87c2f6 100644
> --- a/arch/arm/kernel/vdso.c
> +++ b/arch/arm/kernel/vdso.c
> @@ -54,8 +54,11 @@ static const struct vm_special_mapping vdso_data_mapping = {
>  	.pages = &vdso_data_page,
>  };
>  
> +static int vdso_mremap(const struct vm_special_mapping *sm,
> +		struct vm_area_struct *new_vma);

I'd much rather avoid this forward declaration.  Is there any reason the
function body can't be here?

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH] arm/vdso: introduce vdso_mremap hook

[Dmitry Safonov]

On 11/07/2016 09:27 PM, Russell King - ARM Linux wrote:
> On Tue, Nov 01, 2016 at 08:22:14PM +0300, Dmitry Safonov wrote:
>> diff --git a/arch/arm/kernel/vdso.c b/arch/arm/kernel/vdso.c
>> index 53cf86cf2d1a..d1001f87c2f6 100644
>> --- a/arch/arm/kernel/vdso.c
>> +++ b/arch/arm/kernel/vdso.c
>> @@ -54,8 +54,11 @@ static const struct vm_special_mapping vdso_data_mapping = {
>>  	.pages = &vdso_data_page,
>>  };
>>
>> +static int vdso_mremap(const struct vm_special_mapping *sm,
>> +		struct vm_area_struct *new_vma);
>
> I'd much rather avoid this forward declaration.  Is there any reason the
> function body can't be here?
>

Well, I didn't want it to be in the middle of static file variables -
those looks nice at this moment just on top of the file.
No other than that.

-- 
              Dmitry