Re: [PATCH v2] block: Fix elv_iosched_local_module handling of "none" scheduler

[Andreas Hindborg <a.hindborg@kernel.org>]

Hi Damien,

Damien Le Moal <dlemoal@kernel.org> writes:

> Commit 734e1a860312 ("block: Prevent deadlocks when switching
> elevators") introduced the function elv_iosched_load_module() to allow
> loading an elevator module outside of elv_iosched_store() with the
> target device queue not frozen, to avoid deadlocks. However, the "none"
> scheduler does not have a module and as a result,
> elv_iosched_load_module() always returns an error when trying to switch
> to this valid scheduler.

The commit message here is a bit misleading. The problem is not that
`request_module` can fail, the problem is that some failure modes cause
this function to return a positive integer. This is not caught by
callers and it ends up causing all kinds of problems in user space.

Perhaps it makes sense to check for a positive return value at the call
site of the `load_module` pointer in `queue_attr_store`, so this does
not repeat at some point in the future?

Or maybe document that `load_module` implementations should not return a
positive value unless it actually wants to send this to user space as
the result of a write to the `scheduler` sysfs file?

Best regards,
Andreas