On Tue, Jan 03, 2023 at 07:20:11PM +0000, Yu, Fenghua wrote: > Hi, Lorenzo, > Hey Fenghua :) > access_remote_vm(mm) directly call __access_remote_vm(mm). > access_process_vm(tsk) calls mm=get_task_mm() then __access_remote_vm(mm). > > So instead of access_remote_vm(mm), it's access_process_vm(tsk) that holds > a reference count on the mm, right? Indeed! > > > > > > > Is there a reason you can't use access_process_vm() which is exported > > > and additionally handles the refrencing? > > IDXD interrupt handler starts a work which needs to access remote vm. > The remote mm is found by PASID which is saved in device event log. > > In the work, it's hard to get the remote mm from a task because mm->owner could be NULL > but the mm is still existing. That makes sense, however I do feel nervous about exporting something that that relies on this reference. The issue is ensuring that the mm can't be taken from underneath you, the only user of access_remote_vm(), procfs, does a careful dance using get_task_mm() and mm_access() to ensure this can't happen, if _sometimes_ the remote mm might have an owner and _sometimes_ not it feels like any exported function needs to be equally careful? I definitely don't feel as if simply exporting this is a safe option, and you would in that case need a new function that handles different scenarios of mm ownership/not. I may be missing something here and I will wait for others to chime in but I think we would definitely need something more than simply exporting this.
