Skip to content

Commit

Permalink
scst_lib: Split scst_ext_block_dev() for clarity and easier maintenance
Browse files Browse the repository at this point in the history 
This patch divides the scst_ext_block_dev() function into two separate
functions to improve code readability and simplify maintenance:

1. scst_sync_ext_block_dev() - This function is for synchronous blocking
   and serves as the equivalent of calling the old scst_ext_block_dev()
   function with the SCST_EXT_BLOCK_SYNC flag.

2. scst_ext_block_dev() - This function is for asynchronous blocking.

Additionally, the patch introduces the helper function
scst_dev_ext_block() to reduce code duplication between the
scst_sync_ext_block_dev() and scst_ext_block_dev() functions.

This patch doesn't change any functionality.
  • Loading branch information
@lnocturno
lnocturno committed Jun 20, 2023
1 parent fc282a8 commit a345160
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 60 deletions.
140 changes: 87 additions & 53 deletions scst/src/scst_lib.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14918,30 +14918,12 @@ static void scst_sync_ext_blocking_done(struct scst_device *dev,
return;
}

int scst_ext_block_dev(struct scst_device *dev, ext_blocker_done_fn_t done_fn,
const uint8_t *priv, int priv_len, int flags)
static void scst_dev_ext_block(struct scst_device *dev, bool block_stpg)
{
int res;
struct scst_ext_blocker *b;
lockdep_assert_held(&dev->dev_lock);

TRACE_ENTRY();

if (flags & SCST_EXT_BLOCK_SYNC)
priv_len = sizeof(void *);

b = kzalloc(sizeof(*b) + priv_len, GFP_KERNEL);
if (b == NULL) {
PRINT_ERROR("Unable to alloc struct scst_ext_blocker with data "
"(size %zd)", sizeof(*b) + priv_len);
res = -ENOMEM;
goto out;
}

TRACE_MGMT_DBG("New %d ext blocker %p for dev %s (flags %x)",
dev->ext_blocks_cnt+1, b, dev->virt_name, flags);

spin_lock_bh(&dev->dev_lock);

if (dev->strictly_serialized_cmd_waiting) {
/*
* Avoid deadlock when this strictly serialized cmd
Expand All @@ -14951,63 +14933,115 @@ int scst_ext_block_dev(struct scst_device *dev, ext_blocker_done_fn_t done_fn,
TRACE_DBG("Unstrictlyserialize dev %s", dev->virt_name);
dev->strictly_serialized_cmd_waiting = 0;
/* We will reuse blocking done by the strictly serialized cmd */
} else
} else {
scst_block_dev(dev);
}

