| Message ID | 20171208183034.9027-1-ard.biesheuvel@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Dec 08, 2017 at 06:30:34PM +0000, Ard Biesheuvel wrote: > Commit be55287aa5b ("drm/nouveau/imem/nv50: embed nvkm_instobj directly > into nv04_instobj") introduced some new calls to the refcount api to > the nv50 mapping code. In one particular instance, it does the > following: > > if (!refcount_inc_not_zero(&iobj->maps)) { > ... > refcount_inc(&iobj->maps); > } > > i.e., it calls refcount_inc() to increment the refcount when it is known > to be zero, which is explicitly forbidden by the API. Instead, use > refcount_set() to set it to 1. > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- Awesome! Works for me. > Apologies if this was already found and fixed. I don't usually follow > the DRM or nouveau mailing lists. I see nothing relevant in dri-devel and nouveau archives, except my complaint (GTX 560 Ti (GF114)): https://lists.freedesktop.org/archives/nouveau/2017-December/029264.html and Richard Narron seconding it (MSI GeForce 210): https://lists.freedesktop.org/archives/nouveau/2017-December/029276.html > drivers/gpu/drm/nouveau/nvkm/subdev/instmem/nv50.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/nv50.c b/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/nv50.c > index 1ba7289684aa..db48a1daca0c 100644 > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/nv50.c > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/nv50.c > @@ -249,7 +249,7 @@ nv50_instobj_acquire(struct nvkm_memory *memory) > iobj->base.memory.ptrs = &nv50_instobj_fast; > else > iobj->base.memory.ptrs = &nv50_instobj_slow; > - refcount_inc(&iobj->maps); > + refcount_set(&iobj->maps, 1); > } > > mutex_unlock(&imem->subdev.mutex); > -- I'm just a dumb user here, my tags don't carry a weight, but Tested-by:. Meow!
On 8 December 2017 at 19:30, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > Commit be55287aa5b ("drm/nouveau/imem/nv50: embed nvkm_instobj directly > into nv04_instobj") introduced some new calls to the refcount api to > the nv50 mapping code. In one particular instance, it does the > following: > > if (!refcount_inc_not_zero(&iobj->maps)) { > ... > refcount_inc(&iobj->maps); > } > > i.e., it calls refcount_inc() to increment the refcount when it is known > to be zero, which is explicitly forbidden by the API. Instead, use > refcount_set() to set it to 1. > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > > Apologies if this was already found and fixed. I don't usually follow > the DRM or nouveau mailing lists. > > drivers/gpu/drm/nouveau/nvkm/subdev/instmem/nv50.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/nv50.c b/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/nv50.c > index 1ba7289684aa..db48a1daca0c 100644 > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/nv50.c > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/nv50.c > @@ -249,7 +249,7 @@ nv50_instobj_acquire(struct nvkm_memory *memory) > iobj->base.memory.ptrs = &nv50_instobj_fast; > else > iobj->base.memory.ptrs = &nv50_instobj_slow; > - refcount_inc(&iobj->maps); > + refcount_set(&iobj->maps, 1); > } > > mutex_unlock(&imem->subdev.mutex); > -- > 2.11.0 > Ping?
Hey Ard, It seems that Ben already committed a similar patch to his tree (see [0]). I do not know whether he is planning to have it part of a pull request of fixes for 4.15. Best regards, Pierre [0]: https://github.com/skeggsb/nouveau/commit/9068f1df2394f0e4ab2b2a28cac06b462fe0a0aa On 2017-12-18 — 09:27, Ard Biesheuvel wrote: > On 8 December 2017 at 19:30, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > > Commit be55287aa5b ("drm/nouveau/imem/nv50: embed nvkm_instobj directly > > into nv04_instobj") introduced some new calls to the refcount api to > > the nv50 mapping code. In one particular instance, it does the > > following: > > > > if (!refcount_inc_not_zero(&iobj->maps)) { > > ... > > refcount_inc(&iobj->maps); > > } > > > > i.e., it calls refcount_inc() to increment the refcount when it is known > > to be zero, which is explicitly forbidden by the API. Instead, use > > refcount_set() to set it to 1. > > > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > --- > > > > Apologies if this was already found and fixed. I don't usually follow > > the DRM or nouveau mailing lists. > > > > drivers/gpu/drm/nouveau/nvkm/subdev/instmem/nv50.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/nv50.c b/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/nv50.c > > index 1ba7289684aa..db48a1daca0c 100644 > > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/nv50.c > > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/nv50.c > > @@ -249,7 +249,7 @@ nv50_instobj_acquire(struct nvkm_memory *memory) > > iobj->base.memory.ptrs = &nv50_instobj_fast; > > else > > iobj->base.memory.ptrs = &nv50_instobj_slow; > > - refcount_inc(&iobj->maps); > > + refcount_set(&iobj->maps, 1); > > } > > > > mutex_unlock(&imem->subdev.mutex); > > -- > > 2.11.0 > > > > Ping? > _______________________________________________ > Nouveau mailing list > Nouveau@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/nouveau
On 18 December 2017 at 13:16, Pierre Moreau <pierre.morrow@free.fr> wrote: > Hey Ard, > > It seems that Ben already committed a similar patch to his tree (see [0]). I do > not know whether he is planning to have it part of a pull request of fixes for > 4.15. > Hi Pierre, Thanks for the reply. If a fix has been queued, I don't mind leaving it up to Ben to decide when it gets sent onwards.
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/nv50.c b/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/nv50.c index 1ba7289684aa..db48a1daca0c 100644 --- a/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/nv50.c +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/nv50.c @@ -249,7 +249,7 @@ nv50_instobj_acquire(struct nvkm_memory *memory) iobj->base.memory.ptrs = &nv50_instobj_fast; else iobj->base.memory.ptrs = &nv50_instobj_slow; - refcount_inc(&iobj->maps); + refcount_set(&iobj->maps, 1); } mutex_unlock(&imem->subdev.mutex);
Commit be55287aa5b ("drm/nouveau/imem/nv50: embed nvkm_instobj directly into nv04_instobj") introduced some new calls to the refcount api to the nv50 mapping code. In one particular instance, it does the following: if (!refcount_inc_not_zero(&iobj->maps)) { ... refcount_inc(&iobj->maps); } i.e., it calls refcount_inc() to increment the refcount when it is known to be zero, which is explicitly forbidden by the API. Instead, use refcount_set() to set it to 1. Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- Apologies if this was already found and fixed. I don't usually follow the DRM or nouveau mailing lists. drivers/gpu/drm/nouveau/nvkm/subdev/instmem/nv50.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)