[PATCH 0/5] Revert cleanups for IS_ERR_OR_NULL() usage

[PATCH 0/5] Revert cleanups for IS_ERR_OR_NULL() usage

[Lyude Paul]

Turns out there's many good reasons for this!

Lyude Paul (5):
  Revert "nouveau/gsp/rm: cleanup remaining IS_ERR_OR_NULL usage"
  Revert "nouveau/gsp/rm: cleanup IS_ERR_OR_NULL in core implementation"
  Revert "nouveau/gsp/rm: cleanup WARN_ON(IS_ERR_OR_NULL)"
  Revert "nouveau/gsp: cleanup IS_ERR_OR_NULL in rpc_rd"
  Revert "nouveau/gsp: cleanup IS_ERR_OR_NULL in rm_alloc functions"

 drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h      |  8 ++++----
 .../gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/alloc.c    |  4 ++--
 drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/bar.c  |  2 +-
 drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/ctrl.c |  2 +-
 drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c  | 10 +++++-----
 5 files changed, 13 insertions(+), 13 deletions(-)


base-commit: 61de054a772a1feda6364931ab1baf9038abf1c8
-- 
2.54.0

[PATCH 1/5] Revert "nouveau/gsp/rm: cleanup remaining IS_ERR_OR_NULL usage"

[Lyude Paul]

This reverts commit 1a80c009e27b42e8c202d4c5dbd9dad9e22af742.

Embarassingly, it seems that I completely missed a pretty big issue this
patch causes according to Danilo and Sashiko:

https://lists.freedesktop.org/archives/dri-devel/2026-May/570353.html

Where it seems this causes some machines to segfault during nouveau probe.

So, revert this for the time being.

Signed-off-by: Lyude Paul <lyude@redhat.com>
---
 drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/ctrl.c | 2 +-
 drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c  | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/ctrl.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/ctrl.c
