On 03/01/2016 04:31 AM, Michael Ellerman wrote: > On Mon, 2016-29-02 at 11:52:32 UTC, Anshuman Khandual wrote: >> There are certain condition in which H_PROTECT can return error code >> other than H_NOT_FOUND and H_SUCCESS. One such being an attempt to >> update an hpte owned by adjunct partition. Return 0 in that case so >> that user space will retry the access. In adjunct case this mean we >> will not make much progress in the user space. But atleast we get a >> chance to kill the task and avoid taking down the entire box. > > Why is it OK to do nothing and return 0? > > The function's contract is that it either does the update or returns -1, you
Right, the semantics of the function will change with it. The callers of the function include these places. (1) __hash_page_4K (arch/powerpc/mm/hash64_4k.c) (2) __hash_page_4K (arch/powerpc/mm/hash64_64k.c) (3) __hash_page_64K (arch/powerpc/mm/hash64_64k.c) (4) __hash_page_huge (arch/powerpc/mm/hugetlbpage-hash64.c) (5) __hash_page_thp (arch/powerpc/mm/hugepage-hash64.c) All of them get called from hash_page_mm (in turn from hash_page) which is triggered from page fault interrupt (0x300 or 0x400). A return value of 0 from these individual HPTE management functions will return 0 from hash_page_mm as well. This returns the control back to the user space/kernel which would have caused the page fault but without actually setting the right HPTE. This will cause fault again. The semantics change does not affect these callers in any bad way but fakes a correct page fault handling if I am not missing anything. But only __hash_page_4K and __hash_page_64K functions get called from hash_preload (in turn from update_mmu_cache) which is called from a lot of code path including linux page fault handling path through handle_pte_fault. Inside hash_preload, the return value from __hash_page_4K(|64K) function is checked only for -1, detecting which hash_failure_debug is called. The return value is not passed up in the call chain. With H_RESOURCE, hash_preload will pretend as if it did what it was suppose to do and but none of it's callers will explicitly know about it other than the WARN_RATELIMIT message. I believed that hash_preload is a best attempt approach and anything missed here will be corrected with future page faults when the process starts executing. Please correct me if I am wrong. > can't change that without auditing all callers - and describing that in the > change log. Did not get it, you would like to have something like the above code path description in the change log ? _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev