Skip to content

Commit

Permalink
[TA1652] [DE17][DE37] Fixing these issues & avoid operations on stale…
Browse files Browse the repository at this point in the history 
… io-fd (openzfs#83)


* [TA1652][DE17] [DE37] Atomic inc and decrement of zinfo refcount done.
Closing data fd lazily so that operations on stale fd can be avoided
Wait reference to be drained out before freeing zinfo.

Signed-off-by: satbir <[email protected]>
  • Loading branch information
@satbirchhikara
satbirchhikara authored Jul 10, 2018
1 parent 5d7a3ed commit 78c26c7
Show file tree
Hide file tree
Showing 8 changed files with 113 additions and 84 deletions.
35 changes: 24 additions & 11 deletions cmd/zrepl/zrepl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ open_zvol(int fd, zvol_info_t **zinfopp)
thrd_arg = kmem_alloc(sizeof (thread_args_t), KM_SLEEP);
thrd_arg->fd = fd;
thrd_arg->zinfo = zinfo;
uzfs_zinfo_take_refcnt(zinfo, B_FALSE);
uzfs_zinfo_take_refcnt(zinfo);
thrd_info = zk_thread_create(NULL, 0,
(thread_func_t)uzfs_zvol_io_ack_sender,
(void *)thrd_arg, 0, NULL, TS_RUN, 0,
Expand All @@ -148,8 +148,9 @@ open_zvol(int fd, zvol_info_t **zinfopp)
if (rc == -1)
LOG_ERR("Failed to send reply for open request");
if (hdr.status != ZVOL_OP_STATUS_OK) {
ASSERT3P(*zinfopp, ==, NULL);
if (zinfo != NULL)
uzfs_zinfo_drop_refcnt(zinfo, B_FALSE);
uzfs_zinfo_drop_refcnt(zinfo);
return (-1);
}
return (rc);
Expand All @@ -173,17 +174,23 @@ uzfs_zvol_io_receiver(void *arg)
/* First command should be OPEN */
while (zinfo == NULL) {
if (open_zvol(fd, &zinfo) != 0) {
if ((zinfo != NULL) &&
(zinfo->is_io_ack_sender_created))
goto exit;
shutdown(fd, SHUT_RDWR);
(void) close(fd);
LOG_INFO("Data connection closed");
zk_thread_exit();
return;
}
}

LOG_INFO("Data connection associated with zvol %s", zinfo->name);

while ((rc = uzfs_zvol_socket_read(fd, (char *)&hdr, sizeof (hdr))) ==
0) {
if ((zinfo->state == ZVOL_INFO_STATE_OFFLINE))
break;

if (hdr.opcode != ZVOL_OPCODE_WRITE &&
hdr.opcode != ZVOL_OPCODE_READ &&
Expand Down Expand Up @@ -211,13 +218,17 @@ uzfs_zvol_io_receiver(void *arg)
}
}

if (zinfo->state == ZVOL_INFO_STATE_OFFLINE) {
zio_cmd_free(&zio_cmd);
break;
}
/* Take refcount for uzfs_zvol_worker to work on it */
uzfs_zinfo_take_refcnt(zinfo, B_FALSE);
uzfs_zinfo_take_refcnt(zinfo);
zio_cmd->zv = zinfo;
taskq_dispatch(zinfo->uzfs_zvol_taskq, uzfs_zvol_worker,
zio_cmd, TQ_SLEEP);
}

exit:
(void) pthread_mutex_lock(&zinfo->zinfo_mutex);
zinfo->conn_closed = B_TRUE;
/*
Expand All @@ -237,7 +248,9 @@ uzfs_zvol_io_receiver(void *arg)
(void) pthread_mutex_lock(&zinfo->zinfo_mutex);
}
(void) pthread_mutex_unlock(&zinfo->zinfo_mutex);
uzfs_zinfo_drop_refcnt(zinfo, B_FALSE);

close(fd);
uzfs_zinfo_drop_refcnt(zinfo);
zk_thread_exit();
}

Expand Down Expand Up @@ -331,6 +344,9 @@ uzfs_zvol_io_ack_sender(void *arg)
while (1) {
if ((zinfo->state == ZVOL_INFO_STATE_OFFLINE) ||
(zinfo->conn_closed == B_TRUE)) {
zinfo->is_io_ack_sender_created = B_FALSE;
(void) pthread_mutex_unlock(
&zinfo->zinfo_mutex);
goto exit;
}
if (STAILQ_EMPTY(&zinfo->complete_queue)) {
Expand Down Expand Up @@ -378,7 +394,6 @@ uzfs_zvol_io_ack_sender(void *arg)
*/
if (zio_cmd->conn == fd) {
zio_cmd_free(&zio_cmd);
(void) pthread_mutex_lock(&zinfo->zinfo_mutex);
goto exit;
}
zio_cmd_free(&zio_cmd);
Expand All @@ -394,8 +409,6 @@ uzfs_zvol_io_ack_sender(void *arg)
LOG_ERRNO("socket write err");
if (zio_cmd->conn == fd) {
zio_cmd_free(&zio_cmd);
(void) pthread_mutex_lock(
&zinfo->zinfo_mutex);
goto exit;
}
}
Expand All @@ -413,16 +426,16 @@ uzfs_zvol_io_ack_sender(void *arg)
exit:
zinfo->zio_cmd_in_ack = NULL;
shutdown(fd, SHUT_RDWR);
close(fd);
LOG_INFO("Data connection for zvol %s closed", zinfo->name);

