Messages in this thread | | | Date | Wed, 13 Mar 2024 10:07:06 +0800 | Subject | Re: [PATCH v2] mm/migrate: put dest folio on deferred split list if source was there. | From | "Yin, Fengwei" <> |
| |
On 3/13/2024 2:46 AM, Matthew Wilcox wrote: > On Tue, Mar 12, 2024 at 02:32:43PM -0400, Zi Yan wrote: >> On 12 Mar 2024, at 12:38, Matthew Wilcox wrote: >>> Folios with a positive refcount are >>> removed from the per-node or per-cgroup list _at which point there is >>> an undocumented assumption_ that they will not be removed from the >>> local list because they have a positive refcount. >> >> But that sounds very subtle if not broken. As an outsider of > > I merely deduced this requirement; I didn't come up with it ... My understanding is that this requirement is because of just local list in deferred_split_scan().
Using fbatch instead of local list here as your created for that issue debugging can eliminate this subtlety?
Regards Yin, Fengwei
> >> deferred_split_scan(), only !list_empty(folio->_deferred_list) is checked. >> The condition can be true if the folio is on split_queue or >> local list of deferred_split_scan() with elevated refcount. In that case, >> the folio cannot be removed from the list (either split_queue or local list) >> even if split_queue_lock is held, since local list manipulation is not under >> split_queue_lock. This makes _deferred_list a one-way train to anyone >> except deferred_split_scan(), namely folios can only be added into >> _deferred_list until they are freed or split by deferred_split_scan(). >> >> Is that intended? If yes, maybe we should document it. If not, using >> split_queue_lock to protect local list, or more explicitly folio->_deferred_list >> might be better? > > To be fair, the folio can be split by anybody as > split_huge_page_to_list_to_order() is careful to only manipulate the > deferred list while the refcount is frozen at 0. I'm still trying to > figure out where to document this behaviour of the deferred list that > someone (for example, your good self) would actually see it. > > >
| |