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: > > On Wed, Aug 01, 2018 at 10:38:30AM -0400, Theodore Y. Ts'o wrote: > > > 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
