lkml.org 
[lkml]   [2005]   [Sep]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] Fix commit of ordered data buffers
    > > > An alternative is to just lock the buffer in journal_commit_transaction(),
    > > > if it was locked-and-dirty. And remove the call to ll_rw_block() and
    > > > submit the locked buffers by hand.
    > >
    > > Yes, this has the advantage that we can move the buffer to t_locked_list
    > > in the right time and so we don't change the semantics of t_locked_list.
    > > OTOH the locking will be a bit more complicated (we'd need to acquire and
    > > drop j_list_lock almost for every bh while currently we do it only once
    > > per batch)
    >
    > Only need to drop the spinlock if test_set_buffer_locked() fails.
    Ahh, good point.

    > > spin_lock(&journal->j_list_lock);
    > > while (commit_transaction->t_sync_datalist) {
    > > jh = commit_transaction->t_sync_datalist;
    > > bh = jh2bh(jh);
    > > journal_grab_journal_head(bh);
    > > if (buffer_dirty(bh)) {
    > > get_bh(bh);
    > > spin_unlock(&journal->j_list_lock);
    > > lock_buffer(bh);
    > > if (buffer_dirty(bh))
    > > /* submit the buffer */
    > > jbd_lock_bh_state(bh);
    > > spin_lock(&journal->j_list_lock);
    > > /* Check that somebody did not move the jh elsewhere */
    > > }
    > > else {
    > > if (!inverted_lock(journal, bh))
    > > goto write_out_data;
    > > }
    > > __journal_temp_unlink_buffer(jh);
    > > __journal_file_buffer(jh, commit_transaction, BJ_Locked);
    > > journal_put_journal_head(bh);
    > > jbd_unlock_bh_state(bh);
    > > }
    > >
    > > If you prefer something like this I can code it up...
    >
    > If the code is conceptually simpler then I think it's worth doing, even if
    > the actual implementation is similarly or even more complex.
    >
    > So yes please, let's see how it looks.
    OK, will do.

    > > > That would mean that if someone had redirtied a buffer which was on
    > > > t_sync_datalist *while* it was under writeout, we'd end up waiting on that
    > > > writeout to complete before submitting more I/O. But I suspect that's
    > > > pretty rare.
    > > >
    > > > One thing which concerns me with your approach is livelocks: if some process
    > > > sits in a tight loop writing to the same part of the same file, will it
    > > > cause kjournald to get stuck?
    > >
    > > No, because as soon as we find the buffer in t_sync_datalist we move
    > > it to t_locked_list and submit it for IO - this case is one reason why I
    > > introduced that new meaning to t_locked_list.
    >
    > Right. But the buffer can be redirtied while it's on t_locked_list, even
    > while the I/O is in flight. What happens then? Will kjournald try to
    > rewrite it?
    No. With my patch journaling code writes only data buffers in
    t_sync_data_list and moves them to t_locked_list even before the actual
    submit so we really write each buffer exactly once in the
    journal_commit_transaction().
    Originally it worked as follows: buffer has been first submitted
    for IO, then if we eventually came over it for the second time and found
    it has been locked, we moved it to t_locked_list. If we found a clean
    buffer in t_sync_data_list we just removed it from the transaction.
    Now the livelock you were talking about was prevented by the clever
    code in journal_dirty_data() that has been (and still is) checking
    whether the buffer is a part of committing transaction and if so, it
    sends it to disk and refiles it to the new transaction.

    Honza

    --
    Jan Kara <jack@suse.cz>
    SuSE CR Labs
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2005-09-15 09:29    [W:0.025 / U:89.368 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site