Hi James,
On Fri, 7 Mar 2025 at 01:53, James Houghton <jthoughton@xxxxxxxxxx> wrote:
>
> On Mon, Mar 3, 2025 at 9:10 AM Fuad Tabba <tabba@xxxxxxxxxx> wrote:
> > +static vm_fault_t kvm_gmem_fault(struct vm_fault *vmf)
> > +{
> > + struct inode *inode = file_inode(vmf->vma->vm_file);
> > + struct folio *folio;
> > + vm_fault_t ret = VM_FAULT_LOCKED;
> > +
> > + filemap_invalidate_lock_shared(inode->i_mapping);
> > +
> > + folio = kvm_gmem_get_folio(inode, vmf->pgoff);
> > + if (IS_ERR(folio)) {
> > + switch (PTR_ERR(folio)) {
> > + case -EAGAIN:
> > + ret = VM_FAULT_RETRY;
> > + break;
> > + case -ENOMEM:
> > + ret = VM_FAULT_OOM;
> > + break;
> > + default:
> > + ret = VM_FAULT_SIGBUS;
> > + break;
>
> Tiny nit-pick: This smells almost like an open-coded vmf_error(). For
> the non-EAGAIN case, can we just call vmf_error()?
I wasn't aware of that. I will fix this on the respin, thanks for
pointing it out.
That said, any idea why vmf_error() doesn't convert -EAGAIN to VM_FAULT_RETRY?
Cheers,
/fuad
> > + }
> > + goto out_filemap;
> > + }
> > +
> > + if (folio_test_hwpoison(folio)) {
> > + ret = VM_FAULT_HWPOISON;
> > + goto out_folio;
> > + }
> > +
> > + /* Must be called with folio lock held, i.e., after kvm_gmem_get_folio() */
> > + if (!kvm_gmem_offset_is_shared(vmf->vma->vm_file, vmf->pgoff)) {
> > + ret = VM_FAULT_SIGBUS;
> > + goto out_folio;
> > + }
