On Wed, Sep 25, 2024 at 3:23 PM Huang, Ying <ying.huang@xxxxxxxxx> wrote:
>
> Nhat Pham <nphamcs@xxxxxxxxx> writes:
>
> [snip]
>
> >
> > My understanding now is that there are two for loops. One for loop
> > that checks the entry's states, and one for loop that does the actual
> > incrementing work (or state modification).
> >
> > We can check in the first for loop, if it is safe to proceed:
> >
> > if (!count && !has_cache) {
> > err = -ENOENT;
> > } else if (usage == SWAP_HAS_CACHE) {
> > if (has_cache)
> > err = -EEXIST;
> > } else if ((count & ~COUNT_CONTINUED) > SWAP_MAP_MAX) {
> > err = -EINVAL;
> > } else if (usage == 1 && nr > 1 && (count & ~COUNT_CONTINUED) >=
> > SWAP_MAP_MAX)) {
> > /* the batched variants currently do not support rollback */
> > err = -ENOMEM;
> > }
> >
> > At this point, IIUC, we have not done any incrementing, so no rollback
> > needed? :)
>
> I think that it's better to add a VM_WARN_ONCE() here. If someone
> enabled 'nr > 1' for __swap_duplicate(), the issue will be more
> explicit.
ying, i guess you missed this is the exact case Nhat is enabling
'nr > 1' for __swap_duplicate(). and this warning is functioning.
and he is trying to support the nr>1 case.
https://lore.kernel.org/linux-mm/20240923231142.4155415-2-nphamcs@xxxxxxxxx/
>
> [snip]
>
> --
> Best Regards,
> Huang, Ying
