Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200831203111.4940-1-rcampbell@nvidia.com>
In-Reply-To: <20200831203111.4940-1-rcampbell@nvidia.com>
From:   Ben Skeggs <skeggsb@gmail.com>
Date:   Thu, 3 Sep 2020 15:18:41 +1000
Message-ID: <CACAvsv4yP2xR4aayzFDT8nGfM69j5ogZUxjoV4f0P58pEJq3RQ@mail.gmail.com>
Subject: Re: [Nouveau] [PATCH v2] nouveau: fix the start/end range for migration
To:     Ralph Campbell <rcampbell@nvidia.com>
Cc:     ML nouveau <nouveau@lists.freedesktop.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Ben Skeggs <bskeggs@redhat.com>,
        Jason Gunthorpe <jgg@nvidia.com>,
        Christoph Hellwig <hch@lst.de>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, 1 Sep 2020 at 06:31, Ralph Campbell <rcampbell@nvidia.com> wrote:
>
> The user level OpenCL code shouldn't have to align start and end
> addresses to a page boundary. That is better handled in the nouveau
> driver. The npages field is also redundant since it can be computed
> from the start and end addresses.
>
> Signed-off-by: Ralph Campbell <rcampbell@nvidia.com>
Thanks Ralph,

Queued in my tree.

Ben.

> ---
>
> This is for Ben Skegg's nouveau tree.
>
> I have been working with Karol Herbst on the OpenCL mesa changes for
> nouveau which will be merged upstream soon.
> With or without those changes, the user visible effect of this patch
> only extends the range by one page (round up vs. round down to page
> boundary).
>
> Changes in v2:
> I changed the start/end check to require a size so start has to be less
> than end.
>
>  drivers/gpu/drm/nouveau/nouveau_svm.c | 14 +++-----------
>  1 file changed, 3 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c b/drivers/gpu/drm/nouveau/nouveau_svm.c
> index 2df1c0460559..4f69e4c3dafd 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_svm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_svm.c
> @@ -105,11 +105,11 @@ nouveau_svmm_bind(struct drm_device *dev, void *data,
>         struct nouveau_cli *cli = nouveau_cli(file_priv);
>         struct drm_nouveau_svm_bind *args = data;
>         unsigned target, cmd, priority;
> -       unsigned long addr, end, size;
> +       unsigned long addr, end;
>         struct mm_struct *mm;
>
>         args->va_start &= PAGE_MASK;
> -       args->va_end &= PAGE_MASK;
> +       args->va_end = ALIGN(args->va_end, PAGE_SIZE);
>
>         /* Sanity check arguments */
>         if (args->reserved0 || args->reserved1)
> @@ -118,8 +118,6 @@ nouveau_svmm_bind(struct drm_device *dev, void *data,
>                 return -EINVAL;
>         if (args->va_start >= args->va_end)
>                 return -EINVAL;
> -       if (!args->npages)
> -               return -EINVAL;
>
>         cmd = args->header >> NOUVEAU_SVM_BIND_COMMAND_SHIFT;
>         cmd &= NOUVEAU_SVM_BIND_COMMAND_MASK;
> @@ -151,12 +149,6 @@ nouveau_svmm_bind(struct drm_device *dev, void *data,
>         if (args->stride)
>                 return -EINVAL;
>
> -       size = ((unsigned long)args->npages) << PAGE_SHIFT;
> -       if ((args->va_start + size) <= args->va_start)
> -               return -EINVAL;
> -       if ((args->va_start + size) > args->va_end)
> -               return -EINVAL;
> -
>         /*
>          * Ok we are ask to do something sane, for now we only support migrate
>          * commands but we will add things like memory policy (what to do on
> @@ -171,7 +163,7 @@ nouveau_svmm_bind(struct drm_device *dev, void *data,
>                 return -EINVAL;
>         }
>
> -       for (addr = args->va_start, end = args->va_start + size; addr < end;) {
> +       for (addr = args->va_start, end = args->va_end; addr < end;) {
>                 struct vm_area_struct *vma;
>                 unsigned long next;
>
> --
> 2.20.1
>
> _______________________________________________
> Nouveau mailing list
> Nouveau@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau