Hi Thomas, On 2/20/2018 3:21 PM, Thomas Gleixner wrote: > On Tue, 20 Feb 2018, Reinette Chatre wrote: >> On 2/20/2018 9:15 AM, Thomas Gleixner wrote: >>> On Tue, 13 Feb 2018, Reinette Chatre wrote: >>> >>> Are you really sure that the life time rules of plr are correct vs. an >>> application which still has the locked memory mapped? i.e. the following >>> operation: >> >> You are correct. I am not preventing an administrator from removing the >> pseudo-locked region if it is in use. I will fix that. > > The removal is fine and you cannot prevent it w/o introducing a mess, but > you have to make sure that the PLR and the mapped memory are not > vanishing. The refcount rules I outlined are exactly doing that.
Thank you for catching my misunderstanding. Will do. >> Thank you so much for taking the time to do this thorough review and to >> make these suggestions. While I am still digesting the details I do >> intend to follow all (as well as the ones earlier I did not explicitly >> respond to). > > Make your mind up and tell me where I'm wrong before you implement the crap > I suggested blindly, as that will just cause the next reviewer (me or > someone else) to tell _you_ that it is crap :) Will do. I need more time to digest your suggestions because your thorough review provided me plenty to consider. >> Keeping the CLOSID associated with the pseudo-locked region will surely >> make the above simpler since CLOSID's are association with resource >> groups (represented by the directories). I would like to highlight that >> on some platforms there are only a few (for example, 4) CLOSIDs >> available. Not releasing a CLOSID would thus reduce available CLOSIDs >> that are already limited. These platforms do have smaller possible >> bitmasks though (for example, 8 possible bits), which may make light of >> this concern. I thus just add it as informational to the consequence of >> this simplification. > > Yes. If you have 4 CLOSIDs and only 8 CBM bits it really does not matter > much. > >>> Now the remaining thing is the memory allocation and the mmap itself. I >>> really dislike the preallocation of memory right at setup time. Ideally >>> that should be an allocation of the application itself, but the horrid >>> wbinvd stuff kinda prevents that. With that restriction we are more or less >>> bound to immediate allocation and population. >> >> Acknowledged. I am not sure if the current permissions would support >> such a dynamic setup though. At this time the system administrator is >> the one that sets up the pseudo-locked region and can through >> permissions of the character device provide access to these regions to >> user space applications. > > You still would need some interface, e.g. character device which allows you > to hand in the pointer to the user allocated memory and do the cache > priming. So you could use the same permission setup for that character > device. > > The other problem is that we'd need to have MAP_CONTIG first so you > actually can allocate physically contigous memory from user space. Mike is > working on that, but it's not available today. The only way to do so today > (with lots of waste) would be MAP_HUGETLB, which might be an acceptable > constraint up to the point where MAP_CONTIG is available. I recorded this in a pseudo-locking task list as something to consider if the wbinvd requirement goes away at some point. > Though this all depends on the ability to remove the wbinvd > requirement. But even if we can remove that we'd still need to be aware > that the cache priming loop which needs to run with interrupts disabled is > expensive as well and can introduce undesired latencies. Needs all some > thought... Reinette
