Re: [for-416 PATCH 1/2] bcache: Fix, improve efficiency of closure_sync()

Kent Overstreet
2018-01-05T19:06:46Z

On Fri, Jan 05, 2018 at 09:15:41AM -0800, Michael Lyle wrote:
> Jens & Kent,
>
> On 01/05/2018 08:05 AM, Jens Axboe wrote:
> > On 12/30/17 4:09 PM, Michael Lyle wrote:
> >> +void __closure_sync(struct closure *cl)
> >> +{
> >> + struct closure_syncer s = { .task = current };
> >>
> >> + cl->s = &s;
> >> + continue_at(cl, closure_sync_fn, NULL);
> >> +
> >> + while (1) {
> >> + __set_current_state(TASK_UNINTERRUPTIBLE);
> >> + smp_mb(); /* Ensure task state set before load of done flag */
> >
> > That's why we have set_current_state().
> >
>
> I wrote the comment in question-- it seemed like to me set_current_state
> and a store w/ barrier, but I was nervous since I didn't write the code
> that there might be another dependency/reason.
>
> Kent-- is there any reason to not just set_current_state(...)?

No, set_current_state() is the right way to do it