On Tue, Oct 1, 2024 at 6:58 PM Nhat Pham <nphamcs@xxxxxxxxx> wrote: > > On Tue, Oct 1, 2024 at 6:33 PM Yosry Ahmed <yosryahmed@xxxxxxxxxx> wrote: > > I was debating between WARN-ing here, and returning -ENOMEM and > WARN-ing at shmem's callsite. > > My thinking is that if we return -ENOMEM here, it will work in the > current setup, for both shmem and other callsites. However, in the > future, if we add another user of swap_duplicate_nr(), this time > without guaranteeing that we won't need continuation, I think it won't > work unless we have the fallback logic in place as well: > > while (!err && __swap_duplicate(entry, 1, nr) == -ENOMEM) > err = add_swap_count_continuation(entry, GFP_ATOMIC); Sorry, I accidentally sent out the email without completing my explanation :) Anyway, the point being, with the current implementation, any new user would immediately hit a WARN and the implementer will know to check. Whereas if we return -ENOMEM in __swap_duplicate(), then I think we would just hang, no? We only try to add swap count continuation to the first entry only, which is not sufficient to fix the problem. I can probably whip up the fallback logic here, but it would be dead, untestable code (as it has no users, and I cannot even conceive one to test it). And the swap abstraction might render all of this moot anyway.
