On 03/09/2024 23:05, Yosry Ahmed wrote: > On Tue, Sep 3, 2024 at 2:36 PM Barry Song <21cnbao@xxxxxxxxx> wrote: >> >> On Wed, Sep 4, 2024 at 8:08 AM Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote: >>> >>> On Tue, 3 Sep 2024 11:38:37 -0700 Yosry Ahmed <yosryahmed@xxxxxxxxxx> wrote: >>> >>>>> [ 39.157954] RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000007 >>>>> [ 39.158288] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001 >>>>> [ 39.158634] R13: 0000000000002b9a R14: 0000000000000000 R15: 00007ffd619d5518 >>>>> [ 39.158998] </TASK> >>>>> [ 39.159226] ---[ end trace 0000000000000000 ]--- >>>>> >>>>> After reverting this or Usama's "mm: store zero pages to be swapped >>>>> out in a bitmap", the problem is gone. I think these two patches may >>>>> have some conflict that needs to be resolved. >>>> >>>> Yup. I saw this conflict coming and specifically asked for this >>>> warning to be added in Usama's patch to catch it [1]. It served its >>>> purpose. >>>> >>>> Usama's patch does not handle large folio swapin, because at the time >>>> it was written we didn't have it. We expected Usama's series to land >>>> sooner than this one, so the warning was to make sure that this series >>>> handles large folio swapin in the zeromap code. Now that they are both >>>> in mm-unstable, we are gonna have to figure this out. >>>> >>>> I suspect Usama's patches are closer to land so it's better to handle >>>> this in this series, but I will leave it up to Usama and >>>> Chuanhua/Barry to figure this out :) >> >> I believe handling this in swap-in might violate layer separation. >> `swap_read_folio()` should be a reliable API to call, regardless of >> whether `zeromap` is present. Therefore, the fix should likely be >> within `zeromap` but not this `swap-in`. I’ll take a look at this with >> Usama :-) > > I meant handling it within this series to avoid blocking Usama > patches, not within this code. Thanks for taking a look, I am sure you > and Usama will figure out the best way forward :) Hi Barry and Yosry, Is the best (and quickest) way forward to have a v8 of this with https://lore.kernel.org/all/20240904055522.2376-1-21cnbao@xxxxxxxxx/ as the first patch, and using swap_zeromap_entries_count in alloc_swap_folio in this support large folios swap-in patch? Thanks, Usama
