tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: wapbl_flush() speedup

> The attached diff tries to coalesce writes to the journal in MAXPHYS
> sized and aligned blocks.
First, many thanks to hannken@ for ending my monthlong performance nightmare!

Btw., this also explains my tstile lockups: writing the journal could take
tens of seconds; because the journal was locked, most actions on the fs would
stall for that time.

> Comments or objections anyone?
I have a few questions. But I am by far no file system expert. So identifiers
confusing me may be evident for an expert or it may be just my ignorance that
some invariants are not obvious to me.

> +     daddr_t wl_buffer_addr; /* l:   buffer base address */
With "base address" I was thinking of a memory address, not a disc address.
Is it common practice to use xxx_addr for daddrs?

> +     size_t wl_buffer_len;   /* l:   buffer current use */
"len" made me think of allocated length, not used length.
Again, that use may be common practice.

> +      * If not adjacent to buffered dat flush first.

> +         pbn != wl->wl_buffer_addr + btodb(wl->wl_buffer_len)) {
Is it obvious to anyone but me why wl_buffer_addr is initialized at this point?
I'm now sure it is, it just took me some time and a private mail to learn.

> +     KASSERT(resid > 0);
It took me even more time to learn why resid is non-negative here.
Would the explanation be worth a comment?
However, I'm still not sure why it can't be zero.

> +             error = wapbl_doio(wl->wl_buffer, wl->wl_buffer_len,
> +                 wl->wl_devvp, wl->wl_buffer_addr, B_WRITE);
I suppose it's on purpose not to use wapbl_write() here.

Home | Main Index | Thread Index | Old Index