Skip to content

Commit b922793

Browse files
raghavendrabhatamarts
raghavendrabhat
authored and
amarts
committed
features/bit-rot-stub: clean the mutex after cancelling the signer thread
When bit-rot feature is disabled, the signer thread from the bit-rot-stub xlator (the thread which performs the setxattr of the signature on to the disk) is cancelled. But, if the cancelled signer thread had already held the mutex (&priv->lock) which it uses to monitor the queue of files to be signed, then the mutex is never released. This creates problems in future when the feature is enabled again. Both the new instance of the signer thread and the regular thread which enqueues the files to be signed will be blocked on this mutex. So, as part of cancelling the signer thread, unlock the mutex associated with it as well using pthread_cleanup_push and pthread_cleanup_pop. Change-Id: Ib761910caed90b268e69794ddeb108165487af40 updates: bz#1700078 Signed-off-by: Raghavendra Bhat <raghavendra@redhat.com>
1 parent e5ff6cc commit b922793

2 files changed

Lines changed: 59 additions & 7 deletions

File tree

‎xlators/features/bit-rot/src/stub/bit-rot-stub-messages.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ GLFS_MSGID(BITROT_STUB, BRS_MSG_NO_MEMORY, BRS_MSG_SET_EVENT_FAILED,
3939
BRS_MSG_BAD_HANDLE_DIR_NULL, BRS_MSG_BAD_OBJ_THREAD_FAIL,
4040
BRS_MSG_BAD_OBJ_DIR_CLOSE_FAIL, BRS_MSG_LINK_FAIL,
4141
BRS_MSG_BAD_OBJ_UNLINK_FAIL, BRS_MSG_DICT_SET_FAILED,
42-
BRS_MSG_PATH_GET_FAILED, BRS_MSG_NULL_LOCAL);
42+
BRS_MSG_PATH_GET_FAILED, BRS_MSG_NULL_LOCAL,
43+
BRS_MSG_SPAWN_SIGN_THRD_FAILED, BRS_MSG_KILL_SIGN_THREAD,
44+
BRS_MSG_NON_BITD_PID, BRS_MSG_SIGN_PREPARE_FAIL);
4345

4446
#endif /* !_BITROT_STUB_MESSAGES_H_ */

‎xlators/features/bit-rot/src/stub/bit-rot-stub.c‎

Lines changed: 56 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,15 @@
2626

2727
#define BR_STUB_REQUEST_COOKIE 0x1
2828

29+
void
30+
br_stub_lock_cleaner(void *arg)
31+
{
32+
pthread_mutex_t *clean_mutex = arg;
33+
34+
pthread_mutex_unlock(clean_mutex);
35+
return;
36+
}
37+
2938
void *
3039
br_stub_signth(void *);
3140

@@ -166,8 +175,11 @@ init(xlator_t *this)
166175

167176
ret = gf_thread_create(&priv->signth, NULL, br_stub_signth, this,
168177
"brssign");
169-
if (ret != 0)
178+
if (ret != 0) {
179+
gf_msg(this->name, GF_LOG_WARNING, 0, BRS_MSG_SPAWN_SIGN_THRD_FAILED,
180+
"failed to create the new thread for signer");
170181
goto cleanup_lock;
182+
}
171183

