Re: Status of buffered write path (deadlock fixes)

[Mark Fasheh]

On Tue, Dec 12, 2006 at 02:52:26AM +1100, Nick Piggin wrote:
> Nick Piggin wrote:
> >Mark Fasheh wrote:
> 
> >>->commit_write() would probably do fine. Currently, block_prepare_write()
> >>uses it to know which buffers were newly allocated (the file system 
> >>specific
> >>get_block_t sets the bit after allocation). I think we could safely move
> >>the clearing of that bit to block_commit_write(), thus still allowing 
> >>us to
> >>detect and zero those blocks in generic_file_buffered_write()
> >
> >
> >OK, great, I'll make a few patches and see how they look. What did you
> >think of those other uninitialised buffer problems in my first email?
> 
> Hmm, doesn't look like we can do this either because at least GFS2
> uses BH_New for its own special things.
> 
> Also, I don't know if the trick of only walking over BH_New buffers
> will work anyway, since we may still need to release resources on
> other buffers as well.

Oh, my idea was that only the range passed to ->commit() would be walked,
but any BH_New buffers (regardless of where they are in the page) would be
passed to the journal as well. So the logic would be:

for all the buffers in the page:
  If the buffer is new, or it is within the range passed to commit, pass to
  the journal.

Is there anything I'm still missing here?


> As you say, filesystems are simply not set up to expect this, which is a
> problem.
> 
> Maybe it isn't realistic to change the API this way, no matter how
> bad it is presently.

We definitely agree. It's not intuitive that the range should change
between ->prepare_write() and ->commit_write() and IMHO, all the issues
we've found are good evidence that this particular approach will be
problematic.


> What if we tackle the problem a different way?
> 
> 1. In the case of no page in the pagecache (or an otherwise
> !uptodate page), if the operation is a full-page write then we
> first copy all the user data *then* lock the page *then* insert it
> into pagecache and go on to call into the filesystem.

Silly question - what's preventing a reader from filling the !uptodate page with disk
data while the writer is copying the user buffer into it?


> 2. In the case of a !uptodate page and a partial-page write, then
> we can first bring the page uptodate, then continue (goto 3).
> 
> 3. In the case of an uptodate page, we could perform a full-length
> commit_write so long as we didn't expand i_size further than was
> copied, and were sure to trim off blocks allocated past that
> point.
> 
> This scheme IMO is not as "nice" as the partial commit patches,
> but in practical terms it may be much more realistic.

It seems more realistic in that it makes sure the write is properly setup
before calling into the file system. What do you think is not as nice about
it?
	--Mark

--
Mark Fasheh
Senior Software Developer, Oracle
mark.fasheh@xxxxxxxxxx
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/