+Cc Miguel Ojeda > > > +({ \ > > > + struct efi_runtime_work efi_rts_work; \ > > > + \ > > > + INIT_WORK_ONSTACK(&efi_rts_work.work, efi_call_rts); \ > > > + efi_rts_work.func = _rts; \ > > > + efi_rts_work.arg1 = _arg1; \ > > > + efi_rts_work.arg2 = _arg2; \ > > > + efi_rts_work.arg3 = _arg3; \ > > > + efi_rts_work.arg4 = _arg4; \ > > > + efi_rts_work.arg5 = _arg5; \ > > > + /* \ > > > + * queue_work() returns 0 if work was already on queue, \ > > > + * _ideally_ this should never happen. \ > > > + */ \ > > > + if (queue_work(efi_rts_wq, &efi_rts_work.work)) > > \ > > > + flush_work(&efi_rts_work.work); > > \ > > > + else \ > > > + BUG(); \ > > > > So failure to queue that work is such a critical problem that we need > > to BUG() and can't possibly continue and shoult not attempt recovery at all? > > > > I think it's not critical, we can just return error status. > I think the problem in itself is not at all critical but when I initially thought about > why the problem could have occurred, it sounded like one i.e. ideally (if the > system is running fine) we should always be able to queue work. Failure to queue > means that the previous work is already on queue and that shouldn't be the > case. > So, thought, maybe something bad had happened already (just doubtful). > > But, I see your point. BUG() sounds more like an over kill. Instead of fixing an > existing problem, this patch could completely take down the system. > > > IOW, we should always strive to fail gracefully and not shit in pants > > at the first sign of trouble. > > > > Yes, that makes sense. I will remove BUG() in V3 (in the two places that I > introduced). > > > Even checkpatch warns here: > > > > WARNING: Avoid crashing the kernel - try using WARN_ON & recovery code > > rather than BUG() or BUG_ON() > > #184: FILE: drivers/firmware/efi/runtime-wrappers.c:92: > > + BUG(); \ > > > > Sure! I will fix this > > > > > and by looking at the other output, you should run your patches > > through checkpatch. Some of the things make sense like: > > > > WARNING: quoted string split across lines > > #97: FILE: drivers/firmware/efi/efi.c:341: > > + pr_err("Failed to create efi_rts_workqueue, EFI runtime services " > > + "disabled.\n"); > > > > for example. > > > > I will fix this one too. > > Another warning by checkpatch is "use of in_atomic() in drivers code" > Do you think it's OK to check if were are "in_atomic()" in drivers code. > I wasn't able to decide on other alternative, if possible, could you please suggest > one? > > Regards, > Sai ��.n��������+%������w��{.n�����{����*jg��������ݢj����G�������j:+v���w�m������w�������h�����٥