Return-Path: Received: from mx2.netapp.com ([216.240.18.37]:11176 "EHLO mx2.netapp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751364Ab1GYTNl convert rfc822-to-8bit (ORCPT ); Mon, 25 Jul 2011 15:13:41 -0400 Subject: RE: [PATCH] NFS41: Drop lseg ref before fallthru to MDS From: Trond Myklebust To: tao.peng@emc.com Cc: linux-nfs@vger.kernel.org, bhalevy@tonian.com, bergwolf@gmail.com Date: Mon, 25 Jul 2011 15:13:24 -0400 In-Reply-To: References: <1309743002-1658-1-git-send-email-bergwolf@gmail.com> <4E18614C.4010002@tonian.com> Content-Type: text/plain; charset="UTF-8" Message-ID: <1311621204.28209.14.camel@lade.trondhjem.org> Sender: linux-nfs-owner@vger.kernel.org List-ID: MIME-Version: 1.0 On Wed, 2011-07-20 at 01:52 -0400, tao.peng@emc.com wrote: > Hi, Trond, > > Any comments on this patch? I still get kernel crash when pnfs write is attempted but fails and calls pnfs_ld_write_done(). It seems object layout uses the same code path as well. But I don't find the patch in either your tree or Benny's tree. Are there any concerns? > > Thanks, > Tao The whole pnfs_ld_write_done thing is bogus and needs to be replaced with something sane. It is trying to initiate a WRITE RPC call with the wrong block size, and is calling the MDS rpc_call_done() and rpc_release() with an uninitialised rpc task pointer. Ditto for pnfs_ld_read_done. Cheers Trond > > -----Original Message----- > > From: Benny Halevy [mailto:bhalevy@tonian.com] > > Sent: Saturday, July 09, 2011 10:10 PM > > To: Peng Tao > > Cc: Trond.Myklebust@netapp.com; linux-nfs@vger.kernel.org; Peng, Tao > > Subject: Re: [PATCH] NFS41: Drop lseg ref before fallthru to MDS > > > > On 2011-07-04 04:30, Peng Tao wrote: > > > There is no need to keep lseg reference when read/write through MDS. > > > This fixes a null pointer crash at nfs_post_op_update_inode_force_wcc > > > because nfs4_proc_write_setup will unset wdata->res.fattr if wdata->lseg > > > is not NULL. > > > > > > Signed-off-by: Peng Tao > > > > Looks good to me. > > > > Benny > > > > > --- > > > fs/nfs/pnfs.c | 6 ++++++ > > > 1 files changed, 6 insertions(+), 0 deletions(-) > > > > > > diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c > > > index 30a0394..55fdf02 100644 > > > --- a/fs/nfs/pnfs.c > > > +++ b/fs/nfs/pnfs.c > > > @@ -1193,6 +1193,9 @@ pnfs_ld_write_done(struct nfs_write_data *data) > > > > > > dprintk("%s: pnfs_error=%d, retry via MDS\n", __func__, > > > data->pnfs_error); > > > + > > > + put_lseg(data->lseg); > > > + data->lseg = NULL; > > > status = nfs_initiate_write(data, NFS_CLIENT(data->inode), > > > data->mds_ops, NFS_FILE_SYNC); > > > return status ? : -EAGAIN; > > > @@ -1240,6 +1243,9 @@ pnfs_ld_read_done(struct nfs_read_data *data) > > > > > > dprintk("%s: pnfs_error=%d, retry via MDS\n", __func__, > > > data->pnfs_error); > > > + > > > + put_lseg(data->lseg); > > > + data->lseg = NULL; > > > status = nfs_initiate_read(data, NFS_CLIENT(data->inode), > > > data->mds_ops); > > > return status ? : -EAGAIN; > -- Trond Myklebust Linux NFS client maintainer NetApp Trond.Myklebust@netapp.com www.netapp.com