On 06 Dec 10:29, David Hildenbrand wrote: > > On 06.12.24 10:24, Heiko Carstens wrote: > > On Fri, Dec 06, 2024 at 06:27:09AM +0100, Guillaume Morin wrote: > > > On 05 Dec 21:50, Nathan Chancellor wrote: > > > > This looks to be one of the first uses of pud_soft_dirty() in a generic > > > > part of the tree from what I can tell, which shows that s390 is lacking > > > > it despite setting CONFIG_HAVE_ARCH_SOFT_DIRTY: > > > > > > > > $ make -skj"$(nproc)" ARCH=s390 CROSS_COMPILE=s390-linux- mrproper defconfig mm/gup.o > > > > mm/gup.c: In function 'can_follow_write_pud': > > > > mm/gup.c:665:48: error: implicit declaration of function 'pud_soft_dirty'; did you mean 'pmd_soft_dirty'? [-Wimplicit-function-declaration] > > > > 665 | return !vma_soft_dirty_enabled(vma) || pud_soft_dirty(pud); > > > > | ^~~~~~~~~~~~~~ > > > > | pmd_soft_dirty > > > > > > > > Is this expected? > > > > > > Yikes! It does look like an oversight in the s390 code since as you said > > > it has CONFIG_HAVE_ARCH_SOFT_DIRTY and pud_mkdirty seems to be setting > > > _REGION3_ENTRY_SOFT_DIRTY. But I'll let the s390 folks opine. > > > > > > I don't mind dropping the pud part of the change (even if that's a bit > > > of a shame) if it's causing too many issues. > > > > It would be quite easy to add pud_soft_dirty() etc. helper functions > > for s390, but I think that would be the wrong answer to this problem. > > > > s390 implements pud_mkdirty(), but it is only used in the context of > > HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD, which s390 doesn't support. So > > this function should probably be removed from s390's pgtable.h. > > > > Similar the pud_soft_dirty() and friends helper functions should only > > be implemented if common code support for soft dirty would exist, > > which is currently not the case. Otherwise similar fallbacks like for > > pmd_soft_dirty() (-> include/linux/pgtable.h) would also need to be > > implemented. > > > > So IMHO the right fix (at this time) seems to be to remove the above > > pud part of your patch, and in addition we should probably also drop > > the partially implemented pud level soft dirty bits in s390 code, > > since that is dead code and might cause even more confusion in future. > > > > Does that make sense? > > As hugetlb does not support softdirty, and PUDs are currently only possible > (weird DAX thing put aside) with hugetlb, it makes sense to drop the pud > softdirty thingy. Thanks all. I dropped the check and the dummy definition I had to add for i386 in v4 [1] [1] https://lore.kernel.org/linux-mm/Z1MO5slZh8uWl8LH@xxxxxxxxxxxxxxxxxx/T/#u -- Guillaume Morin <guillaume@xxxxxxxxxxx>
