Commit 44a65a0c authored by Trond Myklebust's avatar Trond Myklebust

NFS: Reverse the submission order of requests in __nfs_pageio_add_request()

If we have to split the request up into subrequests, we have to submit
the request pointed to by the function call parameter last, in case
there is an error or other issue that causes us to exit before the
last request is submitted. The reason is that the caller is expected
to perform cleanup in those cases.
Signed-off-by: default avatarTrond Myklebust <trond.myklebust@hammerspace.com>
parent a62f8e3b
...@@ -454,15 +454,23 @@ nfs_create_request(struct nfs_open_context *ctx, struct page *page, ...@@ -454,15 +454,23 @@ nfs_create_request(struct nfs_open_context *ctx, struct page *page,
} }
static struct nfs_page * static struct nfs_page *
nfs_create_subreq(struct nfs_page *req, struct nfs_page *last, nfs_create_subreq(struct nfs_page *req,
unsigned int pgbase, unsigned int offset, unsigned int pgbase,
unsigned int offset,
unsigned int count) unsigned int count)
{ {
struct nfs_page *last;
struct nfs_page *ret; struct nfs_page *ret;
ret = __nfs_create_request(req->wb_lock_context, req->wb_page, ret = __nfs_create_request(req->wb_lock_context, req->wb_page,
pgbase, offset, count); pgbase, offset, count);
if (!IS_ERR(ret)) { if (!IS_ERR(ret)) {
/* find the last request */
for (last = req->wb_head;
last->wb_this_page != req->wb_head;
last = last->wb_this_page)
;
nfs_lock_request(ret); nfs_lock_request(ret);
ret->wb_index = req->wb_index; ret->wb_index = req->wb_index;
nfs_page_group_init(ret, last); nfs_page_group_init(ret, last);
...@@ -988,7 +996,7 @@ static bool nfs_match_lock_context(const struct nfs_lock_context *l1, ...@@ -988,7 +996,7 @@ static bool nfs_match_lock_context(const struct nfs_lock_context *l1,
} }
/** /**
* nfs_can_coalesce_requests - test two requests for compatibility * nfs_coalesce_size - test two requests for compatibility
* @prev: pointer to nfs_page * @prev: pointer to nfs_page
* @req: pointer to nfs_page * @req: pointer to nfs_page
* @pgio: pointer to nfs_pagio_descriptor * @pgio: pointer to nfs_pagio_descriptor
...@@ -997,41 +1005,36 @@ static bool nfs_match_lock_context(const struct nfs_lock_context *l1, ...@@ -997,41 +1005,36 @@ static bool nfs_match_lock_context(const struct nfs_lock_context *l1,
* page data area they describe is contiguous, and that their RPC * page data area they describe is contiguous, and that their RPC
* credentials, NFSv4 open state, and lockowners are the same. * credentials, NFSv4 open state, and lockowners are the same.
* *
* Return 'true' if this is the case, else return 'false'. * Returns size of the request that can be coalesced
*/ */
static bool nfs_can_coalesce_requests(struct nfs_page *prev, static unsigned int nfs_coalesce_size(struct nfs_page *prev,
struct nfs_page *req, struct nfs_page *req,
struct nfs_pageio_descriptor *pgio) struct nfs_pageio_descriptor *pgio)
{ {
size_t size;
struct file_lock_context *flctx; struct file_lock_context *flctx;
if (prev) { if (prev) {
if (!nfs_match_open_context(nfs_req_openctx(req), nfs_req_openctx(prev))) if (!nfs_match_open_context(nfs_req_openctx(req), nfs_req_openctx(prev)))
return false; return 0;
flctx = d_inode(nfs_req_openctx(req)->dentry)->i_flctx; flctx = d_inode(nfs_req_openctx(req)->dentry)->i_flctx;
if (flctx != NULL && if (flctx != NULL &&
!(list_empty_careful(&flctx->flc_posix) && !(list_empty_careful(&flctx->flc_posix) &&
list_empty_careful(&flctx->flc_flock)) && list_empty_careful(&flctx->flc_flock)) &&
!nfs_match_lock_context(req->wb_lock_context, !nfs_match_lock_context(req->wb_lock_context,
prev->wb_lock_context)) prev->wb_lock_context))
return false; return 0;
if (req_offset(req) != req_offset(prev) + prev->wb_bytes) if (req_offset(req) != req_offset(prev) + prev->wb_bytes)
return false; return 0;
if (req->wb_page == prev->wb_page) { if (req->wb_page == prev->wb_page) {
if (req->wb_pgbase != prev->wb_pgbase + prev->wb_bytes) if (req->wb_pgbase != prev->wb_pgbase + prev->wb_bytes)
return false; return 0;
} else { } else {
if (req->wb_pgbase != 0 || if (req->wb_pgbase != 0 ||
prev->wb_pgbase + prev->wb_bytes != PAGE_SIZE) prev->wb_pgbase + prev->wb_bytes != PAGE_SIZE)
return false; return 0;
} }
} }
size = pgio->pg_ops->pg_test(pgio, prev, req); return pgio->pg_ops->pg_test(pgio, prev, req);
WARN_ON_ONCE(size > req->wb_bytes);
if (size && size < req->wb_bytes)
req->wb_bytes = size;
return size > 0;
} }
/** /**
...@@ -1039,15 +1042,16 @@ static bool nfs_can_coalesce_requests(struct nfs_page *prev, ...@@ -1039,15 +1042,16 @@ static bool nfs_can_coalesce_requests(struct nfs_page *prev,
* @desc: destination io descriptor * @desc: destination io descriptor
* @req: request * @req: request
* *
* Returns true if the request 'req' was successfully coalesced into the * If the request 'req' was successfully coalesced into the existing list
* existing list of pages 'desc'. * of pages 'desc', it returns the size of req.
*/ */
static int nfs_pageio_do_add_request(struct nfs_pageio_descriptor *desc, static unsigned int
nfs_pageio_do_add_request(struct nfs_pageio_descriptor *desc,
struct nfs_page *req) struct nfs_page *req)
{ {
struct nfs_pgio_mirror *mirror = nfs_pgio_current_mirror(desc); struct nfs_pgio_mirror *mirror = nfs_pgio_current_mirror(desc);
struct nfs_page *prev = NULL; struct nfs_page *prev = NULL;
unsigned int size;
if (mirror->pg_count != 0) { if (mirror->pg_count != 0) {
prev = nfs_list_entry(mirror->pg_list.prev); prev = nfs_list_entry(mirror->pg_list.prev);
...@@ -1067,11 +1071,12 @@ static int nfs_pageio_do_add_request(struct nfs_pageio_descriptor *desc, ...@@ -1067,11 +1071,12 @@ static int nfs_pageio_do_add_request(struct nfs_pageio_descriptor *desc,
return 0; return 0;
} }
if (!nfs_can_coalesce_requests(prev, req, desc)) size = nfs_coalesce_size(prev, req, desc);
return 0; if (size < req->wb_bytes)
return size;
nfs_list_move_request(req, &mirror->pg_list); nfs_list_move_request(req, &mirror->pg_list);
mirror->pg_count += req->wb_bytes; mirror->pg_count += req->wb_bytes;
return 1; return req->wb_bytes;
} }
/* /*
...@@ -1111,7 +1116,8 @@ nfs_pageio_cleanup_request(struct nfs_pageio_descriptor *desc, ...@@ -1111,7 +1116,8 @@ nfs_pageio_cleanup_request(struct nfs_pageio_descriptor *desc,
* @req: request * @req: request
* *
* This may split a request into subrequests which are all part of the * This may split a request into subrequests which are all part of the
* same page group. * same page group. If so, it will submit @req as the last one, to ensure
* the pointer to @req is still valid in case of failure.
* *
* Returns true if the request 'req' was successfully coalesced into the * Returns true if the request 'req' was successfully coalesced into the
* existing list of pages 'desc'. * existing list of pages 'desc'.
...@@ -1120,51 +1126,50 @@ static int __nfs_pageio_add_request(struct nfs_pageio_descriptor *desc, ...@@ -1120,51 +1126,50 @@ static int __nfs_pageio_add_request(struct nfs_pageio_descriptor *desc,
struct nfs_page *req) struct nfs_page *req)
{ {
struct nfs_pgio_mirror *mirror = nfs_pgio_current_mirror(desc); struct nfs_pgio_mirror *mirror = nfs_pgio_current_mirror(desc);
struct nfs_page *subreq; struct nfs_page *subreq;
unsigned int bytes_left = 0; unsigned int size, subreq_size;
unsigned int offset, pgbase;
nfs_page_group_lock(req); nfs_page_group_lock(req);
subreq = req; subreq = req;
bytes_left = subreq->wb_bytes; subreq_size = subreq->wb_bytes;
offset = subreq->wb_offset; for(;;) {
pgbase = subreq->wb_pgbase; size = nfs_pageio_do_add_request(desc, subreq);
if (size == subreq_size) {
do { /* We successfully submitted a request */
if (!nfs_pageio_do_add_request(desc, subreq)) { if (subreq == req)
/* make sure pg_test call(s) did nothing */ break;
WARN_ON_ONCE(subreq->wb_bytes != bytes_left); req->wb_pgbase += size;
WARN_ON_ONCE(subreq->wb_offset != offset); req->wb_bytes -= size;
WARN_ON_ONCE(subreq->wb_pgbase != pgbase); req->wb_offset += size;
subreq_size = req->wb_bytes;
subreq = req;
continue;
}
if (WARN_ON_ONCE(subreq != req)) {
nfs_page_group_unlock(req);
nfs_pageio_cleanup_request(desc, subreq);
subreq = req;
subreq_size = req->wb_bytes;
nfs_page_group_lock(req);
}
if (!size) {
/* Can't coalesce any more, so do I/O */
nfs_page_group_unlock(req); nfs_page_group_unlock(req);
desc->pg_moreio = 1; desc->pg_moreio = 1;
nfs_pageio_doio(desc); nfs_pageio_doio(desc);
if (desc->pg_error < 0 || mirror->pg_recoalesce) if (desc->pg_error < 0 || mirror->pg_recoalesce)
goto out_cleanup_subreq; return 0;
/* retry add_request for this subreq */ /* retry add_request for this subreq */
nfs_page_group_lock(req); nfs_page_group_lock(req);
continue; continue;
} }
subreq = nfs_create_subreq(req, req->wb_pgbase,
/* check for buggy pg_test call(s) */ req->wb_offset, size);
WARN_ON_ONCE(subreq->wb_bytes + subreq->wb_pgbase > PAGE_SIZE);
WARN_ON_ONCE(subreq->wb_bytes > bytes_left);
WARN_ON_ONCE(subreq->wb_bytes == 0);
bytes_left -= subreq->wb_bytes;
offset += subreq->wb_bytes;
pgbase += subreq->wb_bytes;
if (bytes_left) {
subreq = nfs_create_subreq(req, subreq, pgbase,
offset, bytes_left);
if (IS_ERR(subreq)) if (IS_ERR(subreq))
goto err_ptr; goto err_ptr;
subreq_size = size;
} }
} while (bytes_left > 0);
nfs_page_group_unlock(req); nfs_page_group_unlock(req);
return 1; return 1;
...@@ -1172,10 +1177,6 @@ static int __nfs_pageio_add_request(struct nfs_pageio_descriptor *desc, ...@@ -1172,10 +1177,6 @@ static int __nfs_pageio_add_request(struct nfs_pageio_descriptor *desc,
desc->pg_error = PTR_ERR(subreq); desc->pg_error = PTR_ERR(subreq);
nfs_page_group_unlock(req); nfs_page_group_unlock(req);
return 0; return 0;
out_cleanup_subreq:
if (req != subreq)
nfs_pageio_cleanup_request(desc, subreq);
return 0;
} }
static int nfs_do_recoalesce(struct nfs_pageio_descriptor *desc) static int nfs_do_recoalesce(struct nfs_pageio_descriptor *desc)
...@@ -1244,7 +1245,7 @@ int nfs_pageio_add_request(struct nfs_pageio_descriptor *desc, ...@@ -1244,7 +1245,7 @@ int nfs_pageio_add_request(struct nfs_pageio_descriptor *desc,
{ {
u32 midx; u32 midx;
unsigned int pgbase, offset, bytes; unsigned int pgbase, offset, bytes;
struct nfs_page *dupreq, *lastreq; struct nfs_page *dupreq;
pgbase = req->wb_pgbase; pgbase = req->wb_pgbase;
offset = req->wb_offset; offset = req->wb_offset;
...@@ -1258,13 +1259,7 @@ int nfs_pageio_add_request(struct nfs_pageio_descriptor *desc, ...@@ -1258,13 +1259,7 @@ int nfs_pageio_add_request(struct nfs_pageio_descriptor *desc,
for (midx = 1; midx < desc->pg_mirror_count; midx++) { for (midx = 1; midx < desc->pg_mirror_count; midx++) {
nfs_page_group_lock(req); nfs_page_group_lock(req);
/* find the last request */ dupreq = nfs_create_subreq(req,
for (lastreq = req->wb_head;
lastreq->wb_this_page != req->wb_head;
lastreq = lastreq->wb_this_page)
;
dupreq = nfs_create_subreq(req, lastreq,
pgbase, offset, bytes); pgbase, offset, bytes);
nfs_page_group_unlock(req); nfs_page_group_unlock(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