(void) pthread_mutex_lock(&zinfo->zinfo_mutex);
while (!STAILQ_EMPTY(&zinfo->complete_queue)) {
zio_cmd = STAILQ_FIRST(&zinfo->complete_queue);
STAILQ_REMOVE_HEAD(&zinfo->complete_queue, cmd_link);
zio_cmd_free(&zio_cmd);
}
zinfo->is_io_ack_sender_created = B_FALSE;
(void) pthread_mutex_unlock(&zinfo->zinfo_mutex);
uzfs_zinfo_drop_refcnt(zinfo, B_FALSE);
uzfs_zinfo_drop_refcnt(zinfo);

zk_thread_exit();
}
Expand Down
23 changes: 20 additions & 3 deletions include/zrepl_mgmt.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ typedef struct zvol_info_s {
zvol_info_state_t state;
char name[MAXPATHLEN];
zvol_state_t *zv;
int refcnt;
uint64_t refcnt;
int is_io_ack_sender_created;
uint32_t timeout; /* iSCSI timeout val for this zvol */
uint64_t zvol_guid;
Expand Down Expand Up @@ -153,8 +153,6 @@ typedef struct zvol_rebuild_s {
extern int uzfs_zinfo_init(void *zv, const char *ds_name,
nvlist_t *create_props);
extern zvol_info_t *uzfs_zinfo_lookup(const char *name);
extern void uzfs_zinfo_drop_refcnt(zvol_info_t *zinfo, int locked);
extern void uzfs_zinfo_take_refcnt(zvol_info_t *zinfo, int locked);
extern void uzfs_zinfo_replay_zil_all(void);
extern int uzfs_zinfo_destroy(const char *ds_name, spa_t *spa);
uint64_t uzfs_zvol_get_last_committed_io_no(zvol_state_t *zv);
Expand All @@ -163,6 +161,25 @@ void uzfs_zvol_store_last_committed_io_no(zvol_state_t *zv,
extern int create_and_bind(const char *port, int bind_needed,
boolean_t nonblocking);

/*
* API to drop refcnt on zinfo. If refcnt
* dropped to zero then free zinfo.
*/
inline void
uzfs_zinfo_drop_refcnt(zvol_info_t *zinfo)
{
atomic_dec_64(&zinfo->refcnt);
}

/*
* API to take refcount on zinfo.
*/
inline void
uzfs_zinfo_take_refcnt(zvol_info_t *zinfo)
{
atomic_inc_64(&zinfo->refcnt);
}

#ifdef __cplusplus
}
#endif
Expand Down
6 changes: 6 additions & 0 deletions lib/libzpool/uzfs_rebuilding.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,8 @@ uzfs_get_io_diff(zvol_state_t *zv, blk_metadata_t *low,
EXECUTE_DIFF_CALLBACK(last_lun_offset,
diff_count, buf, last_index, arg,
last_md, snap_zv, func, ret);
if (ret != 0)
break;
last_lun_offset = lun_offset;
last_md = (blk_metadata_t *)(buf+i);
last_index = i;
Expand All @@ -224,13 +226,17 @@ uzfs_get_io_diff(zvol_state_t *zv, blk_metadata_t *low,
EXECUTE_DIFF_CALLBACK(last_lun_offset,
diff_count, buf, last_index, arg, last_md,
snap_zv, func, ret);
if (ret != 0)
break;
}

lun_offset += zv->zv_metavolblocksize;
}
if (!ret && diff_count) {
EXECUTE_DIFF_CALLBACK(last_lun_offset, diff_count, buf,
last_index, arg, last_md, snap_zv, func, ret);
if (ret != 0)
break;
}
}

Expand Down
70 changes: 26 additions & 44 deletions lib/libzpool/zrepl_mgmt.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,58 +119,20 @@ create_and_bind(const char *port, int bind_needed, boolean_t nonblock)
return (sfd);
}

