Re: [PATCH 0/4] Change return values from queue_work et al.

[Stefan Richter <stefanr@s5r6.in-berlin.de>]

Alan Stern wrote:
[...]
> It turned out these functions were used in ~800 places, and in ~90% of
> them the return value was ignored!  This is perhaps understandble, because
> the only way these functions can fail is if their work_struct argument is
> uninitialized or already in use.  (Whether it's robust for callers to
> depend on this behavior remaining unchanged into the indefinite future is
> more questionable.)

You are changing this behavior right now...

> So I took a short cut which allowed most of the usages to remain as they
> are.  queue_work(), schedule_work(), and their friends still exist and do
> what they did before, but now they return void.  In addition, they call
> WARN_ON if the submission fails; this seems safer than letting the failure
> go silently unnoticed.  
[...]

...by adding a WARN_ON even though "work not enqueued because it is 
already in queue" may not be a "failure" at all.

It _is_ robust for callers to depend on the old behavior. This is 
because /we/ who use these functions will remind /you/ who alters these 
functions to first research what the actual usage is. So please check 
every caller which ignores the return code for the actual intent of the 
caller.

Do not add WARN_ON to queue_work() etc.. Instead add WARN_ON or BUG_ON 
or an actual failure handling to callers which _incorrectly_ expect they 
could add the same instance of work_struct to queues more than once 
before the work was executed.

Furthermore, if you already change the type of widely-used exported 
functions (i.e. you change the workqueue API), why don't you delete 
these functions right away?
-- 
Stefan Richter
-=====-=-==- =--- ===-=
http://arcgraph.de/sr/