Commit 1f28476d authored by Trond Myklebust's avatar Trond Myklebust

NFS: Fix O_DIRECT commit verifier handling

Instead of trying to save the commit verifiers and checking them against
previous writes, adopt the same strategy as for buffered writes, of
just checking the verifiers at commit time.
Signed-off-by: default avatarTrond Myklebust <trond.myklebust@hammerspace.com>
parent fb5f7f20
...@@ -95,7 +95,6 @@ struct nfs_direct_req { ...@@ -95,7 +95,6 @@ struct nfs_direct_req {
/* for read */ /* for read */
#define NFS_ODIRECT_SHOULD_DIRTY (3) /* dirty user-space page after read */ #define NFS_ODIRECT_SHOULD_DIRTY (3) /* dirty user-space page after read */
#define NFS_ODIRECT_DONE INT_MAX /* write verification failed */ #define NFS_ODIRECT_DONE INT_MAX /* write verification failed */
struct nfs_writeverf verf; /* unstable write verifier */
}; };
static const struct nfs_pgio_completion_ops nfs_direct_write_completion_ops; static const struct nfs_pgio_completion_ops nfs_direct_write_completion_ops;
...@@ -152,106 +151,6 @@ nfs_direct_count_bytes(struct nfs_direct_req *dreq, ...@@ -152,106 +151,6 @@ nfs_direct_count_bytes(struct nfs_direct_req *dreq,
dreq->count = dreq_len; dreq->count = dreq_len;
} }
/*
* nfs_direct_select_verf - select the right verifier
* @dreq - direct request possibly spanning multiple servers
* @ds_clp - nfs_client of data server or NULL if MDS / non-pnfs
* @commit_idx - commit bucket index for the DS
*
* returns the correct verifier to use given the role of the server
*/
static struct nfs_writeverf *
nfs_direct_select_verf(struct nfs_direct_req *dreq,
struct nfs_client *ds_clp,
int commit_idx)
{
struct nfs_writeverf *verfp = &dreq->verf;
#ifdef CONFIG_NFS_V4_1
/*
* pNFS is in use, use the DS verf except commit_through_mds is set
* for layout segment where nbuckets is zero.
*/
if (ds_clp && dreq->ds_cinfo.nbuckets > 0) {
if (commit_idx >= 0 && commit_idx < dreq->ds_cinfo.nbuckets)
verfp = &dreq->ds_cinfo.buckets[commit_idx].direct_verf;
else
WARN_ON_ONCE(1);
}
#endif
return verfp;
}
/*
* nfs_direct_set_hdr_verf - set the write/commit verifier
* @dreq - direct request possibly spanning multiple servers
* @hdr - pageio header to validate against previously seen verfs
*
* Set the server's (MDS or DS) "seen" verifier
*/
static void nfs_direct_set_hdr_verf(struct nfs_direct_req *dreq,
struct nfs_pgio_header *hdr)
{
struct nfs_writeverf *verfp;
verfp = nfs_direct_select_verf(dreq, hdr->ds_clp, hdr->ds_commit_idx);
WARN_ON_ONCE(verfp->committed >= 0);
memcpy(verfp, &hdr->verf, sizeof(struct nfs_writeverf));
WARN_ON_ONCE(verfp->committed < 0);
}
static int nfs_direct_cmp_verf(const struct nfs_writeverf *v1,
const struct nfs_writeverf *v2)
{
return nfs_write_verifier_cmp(&v1->verifier, &v2->verifier);
}
/*
* nfs_direct_cmp_hdr_verf - compare verifier for pgio header
* @dreq - direct request possibly spanning multiple servers
* @hdr - pageio header to validate against previously seen verf
*
* set the server's "seen" verf if not initialized.
* returns result of comparison between @hdr->verf and the "seen"
* verf of the server used by @hdr (DS or MDS)
*/
static int nfs_direct_set_or_cmp_hdr_verf(struct nfs_direct_req *dreq,
struct nfs_pgio_header *hdr)
{
struct nfs_writeverf *verfp;
verfp = nfs_direct_select_verf(dreq, hdr->ds_clp, hdr->ds_commit_idx);
if (verfp->committed < 0) {
nfs_direct_set_hdr_verf(dreq, hdr);
return 0;
}
return nfs_direct_cmp_verf(verfp, &hdr->verf);
}
/*
* nfs_direct_cmp_commit_data_verf - compare verifier for commit data
* @dreq - direct request possibly spanning multiple servers
* @data - commit data to validate against previously seen verf
*
* returns result of comparison between @data->verf and the verf of
* the server used by @data (DS or MDS)
*/
static int nfs_direct_cmp_commit_data_verf(struct nfs_direct_req *dreq,
struct nfs_commit_data *data)
{
struct nfs_writeverf *verfp;
verfp = nfs_direct_select_verf(dreq, data->ds_clp,
data->ds_commit_index);
/* verifier not set so always fail */
if (verfp->committed < 0 || data->res.verf->committed <= NFS_UNSTABLE)
return 1;
return nfs_direct_cmp_verf(verfp, data->res.verf);
}
/** /**
* nfs_direct_IO - NFS address space operation for direct I/O * nfs_direct_IO - NFS address space operation for direct I/O
* @iocb: target I/O control block * @iocb: target I/O control block
...@@ -307,7 +206,6 @@ static inline struct nfs_direct_req *nfs_direct_req_alloc(void) ...@@ -307,7 +206,6 @@ static inline struct nfs_direct_req *nfs_direct_req_alloc(void)
init_completion(&dreq->completion); init_completion(&dreq->completion);
INIT_LIST_HEAD(&dreq->mds_cinfo.list); INIT_LIST_HEAD(&dreq->mds_cinfo.list);
pnfs_init_ds_commit_info(&dreq->ds_cinfo); pnfs_init_ds_commit_info(&dreq->ds_cinfo);
dreq->verf.committed = NFS_INVALID_STABLE_HOW; /* not set yet */
INIT_WORK(&dreq->work, nfs_direct_write_schedule_work); INIT_WORK(&dreq->work, nfs_direct_write_schedule_work);
spin_lock_init(&dreq->lock); spin_lock_init(&dreq->lock);
...@@ -637,7 +535,6 @@ static void nfs_direct_write_reschedule(struct nfs_direct_req *dreq) ...@@ -637,7 +535,6 @@ static void nfs_direct_write_reschedule(struct nfs_direct_req *dreq)
dreq->max_count = 0; dreq->max_count = 0;
list_for_each_entry(req, &reqs, wb_list) list_for_each_entry(req, &reqs, wb_list)
dreq->max_count += req->wb_bytes; dreq->max_count += req->wb_bytes;
dreq->verf.committed = NFS_INVALID_STABLE_HOW;
nfs_clear_pnfs_ds_commit_verifiers(&dreq->ds_cinfo); nfs_clear_pnfs_ds_commit_verifiers(&dreq->ds_cinfo);
get_dreq(dreq); get_dreq(dreq);
...@@ -674,6 +571,7 @@ static void nfs_direct_write_reschedule(struct nfs_direct_req *dreq) ...@@ -674,6 +571,7 @@ static void nfs_direct_write_reschedule(struct nfs_direct_req *dreq)
static void nfs_direct_commit_complete(struct nfs_commit_data *data) static void nfs_direct_commit_complete(struct nfs_commit_data *data)
{ {
const struct nfs_writeverf *verf = data->res.verf;
struct nfs_direct_req *dreq = data->dreq; struct nfs_direct_req *dreq = data->dreq;
struct nfs_commit_info cinfo; struct nfs_commit_info cinfo;
struct nfs_page *req; struct nfs_page *req;
...@@ -689,21 +587,19 @@ static void nfs_direct_commit_complete(struct nfs_commit_data *data) ...@@ -689,21 +587,19 @@ static void nfs_direct_commit_complete(struct nfs_commit_data *data)
status = dreq->error; status = dreq->error;
nfs_init_cinfo_from_dreq(&cinfo, dreq); nfs_init_cinfo_from_dreq(&cinfo, dreq);
if (nfs_direct_cmp_commit_data_verf(dreq, data))
dreq->flags = NFS_ODIRECT_RESCHED_WRITES;
while (!list_empty(&data->pages)) { while (!list_empty(&data->pages)) {
req = nfs_list_entry(data->pages.next); req = nfs_list_entry(data->pages.next);
nfs_list_remove_request(req); nfs_list_remove_request(req);
if (dreq->flags == NFS_ODIRECT_RESCHED_WRITES) { if (status >= 0 && !nfs_write_match_verf(verf, req)) {
dreq->flags = NFS_ODIRECT_RESCHED_WRITES;
/* /*
* Despite the reboot, the write was successful, * Despite the reboot, the write was successful,
* so reset wb_nio. * so reset wb_nio.
*/ */
req->wb_nio = 0; req->wb_nio = 0;
/* Note the rewrite will go through mds */
nfs_mark_request_commit(req, NULL, &cinfo, 0); nfs_mark_request_commit(req, NULL, &cinfo, 0);
} else } else /* Error or match */
nfs_release_request(req); nfs_release_request(req);
nfs_unlock_and_release_request(req); nfs_unlock_and_release_request(req);
} }
...@@ -799,20 +695,15 @@ static void nfs_direct_write_completion(struct nfs_pgio_header *hdr) ...@@ -799,20 +695,15 @@ static void nfs_direct_write_completion(struct nfs_pgio_header *hdr)
} }
nfs_direct_count_bytes(dreq, hdr); nfs_direct_count_bytes(dreq, hdr);
if (hdr->good_bytes != 0) { if (hdr->good_bytes != 0 && nfs_write_need_commit(hdr)) {
if (nfs_write_need_commit(hdr)) { switch (dreq->flags) {
if (dreq->flags == NFS_ODIRECT_RESCHED_WRITES) case 0:
request_commit = true;
else if (dreq->flags == 0) {
nfs_direct_set_hdr_verf(dreq, hdr);
request_commit = true;
dreq->flags = NFS_ODIRECT_DO_COMMIT; dreq->flags = NFS_ODIRECT_DO_COMMIT;
} else if (dreq->flags == NFS_ODIRECT_DO_COMMIT) {
request_commit = true; request_commit = true;
if (nfs_direct_set_or_cmp_hdr_verf(dreq, hdr)) break;
dreq->flags = case NFS_ODIRECT_RESCHED_WRITES:
NFS_ODIRECT_RESCHED_WRITES; case NFS_ODIRECT_DO_COMMIT:
} request_commit = true;
} }
} }
spin_unlock(&dreq->lock); spin_unlock(&dreq->lock);
......
...@@ -544,6 +544,14 @@ nfs_write_verifier_cmp(const struct nfs_write_verifier *v1, ...@@ -544,6 +544,14 @@ nfs_write_verifier_cmp(const struct nfs_write_verifier *v1,
return memcmp(v1->data, v2->data, sizeof(v1->data)); return memcmp(v1->data, v2->data, sizeof(v1->data));
} }
static inline bool
nfs_write_match_verf(const struct nfs_writeverf *verf,
struct nfs_page *req)
{
return verf->committed > NFS_UNSTABLE &&
!nfs_write_verifier_cmp(&req->wb_verf, &verf->verifier);
}
/* unlink.c */ /* unlink.c */
extern struct rpc_task * extern struct rpc_task *
nfs_async_rename(struct inode *old_dir, struct inode *new_dir, nfs_async_rename(struct inode *old_dir, struct inode *new_dir,
......
...@@ -1874,8 +1874,7 @@ static void nfs_commit_release_pages(struct nfs_commit_data *data) ...@@ -1874,8 +1874,7 @@ static void nfs_commit_release_pages(struct nfs_commit_data *data)
/* Okay, COMMIT succeeded, apparently. Check the verifier /* Okay, COMMIT succeeded, apparently. Check the verifier
* returned by the server against all stored verfs. */ * returned by the server against all stored verfs. */
if (verf->committed > NFS_UNSTABLE && if (nfs_write_match_verf(verf, req)) {
!nfs_write_verifier_cmp(&req->wb_verf, &verf->verifier)) {
/* We have a match */ /* We have a match */
if (req->wb_page) if (req->wb_page)
nfs_inode_remove_request(req); nfs_inode_remove_request(req);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment