From: Andrew Morton <akpm@linux-foundation.org> To: "Yan, Zheng" <zyan@redhat.com> Cc: LKML <linux-kernel@vger.kernel.org>, linux-fsdevel <linux-fsdevel@vger.kernel.org>, ceph-devel@vger.kernel.org, linux-ext4@vger.kernel.org, viro@zeniv.linux.org.uk, jlayton@redhat.com, linux-mm@kvack.org Subject: Re: [PATCH] mm: save current->journal_info before calling fault/page_mkwrite Date: Wed, 13 Dec 2017 18:30:22 -0800 [thread overview] Message-ID: <20171213183022.adce31de7c5e704b4315e472@linux-foundation.org> (raw) In-Reply-To: <91E1F854-7CE7-4E98-BA87-7E4E55243109@redhat.com> On Thu, 14 Dec 2017 10:20:18 +0800 "Yan, Zheng" <zyan@redhat.com> wrote: > >> + /* > >> + * If the fault happens during write_iter() copies data from > >> + * userspace, filesystem may have set current->journal_info. > >> + * If the userspace memory is mapped to a file on another > >> + * filesystem, fault handler of the later filesystem may want > >> + * to access/modify current->journal_info. > >> + */ > >> + current->journal_info = NULL; > >> ret = vma->vm_ops->fault(vmf); > >> + /* Restore original journal_info */ > >> + current->journal_info = old_journal_info; > >> if (unlikely(ret & (VM_FAULT_ERROR | VM_FAULT_NOPAGE | VM_FAULT_RETRY | > >> VM_FAULT_DONE_COW))) > >> return ret; > > > > Can you explain why you chose these two sites? Rather than, for > > example, way up in handle_mm_fault()? > > I think they are the only two places that code can enter another filesystem hm. Maybe. At this point in time. I'm feeling that doing the save/restore at the highest level is better. It's cheap. > > > > It's hard to believe that a fault handler will alter ->journal_info if > > it is handling a read fault, so perhaps we only need to do this for a > > write fault? Although such an optimization probably isn't worthwhile. > > The whole thing is only about three instructions. > > ceph uses current->journal_info for both read/write operations. I think btrfs also read current->journal_info during read-only operation. (I mentioned this in my previous reply) Quite a lot of filesystems use ->journal_info. Arguably it should be the fs's responsibility to restore the old journal_info value after having used it. But that's a ton of changes :(
WARNING: multiple messages have this Message-ID (diff)
From: Andrew Morton <akpm@linux-foundation.org> To: "Yan, Zheng" <zyan@redhat.com> Cc: LKML <linux-kernel@vger.kernel.org>, linux-fsdevel <linux-fsdevel@vger.kernel.org>, ceph-devel@vger.kernel.org, linux-ext4@vger.kernel.org, viro@zeniv.linux.org.uk, jlayton@redhat.com, linux-mm@kvack.org Subject: Re: [PATCH] mm: save current->journal_info before calling fault/page_mkwrite Date: Wed, 13 Dec 2017 18:30:22 -0800 [thread overview] Message-ID: <20171213183022.adce31de7c5e704b4315e472@linux-foundation.org> (raw) In-Reply-To: <91E1F854-7CE7-4E98-BA87-7E4E55243109@redhat.com> On Thu, 14 Dec 2017 10:20:18 +0800 "Yan, Zheng" <zyan@redhat.com> wrote: > >> + /* > >> + * If the fault happens during write_iter() copies data from > >> + * userspace, filesystem may have set current->journal_info. > >> + * If the userspace memory is mapped to a file on another > >> + * filesystem, fault handler of the later filesystem may want > >> + * to access/modify current->journal_info. > >> + */ > >> + current->journal_info = NULL; > >> ret = vma->vm_ops->fault(vmf); > >> + /* Restore original journal_info */ > >> + current->journal_info = old_journal_info; > >> if (unlikely(ret & (VM_FAULT_ERROR | VM_FAULT_NOPAGE | VM_FAULT_RETRY | > >> VM_FAULT_DONE_COW))) > >> return ret; > > > > Can you explain why you chose these two sites? Rather than, for > > example, way up in handle_mm_fault()? > > I think they are the only two places that code can enter another filesystem hm. Maybe. At this point in time. I'm feeling that doing the save/restore at the highest level is better. It's cheap. > > > > It's hard to believe that a fault handler will alter ->journal_info if > > it is handling a read fault, so perhaps we only need to do this for a > > write fault? Although such an optimization probably isn't worthwhile. > > The whole thing is only about three instructions. > > ceph uses current->journal_info for both read/write operations. I think btrfs also read current->journal_info during read-only operation. (I mentioned this in my previous reply) Quite a lot of filesystems use ->journal_info. Arguably it should be the fs's responsibility to restore the old journal_info value after having used it. But that's a ton of changes :( -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2017-12-14 2:30 UTC|newest] Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-12-13 3:58 [PATCH] mm: save current->journal_info before calling fault/page_mkwrite Yan, Zheng 2017-12-13 13:53 ` Amon Ott 2017-12-14 0:59 ` Andrew Morton 2017-12-14 0:59 ` Andrew Morton 2017-12-14 2:09 ` Yan, Zheng 2017-12-14 2:09 ` Yan, Zheng 2017-12-14 2:18 ` Andrew Morton 2017-12-14 2:18 ` Andrew Morton 2017-12-14 2:20 ` Yan, Zheng 2017-12-14 2:20 ` Yan, Zheng 2017-12-14 2:30 ` Andrew Morton [this message] 2017-12-14 2:30 ` Andrew Morton
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20171213183022.adce31de7c5e704b4315e472@linux-foundation.org \ --to=akpm@linux-foundation.org \ --cc=ceph-devel@vger.kernel.org \ --cc=jlayton@redhat.com \ --cc=linux-ext4@vger.kernel.org \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=viro@zeniv.linux.org.uk \ --cc=zyan@redhat.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.