if (flags & SCST_EXT_BLOCK_STPG) {
if (block_stpg) {
WARN_ON(dev->stpg_ext_blocked);
dev->stpg_ext_blocked = 1;
}

dev->ext_blocks_cnt++;
TRACE_DBG("ext_blocks_cnt %d", dev->ext_blocks_cnt);

if ((flags & SCST_EXT_BLOCK_SYNC) && (dev->on_dev_cmd_count == 0)) {
TRACE_EXIT();
}

int scst_sync_ext_block_dev(struct scst_device *dev)
{
struct scst_ext_blocker *b;
DECLARE_WAIT_QUEUE_HEAD_ONSTACK(w);
int res = 0;

TRACE_ENTRY();

b = kzalloc(sizeof(*b) + sizeof(void *), GFP_KERNEL);
if (unlikely(!b)) {
PRINT_ERROR("Unable to alloc struct scst_ext_blocker with data (size %zd)",
sizeof(*b) + sizeof(void *));
res = -ENOMEM;
goto out;
}

TRACE_MGMT_DBG("New %d sync ext blocker %p for dev %s",
dev->ext_blocks_cnt + 1, b, dev->virt_name);

spin_lock_bh(&dev->dev_lock);

scst_dev_ext_block(dev, false);

if (dev->on_dev_cmd_count == 0) {
TRACE_DBG("No commands to wait for sync blocking (dev %s)",
dev->virt_name);
dev->virt_name);
spin_unlock_bh(&dev->dev_lock);
goto out_free_success;
goto out_free;
}

list_add_tail(&b->ext_blockers_list_entry, &dev->ext_blockers_list);
dev->ext_blocking_pending = 1;

if (flags & SCST_EXT_BLOCK_SYNC) {
DECLARE_WAIT_QUEUE_HEAD_ONSTACK(w);
b->ext_blocker_done_fn = scst_sync_ext_blocking_done;
*((void **)&b->ext_blocker_data[0]) = &w;

b->ext_blocker_done_fn = scst_sync_ext_blocking_done;
*((void **)&b->ext_blocker_data[0]) = &w;
scst_wait_event_lock_bh(w, dev->on_dev_cmd_count == 0, dev->dev_lock);

scst_wait_event_lock_bh(w, dev->on_dev_cmd_count == 0, dev->dev_lock);
spin_unlock_bh(&dev->dev_lock);

spin_unlock_bh(&dev->dev_lock);
} else {
b->ext_blocker_done_fn = done_fn;
if (priv_len > 0) {
b->ext_blocker_data_len = priv_len;
memcpy(b->ext_blocker_data, priv, priv_len);
}
if (dev->on_dev_cmd_count == 0) {
TRACE_DBG("No commands to wait for async blocking "
"(dev %s)", dev->virt_name);
if (!dev->ext_unblock_scheduled)
__scst_ext_blocking_done(dev);
spin_unlock_bh(&dev->dev_lock);
} else
spin_unlock_bh(&dev->dev_lock);
out:
TRACE_EXIT_RES(res);
return res;

out_free:
kfree(b);
goto out;
}

int scst_ext_block_dev(struct scst_device *dev, ext_blocker_done_fn_t done_fn,
const void *priv, size_t priv_len, bool block_stpg)
{
struct scst_ext_blocker *b;
int res = 0;

TRACE_ENTRY();

b = kzalloc(sizeof(*b) + priv_len, GFP_KERNEL);
if (unlikely(!b)) {
PRINT_ERROR("Unable to alloc struct scst_ext_blocker with data (size %zd)",
sizeof(*b) + priv_len);
res = -ENOMEM;
goto out;
}

out_success:
res = 0;
TRACE_MGMT_DBG("New %d ext blocker %p for dev %s (block_stpg %d)",
dev->ext_blocks_cnt + 1, b, dev->virt_name, block_stpg);

spin_lock_bh(&dev->dev_lock);

scst_dev_ext_block(dev, block_stpg);

list_add_tail(&b->ext_blockers_list_entry, &dev->ext_blockers_list);
dev->ext_blocking_pending = 1;

b->ext_blocker_done_fn = done_fn;
if (priv_len > 0) {
b->ext_blocker_data_len = priv_len;
memcpy(b->ext_blocker_data, priv, priv_len);
}

if (dev->on_dev_cmd_count == 0) {
TRACE_DBG("No commands to wait for async blocking (dev %s)",
dev->virt_name);

if (!dev->ext_unblock_scheduled)
__scst_ext_blocking_done(dev);
}

spin_unlock_bh(&dev->dev_lock);

out:
TRACE_EXIT_RES(res);
return res;

out_free_success:
sBUG_ON(!(flags & SCST_EXT_BLOCK_SYNC));
kfree(b);
goto out_success;
}

void scst_ext_unblock_dev(struct scst_device *dev, bool stpg)
Expand Down Expand Up @@ -15044,7 +15078,7 @@ void scst_ext_unblock_dev(struct scst_device *dev, bool stpg)
spin_unlock_bh(&dev->dev_lock);
TRACE_DBG("Ext unblock (dev %s): still pending...",
dev->virt_name);
rc = scst_ext_block_dev(dev, NULL, NULL, 0, SCST_EXT_BLOCK_SYNC);
rc = scst_sync_ext_block_dev(dev);
if (rc != 0) {
/* Oops, have to poll */
PRINT_WARNING("scst_ext_block_dev(dev %s) failed, "
Expand Down
5 changes: 2 additions & 3 deletions scst/src/scst_priv.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -611,10 +611,9 @@ bool __scst_check_blocked_dev(struct scst_cmd *cmd);
void __scst_check_unblock_dev(struct scst_cmd *cmd);
void scst_check_unblock_dev(struct scst_cmd *cmd);

#define SCST_EXT_BLOCK_SYNC 1
#define SCST_EXT_BLOCK_STPG 2
int scst_sync_ext_block_dev(struct scst_device *dev);
int scst_ext_block_dev(struct scst_device *dev, ext_blocker_done_fn_t done_fn,
const uint8_t *priv, int priv_len, int flags);
const void *priv, size_t priv_len, bool block_stpg);
void scst_ext_unblock_dev(struct scst_device *dev, bool stpg);
void __scst_ext_blocking_done(struct scst_device *dev);
void scst_ext_blocking_done(struct scst_device *dev);
Expand Down
4 changes: 2 additions & 2 deletions scst/src/scst_sysfs.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3805,10 +3805,10 @@ static ssize_t scst_dev_block_store(struct kobject *kobj,
"data_len %d)", dev->virt_name, sync, data_start, data_len);

if (sync)
res = scst_ext_block_dev(dev, NULL, NULL, 0, SCST_EXT_BLOCK_SYNC);
res = scst_sync_ext_block_dev(dev);
else
res = scst_ext_block_dev(dev, scst_sysfs_ext_blocking_done,
data_start, data_len, 0);
data_start, data_len, false);
if (res != 0)
goto out;

Expand Down
3 changes: 1 addition & 2 deletions scst/src/scst_tg.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1820,8 +1820,7 @@ static int scst_emit_stpg_event(struct scst_cmd *cmd, struct scst_dev_group *dg,

rc = scst_ext_block_dev(dgd->dev,
scst_stpg_ext_blocking_done,
(uint8_t *)&wait, sizeof(wait),
SCST_EXT_BLOCK_STPG);
&wait, sizeof(wait), true);
if (rc != 0) {
TRACE_DBG("scst_ext_block_dev() failed "
"with %d, reverting (cmd %p)", rc, cmd);
Expand Down

0 comments on commit a345160

Please sign in to comment.