/*
* API to drop refcnt on zinfo. If refcnt
* dropped to zero then free zinfo.
*/
void
uzfs_zinfo_drop_refcnt(zvol_info_t *zinfo, int locked)
{
if (!locked) {
(void) mutex_enter(&zvol_list_mutex);
}

zinfo->refcnt--;
if (zinfo->refcnt == 0) {
(void) uzfs_zinfo_free(zinfo);
}

if (!locked) {
(void) mutex_exit(&zvol_list_mutex);
}
}

/*
* API to take refcount on zinfo.
*/
void
uzfs_zinfo_take_refcnt(zvol_info_t *zinfo, int locked)
{
if (!locked) {
(void) mutex_enter(&zvol_list_mutex);
}
zinfo->refcnt++;
if (!locked) {
(void) mutex_exit(&zvol_list_mutex);
}
}

static void
uzfs_insert_zinfo_list(zvol_info_t *zinfo)
{
LOG_INFO("Instantiating zvol %s", zinfo->name);
/* Base refcount is taken here */
(void) mutex_enter(&zvol_list_mutex);
uzfs_zinfo_take_refcnt(zinfo, B_TRUE);
uzfs_zinfo_take_refcnt(zinfo);
SLIST_INSERT_HEAD(&zvol_list, zinfo, zinfo_next);
(void) mutex_exit(&zvol_list_mutex);
}

static void
uzfs_remove_zinfo_list(zvol_info_t *zinfo)
uzfs_mark_offline_and_free_zinfo(zvol_info_t *zinfo)
{
LOG_INFO("Removing zvol %s", zinfo->name);
SLIST_REMOVE(&zvol_list, zinfo, zvol_info_s, zinfo_next);
(void) pthread_mutex_lock(&zinfo->zinfo_mutex);
zinfo->state = ZVOL_INFO_STATE_OFFLINE;
/* Send signal to ack_sender thread about offline */
Expand All @@ -179,7 +141,17 @@ uzfs_remove_zinfo_list(zvol_info_t *zinfo)
}
(void) pthread_mutex_unlock(&zinfo->zinfo_mutex);
/* Base refcount is droped here */
uzfs_zinfo_drop_refcnt(zinfo, B_TRUE);
uzfs_zinfo_drop_refcnt(zinfo);

/* Wait for refcounts to be drained */
while (zinfo->refcnt > 0) {
LOG_DEBUG("Waiting for refcount to go down to"
" zero on zvol:%s", zinfo->name);
sleep(5);
}

LOG_INFO("Freeing zvol %s", zinfo->name);
(void) uzfs_zinfo_free(zinfo);
}

zvol_info_t *
Expand Down Expand Up @@ -216,7 +188,7 @@ uzfs_zinfo_lookup(const char *name)
}
if (zv != NULL) {
/* Take refcount */
uzfs_zinfo_take_refcnt(zv, B_TRUE);
uzfs_zinfo_take_refcnt(zv);
}
(void) mutex_exit(&zvol_list_mutex);

Expand Down Expand Up @@ -254,9 +226,14 @@ uzfs_zinfo_destroy(const char *name, spa_t *spa)
SLIST_FOREACH_SAFE(zinfo, &zvol_list, zinfo_next, zt) {
if (strncmp(spa_name(spa),
zinfo->name, strlen(spa_name(spa))) == 0) {
SLIST_REMOVE(&zvol_list, zinfo, zvol_info_s,
zinfo_next);

mutex_exit(&zvol_list_mutex);
zv = zinfo->zv;
uzfs_remove_zinfo_list(zinfo);
uzfs_mark_offline_and_free_zinfo(zinfo);
uzfs_close_dataset(zv);
mutex_enter(&zvol_list_mutex);
}
}
} else {
Expand All @@ -265,9 +242,14 @@ uzfs_zinfo_destroy(const char *name, spa_t *spa)
((strncmp(zinfo->name, name, namelen) == 0) &&
zinfo->name[namelen] == '/' &&
zinfo->name[namelen + 1] == '\0')) {
SLIST_REMOVE(&zvol_list, zinfo, zvol_info_s,
zinfo_next);

mutex_exit(&zvol_list_mutex);
zv = zinfo->zv;
uzfs_remove_zinfo_list(zinfo);
uzfs_mark_offline_and_free_zinfo(zinfo);
uzfs_close_dataset(zv);
mutex_enter(&zvol_list_mutex);
break;
}
}
Expand Down
Loading

0 comments on commit 78c26c7

Please sign in to comment.