Re: [PATCH 1/4] xfs: wake up cil->xc_commit_wait while removing ctx from cil->xc_committing

[Jeff Liu <jeff.liu@oracle.com>]

On 12/30 2013 23:20 PM, Mark Tinguely wrote:
> On 12/24/13 06:48, Jeff Liu wrote:
>> From: Jie Liu<jeff.liu@oracle.com>
>>
>> I can easily to hit a hang up while running fsstress and shutting down
>> XFS on SSD via the tests below:
<snip>
>>
>>     Task1                    Task2
>>
>>                      list_add(&ctx->committing,&cil->xc_committing);
>>
>> xlog_wait(&cil->xc_commit_wait..)
>> schedule()...
>>
>>                      Aborting!! list_del(&ctx->committing);
>>                      wake_up_all(&cil->xc_commit_wait);<-- MISSING!
>>
>> As a result, we should handle this situation in xlog_cil_committed().
>>
>> Signed-off-by: Jie Liu<jeff.liu@oracle.com>
>> ---
>>   fs/xfs/xfs_log_cil.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c
>> index 5eb51fc..8c7e9c7 100644
>> --- a/fs/xfs/xfs_log_cil.c
>> +++ b/fs/xfs/xfs_log_cil.c
>> @@ -406,6 +406,8 @@ xlog_cil_committed(
>>
>>       spin_lock(&ctx->cil->xc_push_lock);
>>       list_del(&ctx->committing);
>> +    if (abort)
>> +        wake_up_all(&ctx->cil->xc_commit_wait);
>>       spin_unlock(&ctx->cil->xc_push_lock);
>>
>>       xlog_cil_free_logvec(ctx->lv_chain);
> 
> Hi Jeff, I hope you had a good break,
Thanks :)
> 
> So you are saying the wakeup in the CIL push error path missing?
Yes.

> I agree with that. But I don't like adding a new wake up to
> xlog_cil_committed(), which is after the log buffer is written.
IMO this callback would be called if any problem is happened before
the log buffer is written as well, e.g, 
xlog_cil_push()->xfs_log_notify() <-- failed
			| 
        		|->xlog_cil_committed()

Besides, the CTX will be removed from the committing list here but
there might still have waiters sleeping on it.

Thanks,
-Jeff

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs