Here is the diff (excerpt) that added the code from 1.68 to 1.69:


Index: src/client/sysint/sys-lookup.sm
--- src/client/sysint/sys-lookup.sm     13 Apr 2007 05:14:16 -0000      1.68
+++ src/client/sysint/sys-lookup.sm     20 Jun 2007 06:08:51 -0000
@@ -378,7 +378,16 @@
             cur_seg->seg_name = strdup(cur_seg_name);
             assert(cur_seg->seg_name);
- seg_remaining = strstr(orig_pathname, cur_seg_name);
+            slash_str = orig_pathname;
+            for (i = 0; i < cur_seg_index; i++) {
+                slash_str = strrchr(slash_str, '/');
+                if (slash_str == NULL) {
+                    break;
+                }
+                slash_str++;
+            }
+            //seg_remaining = strstr(orig_pathname, cur_seg_name);
+            seg_remaining = slash_str;
             if (seg_remaining)
             {
                 gossip_debug(GOSSIP_LOOKUP_DEBUG,


If I change the strrchr to strchr, my case works, but it breaks the
bugfix reported previously (but without segv).  If I revert the
change, going back to the original strstr, my case also still works,
and the previous bug also reoccurs (with segv).  I'm not
understanding this fully, but we seem to be getting closer.  Or
maybe the problem is in the interpretation of seg_remaining in other
parts of the lookup code.

In the case where you keep the logic that is currently in head but just change strrchr() to strchr(), what breaks in from the previously reported bug?

I tried making that change and it looks like both cases work fine, but I may be missing something.

Maybe it has something to do with the number of metadata servers? I am currently trying this with just one server.

On a code cleanup note, it looks like maybe we could get rid of the "lookup_init" state and its associated LOOKUP_NCACHE_HIT_FASTPATH. It is trying to look up the entire path up front in the ncache before doing any work, but this isn't likely to gain anything since the ncache now only caches one segment at a time. I guess it could hit in the single segment lookup case, but that would happen anyway in the first iteration of the normal state machine. I guess this extra state isn't technically hurting anything, but it is redundant code that looks strange in the gossip output since it attempts to do ncache lookups with slashes in the name.

-Phil
_______________________________________________
Pvfs2-developers mailing list
[email protected]
http://www.beowulf-underground.org/mailman/listinfo/pvfs2-developers

Reply via email to