On Thu, Sep 6, 2018 at 2:08 PM Jan Kara <j...@suse.cz> wrote: > > On Thu 06-09-18 00:54:50, Souptick Joarder wrote: > > On Wed, Sep 5, 2018 at 7:05 PM Theodore Y. Ts'o <ty...@mit.edu> wrote: > > > > > > On Wed, Sep 05, 2018 at 03:20:16PM +0530, Souptick Joarder wrote: > > > > > > > > "fs: convert return type int to vm_fault_t" is still under > > > > review/discusson and not yet merge > > > > into linux-next. I am not seeing it into linux-next tree.Can you > > > > please share the commit id ? > > > > > > It's at: 83c0adddcc6ed128168e7b87eaed0c21eac908e4 in the Linux Next > > > branch. > > > > > > Dmitry, can you try reverting this commit and see if it makes the > > > problem go away? > > > > > > Souptick, can we just NACK this patch and completely drop it from all > > > trees? > > > > Ok, I will correct it and post v3. > > > > > > > > I think we need to be a *lot* more careful about this vm_fault_t patch > > > thing. If you can't be bothered to run xfstests, we need to introduce > > > a new function which replaces block_page_mkwrite() --- and then let > > > each file system try to convert over to it at their own pace, after > > > they've done regression testing. > > > > > > - Ted > > > > Chris has his opinion, > > > > block_page_mkwrite is only called by ext4 and nilfs2 anyway, so > > converting both callers over should not be a problem, as long as > > it actually is done properly. > > > > Matthew's opinion in other mail thread - > > > > > +vm_fault_t block_page_mkwrite(struct vm_area_struct *vma, struct > > > vm_fault *vmf, > > > + get_block_t get_block, int *err) > > > > I don't like returning both the errno and the vm_fault_t. To me that's a > > sign we need to rethink this interface. > > > > I have two suggestions. First, we could allocate a new VM_FAULT_NOSPC > > bit. Second, we could repurpose one of the existing bits, such as > > VM_FAULT_RETRY for this purpose. > > > > > -int ext4_page_mkwrite(struct vm_fault *vmf) > > > +vm_fault_t ext4_page_mkwrite(struct vm_fault *vmf) > > > > I also think perhaps we could start by _not_ converting > > block_page_mkwrite(). > > Just convert ext4_page_mkwrite(), and save converting block_page_mkwrite() > > for later. >
> Yes, I'd start with converting ext4_page_mkwrite() - that should be pretty > straightforward - and we can leave block_page_mkwrite() as is for now. I > don't think allocating other VM_FAULT_ codes is going to cut it as > generally the filesystem may need to communicate different error codes back > and you don't know in advance which are interesting. Then I need to take care of ext4_page_mkwrite() and ext4_filemap_fault() to migrate to use vm_fault_t return type. Everything else can be removed from this patch and it will go as a separate patch. As block_page_mkwrite() is getting called from 2 places in ext4 and nilfs and both places fault handler code convert errno to VM_FAULT_CODE using block_page_mkwrite_return(), is it required to migrate block_page_mkwrite() to use vm_fault_t return type and further complicate the API or better to leave this API in current state ?? > > One solution for passing error codes we could use with vm_fault_t is a > scheme similar to ERR_PTR. So we can store full error code in vm_fault_t > and still have a plenty of space for the special VM_FAULT_ return codes... > With that scheme converting block_page_mkwrite() would be trivial. > I didn't get this part. Any reference code will be helpful ?
