Hi Triet,

This does not apply to drm-misc-next, which is the tree accel/rocket
goes through:

  error: patch failed: drivers/accel/rocket/rocket_job.c:189

That is the rocket_job_push() hunk, already fixed there by

  https://lore.kernel.org/all/[email protected]/

with the same NULL check, the same check_add_overflow() and the same
Fixes: tag. Please drop that hunk.

The rocket_job_open() part is still needed. I forced the kmalloc_objs()
there to fail on drm-misc-next (debug-only module param, this kernel has
no fault injection) and open(2) oopses on a ROCK 5B+:

  pc : rocket_job_open+0x64/0xd0 [rocket]
   rocket_open / drm_file_alloc / drm_open_helper / accel_open

With your check, open(2) returns -ENOMEM and the log stays clean. Worth
mentioning in the commit message: rocket_open() calls try_module_get()
before rocket_job_open(), so that oops also leaks the module reference
and rmmod is blocked until reboot ("Module rocket is in use").

kfree(scheds) on the drm_sched_entity_init() error path is right too -
rocket_job_close() never runs if rocket_job_open() fails, since
rocket_open() unwinds and .postclose is not called.

>       if (WARN_ON(ret))

Since you are touching this line: ret can be -ENOMEM from
drm_sched_entity_stats_new(). That is an allocation failure, not a driver
bug, so a plain if (ret) seems better than a stack trace.

So a v6 would be just the rocket_job_open() change, rebased. I built and
tested that (this patch minus the push hunk, plus 2/2); happy to give a
Tested-by on it.

Nit: the prefix in this driver is accel/rocket:, not drm/rocket:.

Thanks,
Sidong

Reply via email to