Re: [PATCH v3] dax: Change return type to vm_fault_t
On Fri, 27 Apr 2018, Matthew Wilcox wrote:
> > Hi Matthew/ Ross,
> >
> > There are two changes exist in mm/huge_memory.c as part of this
> > patch. vmf_insert_pfn_pmd() and vmf_insert_pfn_pud() functions are
> > invoked from this patch.
> >
> > Shall we put both in a single patch that it will easy to bisect in case
> > we have any issue ?
> >
>
> Please put them into a single patch, there's no reason to convert
>
> int foo() -> vm_fault_t foo()
>
> but leave
>
> int bar()
> {
> return foo()
> }
>
> It would be best just to convert all callers to also return vm_fault_t as
> I outlined in my response.
Yes, they're all getting converted, but there are too many to do in a
single patch. So it's just a matter of how to split them up. Since the
types are compatible (for now), I advised Souptick to split them by
maintenance area in order to minimise conflicts with other patches.
All I'm saying is that it is 1000 times easier to review and audit if foo
and bar are converted in the same patch above instead of getting feedback
saying "oh, you converted foo() but not bar()" or vice versa and then
referring to another patch posted on some other mailing list as was done
here. How big the patch isn't very important if it's more reviewable.
And converting foo() here and not bar() does nothing but make it harder to
review.