Skip to content

Commit

Permalink
NFS: Fix O_DIRECT commit verifier handling
Browse files Browse the repository at this point in the history 
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: Trond Myklebust <trond.myklebust@hammerspace.com>
  • Loading branch information
Trond Myklebust committed Mar 27, 2020
1 parent fb5f7f2 commit 1f28476
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 124 deletions.
135 changes: 13 additions & 122 deletions fs/nfs/direct.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,6 @@ struct nfs_direct_req {
/* for read */
#define NFS_ODIRECT_SHOULD_DIRTY (3) /* dirty user-space page after read */
#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;
Expand Down Expand Up @@ -152,106 +151,6 @@ nfs_direct_count_bytes(struct nfs_direct_req *dreq,
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
* @iocb: target I/O control block
Expand Down Expand Up @@ -307,7 +206,6 @@ static inline struct nfs_direct_req *nfs_direct_req_alloc(void)
init_completion(&dreq->completion);
INIT_LIST_HEAD(&dreq->mds_cinfo.list);
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);
spin_lock_init(&dreq->lock);

Expand Down Expand Up @@ -637,7 +535,6 @@ static void nfs_direct_write_reschedule(struct nfs_direct_req *dreq)
dreq->max_count = 0;
list_for_each_entry(req, &reqs, wb_list)
dreq->max_count += req->wb_bytes;
dreq->verf.committed = NFS_INVALID_STABLE_HOW;
nfs_clear_pnfs_ds_commit_verifiers(&dreq->ds_cinfo);
get_dreq(dreq);

Expand Down Expand Up @@ -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)
{
const struct nfs_writeverf *verf = data->res.verf;
struct nfs_direct_req *dreq = data->dreq;
struct nfs_commit_info cinfo;
struct nfs_page *req;
Expand All @@ -689,21 +587,19 @@ static void nfs_direct_commit_complete(struct nfs_commit_data *data)
status = dreq->error;

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)) {
req = nfs_list_entry(data->pages.next);
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,
* so reset wb_nio.
*/
req->wb_nio = 0;
/* Note the rewrite will go through mds */
nfs_mark_request_commit(req, NULL, &cinfo, 0);
} else
} else /* Error or match */
nfs_release_request(req);
nfs_unlock_and_release_request(req);
}
Expand Down Expand Up @@ -799,20 +695,15 @@ static void nfs_direct_write_completion(struct nfs_pgio_header *hdr)
}

nfs_direct_count_bytes(dreq, hdr);
if (hdr->good_bytes != 0) {
if (nfs_write_need_commit(hdr)) {
if (dreq->flags == NFS_ODIRECT_RESCHED_WRITES)
request_commit = true;
else if (dreq->flags == 0) {
nfs_direct_set_hdr_verf(dreq, hdr);
request_commit = true;
dreq->flags = NFS_ODIRECT_DO_COMMIT;
} else if (dreq->flags == NFS_ODIRECT_DO_COMMIT) {
request_commit = true;
if (nfs_direct_set_or_cmp_hdr_verf(dreq, hdr))
dreq->flags =
NFS_ODIRECT_RESCHED_WRITES;
}
if (hdr->good_bytes != 0 && nfs_write_need_commit(hdr)) {
switch (dreq->flags) {
case 0:
dreq->flags = NFS_ODIRECT_DO_COMMIT;
request_commit = true;
break;
case NFS_ODIRECT_RESCHED_WRITES:
case NFS_ODIRECT_DO_COMMIT:
request_commit = true;
}
}
spin_unlock(&dreq->lock);
Expand Down
8 changes: 8 additions & 0 deletions fs/nfs/internal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -544,6 +544,14 @@ nfs_write_verifier_cmp(const struct nfs_write_verifier *v1,
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 */
extern struct rpc_task *
nfs_async_rename(struct inode *old_dir, struct inode *new_dir,
Expand Down
3 changes: 1 addition & 2 deletions fs/nfs/write.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1874,8 +1874,7 @@ static void nfs_commit_release_pages(struct nfs_commit_data *data)

/* Okay, COMMIT succeeded, apparently. Check the verifier
* returned by the server against all stored verfs. */
if (verf->committed > NFS_UNSTABLE &&
!nfs_write_verifier_cmp(&req->wb_verf, &verf->verifier)) {
if (nfs_write_match_verf(verf, req)) {
/* We have a match */
if (req->wb_page)
nfs_inode_remove_request(req);
Expand Down

0 comments on commit 1f28476

Please sign in to comment.