On 17 Mar 2022 at 04:54, Dave Chinner wrote:
> On Wed, Mar 16, 2022 at 04:04:55PM +0530, Chandan Babu R wrote:
>> On 15 Mar 2022 at 12:12, Dave Chinner wrote:
>> > From: Dave Chinner <dchinner@xxxxxxxxxx>
>> >
>> > When the AIL tries to flush the CIL, it relies on the CIL push
>> > ending up on stable storage without having to wait for and
>> > manipulate iclog state directly. However, if there is already a
>> > pending CIL push when the AIL tries to flush the CIL, it won't set
>> > the cil->xc_push_commit_stable flag and so the CIL push will not
>> > actively flush the commit record iclog.
>>
>> I think the above sentence maps to the following snippet from
>> xlog_cil_push_now(),
>>
>> if (list_empty(&cil->xc_cil) || push_seq <= cil->xc_push_seq) {
>> spin_unlock(&cil->xc_push_lock);
>> return;
>> }
>>
>> i.e. if the CIL sequence that we are trying to push is already being pushed
>> then xlog_cil_push_now() returns without queuing work on cil->xc_push_wq.
>>
>> However, the push_seq could have been previously pushed by,
>> 1. xfsaild_push()
>> In this case, cil->xc_push_commit_stable is set to true. Hence,
>> xlog_cil_push_work() will definitely make sure to submit the commit record
>> iclog for write I/O.
>> 2. xfs_log_force_seq() => xlog_cil_force_seq()
>> xfs_log_force_seq() invokes xlog_force_lsn() after executing
>> xlog_cil_force_seq(). Here, A partially filled iclog will be in
>> XLOG_STATE_ACTIVE state. This will cause xlog_force_and_check_iclog() to be
>> invoked and hence the iclog is submitted for write I/O.
>>
>> In both the cases listed above, iclog is guaranteed to be submitted for I/O
>> without any help from the log worker task.
>>
>> Looks like I am missing something obvious here.
>
> Pushes triggered by xlog_cil_push_background() can complete leaving
> the partially filled iclog in ACTIVE state. Then xlog_cil_push_now()
> does nothing because it doesn't trigger a new CIL push and so
> setting the cil->xc_push_commit_stable flag doesn't trigger a flush
> of the ACTIVE iclog.
Ah. I had missed xlog_cil_push_background().
>
> The AIL flush does not use xfs_log_force_seq() because that blocks
> waiting for the entire CIL to hit the disk before it can force the
> last iclog to disk. Hence the second piece of this patch is
> necessary, and that is to call xfs_log_force() if the CIL is empty
> (i.e. the case where xlog_cil_push_now() is a no-op because the
> CIL is empty due to background pushes).
>
Thanks for clarifying my doubts.
--
chandan
