On Mon, 20 Jan 2025 at 10:30, Kirill A. Shutemov <kirill@xxxxxxxxxxxxx> wrote:
>
> On Fri, Jan 17, 2025 at 04:29:51PM +0000, Fuad Tabba wrote:
> > +/*
> > + * Marks the range [start, end) as not mappable by the host. If the host doesn't
> > + * have any references to a particular folio, then that folio is marked as
> > + * mappable by the guest.
> > + *
> > + * However, if the host still has references to the folio, then the folio is
> > + * marked and not mappable by anyone. Marking it is not mappable allows it to
> > + * drain all references from the host, and to ensure that the hypervisor does
> > + * not transition the folio to private, since the host still might access it.
> > + *
> > + * Usually called when guest unshares memory with the host.
> > + */
> > +static int gmem_clear_mappable(struct inode *inode, pgoff_t start, pgoff_t end)
> > +{
> > + struct xarray *mappable_offsets = &kvm_gmem_private(inode)->mappable_offsets;
> > + void *xval_guest = xa_mk_value(KVM_GMEM_GUEST_MAPPABLE);
> > + void *xval_none = xa_mk_value(KVM_GMEM_NONE_MAPPABLE);
> > + pgoff_t i;
> > + int r = 0;
> > +
> > + filemap_invalidate_lock(inode->i_mapping);
> > + for (i = start; i < end; i++) {
> > + struct folio *folio;
> > + int refcount = 0;
> > +
> > + folio = filemap_lock_folio(inode->i_mapping, i);
> > + if (!IS_ERR(folio)) {
> > + refcount = folio_ref_count(folio);
> > + } else {
> > + r = PTR_ERR(folio);
> > + if (WARN_ON_ONCE(r != -ENOENT))
> > + break;
> > +
> > + folio = NULL;
> > + }
> > +
> > + /* +1 references are expected because of filemap_lock_folio(). */
> > + if (folio && refcount > folio_nr_pages(folio) + 1) {
>
> Looks racy.
>
> What prevent anybody from obtaining a reference just after check?
>
> Lock on folio doesn't stop random filemap_get_entry() from elevating the
> refcount.
>
> folio_ref_freeze() might be required.
I thought the folio lock would be sufficient, but you're right,
nothing prevents getting a reference after the check. I'll use a
folio_ref_freeze() when I respin.
Thanks,
/fuad
> > + /*
> > + * Outstanding references, the folio cannot be faulted
> > + * in by anyone until they're dropped.
> > + */
> > + r = xa_err(xa_store(mappable_offsets, i, xval_none, GFP_KERNEL));
> > + } else {
> > + /*
> > + * No outstanding references. Transition the folio to
> > + * guest mappable immediately.
> > + */
> > + r = xa_err(xa_store(mappable_offsets, i, xval_guest, GFP_KERNEL));
> > + }
> > +
> > + if (folio) {
> > + folio_unlock(folio);
> > + folio_put(folio);
> > + }
> > +
> > + if (WARN_ON_ONCE(r))
> > + break;
> > + }
> > + filemap_invalidate_unlock(inode->i_mapping);
> > +
> > + return r;
> > +}
>
> --
> Kiryl Shutsemau / Kirill A. Shutemov