172184
ret = br_stub_bad_object_container_init(this, priv);
173185
if (ret) {
@@ -214,11 +226,15 @@ reconfigure(xlator_t *this, dict_t *options)
214226
priv = this->private;
215227

216228
GF_OPTION_RECONF("bitrot", priv->do_versioning, options, bool, err);
217-
if (priv->do_versioning) {
229+
if (priv->do_versioning && !priv->signth) {
218230
ret = gf_thread_create(&priv->signth, NULL, br_stub_signth, this,
219231
"brssign");
220-
if (ret != 0)
232+
if (ret != 0) {
233+
gf_msg(this->name, GF_LOG_WARNING, 0,
234+
BRS_MSG_SPAWN_SIGN_THRD_FAILED,
235+
"failed to create the new thread for signer");
221236
goto err;
237+
}
222238

223239
ret = br_stub_bad_object_container_init(this, priv);
224240
if (ret) {
@@ -232,8 +248,11 @@ reconfigure(xlator_t *this, dict_t *options)
232248
gf_msg(this->name, GF_LOG_ERROR, 0,
233249
BRS_MSG_CANCEL_SIGN_THREAD_FAILED,
234250
"Could not cancel sign serializer thread");
251+
} else {
252+
gf_msg(this->name, GF_LOG_INFO, 0, BRS_MSG_KILL_SIGN_THREAD,
253+
"killed the signer thread");
254+
priv->signth = 0;
235255
}
236-
priv->signth = 0;
237256
}
238257

239258
if (priv->container.thread) {
@@ -902,6 +921,24 @@ br_stub_signth(void *arg)
902921

903922
THIS = this;
904923
while (1) {
924+
/*
925+
* Disabling bit-rot feature leads to this particular thread
926+
* getting cleaned up by reconfigure via a call to the function
927+
* gf_thread_cleanup_xint (which in turn calls pthread_cancel
928+
* and pthread_join). But, if this thread had held the mutex
929+
* &priv->lock at the time of cancellation, then it leads to
930+
* deadlock in future when bit-rot feature is enabled (which
931+
* again spawns this thread which cant hold the lock as the
932+
* mutex is still held by the previous instance of the thread
933+
* which got killed). Also, the br_stub_handle_object_signature
934+
* function which is called whenever file has to be signed
935+
* also gets blocked as it too attempts to acquire &priv->lock.
936+
*
937+
* So, arrange for the lock to be unlocked as part of the
938+
* cleanup of this thread using pthread_cleanup_push and
939+
* pthread_cleanup_pop.
940+
*/
941+
pthread_cleanup_push(br_stub_lock_cleaner, &priv->lock);
905942
pthread_mutex_lock(&priv->lock);
906943
{
907944
while (list_empty(&priv->squeue))
@@ -912,6 +949,7 @@ br_stub_signth(void *arg)
912949
list_del_init(&sigstub->list);
913950
}
914951
pthread_mutex_unlock(&priv->lock);
952+
pthread_cleanup_pop(0);
915953

916954
call_resume(sigstub->stub);
917955

@@ -1042,12 +1080,22 @@ br_stub_handle_object_signature(call_frame_t *frame, xlator_t *this, fd_t *fd,
10421080

10431081
priv = this->private;
10441082

1045-
if (frame->root->pid != GF_CLIENT_PID_BITD)
1083+
if (frame->root->pid != GF_CLIENT_PID_BITD) {
1084+
gf_msg(this->name, GF_LOG_WARNING, op_errno, BRS_MSG_NON_BITD_PID,
1085+
"PID %d from where signature request"
1086+
"came, does not belong to bit-rot daemon."
1087+
"Unwinding the fop",
1088+
frame->root->pid);
10461089
goto dofop;
1090+
}
10471091

10481092
ret = br_stub_prepare_signature(this, dict, fd->inode, sign, &fakesuccess);
1049-
if (ret)
1093+
if (ret) {
1094+
gf_msg(this->name, GF_LOG_WARNING, 0, BRS_MSG_SIGN_PREPARE_FAIL,
1095+
"failed to prepare the signature for %s. Unwinding the fop",
1096+
uuid_utoa(fd->inode->gfid));
10501097
goto dofop;
1098+
}
10511099
if (fakesuccess) {
10521100
op_ret = op_errno = 0;
10531101
goto dofop;
@@ -1387,6 +1435,8 @@ br_stub_fsetxattr(call_frame_t *frame, xlator_t *this, fd_t *fd, dict_t *dict,
13871435
/* object signature request */
13881436
ret = dict_get_bin(dict, GLUSTERFS_SET_OBJECT_SIGNATURE, (void **)&sign);
13891437
if (!ret) {
1438+
gf_msg_debug(this->name, 0, "got SIGNATURE request on %s",
1439+
uuid_utoa(fd->inode->gfid));
13901440
br_stub_handle_object_signature(frame, this, fd, dict, sign, xdata);
13911441
goto done;
13921442
}

0 commit comments

Comments
 (0)