On 12/30 2013 23:20 PM, Mark Tinguely wrote:
> On 12/24/13 06:48, Jeff Liu wrote:
>> From: Jie Liu<jeff.liu@xxxxxxxxxx>
>>
>> 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@xxxxxxxxxx>
>> ---
>> 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@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
