diff options
author | Weston Andros Adamson <dros@primarydata.com> | 2014-05-15 11:56:40 -0400 |
---|---|---|
committer | Trond Myklebust <trond.myklebust@primarydata.com> | 2014-05-29 11:11:42 -0400 |
commit | d201c4de518c1d617aa216664869fa329d562d7d (patch) | |
tree | 696aefe2b87ea892783a0c871688e7a9ce368b0a | |
parent | 41d8d5b7a559a9bfbf9680d1e4777e1a7b0149d5 (diff) |
pnfs: fix race in filelayout commit path
Hold the lock while modifying commit info dataserver buckets.
The following oops can be reproduced by running iozone for a while against
a 2 DS pynfs filelayout server.
general protection fault: 0000 [#1] SMP DEBUG_PAGEALLOC
Modules linked in: nfs_layout_nfsv41_files rpcsec_gss_krb5 nfsv4 nfs fscache
CPU: 0 PID: 903 Comm: iozone Not tainted 3.15.0-rc1-branch-dros_testing+ #44
Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference
task: ffff880078164480 ti: ffff88006e972000 task.ti: ffff88006e972000
RIP: 0010:[<ffffffffa01936e1>] [<ffffffffa01936e1>] nfs_init_commit+0x22/0x
RSP: 0018:ffff88006e973d30 EFLAGS: 00010246
RAX: ffff88006e973e00 RBX: ffff88006e828800 RCX: ffff88006e973e10
RDX: 0000000000000000 RSI: ffff88006e973e00 RDI: dead4ead00000000
RBP: ffff88006e973d38 R08: ffff88006e8289d8 R09: 0000000000000000
R10: ffff88006e8289d8 R11: 0000000000016988 R12: ffff88006e973b98
R13: ffff88007a0a6648 R14: ffff88006e973e10 R15: ffff88006e828800
FS: 00007f2ce396b740(0000) GS:ffff88007f200000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f03278a1000 CR3: 0000000079043000 CR4: 00000000001407f0
Stack:
ffff88006e8289d8 ffff88006e973da8 ffffffffa00f144f ffff88006e9478c0
ffff88006e973e00 ffff88006de21080 0000000100000002 ffff880079be6c48
ffff88006e973d70 ffff88006e973d70 ffff88006e973e10 ffff88006de21080
Call Trace:
[<ffffffffa00f144f>] filelayout_commit_pagelist+0x1ae/0x34a [nfs_layout_nfsv
[<ffffffffa0194f72>] nfs_generic_commit_list+0x92/0xc4 [nfs]
[<ffffffffa0195053>] nfs_commit_inode+0xaf/0x114 [nfs]
[<ffffffffa01892bd>] nfs_file_fsync_commit+0x82/0xbe [nfs]
[<ffffffffa01ceb0d>] nfs4_file_fsync+0x59/0x9b [nfsv4]
[<ffffffff8114ee3c>] vfs_fsync_range+0x18/0x20
[<ffffffff8114ee60>] vfs_fsync+0x1c/0x1e
[<ffffffffa01891c2>] nfs_file_flush+0x7f/0x84 [nfs]
[<ffffffff81127a43>] filp_close+0x3c/0x72
[<ffffffff81140e12>] __close_fd+0x82/0x9a
[<ffffffff81127a9c>] SyS_close+0x23/0x4c
[<ffffffff814acd12>] system_call_fastpath+0x16/0x1b
Code: 5b 41 5c 41 5d 41 5e 5d c3 0f 1f 44 00 00 55 48 89 e5 53 48 89 fb 48 8
RIP [<ffffffffa01936e1>] nfs_init_commit+0x22/0xe1 [nfs]
RSP <ffff88006e973d30>
---[ end trace 732fe6419b235e2f ]---
Suggested-by: Trond Myklebust <trond.myklebust@primarydata.com>
Signed-off-by: Weston Andros Adamson <dros@primarydata.com>
Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
-rw-r--r-- | fs/nfs/nfs4filelayout.c | 20 |
1 files changed, 16 insertions, 4 deletions
diff --git a/fs/nfs/nfs4filelayout.c b/fs/nfs/nfs4filelayout.c index 7954e16a6d83..9fd7cebbff04 100644 --- a/fs/nfs/nfs4filelayout.c +++ b/fs/nfs/nfs4filelayout.c | |||
@@ -1067,6 +1067,7 @@ filelayout_choose_commit_list(struct nfs_page *req, | |||
1067 | */ | 1067 | */ |
1068 | j = nfs4_fl_calc_j_index(lseg, req_offset(req)); | 1068 | j = nfs4_fl_calc_j_index(lseg, req_offset(req)); |
1069 | i = select_bucket_index(fl, j); | 1069 | i = select_bucket_index(fl, j); |
1070 | spin_lock(cinfo->lock); | ||
1070 | buckets = cinfo->ds->buckets; | 1071 | buckets = cinfo->ds->buckets; |
1071 | list = &buckets[i].written; | 1072 | list = &buckets[i].written; |
1072 | if (list_empty(list)) { | 1073 | if (list_empty(list)) { |
@@ -1080,6 +1081,7 @@ filelayout_choose_commit_list(struct nfs_page *req, | |||
1080 | } | 1081 | } |
1081 | set_bit(PG_COMMIT_TO_DS, &req->wb_flags); | 1082 | set_bit(PG_COMMIT_TO_DS, &req->wb_flags); |
1082 | cinfo->ds->nwritten++; | 1083 | cinfo->ds->nwritten++; |
1084 | spin_unlock(cinfo->lock); | ||
1083 | return list; | 1085 | return list; |
1084 | } | 1086 | } |
1085 | 1087 | ||
@@ -1176,6 +1178,7 @@ transfer_commit_list(struct list_head *src, struct list_head *dst, | |||
1176 | return ret; | 1178 | return ret; |
1177 | } | 1179 | } |
1178 | 1180 | ||
1181 | /* Note called with cinfo->lock held. */ | ||
1179 | static int | 1182 | static int |
1180 | filelayout_scan_ds_commit_list(struct pnfs_commit_bucket *bucket, | 1183 | filelayout_scan_ds_commit_list(struct pnfs_commit_bucket *bucket, |
1181 | struct nfs_commit_info *cinfo, | 1184 | struct nfs_commit_info *cinfo, |
@@ -1220,15 +1223,18 @@ static void filelayout_recover_commit_reqs(struct list_head *dst, | |||
1220 | struct nfs_commit_info *cinfo) | 1223 | struct nfs_commit_info *cinfo) |
1221 | { | 1224 | { |
1222 | struct pnfs_commit_bucket *b; | 1225 | struct pnfs_commit_bucket *b; |
1226 | struct pnfs_layout_segment *freeme; | ||
1223 | int i; | 1227 | int i; |
1224 | 1228 | ||
1229 | restart: | ||
1225 | spin_lock(cinfo->lock); | 1230 | spin_lock(cinfo->lock); |
1226 | for (i = 0, b = cinfo->ds->buckets; i < cinfo->ds->nbuckets; i++, b++) { | 1231 | for (i = 0, b = cinfo->ds->buckets; i < cinfo->ds->nbuckets; i++, b++) { |
1227 | if (transfer_commit_list(&b->written, dst, cinfo, 0)) { | 1232 | if (transfer_commit_list(&b->written, dst, cinfo, 0)) { |
1228 | spin_unlock(cinfo->lock); | 1233 | freeme = b->wlseg; |
1229 | pnfs_put_lseg(b->wlseg); | ||
1230 | b->wlseg = NULL; | 1234 | b->wlseg = NULL; |
1231 | spin_lock(cinfo->lock); | 1235 | spin_unlock(cinfo->lock); |
1236 | pnfs_put_lseg(freeme); | ||
1237 | goto restart; | ||
1232 | } | 1238 | } |
1233 | } | 1239 | } |
1234 | cinfo->ds->nwritten = 0; | 1240 | cinfo->ds->nwritten = 0; |
@@ -1243,6 +1249,7 @@ alloc_ds_commits(struct nfs_commit_info *cinfo, struct list_head *list) | |||
1243 | struct nfs_commit_data *data; | 1249 | struct nfs_commit_data *data; |
1244 | int i, j; | 1250 | int i, j; |
1245 | unsigned int nreq = 0; | 1251 | unsigned int nreq = 0; |
1252 | struct pnfs_layout_segment *freeme; | ||
1246 | 1253 | ||
1247 | fl_cinfo = cinfo->ds; | 1254 | fl_cinfo = cinfo->ds; |
1248 | bucket = fl_cinfo->buckets; | 1255 | bucket = fl_cinfo->buckets; |
@@ -1253,8 +1260,10 @@ alloc_ds_commits(struct nfs_commit_info *cinfo, struct list_head *list) | |||
1253 | if (!data) | 1260 | if (!data) |
1254 | break; | 1261 | break; |
1255 | data->ds_commit_index = i; | 1262 | data->ds_commit_index = i; |
1263 | spin_lock(cinfo->lock); | ||
1256 | data->lseg = bucket->clseg; | 1264 | data->lseg = bucket->clseg; |
1257 | bucket->clseg = NULL; | 1265 | bucket->clseg = NULL; |
1266 | spin_unlock(cinfo->lock); | ||
1258 | list_add(&data->pages, list); | 1267 | list_add(&data->pages, list); |
1259 | nreq++; | 1268 | nreq++; |
1260 | } | 1269 | } |
@@ -1264,8 +1273,11 @@ alloc_ds_commits(struct nfs_commit_info *cinfo, struct list_head *list) | |||
1264 | if (list_empty(&bucket->committing)) | 1273 | if (list_empty(&bucket->committing)) |
1265 | continue; | 1274 | continue; |
1266 | nfs_retry_commit(&bucket->committing, bucket->clseg, cinfo); | 1275 | nfs_retry_commit(&bucket->committing, bucket->clseg, cinfo); |
1267 | pnfs_put_lseg(bucket->clseg); | 1276 | spin_lock(cinfo->lock); |
1277 | freeme = bucket->clseg; | ||
1268 | bucket->clseg = NULL; | 1278 | bucket->clseg = NULL; |
1279 | spin_unlock(cinfo->lock); | ||
1280 | pnfs_put_lseg(freeme); | ||
1269 | } | 1281 | } |
1270 | /* Caller will clean up entries put on list */ | 1282 | /* Caller will clean up entries put on list */ |
1271 | return nreq; | 1283 | return nreq; |