[EMAIL PROTECTED] wrote on Thu, 03 Jan 2008 17:30 -0500:
>
>> 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.
I tried it again to remind myself. No segv, but the ls produces the
wrong answer:
am30$ pvfs2-ls /pvfs/dir1/{0..9} | wc
Ignoring path /pvfs/dir1/8
Ignoring path /pvfs/dir1/9
4009 4001 15179
Should be 5000 files, and not say "Ignoring path".
My setup is CVS head with 1 md+io server, and 1 client, tcp, plus removing
exactly one 'r' in sys-lookup.sm.
> 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.
Yes. But I'm so afraid. About to check in another frightening
bugfix to this area that I just discovered.
-- Pete
_______________________________________________
Pvfs2-developers mailing list
[email protected]
http://www.beowulf-underground.org/mailman/listinfo/pvfs2-developers
