From patchwork Sat May 21 08:43:22 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Boaz Harrosh X-Patchwork-Id: 805432 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by demeter2.kernel.org (8.14.4/8.14.3) with ESMTP id p4L8hmlH014771 for ; Sat, 21 May 2011 08:43:48 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753817Ab1EUInb (ORCPT ); Sat, 21 May 2011 04:43:31 -0400 Received: from daytona.panasas.com ([67.152.220.89]:18603 "EHLO daytona.panasas.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753584Ab1EUIn2 (ORCPT ); Sat, 21 May 2011 04:43:28 -0400 Received: from fs2.bhalevy.com ([172.17.33.46]) by daytona.panasas.com with Microsoft SMTPSVC(6.0.3790.4675); Sat, 21 May 2011 04:43:27 -0400 Message-ID: <4DD77B2A.5080707@panasas.com> Date: Sat, 21 May 2011 11:43:22 +0300 From: Boaz Harrosh User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.17) Gecko/20110430 Remi/fc12 Thunderbird/3.1.10 MIME-Version: 1.0 To: Benny Halevy , Trond Myklebust , NFS list Subject: [RFC] Bugs in new pnfs write path References: <4DD772E4.2030505@panasas.com> In-Reply-To: <4DD772E4.2030505@panasas.com> X-OriginalArrivalTime: 21 May 2011 08:43:27.0292 (UTC) FILETIME=[273E2FC0:01CC1793] Sender: linux-nfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-nfs@vger.kernel.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.6 (demeter2.kernel.org [140.211.167.43]); Sat, 21 May 2011 08:43:48 +0000 (UTC) 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 --- 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);