Re: [PATCH 1/5]: ufs: missed brelse and wrong baseblk

[Al Viro <viro@ftp.linux.org.uk>]

On Mon, Jun 19, 2006 at 05:17:50PM +0400, Evgeniy Dushistov wrote:
> In case of 1k fragments, msync of two pages
> cause 8 calls of ufs's get_block_t with create == 1,
> they will be consequent because of synchronization.

_What_ synchronization?

To simplify the analysis, have one of those do msync() and another - write().
One triggers writeback, leading to ufs_writepage().  Another leads to call
of ufs_prepare_write().  Note that the latter call is process-synchronous,
so no implicit serialization could apply.

Now, which lock would, in your opinion, provide serialization between these
two calls?  They apply to different pages, so page locks do not help.

And yes, we do need some serialization between get_block_t here, indeed.
As it is, we don't have any.

Note that there is another problem with use of fs/buffer.c helpers.  They
assume that there are 3 states of buffer_head corresponding to fragment:
	* mapped to known disk block
	* known to be in hole
	* not known
And that's where we have a problem.  block_read_full_page() on page 0
will get all buffer_head on that page (one for each fragment) to
the second state.  block_prepare_write() will get all buffer_head on
page 1 to the first state after the callback allocates the first direct block
of file (they will be mapped to the fragments in that block, at offsets 4Kb
and further).

But now we have the buffer_heads on page 0 in the state inconsistent with
the reality - basically, fs/buffer.c helpers will assume that they are
_still_ in the second state (known to be in hole), while in the reality
they should be either in the first or in the third one (mapped to known
disk block or not known).

It's not a fundamental problem; however, it does mean that using these
helpers means using library functions in situation they'd never been designed
for.  IOW, you need very careful analysis of the assumptions made by
the entire bunch and, quite possibly, need versions modified for UFS.