instmem/gk20a: exclusively acquire instobjs
PATCH 1/1 instmem/gk20a: exclusively acquire instobjs
Although I would not have expected this to happen, we seem to run into race conditions if instobjs are accessed concurrently. Use a global lock for safety.
--- drm/nouveau/nvkm/subdev/instmem/gk20a.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-)
drm/nouveau/nvkm/subdev/instmem/gk20a.c +6 -9
@@ -92,6 +92,7 @@ struct gk20a_instmem {/* protects vaddr_* and gk20a_instobj::vaddr* */ spinlock_t lock; +unsigned long flags;/* CPU mappings LRU */ unsigned int vaddr_use;
@@ -188,12 +189,11 @@ gk20a_instobj_acquire(struct nvkm_memory *memory)struct gk20a_instobj *node =gk20a_instobj (memory);struct gk20a_instmem *imem = node->imem;struct nvkm_ltc *ltc = imem->base.subdev.device->ltc; -const u64 size =nvkm_memory_size (memory); -unsigned long flags; +u64 size;nvkm_ltc_flush (ltc); -spin_lock_irqsave (&imem->lock, flags); +spin_lock_irqsave (&imem->lock, imem->flags);if (node->vaddr) {/* remove us from the LRU list since we cannot be unmapped */
@@ -202,6 +202,8 @@ gk20a_instobj_acquire(struct nvkm_memory *memory) goto out; } + size =nvkm_memory_size (memory); +/* try to free some address space if we reached the limit */ gk20a_instmem_vaddr_gc (imem, size);
@@ -218,8 +220,6 @@ gk20a_instobj_acquire(struct nvkm_memory *memory) imem->vaddr_use, imem->vaddr_max); out: -spin_unlock_irqrestore (&imem->lock, flags); -return node->vaddr; }
@@ -229,14 +229,11 @@ gk20a_instobj_release(struct nvkm_memory *memory)struct gk20a_instobj *node =gk20a_instobj (memory);struct gk20a_instmem *imem = node->imem;struct nvkm_ltc *ltc = imem->base.subdev.device->ltc; -unsigned long flags; - -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); -spin_unlock_irqrestore (&imem->lock, flags); +spin_unlock_irqrestore (&imem->lock, imem->flags);wmb ();nvkm_ltc_invalidate (ltc);
Signature · 6 lines
2.6.1 _______________________________________________ Nouveau mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/nouveau
A message that is not in the archive (sent off-list or not yet received): 1445838895-28342-1-git-send-email-acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org
Quoted 3 lines · Alexandre Courbot
Although I would not have expected this to happen, we seem to run into race conditions if instobjs are accessed concurrently. Use a global lock for safety.
I wouldn't expect this to be an issue either.
Before merging such a large hammer of a fix, I'd strongly prefer to see at least a better justification for why this is happening rather than potentially papering over a larger issue.
Ben.
Quoted 58 lines
Signed-off-by Alexandre Courbot <[email protected]>--- drm/nouveau/nvkm/subdev/instmem/gk20a.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-)drm/nouveau/nvkm/subdev/instmem/gk20a.c +5
-7@@ -92,6 +92,7 @@ struct gk20a_instmem {/* protects vaddr_* and gk20a_instobj::vaddr* */ spinlock_t lock; +unsigned long flags;/* CPU mappings LRU */ unsigned int vaddr_use;@@ -188,12 +189,11 @@ gk20a_instobj_acquire(struct nvkm_memory *memory)struct gk20a_instobj *node =gk20a_instobj (memory);struct gk20a_instmem *imem = node->imem;struct nvkm_ltc *ltc = imem->base.subdev.device->ltc; -const u64 size =nvkm_memory_size (memory); -unsigned long flags; +u64 size;nvkm_ltc_flush (ltc); -spin_lock_irqsave (&imem->lock, flags); +spin_lock_irqsave (&imem->lock, imem->flags);if (node->vaddr) {/* remove us from the LRU list since we cannot be unmapped */ @@ -202,6 +202,8 @@ gk20a_instobj_acquire(struct nvkm_memory *memory) goto out; } + size =nvkm_memory_size (memory); +/* try to free some address space if we reached the limit */ gk20a_instmem_vaddr_gc (imem, size);@@ -218,8 +220,6 @@ gk20a_instobj_acquire(struct nvkm_memory *memory) imem->vaddr_use, imem->vaddr_max); out: -spin_unlock_irqrestore (&imem->lock, flags); -return node->vaddr; }@@ -229,14 +229,11 @@ gk20a_instobj_release(struct nvkm_memory *memory)struct gk20a_instobj *node =gk20a_instobj (memory);struct gk20a_instmem *imem = node->imem;struct nvkm_ltc *ltc = imem->base.subdev.device->ltc; -unsigned long flags; -
drm/nouveau/nvkm/subdev/instmem/gk20a.c gk20a_instobj_release(struct nvkm_memory *memory) · line 239
Quoted 10 lines
drm/nouveau/nvkm/subdev/instmem/gk20a.c +1
-2-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); -spin_unlock_irqrestore (&imem->lock, flags); +spin_unlock_irqrestore (&imem->lock, imem->flags);wmb ();nvkm_ltc_invalidate (ltc);
_______________________________________________
Nouveau mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/nouveau
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 9 lines · Ben Skeggs
Quoted 3 lines · Alexandre Courbot
Although I would not have expected this to happen, we seem to run into race conditions if instobjs are accessed concurrently. Use a global lock for safety.
I wouldn't expect this to be an issue either.
Before merging such a large hammer of a fix, I'd strongly prefer to see at least a better justification for why this is happening rather than potentially papering over a larger issue.
I was afraid you would say that. ;)
But you're right. I am really busy with lots of stuff but will try to rootcause this more precisely...
_______________________________________________
Nouveau mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/nouveau
A message that is not in the archive (sent off-list or not yet received): CAAVeFuJAvb9GDxr=WBcpfvkXbiiTbju=aU2kwzhUA-fXgMXXkQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org
Quoted 7 lines · Alexandre Courbot
Quoted 6 lines · Ben Skeggs
Quoted 3 lines · Alexandre Courbot
Although I would not have expected this to happen, we seem to run into race conditions if instobjs are accessed concurrently. Use a global lock for safety.
I wouldn't expect this to be an issue either.
Quoted 8 lines
Quoted 3 lines
Before merging such a large hammer of a fix, I'd strongly prefer to see at least a better justification for why this is happening rather than potentially papering over a larger issue.
I was afraid you would say that. ;)
But you're right. I am really busy with lots of stuff but will try to rootcause this more precisely...
Turns out this was just me not knowing how to handle concurrency properly. >_<
Fortunately this means that the error was just confined to the gk20a implementation and is not part of something larger. I will send the correct fixup patch later today.
Thanks for refusing my lazy hacks! :)
_______________________________________________
Nouveau mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/nouveau