On 24 July 2018 at 21:28, Sebastian Andrzej Siewior
<bigeasy@xxxxxxxxxxxxx> wrote:
> efi_switch_mm() is a wrapper around switch_mm() which saves current's
> ->active_mm, sets the requests mm as ->active_mm and invokes
> switch_mm().
> I don't think that task_lock() is required during that procedure. It
> protects ->mm which isn't changed here.
>
> It needs to be mentioned that during the whole procedure (switch to
> EFI's mm and back) the preemption needs to be disabled. A context switch
> at this point would reset the cr3 value based on current->mm. Also, this
> function may not be invoked at the same time on a different CPU because
> it would overwrite the efi_scratch.prev_mm information.
>
> Remove task_lock() and also update the comment to reflect it.
>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
> ---
> v1…v2:
> - drop RFC
> - don't remove assignment of ->active_mm (PeterZ)
> - don't worry about concurrent invocations of the function, Ard
> said that the "mixed-mode" version has a lock now.
>
> arch/x86/platform/efi/efi_64.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
> index 448267f1c073..6ab1fdeefa1a 100644
> --- a/arch/x86/platform/efi/efi_64.c
> +++ b/arch/x86/platform/efi/efi_64.c
> @@ -619,18 +619,16 @@ void __init efi_dump_pagetable(void)
>
> /*
> * Makes the calling thread switch to/from efi_mm context. Can be used
> - * for SetVirtualAddressMap() i.e. current->active_mm == init_mm as well
> - * as during efi runtime calls i.e current->active_mm == current_mm.
> - * We are not mm_dropping()/mm_grabbing() any mm, because we are not
> - * losing/creating any references.
> + * in a kernel thread and user context. Preemption needs to remain disabled
> + * while the EFI-mm is borrowed. mmgrab()/mmdrop() is not used because the mm
> + * can not change under us.
> + * It should be ensured that there are no concurent calls to this function.
> */
> void efi_switch_mm(struct mm_struct *mm)
> {
> - task_lock(current);
> efi_scratch.prev_mm = current->active_mm;
> current->active_mm = mm;
> switch_mm(efi_scratch.prev_mm, mm, NULL);
> - task_unlock(current);
> }
>
> #ifdef CONFIG_EFI_MIXED
> --
> 2.18.0
>
Thanks, I'll queue this in efi/next.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
