> > > 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 <[email protected]>
SuSE CR Labs
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
| |
 |