On 8/4/2023 7:38 AM, Yu Zhao wrote: > On Thu, Aug 3, 2023 at 5:27 PM Yin, Fengwei <fengwei.yin@xxxxxxxxx> wrote: >> >> >> >> On 8/4/2023 4:46 AM, Yu Zhao wrote: >>> On Wed, Aug 2, 2023 at 6:56 AM Yin, Fengwei <fengwei.yin@xxxxxxxxx> wrote: >>>> >>>> " >>>> >>>> On 8/2/2023 8:49 PM, Ryan Roberts wrote: >>>>> On 02/08/2023 13:42, Yin, Fengwei wrote: >>>>>> >>>>>> >>>>>> On 8/2/2023 8:40 PM, Ryan Roberts wrote: >>>>>>> On 02/08/2023 13:35, Yin, Fengwei wrote: >>>>>>>> >>>>>>>> >>>>>>>> On 8/2/2023 6:27 PM, Ryan Roberts wrote: >>>>>>>>> On 28/07/2023 17:13, Yin Fengwei wrote: >>>>>>>>>> In madvise_cold_or_pageout_pte_range() and madvise_free_pte_range(), >>>>>>>>>> folio_mapcount() is used to check whether the folio is shared. But it's >>>>>>>>>> not correct as folio_mapcount() returns total mapcount of large folio. >>>>>>>>>> >>>>>>>>>> Use folio_estimated_sharers() here as the estimated number is enough. >>>>>>>>>> >>>>>>>>>> Yin Fengwei (2): >>>>>>>>>> madvise: don't use mapcount() against large folio for sharing check >>>>>>>>>> madvise: don't use mapcount() against large folio for sharing check >>>>>>>>>> >>>>>>>>>> mm/huge_memory.c | 2 +- >>>>>>>>>> mm/madvise.c | 6 +++--- >>>>>>>>>> 2 files changed, 4 insertions(+), 4 deletions(-) >>>>>>>>>> >>>>>>>>> >>>>>>>>> As a set of fixes, I agree this is definitely an improvement, so: >>>>>>>>> >>>>>>>>> Reviewed-By: Ryan Roberts >>>>>>>> Thanks. >>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> But I have a couple of comments around further improvements; >>>>>>>>> >>>>>>>>> Once we have the scheme that David is working on to be able to provide precise >>>>>>>>> exclusive vs shared info, we will probably want to move to that. Although that >>>>>>>>> scheme will need access to the mm_struct of a process known to be mapping the >>>>>>>>> folio. We have that info, but its not passed to folio_estimated_sharers() so we >>>>>>>>> can't just reimplement folio_estimated_sharers() - we will need to rework these >>>>>>>>> call sites again. >>>>>>>> Yes. This could be extra work. Maybe should delay till David's work is done. >>>>>>> >>>>>>> What you have is definitely an improvement over what was there before. And is >>>>>>> probably the best we can do without David's scheme. So I wouldn't delay this. >>>>>>> Just pointing out that we will be able to make it even better later on (if >>>>>>> David's stuff goes in). >>>>>> Yes. I agree that we should wait for David's work ready and do fix based on that. >>>>> >>>>> I was suggesting the opposite - not waiting. Then we can do separate improvement >>>>> later. >>>> Let's wait for David's work ready. >>> >>> Waiting is fine as long as we don't miss the next merge window -- we >>> don't want these two bugs to get into another release. Also I think we >>> should cc stable, since as David mentioned, they have been causing >>> selftest failures. >> >> Stable was CCed. > > Need to add the "Cc: stable@xxxxxxxxxxxxxxx" tag: > Documentation/process/stable-kernel-rules.rst OK. Thanks for clarification. I totally mis-understanded this. :). I'd like to wait for answer from Andrew whether these patches are suitable for stable (I suppose you think so) branch. Regards Yin, Fengwei
