Hi Martin,
Sorry for being so slow to respond: just back from vacation,
and masses of mail to go through.
On Wed, 3 Jul 2013, Martin Schwidefsky wrote:
> Hi Hugh,
>
> I still have the patch below in my patch heap. Should I just go ahead and
> add it to my s390-tree or do you prefer to take care of it yourself ?
I didn't realize that you had taken that in. Mel had acked your original
patch, I raised this concern, but nobody agreed or disagreed with me
(and you didn't persuade rmk to join you in __ARCH_WANT_PTE_WRITE_DIRTY).
When I saw your original go to linux-next, then to Linus, I had to ask
myself how much I still cared about it, given everything else going on.
I decided that I didn't care enough to spend time on it, so just let it
drop; and find myself still feeling that way. I still don't like your
PageDirty buried in mk_pte, and fear we may cause s390 unforeseen trouble
in future because of it; but now that it is upstream, I'm inclined to let
it rest until a problem is demonstrated - it can't hurt anyone but s390.
If you feel differently, and think it is better with the patch below (I
didn't realize that I had persuaded you), then by all means send it in
to Linus. As you know, I personally prefer an explicit CONFIG_S390, but
clearly you disagree with me on that, and I wouldn't be surprised if
Linus and everyone else share your view.
Thanks for caring!
Hugh
>
> --
> Subject: [PATCH] s390/mm: move PageDirty check from mk_pte to common code
>
> Hugh Dickins commented on the software dirty bit implementation and he
> does not like the fact that mk_pte uses PageDirty under the covers.
> His suggestion is to move the PageDirty check into the __do_fault
> function.
>
> Signed-off-by: Martin Schwidefsky <schwidefsky@xxxxxxxxxx>
> ---
> arch/s390/include/asm/pgtable.h | 9 +++------
> mm/memory.c | 12 ++++++++++++
> 2 files changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/arch/s390/include/asm/pgtable.h b/arch/s390/include/asm/pgtable.h
> index 68e6168..d56dc6d 100644
> --- a/arch/s390/include/asm/pgtable.h
> +++ b/arch/s390/include/asm/pgtable.h
> @@ -1260,13 +1260,8 @@ static inline pte_t mk_pte_phys(unsigned long physpage, pgprot_t pgprot)
> static inline pte_t mk_pte(struct page *page, pgprot_t pgprot)
> {
> unsigned long physpage = page_to_phys(page);
> - pte_t __pte = mk_pte_phys(physpage, pgprot);
>
> - if ((pte_val(__pte) & _PAGE_SWW) && PageDirty(page)) {
> - pte_val(__pte) |= _PAGE_SWC;
> - pte_val(__pte) &= ~_PAGE_RO;
> - }
> - return __pte;
> + return mk_pte_phys(physpage, pgprot);
> }
>
> #define pgd_index(address) (((address) >> PGDIR_SHIFT) & (PTRS_PER_PGD-1))
> @@ -1599,6 +1594,8 @@ extern int s390_enable_sie(void);
> static inline void pgtable_cache_init(void) { }
> static inline void check_pgt_cache(void) { }
>
> +#define __ARCH_WANT_PTE_WRITE_DIRTY
> +
> #include <asm-generic/pgtable.h>
>
> #endif /* _S390_PAGE_H */
> diff --git a/mm/memory.c b/mm/memory.c
> index 1207cef..765d5f2 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -3417,6 +3417,18 @@ static int __do_fault(struct mm_struct *mm, struct vm_area_struct *vma,
> dirty_page = page;
> get_page(dirty_page);
> }
> +#ifdef __ARCH_WANT_PTE_WRITE_DIRTY
> + /*
> + * Architectures that use software dirty bits may
> + * want to set the dirty bit in the pte if the pte
> + * is writable and the PageDirty bit is set for the
> + * page. This avoids unnecessary protection faults
> + * for writable mappings which do not use
> + * mapping_cap_account_dirty, e.g. tmpfs and shmem.
> + */
> + else if (pte_write(entry) && PageDirty(page))
> + entry = pte_mkdirty(entry);
> +#endif
> }
> set_pte_at(mm, address, page_table, entry);
>
> --
> blue skies,
> Martin.
>
> "Reality continues to ruin my life." - Calvin.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>