On Wed, Aug 01, 2018 at 09:13:19AM -0700, Matthew Wilcox wrote:

On Wed, Aug 01, 2018 at 12:06:18PM -0400, Theodore Y. Ts'o wrote:

quoted

On Wed, Aug 01, 2018 at 10:38:30AM -0400, Theodore Y. Ts'o wrote:

quoted

I'm going to drop the whole ext4 changes for vm_fault_t for this
cycle, and I'll let you try to fix it up properly for the next cycle.

Here's the fixed up commit that I'm going to drop since you plan to be
making changes in block_page_mkwrite(), and I don't want us to get out
of sync.

This looks sane to me.

Yeah, it's fine except for the cognitive load to the file system
programmer where block_page_mkwrite() returns an int, and not a
vm_fault_t, and you have use block_page_mkwrite_return() in order to
convert from the negative error code convention to a vm_fault_t.

Souptick has a separate patch out which changes block_page_mkpage() to
return a vm_fault_t.  It's broken in that it clobbers the error return
and doesn't provide a way for the caller to get the error return.  So
Souptick, please consider that other patch to have received a NACK
from me, as it *will* break ext4.

Souptick, perhaps you could change block_page_mkwrite() so that its
function signature looks like this instead:

vm_fault_t block_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf,
	   		      get_block_t get_block, int *err)

...that's sane.  Or you can maybe simply change the *name* of the
function so it's clear it's differnt from all other xxx_page_mkwrite()
functions in that it returns an int.

I'll let you decide what you want to do --- since part of your
development to be a sophisticated programmer is to get experience
making these sorts of decisions that involve having good programming
"taste".

Regards,

						- Ted