linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Boaz Harrosh <bharrosh@panasas.com>
To: Benny Halevy <bhalevy@panasas.com>,
	Trond Myklebust <Trond.Myklebust@netapp.com>,
	NFS list <linux-nfs@vger.kernel.org>
Subject: [PATCH 10/13] [RFC] Bugs in new pnfs write path
Date: Sat, 21 May 2011 13:33:02 +0300	[thread overview]
Message-ID: <1305973982-17499-1-git-send-email-bharrosh@panasas.com> (raw)
In-Reply-To: <4DD79273.9030202@panasas.com>

1. In nfs4_write_done_cb: data->write_done_cb comes with a NULL.
   Just as a guess I call nfs4_write_done_cb() just above it
   it looked like the right thing todo. With that in, I'm able
   to write things to file When converting pnfs.c:258 to a WARN_ON.

   Benny we might want to set data->write_done_cb somewhere in the
   none-rpc path? where is it best to do that?

2. In pnfs_ld_write_done:
	put_lseg(data->lseg);
	data->lseg = NULL;
   was done before the call to pnfs_set_layoutcommit()
   which trys to get_lseg() on that same data->lseg.

3. In pnfs_ld_write_done:
   data->mds_ops->rpc_call_done(NULL, data);
   crashes with a NULL task. Just pass it with &data->task

   Which calls for a cleanup. There is bunch of functions
   with [task, write_data] API. And the task is always
   write_data->task

Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
 fs/nfs/nfs4proc.c |    3 ++-
 fs/nfs/pnfs.c     |   10 ++++++----
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 759523a..1a53187 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -3250,7 +3250,8 @@ static int nfs4_write_done(struct rpc_task *task, struct nfs_write_data *data)
 {
 	if (!nfs4_sequence_done(task, &data->res.seq_res))
 		return -EAGAIN;
-	return data->write_done_cb(task, data);
+	return data->write_done_cb ? data->write_done_cb(task, data) :
+		nfs4_write_done_cb(task, data);
 }
 
 /* Reset the the nfs_write_data to send the write to the MDS. */
diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index 17d0c4c..b04cdb4 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -255,7 +255,7 @@ put_lseg_common(struct pnfs_layout_segment *lseg)
 {
 	struct inode *inode = lseg->pls_layout->plh_inode;
 
-	BUG_ON(test_bit(NFS_LSEG_VALID, &lseg->pls_flags));
+ 	WARN_ON(test_bit(NFS_LSEG_VALID, &lseg->pls_flags));
 	list_del_init(&lseg->pls_list);
 	if (list_empty(&lseg->pls_layout->plh_segs)) {
 		set_bit(NFS_LAYOUT_DESTROYED, &lseg->pls_layout->plh_flags);
@@ -1124,15 +1124,17 @@ pnfs_ld_write_done(struct nfs_write_data *data)
 {
 	int status;
 
-	put_lseg(data->lseg);
-	data->lseg = NULL;
 	if (!data->pnfs_error) {
 		pnfs_set_layoutcommit(data);
-		data->mds_ops->rpc_call_done(NULL, data);
+		data->mds_ops->rpc_call_done(&data->task, data);
 		data->mds_ops->rpc_release(data);
+		put_lseg(data->lseg);
+		data->lseg = NULL;
 		return 0;
 	}
 
+	put_lseg(data->lseg);
+	data->lseg = NULL;
 	dprintk("%s: pnfs_error=%d, retry via MDS\n", __func__,
 		data->pnfs_error);
 	status = nfs_initiate_write(data, NFS_CLIENT(data->inode), data->mds_ops, NFS_FILE_SYNC);
-- 
1.7.2.3


  parent reply	other threads:[~2011-05-21 10:33 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-21 10:22 [PATCHSET 00/13] SQUASHME pnfs-obj: Lots of changes addressing comments by Trond and Benny Boaz Harrosh
2011-05-21 10:29 ` [PATCH 01/13] SQUASHME: re-reorder the functions so it compares better with raids base Boaz Harrosh
2011-05-21 11:06   ` pnfs-obj: git diff pnfs-all-2.6.38 to pnfs-submit. Without the reordering Boaz Harrosh
2011-05-21 10:30 ` [PATCH 02/13] SQUASHME: pnfs-obj: objio_segment only needs the pnfs_osd_layout->olo_comps Boaz Harrosh
2011-05-21 10:30 ` [PATCH 03/13] SQUASHME: pnfs-obj: Rename ios->objio_seg => ios->layout Boaz Harrosh
2011-05-21 10:31 ` [PATCH 05/13] SQUASHME: pnfs-obj: Convert layout and deviceinfo decoding to new XDR Boaz Harrosh
2011-05-21 10:31 ` [PATCH 06/13] SQUASHME: pnfs-obj: Change API of objlayout_io_set_result Boaz Harrosh
2011-05-21 10:31 ` [PATCH 07/13] SQUASHME: pnfs-obj: Avoid double allocation logic in objlayout_alloc_lseg Boaz Harrosh
2011-05-21 10:32 ` [PATCH 08/13] SQUASHME: pnfs_osd_xdr: Remove Server API declarations Boaz Harrosh
2011-05-21 10:32 ` [PATCH 09/13] SQUASHME: pnfs_osd_xdr: Avoid using xdr_rewind_stream Boaz Harrosh
2011-05-21 10:33 ` Boaz Harrosh [this message]
2011-05-22 18:08   ` [PATCH 10/13] [RFC] Bugs in new pnfs write path Benny Halevy
2011-05-23  4:19     ` Boaz Harrosh
2011-05-23  4:26       ` Boaz Harrosh
2011-05-21 10:33 ` [PATCH 11/13] SQUASHME: pnfs_osd_xdr: Add Server API for encoding/decoding osd XDRs Boaz Harrosh
2011-05-21 10:34 ` [PATCH 12/13] SQUASHME: XDR API changes to pnfs_osd_xdr_decode_ioerr() Boaz Harrosh
2011-05-21 10:34 ` [PATCH 13/13] SQUASHME: dbg Print the full device_id returned Boaz Harrosh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1305973982-17499-1-git-send-email-bharrosh@panasas.com \
    --to=bharrosh@panasas.com \
    --cc=Trond.Myklebust@netapp.com \
    --cc=bhalevy@panasas.com \
    --cc=linux-nfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).