Re: [patch 2/2] reiser4: new export ops

[Christoph Hellwig]

This code looks a little confusing to me..

>   */
>  static char *decode_inode(struct super_block *s, char *addr,
>  			  reiser4_object_on_wire * obj)
> @@ -41,7 +42,8 @@
>  	fplug = file_plugin_by_disk_id(reiser4_get_tree(s), (d16 *) addr);
>  	if (fplug != NULL) {
>  		addr += sizeof(d16);
> -		obj->plugin = fplug;
> +		if (obj)
> +			obj->plugin = fplug;

You are adding quite a few of those if (obj) clauses.  I can't see a
reason for that - care to explain?  The new aops should not disallow for
any functionality that has been there before.

> static struct dentry *reiser4_decode_fh(struct super_block *super, __u32 *fh,
> +					int len, int fhtype, int parent)
>  {
>  	reiser4_context *ctx;
>  	reiser4_object_on_wire object;
>  	char *addr;
>  
>  	ctx = reiser4_init_context(super);
>  	if (IS_ERR(ctx))
> @@ -80,25 +77,19 @@
>  	assert("vs-1482",
>  	       fhtype == FH_WITH_PARENT || fhtype == FH_WITHOUT_PARENT);
>  
> 	addr = (char *)fh;
> 
> 	object_on_wire_init(&object);
> +
> +	if (parent)
> +		/* skip first onwire object */
> +		addr = decode_inode(super, addr, NULL);
> 	if (!IS_ERR(addr)) {
> +		addr = decode_inode(super, addr, &object);
> 		if (!IS_ERR(addr)) {
> 			struct dentry *d;
> 
> +			d = reiser4_get_dentry(super, &object);

I'd suggest to directly poke into the place where the parent handle
is stored.  XFS used a similar construct to the decode_inode helper,
but with the new aops it's faster and easier to read if you just have
a helper on how many bytes to skip.  Did you take a look at how the
various other filesystem handle the export ops?

> --- linux-2.6.23-mm1/fs/reiser4/dscale.c.orig
> +++ linux-2.6.23-mm1/fs/reiser4/dscale.c
> @@ -126,6 +126,24 @@

How are the changes to all these other files related to the export
operations changes?

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/