On Thu 06-09-18 00:54:50, Souptick Joarder wrote: > On Wed, Sep 5, 2018 at 7:05 PM Theodore Y. Ts'o <tytso@xxxxxxx> 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. 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. Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
