在 2016/7/1 12:21, Coly Li 写道:
> 在 16/7/1 上午9:51, wangyijing 写道:
>> Hi Coly, thanks to your review and comments.
>>
>> Commit 77b5a08427e875 ("bcache: don't embed 'return' statements in closure macros")
>> remove the return in continue_at(), so I think we should update the document info
>> about continue_at().
>>
>> Thanks!
>> Yijing.
>
> Hi Yijing,
>
> The original version of continue_at() returns to caller function inside
> the macro, Jens thinks this macro breaks code execution flow implicitly,
> so he moves 'return' out of continue_at() and to follow continue_at() at
> the location where continue_at() is referenced.
>
> So as I suggested, the original author means the code should return to
> the calling function.
>
> But YES, I agree that the comments should be updated, because there is
> no 'return' inside macro continue_at(). We should explicitly point out
> that there should be a 'return' immediately following macro continue_at().
Yes, you are right, it's better to explicitly point out a return needed to follow continue_at()
than remove this document info, I will update this patch, thanks very much!
>
> Thanks.
>
> Coly
>
>
>> 在 2016/6/29 18:16, Coly Li 写道:
>>> 在 16/6/22 上午10:12, Yijing Wang 写道:
>>>> There is no return in continue_at(), update the documentation.
>>>>
>>>
>>> There are 2 modification of this patch. The first one is about a typo,
>>> it is correct.
>>>
>>> But I doubt your second modification is proper. The line removed in your
>>> patch is,
>>>> - * continue_at() also, critically, is a macro that returns the
>>> calling function.
>>>> - * There's good reason for this.
>>>> - *
>>>
>>> I think this is exactly what original author wants to say. It does not
>>> mean return a value, it means return to the calling function. And the
>>> bellowed lines explains the reason.
>>>
>>>> Signed-off-by: Yijing Wang <wangyijing@xxxxxxxxxx>
>>>> ---
>>>> drivers/md/bcache/closure.c | 2 +-
>>>> drivers/md/bcache/closure.h | 3 ---
>>>> 2 files changed, 1 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/md/bcache/closure.c b/drivers/md/bcache/closure.c
>>>> index 9eaf1d6..864e673 100644
>>>> --- a/drivers/md/bcache/closure.c
>>>> +++ b/drivers/md/bcache/closure.c
>>>> @@ -112,7 +112,7 @@ bool closure_wait(struct closure_waitlist *waitlist, struct closure *cl)
>>>> EXPORT_SYMBOL(closure_wait);
>>>>
>>>> /**
>>>> - * closure_sync - sleep until a closure a closure has nothing left to wait on
>>>> + * closure_sync - sleep until a closure has nothing left to wait on
>>>
>>> Yes, this modification is good.
>>>
>>>> *
>>>> * Sleeps until the refcount hits 1 - the thread that's running the closure owns
>>>> * the last refcount.
>>>> diff --git a/drivers/md/bcache/closure.h b/drivers/md/bcache/closure.h
>>>> index 782cc2c..f51188d 100644
>>>> --- a/drivers/md/bcache/closure.h
>>>> +++ b/drivers/md/bcache/closure.h
>>>> @@ -31,9 +31,6 @@
>>>> * passing it, as you might expect, the function to run when nothing is pending
>>>> * and the workqueue to run that function out of.
>>>> *
>>>> - * continue_at() also, critically, is a macro that returns the calling function.
>>>> - * There's good reason for this.
>>>> - *
>>>> * To use safely closures asynchronously, they must always have a refcount while
>>>> * they are running owned by the thread that is running them. Otherwise, suppose
>>>> * you submit some bios and wish to have a function run when they all complete:
>>>>
>>>
>>>
>>
>
>
> .
>
--
To unsubscribe from this list: send the line "unsubscribe linux-bcache" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
