On Thu, Aug 29, 2019 at 12:39:11PM -0700, Sukadev Bhattiprolu wrote: > Bharata B Rao [bharata@xxxxxxxxxxxxx] wrote: > > On Wed, Aug 28, 2019 at 08:02:19PM -0700, Sukadev Bhattiprolu wrote: > Where do we serialize two threads attempting to H_SVM_PAGE_IN the same gfn > at the same time? Or one thread issuing a H_SVM_PAGE_IN and another a > H_SVM_PAGE_OUT for the same page? I am not not serializing page-in/out calls on same gfn, I thought you take care of that in UV, guess UV doesn't yet. I can probably use rmap_lock() and serialize such calls in HV if UV can't prevent such calls easily. > > > > + > > > > + if (!trylock_page(dpage)) > > > > + goto out_clear; > > > > + > > > > + *rmap = devm_pfn | KVMPPC_RMAP_DEVM_PFN; > > > > + pvt = kzalloc(sizeof(*pvt), GFP_ATOMIC); > > > > + if (!pvt) > > > > + goto out_unlock; > > If we fail to alloc, we don't clear the KVMPPC_RMAP_DEVM_PFN? Right, I will move the assignment to *rmap to after kzalloc. > > Also, when/where do we clear this flag on an uv-page-out? > kvmppc_devm_drop_pages() drops the flag on a local variable but not > in the rmap? If we don't clear the flag on page-out, would the > subsequent H_SVM_PAGE_IN of this page fail? It gets cleared in kvmppc_devm_page_free(). > > Ok. Nit. thought we can drop the "_fault" in the function name but would > collide the other "alloc_and_copy" function used during H_SVM_PAGE_IN. > If the two alloc_and_copy functions are symmetric, maybe they could > have "page_in" and "page_out" in the (already long) names. Christoph also suggested to reorganize these two calls. Will take care. Regards, Bharata.