2006-05-18 08:28:55

by Menyhart Zoltan

[permalink] [raw]
Subject: write_out_data in JBD

Stephen,

Here is a code fragment starting at "write_out_data:" in
"journal_commit_transaction()":

Let's assume that there is a single "jh" on the list.

write_out_data:
while (commit_transaction->t_sync_datalist) {

jh = commit_transaction->t_sync_datalist;
commit_transaction->t_sync_datalist = jh->b_tnext;

// "commit_transaction->t_sync_datalist" happens always
// to point at our single "jh"

bh = jh2bh(jh);

// Assume not locked
// Assume dirty

if (buffer_dirty(bh)) {
get_bh(bh);
wbuf[bufs++] = bh;
if (bufs == journal->j_wbufsize) {
...
goto write_out_data;
}
} else ...
}

I think our single "jh" will be put on "wbuf[]" repeatedly
("journal->j_wbufsize" times).

Regards,

Zoltan Menyhart


2006-05-19 20:23:10

by Jan Kara

[permalink] [raw]
Subject: Re: write_out_data in JBD

Hello,

> Stephen,
I'm not Stephen but I guess my answer would suffice ;)

> Here is a code fragment starting at "write_out_data:" in
> "journal_commit_transaction()":
>
> Let's assume that there is a single "jh" on the list.
OK.

>
> while (commit_transaction->t_sync_datalist) {
>
> jh = commit_transaction->t_sync_datalist;
> commit_transaction->t_sync_datalist = jh->b_tnext;
>
> // "commit_transaction->t_sync_datalist" happens always
> // to point at our single "jh"
>
> bh = jh2bh(jh);
>
> // Assume not locked
> // Assume dirty
>
> if (buffer_dirty(bh)) {
> get_bh(bh);
> wbuf[bufs++] = bh;
> if (bufs == journal->j_wbufsize) {
Now this would not happen as j_wbufsize is larger than 1. But still
even if it would happen ll_rw_block() would be called, that would clear
a dirty bit and and the next time we see the buffer we take a different
route.
But actually you are right that if we fail with
bufs==journal->j_wbufsize we would go into the next iteration of the
loop and queue the buffer again. Anyway we agreed to rewrite this code
in another thread :). I just have to make sure we don't have a similar
problem in the new version.

> ...
> goto write_out_data;
> }
> } else ...
> }
>
> I think our single "jh" will be put on "wbuf[]" repeatedly
> ("journal->j_wbufsize" times).


Honza
--
Jan Kara <[email protected]>
SuSE CR Labs