[pnfs] [PATCH 1/1] Replaced BUG() with BUG_ON()

[Iyer, Rahul]

> -----Original Message-----
> From: Benny Halevy [mailto:bhalevy at panasas.com] 
> Sent: Wednesday, May 02, 2007 1:20 PM
> To: Iyer, Rahul
> Cc: pnfs at linux-nfs.org
> Subject: Re: [pnfs] [PATCH 1/1] Replaced BUG() with BUG_ON()
> 
> The problem I hit is caused by the fact that that I'm not 
> using the file layout.
> 
> nfs_write_rpcsetup() does not set data->session before 
> calling pnfs_try_to_write_data().
> 
> When we do the pnfs write via the layout driver there's 
> indeed no session in my case.
> 
> The files layout driver sets data->session to the DS session as
> 		data->session = dserver.dev_item->clp->cl_session;
> 
> and on the async callback path pnfs4_write_done() is called.
> [note, without properly going through pnfs4_proc_write_setup 
> in my case, it is called only when writing to the MDS to to 
> the DS via calls to nfs_initiate_write()]
> 
> pnfs4_write_done(), in turn, calls the 
> NFS_SERVER(inode)->rpc_ops-> sequence_done if not null, 
> unconditionally in all cases.
> 
> The fix I tested is attached.  It calls sequence_done only if 
> data->session is not NULL.
>

Yep! The patch looks good.
 
> Another question is whether calling 
> server->rpc_ops->sequence_done is appropriate both for the 
> MDS and for the DS case.
>
Isn't it appropriate? There should always be a session between the MDS
and the client regardless of the protocol being used between the client
and the DS.
Thanks
Regards
Rahul

 
> Benny
>  
> Benny Halevy wrote:
> > iyer at netapp.com wrote:
> >> From: Rahul Iyer <iyer at netapp.com>
> >>
> >> Replaced instances of
> >>
> >> if (....)
> >> 	BUG();
> >>
> >> with
> >>
> >> BUG_on(...);
> >>
> >> Signed-off-by: Rahul Iyer <iyer at netapp.com>
> >> ---
> >>  fs/nfs/nfs4proc.c |   31 +++++++++++++------------------
> >>  1 files changed, 13 insertions(+), 18 deletions(-)
> >>
> >> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 
> >> b4ce093..89702cf 100644
> >> --- a/fs/nfs/nfs4proc.c
> >> +++ b/fs/nfs/nfs4proc.c
> >> @@ -222,10 +222,9 @@ static int 
> nfs41_proc_sequence_done(struct nfs4_session *session, struct nfs41_s
> >>  	unsigned long timestamp;
> >>  	struct nfs4_client *clp;
> >>  
> >> -	if (!session || !(clp = session->client)) {
> >> -		printk(KERN_EMERG "%s is NULL!!!\n", 
> (!session)?"session":"clp");
> >> -		BUG();
> >> -	}
> >> +	BUG_ON(!session);
> > 
> > Rahul, this BUG_ON introduces a bug :-( Previously the 
> function just 
> > returned if !session, now it BUG()'s.
> > 
> >> +	clp = session->client;
> >> +	BUG_ON(!clp);
> >>  
> > 
> > _______________________________________________
> > pNFS mailing list
> > pNFS at linux-nfs.org
> > http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs
> 
>