index aa0ebd3dfb17d..70b9ee911c5e7 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/ctrl.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/ctrl.c
@@ -42,7 +42,7 @@ r535_gsp_rpc_rm_ctrl_push(struct nvkm_gsp_object *object, void **params, u32 rep
 	int ret = 0;
 
 	rpc = nvkm_gsp_rpc_push(gsp, rpc, NVKM_GSP_RPC_REPLY_RECV, repc);
-	if (IS_ERR(rpc)) {
+	if (IS_ERR_OR_NULL(rpc)) {
 		*params = NULL;
 		return PTR_ERR(rpc);
 	}
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c
index 0c9657cb2dd72..41301f19729cf 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c
@@ -459,11 +459,11 @@ r535_gsp_msg_recv(struct nvkm_gsp *gsp, int fn, u32 gsp_rpc_len)
 
 retry:
 	rpc = r535_gsp_msgq_peek(gsp, sizeof(*rpc), &retries);
-	if (IS_ERR(rpc))
+	if (IS_ERR_OR_NULL(rpc))
 		return rpc;
 
 	rpc = r535_gsp_msgq_recv(gsp, gsp_rpc_len, &retries);
-	if (IS_ERR(rpc))
+	if (IS_ERR_OR_NULL(rpc))
 		return rpc;
 
 	if (rpc->rpc_result) {
@@ -561,7 +561,7 @@ r535_gsp_rpc_handle_reply(struct nvkm_gsp *gsp, u32 fn,
 		break;
 	case NVKM_GSP_RPC_REPLY_RECV:
 		reply = r535_gsp_msg_recv(gsp, fn, gsp_rpc_len);
-		if (!IS_ERR(reply))
+		if (!IS_ERR_OR_NULL(reply))
 			repv = reply->data;
 		else
 			repv = reply;
-- 
2.54.0

[PATCH 2/5] Revert "nouveau/gsp/rm: cleanup IS_ERR_OR_NULL in core implementation"

[Lyude Paul]

This is probably too risky, see the discussion here:

  https://lists.freedesktop.org/archives/dri-devel/2026-May/570353.html

This reverts commit 67346c90ce275e835e93a4a13041afee47bd3f9e.

Signed-off-by: Lyude Paul <lyude@redhat.com>
---
 drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/alloc.c | 4 ++--
 drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c   | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/alloc.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/alloc.c
index 27f275d2e1510..46e3a29f2ad7f 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/alloc.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/alloc.c
@@ -35,7 +35,7 @@ r535_gsp_rpc_rm_free(struct nvkm_gsp_object *object)
 		   client->object.handle, object->handle);
 
 	rpc = nvkm_gsp_rpc_get(gsp, NV_VGPU_MSG_FUNCTION_FREE, sizeof(*rpc));
-	if (WARN_ON(IS_ERR(rpc)))
+	if (WARN_ON(IS_ERR_OR_NULL(rpc)))
 		return -EIO;
 
 	rpc->params.hRoot = client->object.handle;
@@ -60,7 +60,7 @@ r535_gsp_rpc_rm_alloc_push(struct nvkm_gsp_object *object, void *params)
 	void *ret = NULL;
 
 	rpc = nvkm_gsp_rpc_push(gsp, rpc, NVKM_GSP_RPC_REPLY_RECV, sizeof(*rpc));
-	if (IS_ERR(rpc))
+	if (IS_ERR_OR_NULL(rpc))
 		return rpc;
 
 	if (rpc->status) {
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c
index 41301f19729cf..3ca3de8f43408 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c
@@ -324,7 +324,7 @@ r535_gsp_msgq_recv(struct nvkm_gsp *gsp, u32 gsp_rpc_len, int *retries)
 		u32 size;
 
 		rpc = r535_gsp_msgq_peek(gsp, sizeof(*rpc), info.retries);
-		if (IS_ERR(rpc)) {
+		if (IS_ERR_OR_NULL(rpc)) {
 			kvfree(buf);
 			return rpc;
 		}
@@ -333,7 +333,7 @@ r535_gsp_msgq_recv(struct nvkm_gsp *gsp, u32 gsp_rpc_len, int *retries)
 		info.continuation = true;
 
 		rpc = r535_gsp_msgq_recv_one_elem(gsp, &info);
-		if (IS_ERR(rpc)) {
+		if (IS_ERR_OR_NULL(rpc)) {
 			kvfree(buf);
 			return rpc;
 		}
-- 
2.54.0

[PATCH 3/5] Revert "nouveau/gsp/rm: cleanup WARN_ON(IS_ERR_OR_NULL)"

[Lyude Paul]

This is probably too risky, see the discussion here:

  https://lists.freedesktop.org/archives/dri-devel/2026-May/570353.html

This reverts commit 6198977a78af8769d3f3108e830901325b97cf03.

Signed-off-by: Lyude Paul <lyude@redhat.com>
---
 drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/bar.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/bar.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/bar.c
index 9cd68f8622d36..fae08ac3b18c7 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/bar.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/bar.c
@@ -55,7 +55,7 @@ r535_bar_bar2_update_pde(struct nvkm_gsp *gsp, u8 page_shift, u64 pdbe)
 	rpc_update_bar_pde_v15_00 *rpc;
 
 	rpc = nvkm_gsp_rpc_get(gsp, NV_VGPU_MSG_FUNCTION_UPDATE_BAR_PDE, sizeof(*rpc));
-	if (WARN_ON(IS_ERR(rpc)))
+	if (WARN_ON(IS_ERR_OR_NULL(rpc)))
 		return -EIO;
 
 	rpc->info.barType = NV_RPC_UPDATE_PDE_BAR_2;
-- 
2.54.0

[PATCH 4/5] Revert "nouveau/gsp: cleanup IS_ERR_OR_NULL in rpc_rd"

[Lyude Paul]

This is probably much too risky. See the discussion here:

  https://lists.freedesktop.org/archives/dri-devel/2026-May/570353.html

This reverts commit 47f15f6cf068c14d1a5054066c445bee23f6047e.

Signed-off-by: Lyude Paul <lyude@redhat.com>
---
 drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h b/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
index d771134fa4107..ab2bd88eebce5 100644
--- a/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
+++ b/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
@@ -293,7 +293,7 @@ nvkm_gsp_rpc_rd(struct nvkm_gsp *gsp, u32 fn, u32 argc)
 {
 	void *argv = nvkm_gsp_rpc_get(gsp, fn, argc);
 
-	if (IS_ERR(argv))
+	if (IS_ERR_OR_NULL(argv))
 		return argv;
 
 	return nvkm_gsp_rpc_push(gsp, argv, NVKM_GSP_RPC_REPLY_RECV, argc);
-- 
2.54.0

[PATCH 5/5] Revert "nouveau/gsp: cleanup IS_ERR_OR_NULL in rm_alloc functions"

[Lyude Paul]

This is probably too risky, see the discussion here:

  https://lists.freedesktop.org/archives/dri-devel/2026-May/570353.html

This reverts commit 281fe11c6c4aebc1a1eb9d21eaab7323ee5af979.

Signed-off-by: Lyude Paul <lyude@redhat.com>
---
 drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h b/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
index ab2bd88eebce5..64fed208e4cf8 100644
--- a/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
+++ b/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
@@ -373,7 +373,7 @@ nvkm_gsp_rm_alloc_get(struct nvkm_gsp_object *parent, u32 handle, u32 oclass, u3
 	object->handle = handle;
 
 	argv = gsp->rm->api->alloc->get(object, oclass, argc);
-	if (IS_ERR(argv)) {
+	if (IS_ERR_OR_NULL(argv)) {
 		object->client = NULL;
 		return argv;
 	}
@@ -415,8 +415,8 @@ nvkm_gsp_rm_alloc(struct nvkm_gsp_object *parent, u32 handle, u32 oclass, u32 ar
 {
 	void *argv = nvkm_gsp_rm_alloc_get(parent, handle, oclass, argc, object);
 
-	if (IS_ERR(argv))
-		return PTR_ERR(argv);
+	if (IS_ERR_OR_NULL(argv))
+		return argv ? PTR_ERR(argv) : -EIO;
 
 	return nvkm_gsp_rm_alloc_wr(object, argv);
 }
-- 
2.54.0

Re: [PATCH 0/5] Revert cleanups for IS_ERR_OR_NULL() usage

[Danilo Krummrich]

On Thu May 28, 2026 at 9:27 PM CEST, Lyude Paul wrote:
> Lyude Paul (5):
>   Revert "nouveau/gsp/rm: cleanup remaining IS_ERR_OR_NULL usage"
>   Revert "nouveau/gsp/rm: cleanup IS_ERR_OR_NULL in core implementation"
>   Revert "nouveau/gsp/rm: cleanup WARN_ON(IS_ERR_OR_NULL)"
>   Revert "nouveau/gsp: cleanup IS_ERR_OR_NULL in rpc_rd"
>   Revert "nouveau/gsp: cleanup IS_ERR_OR_NULL in rm_alloc functions"

Reverting the whole thing for now is the right call, as it needs some more
review.

@Timur: I do think cleaning this up is the right call in general though, and I
also don't think that the whole driver necessarily needs to be consistent on
whether IS_ERR_OR_NULL() or IS_ERR() is used -- it depends on the context
(although I usually prefer not to mix up NULL and ERR semantics in the first
place).

It should however be consistent in terms of what functions can actually return.

	ret = foo();
	if (IS_ERR_OR_NULL(ret))
		return ret;

If foo() can never return NULL, the above is misleading, as it puts an
obligation on the caller to somehow handle the NULL case and come up with an
actual error code for it.

So, I think it is the right call to align that to what functions can actually
return, but while doing this, the contract should be properly documented, such
that subsequent changes can be properly validated.

I can pick this up later, but in case you want to pick it up Lyude, note that
this now has to go into drm-misc-next-fixes.

Thanks,
Danilo

Re: [PATCH 0/5] Revert cleanups for IS_ERR_OR_NULL() usage

[lyude]

On Thu, 2026-05-28 at 21:44 +0200, Danilo Krummrich wrote:
> 
> I can pick this up later, but in case you want to pick it up Lyude,
> note that
> this now has to go into drm-misc-next-fixes.

Whatever works for you and is easier, I also didn't realize we'd hit
rc6 yet

Re: [PATCH 0/5] Revert cleanups for IS_ERR_OR_NULL() usage

[Timur Tabi]

On Thu, 2026-05-28 at 21:44 +0200, Danilo Krummrich wrote:
> 
> @Timur: I do think cleaning this up is the right call in general though, and I
> also don't think that the whole driver necessarily needs to be consistent on
> whether IS_ERR_OR_NULL() or IS_ERR() is used -- it depends on the context
> (although I usually prefer not to mix up NULL and ERR semantics in the first
> place).
> 
> It should however be consistent in terms of what functions can actually return.
> 
> 	ret = foo();
> 	if (IS_ERR_OR_NULL(ret))
> 		return ret;
> 
> If foo() can never return NULL, the above is misleading, as it puts an
> obligation on the caller to somehow handle the NULL case and come up with an
> actual error code for it.

Sure, I get that.  My point is that it's often not clear whether foo() actually can never return
NULL.  

It's been a while since I've dug through the RPC call chains in Nouveau, so my memory is a little
hazy here.  I do remember noticing that Nouveau frequently has situations where foo() call bar1()
and bar2(), where bar1() can return NULL but bar2() can't.  So the question is not whether foo() can
return NULL, it's whether bar1() should not return NULL, or whether bar2() should.

> So, I think it is the right call to align that to what functions can actually
> return, but while doing this, the contract should be properly documented, such
> that subsequent changes can be properly validated.

"Properly documented" and "Nouveau" are not two things that go together.  

Re: [PATCH 0/5] Revert cleanups for IS_ERR_OR_NULL() usage

[Danilo Krummrich]

On Thu May 28, 2026 at 10:23 PM CEST, Timur Tabi wrote:
> On Thu, 2026-05-28 at 21:44 +0200, Danilo Krummrich wrote:
>> 
>> @Timur: I do think cleaning this up is the right call in general though, and I
>> also don't think that the whole driver necessarily needs to be consistent on
>> whether IS_ERR_OR_NULL() or IS_ERR() is used -- it depends on the context
>> (although I usually prefer not to mix up NULL and ERR semantics in the first
>> place).
>> 
>> It should however be consistent in terms of what functions can actually return.
>> 
>> 	ret = foo();
>> 	if (IS_ERR_OR_NULL(ret))
>> 		return ret;
>> 
>> If foo() can never return NULL, the above is misleading, as it puts an
>> obligation on the caller to somehow handle the NULL case and come up with an
>> actual error code for it.
>
> Sure, I get that.  My point is that it's often not clear whether foo() actually can never return
> NULL.  
>
> It's been a while since I've dug through the RPC call chains in Nouveau, so my memory is a little
> hazy here.  I do remember noticing that Nouveau frequently has situations where foo() call bar1()
> and bar2(), where bar1() can return NULL but bar2() can't.  So the question is not whether foo() can
> return NULL, it's whether bar1() should not return NULL, or whether bar2() should.

If there are multiple, it has to be the superset of course.

>> So, I think it is the right call to align that to what functions can actually
>> return, but while doing this, the contract should be properly documented, such
>> that subsequent changes can be properly validated.
>
> "Properly documented" and "Nouveau" are not two things that go together.

Unfortunately -- but the changes submitted by Hongling can add the documentation
for the places that are touched.

@Hongling, can you consider this in a v2 please?

Thanks,
Danilo

Re: [PATCH 0/5] Revert cleanups for IS_ERR_OR_NULL() usage

[Timur Tabi]

On Thu, 2026-05-28 at 15:27 -0400, Lyude Paul wrote:
> Turns out there's many good reasons for this!
> 
> Lyude Paul (5):
>   Revert "nouveau/gsp/rm: cleanup remaining IS_ERR_OR_NULL usage"
>   Revert "nouveau/gsp/rm: cleanup IS_ERR_OR_NULL in core implementation"
>   Revert "nouveau/gsp/rm: cleanup WARN_ON(IS_ERR_OR_NULL)"
>   Revert "nouveau/gsp: cleanup IS_ERR_OR_NULL in rpc_rd"
>   Revert "nouveau/gsp: cleanup IS_ERR_OR_NULL in rm_alloc functions"

Reviewed-by: Timur Tabi <ttabi@nvidia.com>

Re: [PATCH 0/5] Revert cleanups for IS_ERR_OR_NULL() usage

[Danilo Krummrich]

On Thu, 28 May 2026 15:27:14 -0400, Lyude Paul wrote:
> [PATCH 0/5] Revert cleanups for IS_ERR_OR_NULL() usage

Applied, thanks!

  Branch: drm-misc-next-fixes
  Tree:   https://gitlab.freedesktop.org/drm/misc/kernel.git

[1/5] Revert "nouveau/gsp/rm: cleanup remaining IS_ERR_OR_NULL usage"
      commit: ebddb00faa82
[2/5] Revert "nouveau/gsp/rm: cleanup IS_ERR_OR_NULL in core implementation"
      commit: 1a8117455d0d
[3/5] Revert "nouveau/gsp/rm: cleanup WARN_ON(IS_ERR_OR_NULL)"
      commit: dd516da348f1
[4/5] Revert "nouveau/gsp: cleanup IS_ERR_OR_NULL in rpc_rd"
      commit: 80fa10569b90
[5/5] Revert "nouveau/gsp: cleanup IS_ERR_OR_NULL in rm_alloc functions"
      commit: 18178f2ac176

The patches will appear in the next linux-next integration (typically within 24
hours on weekdays).

The patches are queued up for the upcoming merge window for the next major
kernel release.