On 07/27/2011 01:59 PM, Trond Myklebust wrote: > On Wed, 2011-07-27 at 13:42 -0700, Boaz Harrosh wrote: >> On 07/27/2011 01:25 PM, Trond Myklebust wrote: >>> On Wed, 2011-07-27 at 13:20 -0700, Boaz Harrosh wrote: >>>> On 07/27/2011 12:53 PM, Trond Myklebust wrote: >>>>>> diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h >>>>>> index bddd8b9..f271425 100644 >>>>>> --- a/fs/nfs/pnfs.h >>>>>> +++ b/fs/nfs/pnfs.h >>>>>> @@ -113,6 +113,9 @@ struct pnfs_layoutdriver_type { >>>>>> struct xdr_stream *xdr, >>>>>> const struct nfs4_layoutreturn_args *args); >>>>>> >>>>>> + void (*cleanup_layoutcommit) (struct pnfs_layout_hdr *layoutid, >>>>>> + struct nfs4_layoutcommit_data *data); >>>>>> + >>>>>> void (*encode_layoutcommit) (struct pnfs_layout_hdr *layoutid, >>>>>> struct xdr_stream *xdr, >>>>>> const struct nfs4_layoutcommit_args *args); >>>>> >>>>> This really needs to go. We should have >>>>> >>>>> int (*layoutcommit)()... >>>>> >>>>> instead of 'encode' and 'cleanup' methods... >>>>> >>>> >>>> Theoretically it is not possible because the blocks-layout protocol mandates >>>> different handling depending on the "error" response from the Server which >>>> will be received on RPC done. >>> >>> ???? If the blocks code is in charge of actually doing the RPC call, why >>> would it not be able to perform its own error handling? >>> >> >> Is it? I thought it was the Generic layer that was Initiating the layoutcommit >> (From the pnfs_layoutcommit_inode called from nfs_write_inode) >> >> The LD only has a chance to encode the payload on rpc-setup and here the blocks >> code needs cleanup depending on the return-status of rpc-done >> >> [I do think that setup and done might be better names to reflect the rpc states) > > I'm suggesting replacing the version in the generic layer with > per-layout-type variants. When the only thing that is common between the > 3 variants is a couple of lines of xdr, then it doesn't make sense IMO > to try to share. > You lost me. What are you suggesting to replace? pnfs_layoutcommit_inode? nfs4_proc_layoutcommit? nfs4_xdr_enc_layoutcommit? At what level do you want to switch to a per LD handling. If I look at the all layoutcommit stack the actual xdr encoding is the least of the common code. The housekeeping is the most of it. I do not see a clear point in current code that we can make a clean cut. And that gives us a place that: 1. Already have an xdr buffer to encode into. 2. Also sees the return code from layoutcommit_done We used to allocated the layoutcommit buffer twice and that didn't solve that problem either. It is two stages of the rpc-state. I don't see how it can be briged Boaz -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html