From: "Matthew R. Ochs" <mrochs@linux.vnet.ibm.com>
To: linux-scsi@vger.kernel.org,
James Bottomley <James.Bottomley@HansenPartnership.com>,
"Nicholas A. Bellinger" <nab@linux-iscsi.org>,
Brian King <brking@linux.vnet.ibm.com>,
Ian Munsie <imunsie@au1.ibm.com>,
Daniel Axtens <dja@ozlabs.au.ibm.com>,
Andrew Donnellan <andrew.donnellan@au1.ibm.com>,
Tomas Henzl <thenzl@redhat.com>,
David Laight <David.Laight@ACULAB.COM>
Cc: Michael Neuling <mikey@neuling.org>,
linuxppc-dev@lists.ozlabs.org,
"Manoj N. Kumar" <manoj@linux.vnet.ibm.com>
Subject: [PATCH v4 30/32] cxlflash: Fix to avoid corrupting adapter fops
Date: Fri, 25 Sep 2015 18:19:24 -0500 [thread overview]
Message-ID: <1443223164-10077-1-git-send-email-mrochs@linux.vnet.ibm.com> (raw)
In-Reply-To: <1443222593-8828-1-git-send-email-mrochs@linux.vnet.ibm.com>
The fops owned by the adapter can be corrupted in certain scenarios,
opening a window where certain fops are temporarily NULLed before being
reset to their proper value. This can potentially lead software to make
incorrect decisions, leaving the user with the inability to function as
intended.
An example of this behavior can be observed when there are a number of
users with a high rate of turn around (attach to LUN, perform an I/O,
detach from LUN, repeat). Every so often a user is given a valid
context and adapter file descriptor, but the file associated with the
descriptor lacks the correct read permission bit (FMODE_CAN_READ) and
thus the read system call bails before calling the valid read fop.
Background:
The fops is stored in the adapter structure to provide the ability to
lookup the adapter structure from within the fop handler. CXL services
use the file's private_data and at present, the CXL context does not
have a private section. In an effort to limit areas of the cxlflash
driver with code specific the superpipe function, a design choice was
made to keep the details of the fops situated away from the legacy
portions of the driver. This drove the behavior that the adapter fops
is set at the beginning of the disk attach ioctl handler when there
are no users present.
The corruption that this fix remedies is due to the fact that the fops
is initially defaulted to values found within a static structure. When
the fops is handed down to the CXL services later in the attach path,
certain services are patched. The fops structure remains correct until
the user count drops to 0 and the fops is reset, triggering the process
to repeat again. The user counts are tightly coupled with the creation
and deletion of the user context. If multiple users perform a disk
attach at the same time, when the user count is currently 0, some users
can be in the middle of obtaining a file descriptor and have not yet
reached the context creation code that [in addition to creating the
context] increments the user count. Subsequent users coming in to
perform the attach see that the user count is still 0, and reinitialize
the fops, temporarily removing the patched fops. The users that are in
the middle obtaining their file descriptor may then receive an invalid
descriptor.
The fix simply removes the user count altogether and moves the fops
initialization to probe time such that it is only performed one time
for the life of the adapter. In the future, if the CXL services adopt
a private member for their context, that could be used to store the
adapter structure reference and cxlflash could revert to a model that
does not require an embedded fops.
Signed-off-by: Matthew R. Ochs <mrochs@linux.vnet.ibm.com>
Signed-off-by: Manoj N. Kumar <manoj@linux.vnet.ibm.com>
---
drivers/scsi/cxlflash/common.h | 3 +--
drivers/scsi/cxlflash/main.c | 1 +
drivers/scsi/cxlflash/superpipe.c | 11 +----------
3 files changed, 3 insertions(+), 12 deletions(-)
diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h
index bbfe711..c11cd19 100644
--- a/drivers/scsi/cxlflash/common.h
+++ b/drivers/scsi/cxlflash/common.h
@@ -21,6 +21,7 @@
#include <scsi/scsi.h>
#include <scsi/scsi_device.h>
+extern const struct file_operations cxlflash_cxl_fops;
#define MAX_CONTEXT CXLFLASH_MAX_CONTEXT /* num contexts per afu */
@@ -115,8 +116,6 @@ struct cxlflash_cfg {
struct list_head ctx_err_recovery; /* contexts w/ recovery pending */
struct file_operations cxl_fops;
- atomic_t num_user_contexts;
-
/* Parameters that are LUN table related */
int last_lun_index[CXLFLASH_NUM_FC_PORTS];
int promote_lun_index;
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index be78906..38e7edc 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -2386,6 +2386,7 @@ static int cxlflash_probe(struct pci_dev *pdev,
cfg->init_state = INIT_STATE_NONE;
cfg->dev = pdev;
+ cfg->cxl_fops = cxlflash_cxl_fops;
/*
* The promoted LUNs move to the top of the LUN table. The rest stay
diff --git a/drivers/scsi/cxlflash/superpipe.c b/drivers/scsi/cxlflash/superpipe.c
index 3cc8609..f625e07 100644
--- a/drivers/scsi/cxlflash/superpipe.c
+++ b/drivers/scsi/cxlflash/superpipe.c
@@ -712,7 +712,6 @@ static void destroy_context(struct cxlflash_cfg *cfg,
kfree(ctxi->rht_needs_ws);
kfree(ctxi->rht_lun);
kfree(ctxi);
- atomic_dec_if_positive(&cfg->num_user_contexts);
}
/**
@@ -769,7 +768,6 @@ static struct ctx_info *create_context(struct cxlflash_cfg *cfg,
INIT_LIST_HEAD(&ctxi->luns);
INIT_LIST_HEAD(&ctxi->list); /* initialize for list_empty() */
- atomic_inc(&cfg->num_user_contexts);
mutex_lock(&ctxi->mutex);
out:
return ctxi;
@@ -1164,10 +1162,7 @@ out:
return rc;
}
-/*
- * Local fops for adapter file descriptor
- */
-static const struct file_operations cxlflash_cxl_fops = {
+const struct file_operations cxlflash_cxl_fops = {
.owner = THIS_MODULE,
.mmap = cxlflash_cxl_mmap,
.release = cxlflash_cxl_release,
@@ -1286,10 +1281,6 @@ static int cxlflash_disk_attach(struct scsi_device *sdev,
int fd = -1;
- /* On first attach set fileops */
- if (atomic_read(&cfg->num_user_contexts) == 0)
- cfg->cxl_fops = cxlflash_cxl_fops;
-
if (attach->num_interrupts > 4) {
dev_dbg(dev, "%s: Cannot support this many interrupts %llu\n",
__func__, attach->num_interrupts);
--
2.1.0
next prev parent reply other threads:[~2015-09-25 23:20 UTC|newest]
Thread overview: 68+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-09-25 23:09 [PATCH v4 00/32] cxlflash: Miscellaneous bug fixes and corrections Matthew R. Ochs
2015-09-25 23:12 ` [PATCH v4 01/32] cxlflash: Fix to avoid invalid port_sel value Matthew R. Ochs
2015-09-25 23:12 ` [PATCH v4 02/32] cxlflash: Replace magic numbers with literals Matthew R. Ochs
2015-09-29 5:40 ` Andrew Donnellan
2015-09-25 23:12 ` [PATCH v4 03/32] cxlflash: Fix read capacity timeout Matthew R. Ochs
2015-09-25 23:13 ` [PATCH v4 04/32] cxlflash: Fix potential oops following LUN removal Matthew R. Ochs
2015-09-25 23:13 ` [PATCH v4 05/32] cxlflash: Fix data corruption when vLUN used over multiple cards Matthew R. Ochs
2015-09-25 23:14 ` [PATCH v4 06/32] cxlflash: Fix to avoid sizeof(bool) Matthew R. Ochs
2015-09-28 22:35 ` Daniel Axtens
2015-09-25 23:14 ` [PATCH v4 07/32] cxlflash: Fix context encode mask width Matthew R. Ochs
2015-09-28 22:39 ` Daniel Axtens
2015-09-25 23:14 ` [PATCH v4 08/32] cxlflash: Fix to avoid CXL services during EEH Matthew R. Ochs
2015-09-28 22:07 ` Brian King
2015-09-28 23:05 ` Daniel Axtens
2015-09-29 19:28 ` Matthew R. Ochs
2015-09-25 23:14 ` [PATCH v4 09/32] cxlflash: Correct naming of limbo state and waitq Matthew R. Ochs
2015-09-28 23:09 ` Daniel Axtens
2015-09-25 23:14 ` [PATCH v4 10/32] cxlflash: Make functions static Matthew R. Ochs
2015-09-25 23:14 ` [PATCH v4 11/32] cxlflash: Refine host/device attributes Matthew R. Ochs
2015-09-29 4:29 ` Andrew Donnellan
2015-09-25 23:15 ` [PATCH v4 12/32] cxlflash: Fix to avoid spamming the kernel log Matthew R. Ochs
2015-09-29 5:05 ` Andrew Donnellan
2015-09-29 20:37 ` Matthew R. Ochs
2015-09-25 23:16 ` [PATCH v4 13/32] cxlflash: Fix to avoid stall while waiting on TMF Matthew R. Ochs
2015-09-25 23:16 ` [PATCH v4 14/32] cxlflash: Fix location of setting resid Matthew R. Ochs
2015-09-25 23:16 ` [PATCH v4 15/32] cxlflash: Fix host link up event handling Matthew R. Ochs
2015-09-25 23:16 ` [PATCH v4 16/32] cxlflash: Fix async interrupt bypass logic Matthew R. Ochs
2015-09-25 23:16 ` [PATCH v4 17/32] cxlflash: Remove dual port online dependency Matthew R. Ochs
2015-09-28 23:37 ` Daniel Axtens
2015-09-29 19:38 ` Matthew R. Ochs
2015-09-30 23:50 ` Daniel Axtens
2015-10-01 15:00 ` Matthew R. Ochs
2015-09-25 23:17 ` [PATCH v4 18/32] cxlflash: Fix AFU version access/storage and add check Matthew R. Ochs
2015-09-25 23:17 ` [PATCH v4 19/32] cxlflash: Correct usage of scsi_host_put() Matthew R. Ochs
2015-09-25 23:17 ` [PATCH v4 20/32] cxlflash: Fix to prevent workq from accessing freed memory Matthew R. Ochs
2015-09-25 23:17 ` [PATCH v4 21/32] cxlflash: Correct behavior in device reset handler following EEH Matthew R. Ochs
2015-09-25 23:17 ` [PATCH v4 22/32] cxlflash: Remove unnecessary scsi_block_requests Matthew R. Ochs
2015-09-25 23:18 ` [PATCH v4 23/32] cxlflash: Fix function prolog parameters and return codes Matthew R. Ochs
2015-09-29 4:36 ` Andrew Donnellan
2015-09-29 20:31 ` Matthew R. Ochs
2015-09-25 23:18 ` [PATCH v4 24/32] cxlflash: Fix MMIO and endianness errors Matthew R. Ochs
2015-09-29 1:52 ` Andrew Donnellan
2015-09-25 23:18 ` [PATCH v4 25/32] cxlflash: Fix to prevent EEH recovery failure Matthew R. Ochs
2015-09-29 1:25 ` Daniel Axtens
2015-09-29 20:11 ` Matthew R. Ochs
2015-09-30 23:53 ` Daniel Axtens
2015-09-25 23:18 ` [PATCH v4 26/32] cxlflash: Correct spelling, grammar, and alignment mistakes Matthew R. Ochs
2015-09-29 1:18 ` Andrew Donnellan
2015-09-25 23:19 ` [PATCH v4 27/32] cxlflash: Fix to prevent stale AFU RRQ Matthew R. Ochs
2015-09-29 1:36 ` Daniel Axtens
2015-09-29 20:22 ` Matthew R. Ochs
2015-09-30 23:51 ` Daniel Axtens
2015-09-25 23:19 ` [PATCH v4 28/32] MAINTAINERS: Add cxlflash driver Matthew R. Ochs
2015-09-25 23:19 ` [PATCH v4 29/32] cxlflash: Fix to double the delay each time Matthew R. Ochs
2015-09-29 1:19 ` Andrew Donnellan
2015-09-29 1:40 ` Daniel Axtens
2015-09-29 20:28 ` Matthew R. Ochs
2015-09-30 0:08 ` Daniel Axtens
2015-09-25 23:19 ` Matthew R. Ochs [this message]
2015-09-28 22:13 ` [PATCH v4 30/32] cxlflash: Fix to avoid corrupting adapter fops Brian King
2015-09-29 0:54 ` Andrew Donnellan
2015-09-30 0:18 ` Daniel Axtens
2015-09-25 23:19 ` [PATCH v4 31/32] cxlflash: Correct trace string Matthew R. Ochs
2015-09-29 1:20 ` Andrew Donnellan
2015-09-25 23:19 ` [PATCH v4 32/32] cxlflash: Fix to avoid potential deadlock on EEH Matthew R. Ochs
2015-09-28 23:41 ` Brian King
2015-09-29 19:40 ` Matthew R. Ochs
2015-09-30 0:33 ` Daniel Axtens
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1443223164-10077-1-git-send-email-mrochs@linux.vnet.ibm.com \
--to=mrochs@linux.vnet.ibm.com \
--cc=David.Laight@ACULAB.COM \
--cc=James.Bottomley@HansenPartnership.com \
--cc=andrew.donnellan@au1.ibm.com \
--cc=brking@linux.vnet.ibm.com \
--cc=dja@ozlabs.au.ibm.com \
--cc=imunsie@au1.ibm.com \
--cc=linux-scsi@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=manoj@linux.vnet.ibm.com \
--cc=mikey@neuling.org \
--cc=nab@linux-iscsi.org \
--cc=thenzl@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).