xfs
[Top] [All Lists]

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

To: Mark Tinguely <tinguely@xxxxxxx>
Subject: Re: [PATCH 1/4] xfs: wake up cil->xc_commit_wait while removing ctx from cil->xc_committing
From: Jeff Liu <jeff.liu@xxxxxxxxxx>
Date: Wed, 01 Jan 2014 22:38:36 +0800
Cc: "xfs@xxxxxxxxxxx" <xfs@xxxxxxxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <52C18F56.70709@xxxxxxx>
References: <52B98292.5040002@xxxxxxxxxx> <52C18F56.70709@xxxxxxx>
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.1.0
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

<Prev in Thread] Current Thread [Next in Thread>