On Fri, Aug 13, 2021 at 3:38 PM Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote: > > [???? ???? ???? ???????? ????? ?????? ?- dan.carpenter@xxxxxxxxxx. ??? ???? ???? ???? ?- http://aka.ms/LearnAboutSenderIdentification.] > > Hello Ofir Bitton, > > The patch d5eb8373b2ce: "habanalabs: replace GFP_ATOMIC with GFP_KERNEL" from Feb 14, 2021, leads to the following Smatch static checker warning: > > drivers/misc/habanalabs/common/command_buffer.c:318 hl_cb_create() > warn: sleeping in atomic context > > drivers/misc/habanalabs/common/command_buffer.c > 311 if (rc) > 312 goto release_cb; > 313 } > 314 > 315 spin_lock(&mgr->cb_lock); > ^^^^^^^^^^^^^ > 316 rc = idr_alloc(&mgr->cb_handles, cb, 1, 0, GFP_ATOMIC); > 317 if (rc < 0) > --> 318 rc = idr_alloc(&mgr->cb_handles, cb, 1, 0, GFP_KERNEL); > ^^^^^^^^^^ This falls back to GFP_KERNEL if GFP_ATOMIC fails. I don't understand that. If it's at all a performance improvement to call idr_alloc() with ATOMIC first, then the correct thing to do is to fix idr_alloc() instead of working around it in the caller. > > 319 spin_unlock(&mgr->cb_lock); > 320 > > The other warning is also idr_alloc() but holding a different spin lock. > > drivers/misc/habanalabs/common/memory.c:126 alloc_device_memory() > warn: sleeping in atomic context > > 124 > 125 spin_lock(&vm->idr_lock); > ^^^^^^^^^^^^^^^^^^^^^^^^ > 126 handle = idr_alloc(&vm->phys_pg_pack_handles, phys_pg_pack, 1, 0, > 127 GFP_KERNEL); > ^^^^^^^^^^ > 128 spin_unlock(&vm->idr_lock); > 129 > 130 if (handle < 0) { > 131 dev_err(hdev->dev, "Failed to get handle for page\n"); > 132 rc = -EFAULT; > 133 goto idr_err; > 134 } > > regards, > dan carpenter Thanks for noticing I will handle it. Ofir
