* [PATCH v3 1/1] NFSv4.2 COPY do not allocate memory under the lock
@ 2018-11-16 19:01 Olga Kornievskaia
2018-11-20 5:13 ` kbuild test robot
0 siblings, 1 reply; 2+ messages in thread
From: Olga Kornievskaia @ 2018-11-16 19:01 UTC (permalink / raw)
To: trond.myklebust, anna.schumaker; +Cc: linux-nfs
From: Olga Kornievskaia <kolga@netapp.com>
Bruce pointed out that we shouldn't allocate memory while holding
a lock in the nfs4_callback_offload() and handle_async_copy()
that deal with a racing CB_OFFLOAD and reply to COPY case.
Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
fs/nfs/callback_proc.c | 22 +++++++++++-----------
fs/nfs/nfs42proc.c | 21 ++++++++++++---------
2 files changed, 23 insertions(+), 20 deletions(-)
diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
index fa515d5..48b2e90 100644
--- a/fs/nfs/callback_proc.c
+++ b/fs/nfs/callback_proc.c
@@ -686,20 +686,24 @@ __be32 nfs4_callback_offload(void *data, void *dummy,
{
struct cb_offloadargs *args = data;
struct nfs_server *server;
- struct nfs4_copy_state *copy;
+ struct nfs4_copy_state *copy, *tmp_copy;
bool found = false;
+ copy = kzalloc(sizeof(struct nfs4_copy_state), GFP_NOFS);
+ if (!copy)
+ return htonl(NFS4ERR_SERVERFAULT);
+
spin_lock(&cps->clp->cl_lock);
rcu_read_lock();
list_for_each_entry_rcu(server, &cps->clp->cl_superblocks,
client_link) {
- list_for_each_entry(copy, &server->ss_copies, copies) {
+ list_for_each_entry(tmp_copy, &server->ss_copies, copies) {
if (memcmp(args->coa_stateid.other,
- copy->stateid.other,
+ tmp_copy->stateid.other,
sizeof(args->coa_stateid.other)))
continue;
- nfs4_copy_cb_args(copy, args);
- complete(©->completion);
+ nfs4_copy_cb_args(tmp_copy, args);
+ complete(&tmp_copy->completion);
found = true;
goto out;
}
@@ -707,15 +711,11 @@ __be32 nfs4_callback_offload(void *data, void *dummy,
out:
rcu_read_unlock();
if (!found) {
- copy = kzalloc(sizeof(struct nfs4_copy_state), GFP_NOFS);
- if (!copy) {
- spin_unlock(&cps->clp->cl_lock);
- return htonl(NFS4ERR_SERVERFAULT);
- }
memcpy(©->stateid, &args->coa_stateid, NFS4_STATEID_SIZE);
nfs4_copy_cb_args(copy, args);
list_add_tail(©->copies, &cps->clp->pending_cb_stateids);
- }
+ } else
+ kfree(copy);
spin_unlock(&cps->clp->cl_lock);
return 0;
diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
index c7c2ffa..7f2b716 100644
--- a/fs/nfs/nfs42proc.c
+++ b/fs/nfs/nfs42proc.c
@@ -158,31 +158,34 @@ static int handle_async_copy(struct nfs42_copy_res *res,
struct file *dst,
nfs4_stateid *src_stateid)
{
- struct nfs4_copy_state *copy;
+ struct nfs4_copy_state *copy, *tmp_copy;
int status = NFS4_OK;
bool found_pending = false;
struct nfs_open_context *ctx = nfs_file_open_context(dst);
+ copy = kzalloc(sizeof(struct nfs4_copy_state), GFP_NOFS);
+ if (!copy) {
+ spin_unlock(&server->nfs_client->cl_lock);
+ return -ENOMEM;
+ }
+
spin_lock(&server->nfs_client->cl_lock);
- list_for_each_entry(copy, &server->nfs_client->pending_cb_stateids,
+ list_for_each_entry(tmp_copy, &server->nfs_client->pending_cb_stateids,
copies) {
- if (memcmp(&res->write_res.stateid, ©->stateid,
+ if (memcmp(&res->write_res.stateid, &tmp_copy->stateid,
NFS4_STATEID_SIZE))
continue;
found_pending = true;
- list_del(©->copies);
+ list_del(&tmp_copy->copies);
break;
}
if (found_pending) {
spin_unlock(&server->nfs_client->cl_lock);
+ kfree(copy);
+ copy = tmp_copy;
goto out;
}
- copy = kzalloc(sizeof(struct nfs4_copy_state), GFP_NOFS);
- if (!copy) {
- spin_unlock(&server->nfs_client->cl_lock);
- return -ENOMEM;
- }
memcpy(©->stateid, &res->write_res.stateid, NFS4_STATEID_SIZE);
init_completion(©->completion);
copy->parent_state = ctx->state;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v3 1/1] NFSv4.2 COPY do not allocate memory under the lock
2018-11-16 19:01 [PATCH v3 1/1] NFSv4.2 COPY do not allocate memory under the lock Olga Kornievskaia
@ 2018-11-20 5:13 ` kbuild test robot
0 siblings, 0 replies; 2+ messages in thread
From: kbuild test robot @ 2018-11-20 5:13 UTC (permalink / raw)
To: Olga Kornievskaia; +Cc: kbuild-all, trond.myklebust, anna.schumaker, linux-nfs
[-- Attachment #1: Type: text/plain, Size: 3374 bytes --]
Hi Olga,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on nfs/linux-next]
[also build test WARNING on v4.20-rc3 next-20181119]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Olga-Kornievskaia/NFSv4-2-COPY-do-not-allocate-memory-under-the-lock/20181119-202855
base: git://git.linux-nfs.org/projects/trondmy/linux-nfs.git linux-next
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All warnings (new ones prefixed by >>):
include/linux/slab.h:332:43: warning: dubious: x & !y
include/linux/slab.h:332:43: warning: dubious: x & !y
include/linux/slab.h:332:43: warning: dubious: x & !y
include/linux/slab.h:332:43: warning: dubious: x & !y
>> fs/nfs/nfs42proc.c:147:28: warning: context imbalance in 'handle_async_copy' - unexpected unlock
vim +/handle_async_copy +147 fs/nfs/nfs42proc.c
133
134 static int handle_async_copy(struct nfs42_copy_res *res,
135 struct nfs_server *server,
136 struct file *src,
137 struct file *dst,
138 nfs4_stateid *src_stateid)
139 {
140 struct nfs4_copy_state *copy, *tmp_copy;
141 int status = NFS4_OK;
142 bool found_pending = false;
143 struct nfs_open_context *ctx = nfs_file_open_context(dst);
144
145 copy = kzalloc(sizeof(struct nfs4_copy_state), GFP_NOFS);
146 if (!copy) {
> 147 spin_unlock(&server->nfs_client->cl_lock);
148 return -ENOMEM;
149 }
150
151 spin_lock(&server->nfs_client->cl_lock);
152 list_for_each_entry(tmp_copy, &server->nfs_client->pending_cb_stateids,
153 copies) {
154 if (memcmp(&res->write_res.stateid, &tmp_copy->stateid,
155 NFS4_STATEID_SIZE))
156 continue;
157 found_pending = true;
158 list_del(&tmp_copy->copies);
159 break;
160 }
161 if (found_pending) {
162 spin_unlock(&server->nfs_client->cl_lock);
163 kfree(copy);
164 copy = tmp_copy;
165 goto out;
166 }
167
168 memcpy(©->stateid, &res->write_res.stateid, NFS4_STATEID_SIZE);
169 init_completion(©->completion);
170 copy->parent_state = ctx->state;
171
172 list_add_tail(©->copies, &server->ss_copies);
173 spin_unlock(&server->nfs_client->cl_lock);
174
175 status = wait_for_completion_interruptible(©->completion);
176 spin_lock(&server->nfs_client->cl_lock);
177 list_del_init(©->copies);
178 spin_unlock(&server->nfs_client->cl_lock);
179 if (status == -ERESTARTSYS) {
180 goto out_cancel;
181 } else if (copy->flags) {
182 status = -EAGAIN;
183 goto out_cancel;
184 }
185 out:
186 res->write_res.count = copy->count;
187 memcpy(&res->write_res.verifier, ©->verf, sizeof(copy->verf));
188 status = -copy->error;
189
190 kfree(copy);
191 return status;
192 out_cancel:
193 nfs42_do_offload_cancel_async(dst, ©->stateid);
194 kfree(copy);
195 return status;
196 }
197
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 66620 bytes --]
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2018-11-20 5:15 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-16 19:01 [PATCH v3 1/1] NFSv4.2 COPY do not allocate memory under the lock Olga Kornievskaia
2018-11-20 5:13 ` kbuild test robot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.