On 03/07/2016 07:07 AM, Khalid Aziz wrote:

On 03/05/2016 09:07 PM, David Miller wrote:

quoted

From: Khalid Aziz <redacted>
Date: Wed,  2 Mar 2016 13:39:37 -0700

quoted

    In this
    first implementation I am enabling ADI for hugepages only
    since these pages are locked in memory and hence avoid the
    issue of saving and restoring tags.

This makes the feature almost entire useless.

Non-hugepages must be in the initial implementation.

Hi David,

Thanks for the feedback. I will get this working for non-hugepages as 
well. ADI state of each VMA region is already stored in the VMA itself 
in my first implementation, so I do not lose it when the page is 
swapped out. The trouble is ADI version tags for each VMA region have 
to be stored on the swapped out pages since the ADI version tags are 
flushed when TLB entry for a page is flushed. 

Khalid,

Are you sure about that last statement? My understanding is that the 
tags are stored in physical memory, and remain there until explicitly 
changed or removed, and so flushing a TLB entry has no effect on the ADI 
tags. If it worked the way you think, then somebody would have to 
potentially reload a long list of ADI tags on every TLB miss.

Rob

When that page is brought back in, its version tags have to be set up 
again. Version tags are set on cacheline boundary and hence there can 
be multiple version tags for a single page. Version tags have to be 
stored in the swap space somehow along with the page. I can start out 
with allowing ADI to be enabled only on pages locked in memory.

quoted

quoted

+    PR_ENABLE_SPARC_ADI - Enable ADI checking in all pages in the 
address
+        range specified. The pages in the range must be already
+        locked. This operation enables the TTE.mcd bit for the
+        pages specified. arg2 is the starting address for address
+        range and must be page aligned. arg3 is the length of
+        memory address range and must be a multiple of page size.

I strongly dislike this interface, and it makes the prtctl cases look
extremely ugly and hide to the casual reader what the code is actually
doing.

This is an mprotect() operation, so add a new flag bit and implement
this via mprotect please.

That is an interesting idea. Adding a PROT_ADI protection to 
mprotect() sounds cleaner. There are three steps to enabling ADI - (1) 
set PSTATE.mcde bit which is not tied to any VMA, (2) set TTE.mcd for 
each VMA, and (3) set the version tag on cacheline using MCD ASI. I 
can combine steps 1 and 2 in one mprotect() call. That will leave 
PR_GET_SPARC_ADICAPS and PR_GET_SPARC_ADI_STATUS prctl commands still 
to be implemented. PR_SET_SPARC_ADI is also used to check if the 
process has PSTATE.mcde bit set. I could use PR_GET_SPARC_ADI_STATUS 
to do that where return values of 0 and 1 mean the same as before and 
possibly add return value of 2 to mean PSTATE.mcde is not set?

quoted

Then since you are guarenteed to have a consistent ADI setting for
every single VMA region, you never "lose" the ADI state when you swap
out.  It's implicit in the VMA itself, because you'll store in the VMA
that this is an ADI region.

I also want this enabled unconditionally, without any Kconfig knobs.

I can remove CONFIG_SPARC_ADI. It does mean this code will be built 
into 32-bit kernels as well but it will be inactive code.

Thanks,
Khalid

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>