[email protected] wrote:
>The patch titled
> git-nfsd-broke-reiser4
>has been removed from the -mm tree. Its filename was
> git-nfsd-broke-reiser4.patch
>
>This patch was dropped because it was folded into reiser4.patch
>
>------------------------------------------------------
>Subject: git-nfsd-broke-reiser4
>From: Andrew Morton <[email protected]>
>
>fs/reiser4/export_ops.c: In function 'reiser4_decode_fh':
>fs/reiser4/export_ops.c:96: error: 'const struct export_operations' has no member named 'find_exported_dentry'
>fs/reiser4/export_ops.c:96: warning: type defaults to 'int' in declaration of 'fn'
>fs/reiser4/export_ops.c:98: error: 'const struct export_operations' has no member named 'find_exported_dentry'
>fs/reiser4/export_ops.c:99: warning: comparison between pointer and integer
>fs/reiser4/export_ops.c:101: error: called object 'fn' is not a function
>fs/reiser4/export_ops.c: At top level:
>fs/reiser4/export_ops.c:282: error: unknown field 'decode_fh' specified in initializer
>fs/reiser4/export_ops.c:282: warning: initialization from incompatible pointer type
>fs/reiser4/export_ops.c:284: error: unknown field 'get_dentry' specified in initializer
>fs/reiser4/export_ops.c:285: warning: excess elements in struct initializer
>fs/reiser4/export_ops.c:285: warning: (near initialization for 'reiser4_export_operations')
>
>help!
>
done
Thanks,
Edward.
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?
Christoph Hellwig wrote:
>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
>
new _export_ ops
> should not disallow for
>any functionality that has been there before.
>
>
>
Actually they don't disallow existing functionality, but
require a new one: earlier we performed decoding in
_each_ iteration (decode_fh), and now you want it to be
done separately (fh_to_dentry, fh_to_parent). So we need
something like "empty" iteration, which only moves to the
next position.
Every object in reiser4, that can be serialized, has
special non-zero "wire" methods. I guess that "empty"
iteration should be performed via wire->read() method
which is responsible for extracting on-wire object.
Can not promise to avoid "if" here: I think it is not
a good reason to add a new plugin method for such
empty skip.
The attached patch avoids other two ifs.
>>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?
>
>
>
We don't have a number of u32s to poke, like other filesystems do;
packing/extracting serialized objects in reiser4 is more complex:
first extract object's plugin, which knows how to extract further, etc..
>>--- 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?
>
>
>
So we have to define an "if" branch for wire_read_common()
(plugin/file_plugin_common.c). The best place to do it is kassign.c
(as we actually pack/extract key components). Plus a small change in
dscale.c where a packing taxonomy is implemented. Have I missed
something?
Thanks,
Edward.