Re: [PATCH] zsmalloc: return -EAGAIN for zspage migration lock contention

[Sergey Senozhatsky <senozhatsky@chromium.org>]

On (26/03/18 19:21), Sergey Senozhatsky wrote:
> On (26/03/18 15:31), Hui Zhu wrote:
> > movable_operations::migrate_page() should return -EAGAIN for temporary
> > migration failures so the migration core can retry. Other negative
> > errors are treated as permanent failures.
> > 
> > zs_page_migrate() currently returns -EINVAL when zspage_write_trylock()
> > fails. That path reflects transient lock contention, not invalid input.
> > Returning -EINVAL misclassifies the failure and can reduce migration
> > success under contention.
> 
> The zspage's reader-lock owner can be preempted for unknown duration,
> so I don't know if returning -EAGAIN is safe.  On one hand, migration
> attempts are limited by NR_MAX_MIGRATE_PAGES_RETRY, on the other hand,
> those attempts are performed basically immediately (?), I'm not sure
> if we have good chances for preempted reader-lock owner to get scheduled
> and release the lock in time.

.. or reader-lock owner can simply perform decompression with
a very slow algorithm (e.g. deflate) which can taks significantly
more time than NR_MAX_MIGRATE_PAGES_RETRY migration atteempts.

So I agree that -EINVAL is not the best error code to return here.
Not sure if -EAGAIN is the best one either.  Technically, -EBUSY can
be a candidate (this is not exactly what migration code expects and
-EBUSY is not any better than -EINVAL as it's still going to be a
terminal error).