Re: [PATCH 5/5] ceph: fold ceph_update_writeable_page into ceph_write_begin

[David Howells <dhowells@redhat.com>]

Matthew Wilcox <willy@infradead.org> wrote:

> Anyway, looking at netfs_write_begin(), it's wrong too, in a bunch of
> ways.  You don't need to zero out the part of the page you're going to
> copy into.

Zeroing it out isn't 'wrong', per se, just inefficient.  Fixing that needs the
filesystem to deal with it if the copy fails.

> And the condition is overly complicated which makes it
> hard to know what's going on.  Setting aside the is_cache_enabled part,
> I think you want:
> 
> 	if (offset == 0 && len >= thp_size(page))
> 		goto have_page_no_wait;
> 	if (page_offset(page) >= size) {
> 		zero_user_segments(page, 0, offset,
> 				   offset + len, thp_size(page));

There's a third case too: where the write starts at the beginning of the page
and goes to/straddles the EOF - but doesn't continue to the end of the page.

You also didn't set PG_uptodate - presumably deliberately because there's a
hole potentially containing random rubbish in the middle.

> 		goto have_page_no_wait;
> 	}
> 	... read the interesting chunks of page ...

David