On 11/09/2015 05:37 PM, Alexandre Courbot wrote: > The LRU list used for recycling CPU mappings was handling concurrency > very poorly. For instance, if an instobj was acquired twice before being > released once, it would end up into the LRU list even though there is > still a client accessing it. > > This patch fixes this by properly counting how many clients are > currently using a given instobj. Out of curiosity, which instobjs are being accessed concurrently?
> > While at it, we also raise errors when inconsistencies are detected, and > factorize some code. > > Signed-off-by: Alexandre Courbot <[email protected]> > --- > drm/nouveau/nvkm/subdev/instmem/gk20a.c | 66 > ++++++++++++++++++--------------- > 1 file changed, 37 insertions(+), 29 deletions(-) > > diff --git a/drm/nouveau/nvkm/subdev/instmem/gk20a.c > b/drm/nouveau/nvkm/subdev/instmem/gk20a.c > index fc419bb8eab7..681b2541229a 100644 > --- a/drm/nouveau/nvkm/subdev/instmem/gk20a.c > +++ b/drm/nouveau/nvkm/subdev/instmem/gk20a.c > @@ -57,6 +57,8 @@ struct gk20a_instobj { > /* CPU mapping */ > u32 *vaddr; > struct list_head vaddr_node; > + /* How many clients are using vaddr? */ > + u32 use_cpt; > }; > #define gk20a_instobj(p) container_of((p), struct gk20a_instobj, memory) > > @@ -158,27 +160,35 @@ gk20a_instobj_cpu_map_iommu(struct nvkm_memory *memory) > } > > /* > - * Must be called while holding gk20a_instmem_lock > + * Recycle the vaddr of obj. Must be called with gk20a_instmem::lock held. > + */ > +static void > +gk20a_instobj_recycle_vaddr(struct gk20a_instobj *obj) > +{ > + struct gk20a_instmem *imem = obj->imem; > + /* there should not be any user left... */ > + WARN_ON(obj->use_cpt); > + list_del(&obj->vaddr_node); > + vunmap(obj->vaddr); > + obj->vaddr = NULL; > + imem->vaddr_use -= nvkm_memory_size(&obj->memory); > + nvkm_debug(&imem->base.subdev, "vaddr used: %x/%x\n", imem->vaddr_use, > + imem->vaddr_max); > +} > + > +/* > + * Must be called while holding gk20a_instmem::lock > */ > static void > gk20a_instmem_vaddr_gc(struct gk20a_instmem *imem, const u64 size) > { > while (imem->vaddr_use + size > imem->vaddr_max) { > - struct gk20a_instobj *obj; > - > /* no candidate that can be unmapped, abort... */ > if (list_empty(&imem->vaddr_lru)) > break; > > - obj = list_first_entry(&imem->vaddr_lru, struct gk20a_instobj, > - vaddr_node); > - list_del(&obj->vaddr_node); > - vunmap(obj->vaddr); > - obj->vaddr = NULL; > - imem->vaddr_use -= nvkm_memory_size(&obj->memory); > - nvkm_debug(&imem->base.subdev, "(GC) vaddr used: %x/%x\n", > - imem->vaddr_use, imem->vaddr_max); > - > + gk20a_instobj_recycle_vaddr(list_first_entry(&imem->vaddr_lru, > + struct gk20a_instobj, vaddr_node)); > } > } > > @@ -196,9 +206,10 @@ gk20a_instobj_acquire(struct nvkm_memory *memory) > spin_lock_irqsave(&imem->lock, flags); > > if (node->vaddr) { > - /* remove us from the LRU list since we cannot be unmapped */ > - list_del(&node->vaddr_node); > - > + if (!node->use_cpt) { > + /* remove from LRU list since mapping in use again */ > + list_del(&node->vaddr_node); > + } > goto out; > } > > @@ -218,6 +229,7 @@ gk20a_instobj_acquire(struct nvkm_memory *memory) > imem->vaddr_use, imem->vaddr_max); > > out: > + node->use_cpt++; > spin_unlock_irqrestore(&imem->lock, flags); > > return node->vaddr; > @@ -233,9 +245,15 @@ gk20a_instobj_release(struct nvkm_memory *memory) > > spin_lock_irqsave(&imem->lock, flags); > > - /* add ourselves to the LRU list so our CPU mapping can be freed */ > - list_add_tail(&node->vaddr_node, &imem->vaddr_lru); > + /* we should at least have one user to release... */ > + if (WARN_ON(node->use_cpt == 0)) > + goto out; > + > + /* add unused objs to the LRU list to recycle their mapping */ > + if (--node->use_cpt == 0) > + list_add_tail(&node->vaddr_node, &imem->vaddr_lru); > > +out: > spin_unlock_irqrestore(&imem->lock, flags); > > wmb(); > @@ -273,25 +291,15 @@ static void > gk20a_instobj_dtor(struct gk20a_instobj *node) > { > struct gk20a_instmem *imem = node->imem; > - struct gk20a_instobj *obj; > unsigned long flags; > > spin_lock_irqsave(&imem->lock, flags); > > + /* vaddr has already been recycled */ > if (!node->vaddr) > goto out; > > - list_for_each_entry(obj, &imem->vaddr_lru, vaddr_node) { > - if (obj == node) { > - list_del(&obj->vaddr_node); > - break; > - } > - } > - vunmap(node->vaddr); > - node->vaddr = NULL; > - imem->vaddr_use -= nvkm_memory_size(&node->memory); > - nvkm_debug(&imem->base.subdev, "vaddr used: %x/%x\n", > - imem->vaddr_use, imem->vaddr_max); > + gk20a_instobj_recycle_vaddr(node); > > out: > spin_unlock_irqrestore(&imem->lock, flags); >
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Nouveau mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/nouveau
