From: Oleg Drokin Date: Tue, 22 Apr 2014 17:06:19 +0000 (+0000) Subject: Revert "LU-3333 ptlrpc: Protect request buffer changing" X-Git-Tag: 2.5.59~100 X-Git-Url: https://git.whamcloud.com/?a=commitdiff_plain;h=ed3d0669da25b47fb35180019a43820d20a30a71;p=fs%2Flustre-release.git Revert "LU-3333 ptlrpc: Protect request buffer changing" It turns out that sec_null message format is is not double wrapped like sec_plain and sec_gss cases. As such a conversion from rq_buf to rq_msg is different in those cases. The older way of doing in-place locking seems to be the way to go here I think. This reverts commit b2bb3b247d1dc75e25f1b5c14a333905909b5e70. Change-Id: Icd84a696aafed17a275d0576dad639b929618190 Reviewed-on: http://review.whamcloud.com/10052 Tested-by: Jenkins Reviewed-by: Oleg Drokin Tested-by: Oleg Drokin --- diff --git a/lustre/ptlrpc/gss/sec_gss.c b/lustre/ptlrpc/gss/sec_gss.c index 69afcf0..eb9debb 100644 --- a/lustre/ptlrpc/gss/sec_gss.c +++ b/lustre/ptlrpc/gss/sec_gss.c @@ -68,7 +68,6 @@ #include #include -#include "../ptlrpc_internal.h" #include "gss_err.h" #include "gss_internal.h" #include "gss_api.h" @@ -1652,9 +1651,9 @@ int gss_enlarge_reqbuf_intg(struct ptlrpc_sec *sec, int svc, int segment, int newsize) { + struct lustre_msg *newbuf; int txtsize, sigsize = 0, i; int newmsg_size, newbuf_size; - int rc; /* * gss header is at seg 0; @@ -1698,10 +1697,19 @@ int gss_enlarge_reqbuf_intg(struct ptlrpc_sec *sec, LASSERT(!req->rq_pool || req->rq_reqbuf_len >= newbuf_size); if (req->rq_reqbuf_len < newbuf_size) { - rc = ptlrpc_enlarge_req_buffer(req, newbuf_size); - if (rc != 0) - RETURN(rc); - } + newbuf_size = size_roundup_power2(newbuf_size); + + OBD_ALLOC_LARGE(newbuf, newbuf_size); + if (newbuf == NULL) + RETURN(-ENOMEM); + + memcpy(newbuf, req->rq_reqbuf, req->rq_reqbuf_len); + + OBD_FREE_LARGE(req->rq_reqbuf, req->rq_reqbuf_len); + req->rq_reqbuf = newbuf; + req->rq_reqbuf_len = newbuf_size; + req->rq_reqmsg = lustre_msg_buf(req->rq_reqbuf, 1, 0); + } /* do enlargement, from wrapper to embedded, from end to begin */ if (svc != SPTLRPC_SVC_NULL) @@ -1761,8 +1769,6 @@ int gss_enlarge_reqbuf_priv(struct ptlrpc_sec *sec, if (newclrbuf_size + newcipbuf_size <= req->rq_reqbuf_len) { void *src, *dst; - if (req->rq_import) - spin_lock(&req->rq_import->imp_lock); /* move clear text backward. */ src = req->rq_clrbuf; dst = (char *) req->rq_reqbuf + newcipbuf_size; @@ -1772,9 +1778,6 @@ int gss_enlarge_reqbuf_priv(struct ptlrpc_sec *sec, req->rq_clrbuf = (struct lustre_msg *) dst; req->rq_clrbuf_len = newclrbuf_size; req->rq_reqmsg = lustre_msg_buf(req->rq_clrbuf, 0, 0); - - if (req->rq_import) - spin_unlock(&req->rq_import->imp_lock); } else { /* sadly we have to split out the clear buffer */ LASSERT(req->rq_reqbuf_len >= newcipbuf_size); @@ -1789,15 +1792,6 @@ int gss_enlarge_reqbuf_priv(struct ptlrpc_sec *sec, if (newclrbuf == NULL) RETURN(-ENOMEM); - /* Must lock this, so that otherwise unprotected change of - * rq_reqmsg is not racing with parallel processing of - * imp_replay_list traversing threads. See LU-3333 - * This is a bandaid at best, we really need to deal with this - * in request enlarging code before unpacking that's already - * there */ - if (req->rq_import) - spin_lock(&req->rq_import->imp_lock); - memcpy(newclrbuf, req->rq_clrbuf, req->rq_clrbuf_len); if (req->rq_reqbuf == NULL || @@ -1810,9 +1804,6 @@ int gss_enlarge_reqbuf_priv(struct ptlrpc_sec *sec, req->rq_clrbuf = newclrbuf; req->rq_clrbuf_len = newclrbuf_size; req->rq_reqmsg = lustre_msg_buf(req->rq_clrbuf, 0, 0); - - if (req->rq_import) - spin_unlock(&req->rq_import->imp_lock); } _sptlrpc_enlarge_msg_inplace(req->rq_clrbuf, 0, newmsg_size); diff --git a/lustre/ptlrpc/pack_generic.c b/lustre/ptlrpc/pack_generic.c index 262ee3b..d12885d 100644 --- a/lustre/ptlrpc/pack_generic.c +++ b/lustre/ptlrpc/pack_generic.c @@ -96,46 +96,6 @@ int ptlrpc_buf_need_swab(struct ptlrpc_request *req, const int inout, } EXPORT_SYMBOL(ptlrpc_buf_need_swab); -/* This enlarges the req buffer of request \a req to the next power of 2 - * multiple of \a newbuf_size. - * Returns zero on success or ENOMEM if it failed to allocate the new buffer. - * - * This is used in the reply path on the client if the server responded - * with a bigger message than we expected so we can save the new state for - * a possible future replay where we'll need to present this new info - * (usually striping that's not available at create time) */ -int ptlrpc_enlarge_req_buffer(struct ptlrpc_request *req, int newbuf_size) -{ - struct lustre_msg *newbuf; - - newbuf_size = size_roundup_power2(newbuf_size); - - OBD_ALLOC_LARGE(newbuf, newbuf_size); - if (newbuf == NULL) - return -ENOMEM; - - /* Must lock this, so that otherwise unprotected change of - * rq_reqmsg is not racing with parallel processing of - * imp_replay_list traversing threads. See LU-3333 - * This is a bandaid at best, we really need to deal with this - * in request enlarging code before unpacking what's already - * there */ - if (req->rq_import) - spin_lock(&req->rq_import->imp_lock); - - memcpy(newbuf, req->rq_reqbuf, req->rq_reqbuf_len); - - OBD_FREE_LARGE(req->rq_reqbuf, req->rq_reqbuf_len); - req->rq_reqbuf = newbuf; - req->rq_reqbuf_len = newbuf_size; - req->rq_reqmsg = lustre_msg_buf(req->rq_reqbuf, 1, 0); - - if (req->rq_import) - spin_unlock(&req->rq_import->imp_lock); - - return 0; -} - static inline int lustre_msg_check_version_v2(struct lustre_msg_v2 *msg, __u32 version) { diff --git a/lustre/ptlrpc/ptlrpc_internal.h b/lustre/ptlrpc/ptlrpc_internal.h index f21a0a7..ab8a1dd 100644 --- a/lustre/ptlrpc/ptlrpc_internal.h +++ b/lustre/ptlrpc/ptlrpc_internal.h @@ -238,7 +238,6 @@ void ptlrpc_add_bulk_page(struct ptlrpc_bulk_desc *desc, struct page *page, struct ptlrpc_reply_state * lustre_get_emerg_rs(struct ptlrpc_service_part *svcpt); void lustre_put_emerg_rs(struct ptlrpc_reply_state *rs); -int ptlrpc_enlarge_req_buffer(struct ptlrpc_request *req, int newbuf_size); /* pinger.c */ int ptlrpc_start_pinger(void); diff --git a/lustre/ptlrpc/sec_null.c b/lustre/ptlrpc/sec_null.c index 4c01682..c62ab83 100644 --- a/lustre/ptlrpc/sec_null.c +++ b/lustre/ptlrpc/sec_null.c @@ -49,7 +49,6 @@ #include #include #include -#include "ptlrpc_internal.h" static struct ptlrpc_sec_policy null_policy; static struct ptlrpc_sec null_sec; @@ -239,9 +238,9 @@ int null_enlarge_reqbuf(struct ptlrpc_sec *sec, struct ptlrpc_request *req, int segment, int newsize) { + struct lustre_msg *newbuf; struct lustre_msg *oldbuf = req->rq_reqmsg; - int oldsize, newmsg_size; - int rc; + int oldsize, newmsg_size, alloc_size; LASSERT(req->rq_reqbuf); LASSERT(req->rq_reqbuf == req->rq_reqmsg); @@ -258,10 +257,18 @@ int null_enlarge_reqbuf(struct ptlrpc_sec *sec, LASSERT(!req->rq_pool || req->rq_reqbuf_len >= newmsg_size); if (req->rq_reqbuf_len < newmsg_size) { - rc = ptlrpc_enlarge_req_buffer(req, newmsg_size); - if (rc != 0) - return rc; - } + alloc_size = size_roundup_power2(newmsg_size); + + OBD_ALLOC_LARGE(newbuf, alloc_size); + if (newbuf == NULL) + return -ENOMEM; + + memcpy(newbuf, req->rq_reqbuf, req->rq_reqlen); + + OBD_FREE_LARGE(req->rq_reqbuf, req->rq_reqbuf_len); + req->rq_reqbuf = req->rq_reqmsg = newbuf; + req->rq_reqbuf_len = alloc_size; + } _sptlrpc_enlarge_msg_inplace(req->rq_reqmsg, segment, newsize); req->rq_reqlen = newmsg_size; diff --git a/lustre/ptlrpc/sec_plain.c b/lustre/ptlrpc/sec_plain.c index 45c8c9f..bf02263 100644 --- a/lustre/ptlrpc/sec_plain.c +++ b/lustre/ptlrpc/sec_plain.c @@ -49,7 +49,6 @@ #include #include #include -#include "ptlrpc_internal.h" struct plain_sec { struct ptlrpc_sec pls_base; @@ -672,9 +671,9 @@ int plain_enlarge_reqbuf(struct ptlrpc_sec *sec, struct ptlrpc_request *req, int segment, int newsize) { + struct lustre_msg *newbuf; int oldsize; int newmsg_size, newbuf_size; - int rc; ENTRY; LASSERT(req->rq_reqbuf); @@ -700,10 +699,20 @@ int plain_enlarge_reqbuf(struct ptlrpc_sec *sec, LASSERT(!req->rq_pool || req->rq_reqbuf_len >= newbuf_size); if (req->rq_reqbuf_len < newbuf_size) { - rc = ptlrpc_enlarge_req_buffer(req, newbuf_size); - if (rc != 0) - RETURN(rc); - } + newbuf_size = size_roundup_power2(newbuf_size); + + OBD_ALLOC_LARGE(newbuf, newbuf_size); + if (newbuf == NULL) + RETURN(-ENOMEM); + + memcpy(newbuf, req->rq_reqbuf, req->rq_reqbuf_len); + + OBD_FREE_LARGE(req->rq_reqbuf, req->rq_reqbuf_len); + req->rq_reqbuf = newbuf; + req->rq_reqbuf_len = newbuf_size; + req->rq_reqmsg = lustre_msg_buf(req->rq_reqbuf, + PLAIN_PACK_MSG_OFF, 0); + } _sptlrpc_enlarge_msg_inplace(req->rq_reqbuf, PLAIN_PACK_MSG_OFF, newmsg_size);