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>