diff --git a/scst/include/scst.h b/scst/include/scst.h index f8393a340..16990b147 100644 --- a/scst/include/scst.h +++ b/scst/include/scst.h @@ -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 diff --git a/scst/src/scst_lib.c b/scst/src/scst_lib.c index fb9210385..333b5236d 100644 --- a/scst/src/scst_lib.c +++ b/scst/src/scst_lib.c @@ -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 diff --git a/scst/src/scst_pres.c b/scst/src/scst_pres.c index c08378e01..e7210d433 100644 --- a/scst/src/scst_pres.c +++ b/scst/src/scst_pres.c @@ -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; diff --git a/scst/src/scst_pres.h b/scst/src/scst_pres.h index 9829d4fac..acb0e7e49 100644 --- a/scst/src/scst_pres.h +++ b/scst/src/scst_pres.h @@ -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); diff --git a/scst/src/scst_targ.c b/scst/src/scst_targ.c index bf8e641f6..3c0d2afe9 100644 --- a/scst/src/scst_targ.c +++ b/scst/src/scst_targ.c @@ -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);