On Thu, Aug 8, 2024 at 1:40 AM Huang, Ying <ying.huang@xxxxxxxxx> wrote: > > Kairui Song <ryncsn@xxxxxxxxx> writes: > > [snip] > > > --- a/mm/swapfile.c > > +++ b/mm/swapfile.c > > @@ -450,7 +450,10 @@ static void __free_cluster(struct swap_info_struct *si, struct swap_cluster_info > > lockdep_assert_held(&si->lock); > > lockdep_assert_held(&ci->lock); > > > > - list_move_tail(&ci->list, &si->free_clusters); > > + if (ci->flags) > > + list_move_tail(&ci->list, &si->free_clusters); > > + else > > + list_add_tail(&ci->list, &si->free_clusters); > > If we use list_del_init() to delete the cluster, we can always use > list_move_tail()? If so, the logic can be simplified. Thanks for the suggestion. I feel that list_del_init() generates more instruction than necessary. It is my bad that I leave the discard list without not a list flag bit for it. I do want to clean this up. While we are at it, because the cluster can only belong to one list at a time. We can use a list indicator as integer rather than bits mask. If we give the discard list the same treatment, that should remove the special condition to add a cluster to another list as well. Chris > > ci->flags = CLUSTER_FLAG_FREE; > > ci->order = 0; > > } > > @@ -474,7 +477,6 @@ static void swap_do_scheduled_discard(struct swap_info_struct *si) > > SWAPFILE_CLUSTER); > > > > spin_lock(&si->lock); > > - > > spin_lock(&ci->lock); > > __free_cluster(si, ci); > > memset(si->swap_map + idx * SWAPFILE_CLUSTER, > > @@ -666,7 +668,7 @@ static void cluster_alloc_range(struct swap_info_struct *si, struct swap_cluster > > if (ci->flags & CLUSTER_FLAG_FRAG) > > si->frag_cluster_nr[ci->order]--; > > list_move_tail(&ci->list, &si->full_clusters); > > - ci->flags = 0; > > + ci->flags = CLUSTER_FLAG_FULL; > > } > > } > > -- > Best Regards, > Huang, Ying
