Messages in this thread | | | Date | Tue, 24 Mar 2015 11:55:09 -0400 | From | Tejun Heo <> | Subject | Re: [PATCH 2/4 V5] workqueue: split apply_workqueue_attrs() into 3 stages |
| |
On Wed, Mar 18, 2015 at 12:40:15PM +0800, Lai Jiangshan wrote: > +static void wq_unbound_install_ctx_free(struct wq_unbound_install_ctx *ctx)
Maybe naminig it more consistently with apply_workqueue_attrs() is better? apply_wqattrs_cleanup()?
> { > + int node; > + > + if (ctx) { > + /* put the pwqs */ > + for_each_node(node) > + put_pwq_unlocked(ctx->pwq_tbl[node]); > + put_pwq_unlocked(ctx->dfl_pwq); > + > + free_workqueue_attrs(ctx->attrs); > + } > + > + kfree(ctx); > +}
Wouldn't the following be better? Or at least put kfree(ctx) together with the rest?
if (!ctx) return; the rest;
> + > +/* Allocates the attrs and pwqs for later installment */ > +static struct wq_unbound_install_ctx * > +wq_unbound_install_ctx_prepare(struct workqueue_struct *wq, > + const struct workqueue_attrs *attrs) > +{
apply_wqattrs_prepare()?
... > +out_free: > + free_workqueue_attrs(tmp_attrs); > + > + if (!ctx || !ctx->wq) { > + kfree(new_attrs); > + wq_unbound_install_ctx_free(ctx); > + return NULL; > + } else { > + return ctx; > + } > +}
Let's separate out error return path even if that takes another goto or a duplicate free_workqueue_attrs() call.
> +/* Set the unbound_attr and install the prepared pwqs. Should not fail */ > +static void wq_unbound_install_ctx_commit(struct wq_unbound_install_ctx *ctx)
apply_wqattrs_commit()?
Thanks.
-- tejun
| |