On 11/06/2024 20:33, Nhat Pham wrote:
On Tue, Jun 11, 2024 at 11:50 AM Usama Arif <usamaarif642@xxxxxxxxx> wrote:In swap_writepage, with this patch you have: if (is_folio_zero_filled(folio)) { swap_zeromap_folio_set(folio); folio_unlock(folio); return 0; } swap_zeromap_folio_clear(folio);I was concerned with the swap slot being freed and reused, without ever being read :) But looks like it has to be properly reset before being reused, so all is well on that front. What about the put_swap_folio() -> swap_free_cluster() case - do we need to handle zeromap bit clearing here too? Looks like it's clearing the swap_map (i.e returning it directly to the swapfile, allowing those slots to be reused) here, and I notice that you clear the zeromap bitmap wherever the swap_map is cleared as well :) I jumped around the code a bit - in free_cluster() (called by swap_free_cluster()), there's this chunk: if ((si->flags & (SWP_WRITEOK | SWP_PAGE_DISCARD)) == (SWP_WRITEOK | SWP_PAGE_DISCARD)) { swap_cluster_schedule_discard(si, idx); return; } swap_cluster_schedule_discard() does clear_bit() on the zeromap on the entire cluster. We also clear_bit() in the work function swap_do_scheduled_discard() (is this redundant?). But what if this check is false, i.e the swap device does not have the SWP_PAGE_DISCARD flag set? Are we not clearing the bits in the zeromap here?
Yes, should add in swap_free_cluster as well, will do in next revision. Thanks!
