mirror of
https://github.com/SCST-project/scst.git
synced 2026-05-18 11:11:27 +00:00
scst_pres: Simplify PR locking
Since the time during which a PR read or write lock is held is short, use a mutex to implement PR read and write locking. So although this patch excludes multiple simultaneous readers that shouldn't affect the time needed to process a PR operation measurably. Signed-off-by: Bart Van Assche <bvanassche@acm.org> git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@5254 d57e44dd-8a1f-0410-8b47-8ef2f437770f
This commit is contained in:
@@ -1942,9 +1942,6 @@ struct scst_cmd {
|
||||
/* Set if the device was blocked by scst_check_blocked_dev() */
|
||||
unsigned int unblock_dev:1;
|
||||
|
||||
/* Set if this cmd incremented dev->pr_readers_count */
|
||||
unsigned int dec_pr_readers_count_needed:1;
|
||||
|
||||
/* Set if scst_dec_on_dev_cmd() call is needed on the cmd's finish */
|
||||
unsigned int dec_on_dev_needed:1;
|
||||
|
||||
@@ -2420,18 +2417,6 @@ struct scst_device {
|
||||
int block_size;
|
||||
int block_shift;
|
||||
|
||||
/*
|
||||
* Set if dev is persistently reserved. Protected by dev_pr_mutex.
|
||||
* Modified independently to the above field, hence the alignment.
|
||||
*/
|
||||
unsigned int pr_is_set:1 __aligned(sizeof(long));
|
||||
|
||||
/*
|
||||
* Set if there is a thread changing or going to change PR state(s).
|
||||
* Protected by dev_pr_mutex.
|
||||
*/
|
||||
unsigned int pr_writer_active:1;
|
||||
|
||||
struct scst_dev_type *handler; /* corresponding dev handler */
|
||||
|
||||
/* Used for storage of dev handler private stuff */
|
||||
@@ -2460,27 +2445,28 @@ struct scst_device {
|
||||
*/
|
||||
int on_dev_cmd_count;
|
||||
|
||||
/*
|
||||
* How many threads are checking commands for PR allowance.
|
||||
* Protected by dev_lock.
|
||||
*/
|
||||
int pr_readers_count;
|
||||
|
||||
/* Memory limits for this device */
|
||||
struct scst_mem_lim dev_mem_lim;
|
||||
|
||||
/* List of commands with lock, if dedicated threads are used */
|
||||
struct scst_cmd_threads dev_cmd_threads;
|
||||
|
||||
/*************************************************************
|
||||
** Persistent reservation fields. Protected by dev_pr_mutex.
|
||||
*************************************************************/
|
||||
/**********************************************************************
|
||||
* Persistent reservation fields. Protected as follows:
|
||||
* - Reading PR data must be protected via scst_pr_read_lock() /
|
||||
* scst_pr_read_unlock().
|
||||
* - Modifying PR data modifications must be protected via
|
||||
* scst_pr_write_lock() / scst_pr_write_unlock().
|
||||
**********************************************************************/
|
||||
|
||||
/*
|
||||
* True if persist through power loss is activated. Modified
|
||||
* independently to the above field, hence the alignment.
|
||||
* Set if dev is persistently reserved. Modified independently
|
||||
* to the above field, hence the alignment.
|
||||
*/
|
||||
unsigned short pr_aptpl:1 __aligned(sizeof(long));
|
||||
unsigned short pr_is_set:1 __aligned(sizeof(long));
|
||||
|
||||
/* True if persist through power loss is activated. */
|
||||
unsigned short pr_aptpl:1;
|
||||
|
||||
/* Persistent reservation type */
|
||||
uint8_t pr_type;
|
||||
@@ -2500,6 +2486,8 @@ struct scst_device {
|
||||
/* List of dev's registrants */
|
||||
struct list_head dev_registrants_list;
|
||||
|
||||
/* End of persistent reservation fields protected by dev_pr_mutex. */
|
||||
|
||||
/*
|
||||
* Count of connected tgt_devs from transports, which don't support
|
||||
* PRs, i.e. don't have get_initiator_port_transport_id(). Protected
|
||||
|
||||
@@ -5433,8 +5433,7 @@ void scst_free_cmd(struct scst_cmd *cmd)
|
||||
if (unlikely(test_bit(SCST_CMD_ABORTED, &cmd->cmd_flags)))
|
||||
TRACE_MGMT_DBG("Freeing aborted cmd %p", cmd);
|
||||
|
||||
EXTRACHECKS_BUG_ON(cmd->unblock_dev || cmd->dec_on_dev_needed ||
|
||||
cmd->dec_pr_readers_count_needed);
|
||||
EXTRACHECKS_BUG_ON(cmd->unblock_dev || cmd->dec_on_dev_needed);
|
||||
|
||||
/*
|
||||
* Target driver can already free sg buffer before calling
|
||||
|
||||
@@ -1126,15 +1126,16 @@ out:
|
||||
/* Called under scst_mutex */
|
||||
void scst_pr_clear_tgt_dev(struct scst_tgt_dev *tgt_dev)
|
||||
{
|
||||
struct scst_device *dev = tgt_dev->dev;
|
||||
struct scst_dev_registrant *reg;
|
||||
struct scst_tgt_dev *t;
|
||||
|
||||
TRACE_ENTRY();
|
||||
|
||||
if (tgt_dev->registrant != NULL) {
|
||||
struct scst_dev_registrant *reg = tgt_dev->registrant;
|
||||
struct scst_device *dev = tgt_dev->dev;
|
||||
struct scst_tgt_dev *t;
|
||||
|
||||
scst_pr_write_lock(dev);
|
||||
scst_pr_write_lock(dev);
|
||||
|
||||
reg = tgt_dev->registrant;
|
||||
if (reg) {
|
||||
tgt_dev->registrant = NULL;
|
||||
reg->tgt_dev = NULL;
|
||||
|
||||
@@ -1155,10 +1156,10 @@ void scst_pr_clear_tgt_dev(struct scst_tgt_dev *tgt_dev)
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
scst_pr_write_unlock(dev);
|
||||
}
|
||||
|
||||
scst_pr_write_unlock(dev);
|
||||
|
||||
TRACE_EXIT();
|
||||
return;
|
||||
}
|
||||
@@ -2324,11 +2325,10 @@ bool scst_pr_is_cmd_allowed(struct scst_cmd *cmd)
|
||||
struct scst_tgt_dev *tgt_dev = cmd->tgt_dev;
|
||||
struct scst_dev_registrant *reg;
|
||||
uint8_t type;
|
||||
bool unlock;
|
||||
|
||||
TRACE_ENTRY();
|
||||
|
||||
unlock = scst_pr_read_lock(cmd);
|
||||
scst_pr_read_lock(dev);
|
||||
|
||||
TRACE_DBG("Testing if command %s (0x%x) from %s allowed to execute",
|
||||
cmd->op_name, cmd->cdb[0], cmd->sess->initiator_name);
|
||||
@@ -2388,7 +2388,7 @@ bool scst_pr_is_cmd_allowed(struct scst_cmd *cmd)
|
||||
cmd->op_name, cmd->cdb[0], cmd->sess->initiator_name);
|
||||
|
||||
out_unlock:
|
||||
scst_pr_read_unlock(cmd, unlock);
|
||||
scst_pr_read_unlock(dev);
|
||||
|
||||
TRACE_EXIT_RES(allowed);
|
||||
return allowed;
|
||||
|
||||
@@ -54,63 +54,6 @@
|
||||
/* Persistent reservation SCOPE field */
|
||||
#define SCOPE_LU 0x00
|
||||
|
||||
static inline void scst_inc_pr_readers_count(struct scst_cmd *cmd,
|
||||
bool locked)
|
||||
{
|
||||
struct scst_device *dev = cmd->dev;
|
||||
|
||||
EXTRACHECKS_BUG_ON(cmd->dec_pr_readers_count_needed);
|
||||
|
||||
if (!locked)
|
||||
spin_lock_bh(&dev->dev_lock);
|
||||
|
||||
#ifdef CONFIG_SMP
|
||||
EXTRACHECKS_BUG_ON(!spin_is_locked(&dev->dev_lock));
|
||||
#endif
|
||||
|
||||
dev->pr_readers_count++;
|
||||
cmd->dec_pr_readers_count_needed = 1;
|
||||
TRACE_DBG("New inc pr_readers_count %d (cmd %p)", dev->pr_readers_count,
|
||||
cmd);
|
||||
|
||||
if (!locked)
|
||||
spin_unlock_bh(&dev->dev_lock);
|
||||
return;
|
||||
}
|
||||
|
||||
static inline void scst_dec_pr_readers_count(struct scst_cmd *cmd,
|
||||
bool locked)
|
||||
{
|
||||
struct scst_device *dev = cmd->dev;
|
||||
|
||||
if (unlikely(!cmd->dec_pr_readers_count_needed)) {
|
||||
PRINT_ERROR("__scst_check_local_events(x, false) should not "
|
||||
"be called twice (cmd %p, op %x)! Use "
|
||||
"scst_check_local_events() instead.", cmd, cmd->cdb[0]);
|
||||
WARN_ON(1);
|
||||
goto out;
|
||||
}
|
||||
|
||||
if (!locked)
|
||||
spin_lock_bh(&dev->dev_lock);
|
||||
|
||||
#ifdef CONFIG_SMP
|
||||
EXTRACHECKS_BUG_ON(!spin_is_locked(&dev->dev_lock));
|
||||
#endif
|
||||
|
||||
dev->pr_readers_count--;
|
||||
cmd->dec_pr_readers_count_needed = 0;
|
||||
TRACE_DBG("New dec pr_readers_count %d (cmd %p)", dev->pr_readers_count,
|
||||
cmd);
|
||||
|
||||
if (!locked)
|
||||
spin_unlock_bh(&dev->dev_lock);
|
||||
|
||||
out:
|
||||
EXTRACHECKS_BUG_ON(dev->pr_readers_count < 0);
|
||||
return;
|
||||
}
|
||||
|
||||
static inline bool scst_pr_type_valid(uint8_t type)
|
||||
{
|
||||
switch (type) {
|
||||
@@ -126,73 +69,24 @@ static inline bool scst_pr_type_valid(uint8_t type)
|
||||
}
|
||||
}
|
||||
|
||||
static inline bool scst_pr_read_lock(struct scst_cmd *cmd)
|
||||
static inline void scst_pr_read_lock(struct scst_device *dev)
|
||||
{
|
||||
struct scst_device *dev = cmd->dev;
|
||||
bool unlock = false;
|
||||
|
||||
TRACE_ENTRY();
|
||||
|
||||
smp_mb(); /* to sync with scst_pr_write_lock() */
|
||||
if (unlikely(dev->pr_writer_active)) {
|
||||
unlock = true;
|
||||
scst_dec_pr_readers_count(cmd, false);
|
||||
mutex_lock(&dev->dev_pr_mutex);
|
||||
}
|
||||
|
||||
TRACE_EXIT_RES(unlock);
|
||||
return unlock;
|
||||
mutex_lock(&dev->dev_pr_mutex);
|
||||
}
|
||||
|
||||
static inline void scst_pr_read_unlock(struct scst_cmd *cmd, bool unlock)
|
||||
static inline void scst_pr_read_unlock(struct scst_device *dev)
|
||||
{
|
||||
struct scst_device *dev = cmd->dev;
|
||||
|
||||
TRACE_ENTRY();
|
||||
|
||||
if (unlikely(unlock))
|
||||
mutex_unlock(&dev->dev_pr_mutex);
|
||||
else
|
||||
scst_dec_pr_readers_count(cmd, false);
|
||||
|
||||
TRACE_EXIT();
|
||||
return;
|
||||
mutex_unlock(&dev->dev_pr_mutex);
|
||||
}
|
||||
|
||||
static inline void scst_pr_write_lock(struct scst_device *dev)
|
||||
{
|
||||
TRACE_ENTRY();
|
||||
|
||||
mutex_lock(&dev->dev_pr_mutex);
|
||||
|
||||
dev->pr_writer_active = 1;
|
||||
/* to sync with scst_pr_read_lock() and unlock() */
|
||||
smp_mb();
|
||||
|
||||
while (true) {
|
||||
int readers;
|
||||
spin_lock_bh(&dev->dev_lock);
|
||||
readers = dev->pr_readers_count;
|
||||
spin_unlock_bh(&dev->dev_lock);
|
||||
if (readers == 0)
|
||||
break;
|
||||
TRACE_DBG("Waiting for %d readers (dev %p)", readers, dev);
|
||||
msleep(1);
|
||||
}
|
||||
|
||||
TRACE_EXIT();
|
||||
return;
|
||||
}
|
||||
|
||||
static inline void scst_pr_write_unlock(struct scst_device *dev)
|
||||
{
|
||||
TRACE_ENTRY();
|
||||
|
||||
dev->pr_writer_active = 0;
|
||||
mutex_unlock(&dev->dev_pr_mutex);
|
||||
|
||||
TRACE_EXIT();
|
||||
return;
|
||||
}
|
||||
|
||||
int scst_pr_init_dev(struct scst_device *dev);
|
||||
|
||||
@@ -119,8 +119,6 @@ static bool scst_check_blocked_dev(struct scst_cmd *cmd)
|
||||
cmd->dec_on_dev_needed = 1;
|
||||
TRACE_DBG("New inc on_dev_count %d (cmd %p)", dev->on_dev_cmd_count, cmd);
|
||||
|
||||
scst_inc_pr_readers_count(cmd, true);
|
||||
|
||||
if (unlikely(dev->block_count > 0) ||
|
||||
unlikely(dev->dev_double_ua_possible) ||
|
||||
unlikely((cmd->op_flags & SCST_SERIALIZED) != 0))
|
||||
@@ -134,8 +132,6 @@ static bool scst_check_blocked_dev(struct scst_cmd *cmd)
|
||||
cmd->dec_on_dev_needed = 0;
|
||||
TRACE_DBG("New dec on_dev_count %d (cmd %p)",
|
||||
dev->on_dev_cmd_count, cmd);
|
||||
|
||||
scst_dec_pr_readers_count(cmd, true);
|
||||
}
|
||||
|
||||
spin_unlock_bh(&dev->dev_lock);
|
||||
@@ -159,9 +155,6 @@ static void scst_check_unblock_dev(struct scst_cmd *cmd)
|
||||
dev->on_dev_cmd_count, cmd);
|
||||
}
|
||||
|
||||
if (unlikely(cmd->dec_pr_readers_count_needed))
|
||||
scst_dec_pr_readers_count(cmd, true);
|
||||
|
||||
if (unlikely(cmd->unblock_dev)) {
|
||||
TRACE_BLOCK("cmd %p (tag %llu): unblocking dev %s", cmd,
|
||||
(long long unsigned int)cmd->tag, dev->virt_name);
|
||||
@@ -2211,7 +2204,7 @@ static int scst_persistent_reserve_in_local(struct scst_cmd *cmd)
|
||||
if (unlikely(buffer_size <= 0))
|
||||
goto out_done;
|
||||
|
||||
scst_pr_write_lock(dev);
|
||||
scst_pr_read_lock(dev);
|
||||
|
||||
/* We can be aborted by another PR command while waiting for the lock */
|
||||
if (unlikely(test_bit(SCST_CMD_ABORTED, &cmd->cmd_flags))) {
|
||||
@@ -2246,7 +2239,7 @@ out_complete:
|
||||
cmd->completed = 1;
|
||||
|
||||
out_unlock:
|
||||
scst_pr_write_unlock(dev);
|
||||
scst_pr_read_unlock(dev);
|
||||
|
||||
scst_put_buf_full(cmd, buffer);
|
||||
|
||||
@@ -2303,6 +2296,12 @@ static int scst_persistent_reserve_out_local(struct scst_cmd *cmd)
|
||||
goto out_done;
|
||||
}
|
||||
|
||||
buffer_size = scst_get_buf_full_sense(cmd, &buffer);
|
||||
if (unlikely(buffer_size <= 0))
|
||||
goto out_done;
|
||||
|
||||
scst_pr_write_lock(dev);
|
||||
|
||||
/*
|
||||
* Check if tgt_dev already registered. Also by this check we make
|
||||
* sure that table "PERSISTENT RESERVE OUT service actions that are
|
||||
@@ -2314,20 +2313,16 @@ static int scst_persistent_reserve_out_local(struct scst_cmd *cmd)
|
||||
(tgt_dev->registrant == NULL)) {
|
||||
TRACE_PR("'%s' not registered", cmd->sess->initiator_name);
|
||||
scst_set_cmd_error_status(cmd, SAM_STAT_RESERVATION_CONFLICT);
|
||||
goto out_done;
|
||||
goto out_unlock;
|
||||
}
|
||||
|
||||
buffer_size = scst_get_buf_full_sense(cmd, &buffer);
|
||||
if (unlikely(buffer_size <= 0))
|
||||
goto out_done;
|
||||
|
||||
/* Check scope */
|
||||
if ((action != PR_REGISTER) && (action != PR_REGISTER_AND_IGNORE) &&
|
||||
(action != PR_CLEAR) && (cmd->cdb[2] >> 4) != SCOPE_LU) {
|
||||
TRACE_PR("Scope must be SCOPE_LU for action %x", action);
|
||||
scst_set_cmd_error(cmd,
|
||||
SCST_LOAD_SENSE(scst_sense_invalid_field_in_cdb));
|
||||
goto out_put_buf_full;
|
||||
goto out_unlock;
|
||||
}
|
||||
|
||||
/* Check SPEC_I_PT (PR_REGISTER_AND_MOVE has another format) */
|
||||
@@ -2336,7 +2331,7 @@ static int scst_persistent_reserve_out_local(struct scst_cmd *cmd)
|
||||
TRACE_PR("SPEC_I_PT must be zero for action %x", action);
|
||||
scst_set_cmd_error(cmd, SCST_LOAD_SENSE(
|
||||
scst_sense_invalid_field_in_cdb));
|
||||
goto out_put_buf_full;
|
||||
goto out_unlock;
|
||||
}
|
||||
|
||||
/* Check ALL_TG_PT (PR_REGISTER_AND_MOVE has another format) */
|
||||
@@ -2345,11 +2340,9 @@ static int scst_persistent_reserve_out_local(struct scst_cmd *cmd)
|
||||
TRACE_PR("ALL_TG_PT must be zero for action %x", action);
|
||||
scst_set_cmd_error(cmd,
|
||||
SCST_LOAD_SENSE(scst_sense_invalid_field_in_cdb));
|
||||
goto out_put_buf_full;
|
||||
goto out_unlock;
|
||||
}
|
||||
|
||||
scst_pr_write_lock(dev);
|
||||
|
||||
/* We can be aborted by another PR command while waiting for the lock */
|
||||
aborted = test_bit(SCST_CMD_ABORTED, &cmd->cmd_flags);
|
||||
if (unlikely(aborted)) {
|
||||
@@ -2400,7 +2393,6 @@ static int scst_persistent_reserve_out_local(struct scst_cmd *cmd)
|
||||
out_unlock:
|
||||
scst_pr_write_unlock(dev);
|
||||
|
||||
out_put_buf_full:
|
||||
scst_put_buf_full(cmd, buffer);
|
||||
|
||||
out_done:
|
||||
@@ -2438,7 +2430,6 @@ int __scst_check_local_events(struct scst_cmd *cmd, bool preempt_tests_only)
|
||||
/*
|
||||
* The original command passed all checks and not finished yet
|
||||
*/
|
||||
sBUG_ON(cmd->dec_pr_readers_count_needed);
|
||||
res = 0;
|
||||
goto out;
|
||||
}
|
||||
@@ -2455,7 +2446,7 @@ int __scst_check_local_events(struct scst_cmd *cmd, bool preempt_tests_only)
|
||||
if ((cmd->op_flags & SCST_REG_RESERVE_ALLOWED) == 0) {
|
||||
scst_set_cmd_error_status(cmd,
|
||||
SAM_STAT_RESERVATION_CONFLICT);
|
||||
goto out_dec_pr_readers_count;
|
||||
goto out_complete;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2466,8 +2457,7 @@ int __scst_check_local_events(struct scst_cmd *cmd, bool preempt_tests_only)
|
||||
SAM_STAT_RESERVATION_CONFLICT);
|
||||
goto out_complete;
|
||||
}
|
||||
} else
|
||||
scst_dec_pr_readers_count(cmd, false);
|
||||
}
|
||||
}
|
||||
|
||||
/*
|
||||
@@ -2521,10 +2511,6 @@ out:
|
||||
TRACE_EXIT_RES(res);
|
||||
return res;
|
||||
|
||||
out_dec_pr_readers_count:
|
||||
if (cmd->dec_pr_readers_count_needed)
|
||||
scst_dec_pr_readers_count(cmd, false);
|
||||
|
||||
out_complete:
|
||||
res = 1;
|
||||
sBUG_ON(!cmd->completed);
|
||||
|
||||
Reference in New Issue
Block a user