Khalid Aziz <khalid.aziz@xxxxxxxxxx> writes: > On 02/07/2018 12:38 AM, ebiederm@xxxxxxxxxxxx wrote: >> Khalid Aziz <khalid.aziz@xxxxxxxxxx> writes: >> >>> On 02/01/2018 07:29 PM, ebiederm@xxxxxxxxxxxx wrote: >>>> Khalid Aziz <khalid.aziz@xxxxxxxxxx> writes: >>>> >>>>> V11 changes: >>>>> This series is same as v10 and was simply rebased on 4.15 kernel. Can >>>>> mm maintainers please review patches 2, 7, 8 and 9 which are arch >>>>> independent, and include/linux/mm.h and mm/ksm.c changes in patch 10 >>>>> and ack these if everything looks good? >>>> >>>> I am a bit puzzled how this differs from the pkey's that other >>>> architectures are implementing to achieve a similar result. >>>> >>>> I am a bit mystified why you don't store the tag in a vma >>>> instead of inventing a new way to store data on page out. >>> >>> Hello Eric, >>> >>> As Steven pointed out, sparc sets tags per cacheline unlike pkey. This results >>> in much finer granularity for tags that pkey and hence requires larger tag >>> storage than what we can do in a vma. >> >> *Nod* I am a bit mystified where you keep the information in memory. >> I would think the tags would need to be stored per cacheline or per >> tlb entry, in some kind of cache that could overflow. So I would be >> surprised if swapping is the only time this information needs stored >> in memory. Which makes me wonder if you have the proper data >> structures. >> >> I would think an array per vma or something in the page tables would >> tend to make sense. >> >> But perhaps I am missing something. > > The ADI tags are stored in spare bits in the RAM. ADI tag storage is > managed entirely by memory controller which maintains these tags per > ADI block. An ADI block is the same size as cacheline on M7. Tags for > each ADI block are associated with the physical ADI block, not the > virtual address. When a physical page is reused, the physical ADI tag > storage for that page is overwritten with new ADI tags, hence we need > to store away the tags when we swap out a page. Kernel updates the ADI > tags for physical page when it swaps a new page in. Each vma can cover > variable number of pages so it is best to store a pointer to the tag > storage in vma as opposed to actual tags in an array. Each 8K page can > have 128 tags on it. Since each tag is 4 bits, we need 64 bytes per > page to store the tags. That can add up for a large vma. If the tags are already stored in RAM I can see why it does not make any sense to store them except on page out. Management wise this feels a lot like the encrypted memory options I have been seeing on x86. >>>> Can you please use force_sig_fault to send these signals instead >>>> of force_sig_info. Emperically I have found that it is very >>>> error prone to generate siginfo's by hand, especially on code >>>> paths where several different si_codes may apply. So it helps >>>> to go through a helper function to ensure the fiddly bits are >>>> all correct. AKA the unused bits all need to be set to zero before >>>> struct siginfo is copied to userspace. >>>> >>> >>> What you say makes sense. I followed the same code as other fault handlers for >>> sparc. I could change just the fault handlers for ADI related faults. Would it >>> make more sense to change all the fault handlers in a separate patch and keep >>> the code in arch/sparc/kernel/traps_64.c consistent? Dave M, do you have a >>> preference? >> >> It is my intention post -rc1 to start sending out patches to get the >> rest of not just sparc but all of the architectures using the new >> helpers. I have the code I just ran out of time befor the merge >> window opened to ensure everything had a good thorough review. >> >> So if you can handle the your new changes I expect I will handle the >> rest. >> > > I can add a patch at the end of my series to update all > force_sig_info() in my patchset to force_sig_fault(). That will sync > my patches up with your changes cleanly. Does that work for you? I can > send an updated series with this change. Can you review and ack the > patches after this change. One additional patch would be fine. I can certainly review and ack that part. You probably want to wait until post -rc1 so that you have a clean base to work off of. Eric -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html