Messages in this thread | | | Date | Tue, 29 Aug 2006 00:31:21 +0200 | From | Stefan Richter <> | Subject | Re: [PATCH 0/4] Change return values from queue_work et al. |
| |
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/ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |