cxlflash: Fix to avoid bypassing context cleanup
authorMatthew R. Ochs <mrochs@linux.vnet.ibm.com>
Wed, 21 Oct 2015 20:16:32 +0000 (15:16 -0500)
committerJames Bottomley <JBottomley@Odin.com>
Fri, 30 Oct 2015 08:25:34 +0000 (17:25 +0900)
Contexts may be skipped over for cleanup in situations where contention
for the adapter's table-list mutex is experienced in the presence of a
signal during the execution of the release handler.

This can lead to two known issues:

 - A hang condition on remove as that path tries to wait for users to
   cleanup - something that will never complete should this scenario play
   out as the user has already cleaned up from their perspective.

 - An Oops in the unmap_mapping_range() call that is made as part of
   the user waiting mechanism that is invoked on remove when contexts
   are found to still exist.

The root cause of this issue can be found in get_context() and how the
table-list mutex is acquired. As this code path is shared by several
different access points within the driver, a decision was made during
the development cycle to acquire this mutex in this location using the
interruptible version of the mutex locking service. In almost all of
the use-cases and environmental scenarios this holds up, even when the
mutex is contended. However, for critical system threads (such as the
release handler), failing to acquire the mutex and bailing with the
intention of the user being able to try again later is unacceptable.

In such a scenario, the context _must_ be derived as it is on an
irreversible path to being freed. Without being able to derive the
context, the code mistakenly assumes that it has already been freed
and proceeds to free up the underlying CXL context resources. From
this point on, any usage of [the now stale] CXL context resources
will result in undefined behavior. This is root cause of the Oops
mentioned as the second known issue as the mapping passed to the
unmap_mapping_range() service is owned by the CXL context.

To fix this problem, acquisition of the table-list mutex within
get_context() is simply changed to use the uninterruptible version
of the mutex locking service. This is safe as the timing windows for
holding this mutex are short and also protected against blocking.

Signed-off-by: Matthew R. Ochs <mrochs@linux.vnet.ibm.com>
Acked-by: Manoj Kumar <manoj@linux.vnet.ibm.com>
Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
Signed-off-by: James Bottomley <JBottomley@Odin.com>
drivers/scsi/cxlflash/superpipe.c

index 18a5e11aac3a0d43fd2741eca2d918b7f1b34542..cac2e6a50efd83ab001bdb84cf56fb2d8d1d35b2 100644 (file)
@@ -162,10 +162,7 @@ struct ctx_info *get_context(struct cxlflash_cfg *cfg, u64 rctxid,
 
        if (likely(ctxid < MAX_CONTEXT)) {
                while (true) {
-                       rc = mutex_lock_interruptible(&cfg->ctx_tbl_list_mutex);
-                       if (rc)
-                               goto out;
-
+                       mutex_lock(&cfg->ctx_tbl_list_mutex);
                        ctxi = cfg->ctx_tbl[ctxid];
                        if (ctxi)
                                if ((file && (ctxi->file != file)) ||