lkml.org 
[lkml]   [2022]   [Jan]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.15 549/846] drm/amdkfd: Fix error handling in svm_range_add
    Date
    From: Felix Kuehling <Felix.Kuehling@amd.com>

    [ Upstream commit 726be40607264b180a2b336c81e1dcff941de618 ]

    Add null-pointer check after the last svm_range_new call. This was
    originally reported by Zhou Qingyang <zhou1615@umn.edu> based on a
    static analyzer.

    To avoid duplicating the unwinding code from svm_range_handle_overlap,
    I merged the two functions into one.

    Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
    Cc: Zhou Qingyang <zhou1615@umn.edu>
    Reviewed-by: Philip Yang <Philip.Yang@amd.com>
    Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
    Signed-off-by: Sasha Levin <sashal@kernel.org>
    ---
    drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 138 ++++++++++-----------------
    1 file changed, 49 insertions(+), 89 deletions(-)

    diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
    index 5a674235ae41a..830809b694dd9 100644
    --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
    +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
    @@ -936,7 +936,7 @@ svm_range_split(struct svm_range *prange, uint64_t start, uint64_t last,
    }

    static int
    -svm_range_split_tail(struct svm_range *prange, struct svm_range *new,
    +svm_range_split_tail(struct svm_range *prange,
    uint64_t new_last, struct list_head *insert_list)
    {
    struct svm_range *tail;
    @@ -948,7 +948,7 @@ svm_range_split_tail(struct svm_range *prange, struct svm_range *new,
    }

    static int
    -svm_range_split_head(struct svm_range *prange, struct svm_range *new,
    +svm_range_split_head(struct svm_range *prange,
    uint64_t new_start, struct list_head *insert_list)
    {
    struct svm_range *head;
    @@ -1755,49 +1755,54 @@ static struct svm_range *svm_range_clone(struct svm_range *old)
    }

    /**
    - * svm_range_handle_overlap - split overlap ranges
    - * @svms: svm range list header
    - * @new: range added with this attributes
    - * @start: range added start address, in pages
    - * @last: range last address, in pages
    - * @update_list: output, the ranges attributes are updated. For set_attr, this
    - * will do validation and map to GPUs. For unmap, this will be
    - * removed and unmap from GPUs
    - * @insert_list: output, the ranges will be inserted into svms, attributes are
    - * not changes. For set_attr, this will add into svms.
    - * @remove_list:output, the ranges will be removed from svms
    - * @left: the remaining range after overlap, For set_attr, this will be added
    - * as new range.
    + * svm_range_add - add svm range and handle overlap
    + * @p: the range add to this process svms
    + * @start: page size aligned
    + * @size: page size aligned
    + * @nattr: number of attributes
    + * @attrs: array of attributes
    + * @update_list: output, the ranges need validate and update GPU mapping
    + * @insert_list: output, the ranges need insert to svms
    + * @remove_list: output, the ranges are replaced and need remove from svms
    *
    - * Total have 5 overlap cases.
    + * Check if the virtual address range has overlap with any existing ranges,
    + * split partly overlapping ranges and add new ranges in the gaps. All changes
    + * should be applied to the range_list and interval tree transactionally. If
    + * any range split or allocation fails, the entire update fails. Therefore any
    + * existing overlapping svm_ranges are cloned and the original svm_ranges left
    + * unchanged.
    *
    - * This function handles overlap of an address interval with existing
    - * struct svm_ranges for applying new attributes. This may require
    - * splitting existing struct svm_ranges. All changes should be applied to
    - * the range_list and interval tree transactionally. If any split operation
    - * fails, the entire update fails. Therefore the existing overlapping
    - * svm_ranges are cloned and the original svm_ranges left unchanged. If the
    - * transaction succeeds, the modified clones are added and the originals
    - * freed. Otherwise the clones are removed and the old svm_ranges remain.
    + * If the transaction succeeds, the caller can update and insert clones and
    + * new ranges, then free the originals.
    *
    - * Context: The caller must hold svms->lock
    + * Otherwise the caller can free the clones and new ranges, while the old
    + * svm_ranges remain unchanged.
    + *
    + * Context: Process context, caller must hold svms->lock
    + *
    + * Return:
    + * 0 - OK, otherwise error code
    */
    static int
    -svm_range_handle_overlap(struct svm_range_list *svms, struct svm_range *new,
    - unsigned long start, unsigned long last,
    - struct list_head *update_list,
    - struct list_head *insert_list,
    - struct list_head *remove_list,
    - unsigned long *left)
    +svm_range_add(struct kfd_process *p, uint64_t start, uint64_t size,
    + uint32_t nattr, struct kfd_ioctl_svm_attribute *attrs,
    + struct list_head *update_list, struct list_head *insert_list,
    + struct list_head *remove_list)
    {
    + unsigned long last = start + size - 1UL;
    + struct svm_range_list *svms = &p->svms;
    struct interval_tree_node *node;
    + struct svm_range new = {0};
    struct svm_range *prange;
    struct svm_range *tmp;
    int r = 0;

    + pr_debug("svms 0x%p [0x%llx 0x%lx]\n", &p->svms, start, last);
    +
    INIT_LIST_HEAD(update_list);
    INIT_LIST_HEAD(insert_list);
    INIT_LIST_HEAD(remove_list);
    + svm_range_apply_attrs(p, &new, nattr, attrs);

    node = interval_tree_iter_first(&svms->objects, start, last);
    while (node) {
    @@ -1825,14 +1830,14 @@ svm_range_handle_overlap(struct svm_range_list *svms, struct svm_range *new,

    if (node->start < start) {
    pr_debug("change old range start\n");
    - r = svm_range_split_head(prange, new, start,
    + r = svm_range_split_head(prange, start,
    insert_list);
    if (r)
    goto out;
    }
    if (node->last > last) {
    pr_debug("change old range last\n");
    - r = svm_range_split_tail(prange, new, last,
    + r = svm_range_split_tail(prange, last,
    insert_list);
    if (r)
    goto out;
    @@ -1844,7 +1849,7 @@ svm_range_handle_overlap(struct svm_range_list *svms, struct svm_range *new,
    prange = old;
    }

    - if (!svm_range_is_same_attrs(prange, new))
    + if (!svm_range_is_same_attrs(prange, &new))
    list_add(&prange->update_list, update_list);

    /* insert a new node if needed */
    @@ -1864,8 +1869,16 @@ svm_range_handle_overlap(struct svm_range_list *svms, struct svm_range *new,
    start = next_start;
    }

    - if (left && start <= last)
    - *left = last - start + 1;
    + /* add a final range at the end if needed */
    + if (start <= last) {
    + prange = svm_range_new(svms, start, last);
    + if (!prange) {
    + r = -ENOMEM;
    + goto out;
    + }
    + list_add(&prange->insert_list, insert_list);
    + list_add(&prange->update_list, update_list);
    + }

    out:
    if (r)
    @@ -2693,59 +2706,6 @@ svm_range_is_valid(struct mm_struct *mm, uint64_t start, uint64_t size)
    return true;
    }

    -/**
    - * svm_range_add - add svm range and handle overlap
    - * @p: the range add to this process svms
    - * @start: page size aligned
    - * @size: page size aligned
    - * @nattr: number of attributes
    - * @attrs: array of attributes
    - * @update_list: output, the ranges need validate and update GPU mapping
    - * @insert_list: output, the ranges need insert to svms
    - * @remove_list: output, the ranges are replaced and need remove from svms
    - *
    - * Check if the virtual address range has overlap with the registered ranges,
    - * split the overlapped range, copy and adjust pages address and vram nodes in
    - * old and new ranges.
    - *
    - * Context: Process context, caller must hold svms->lock
    - *
    - * Return:
    - * 0 - OK, otherwise error code
    - */
    -static int
    -svm_range_add(struct kfd_process *p, uint64_t start, uint64_t size,
    - uint32_t nattr, struct kfd_ioctl_svm_attribute *attrs,
    - struct list_head *update_list, struct list_head *insert_list,
    - struct list_head *remove_list)
    -{
    - uint64_t last = start + size - 1UL;
    - struct svm_range_list *svms;
    - struct svm_range new = {0};
    - struct svm_range *prange;
    - unsigned long left = 0;
    - int r = 0;
    -
    - pr_debug("svms 0x%p [0x%llx 0x%llx]\n", &p->svms, start, last);
    -
    - svm_range_apply_attrs(p, &new, nattr, attrs);
    -
    - svms = &p->svms;
    -
    - r = svm_range_handle_overlap(svms, &new, start, last, update_list,
    - insert_list, remove_list, &left);
    - if (r)
    - return r;
    -
    - if (left) {
    - prange = svm_range_new(svms, last - left + 1, last);
    - list_add(&prange->insert_list, insert_list);
    - list_add(&prange->update_list, update_list);
    - }
    -
    - return 0;
    -}
    -
    /**
    * svm_range_best_prefetch_location - decide the best prefetch location
    * @prange: svm range structure
    --
    2.34.1


    \
     
     \ /
      Last update: 2022-01-24 22:40    [W:4.274 / U:0.780 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site