| Message ID | 520C50F7.3010209@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 14 Aug 2013 at 22:54, Dave Kleikamp wrote: > It looks like the problem is that jfs was using a cookie value of 2 for > a real directory entry, where NFSv4 expect 2 to represent "..". This > patch has so far only been lightly tested. Hm, a first compile of 3.11-rc5 errors out with: CC [M] fs/jfs/jfs_dtree.o /usr/local/src/linux-git/fs/jfs/jfs_dtree.c: In function ‘add_index’: /usr/local/src/linux-git/fs/jfs/jfs_dtree.c:493:13: error: invalid storage class for function ‘free_index’ /usr/local/src/linux-git/fs/jfs/jfs_dtree.c:493:1: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement] /usr/local/src/linux-git/fs/jfs/jfs_dtree.c:521:13: error: invalid storage class for function ‘modify_index’ /usr/local/src/linux-git/fs/jfs/jfs_dtree.c:546:12: error: invalid storage class for function ‘read_index’ /usr/local/src/linux-git/fs/jfs/jfs_dtree.c:927:12: error: invalid storage class for function ‘dtSplitUp’ /usr/local/src/linux-git/fs/jfs/jfs_dtree.c:1327:12: error: invalid storage class for function ‘dtSplitPage’ /usr/local/src/linux-git/fs/jfs/jfs_dtree.c:1639:12: error: invalid storage class for function ‘dtExtendPage’ /usr/local/src/linux-git/fs/jfs/jfs_dtree.c:1872:12: error: invalid storage class for function ‘dtSplitRoot’ /usr/local/src/linux-git/fs/jfs/jfs_dtree.c:2234:12: error: invalid storage class for function ‘dtDeleteUp’ /usr/local/src/linux-git/fs/jfs/jfs_dtree.c:2744:12: error: invalid storage class for function ‘dtRelink’ /usr/local/src/linux-git/fs/jfs/jfs_dtree.c:2915:13: error: invalid storage class for function ‘add_missing_indices’ /usr/local/src/linux-git/fs/jfs/jfs_dtree.c:2982:34: error: invalid storage class for function ‘next_jfs_dirent’ /usr/local/src/linux-git/fs/jfs/jfs_dtree.c:3333:12: error: invalid storage class for function ‘dtReadFirst’ /usr/local/src/linux-git/fs/jfs/jfs_dtree.c:3405:12: error: invalid storage class for function ‘dtReadNext’ /usr/local/src/linux-git/fs/jfs/jfs_dtree.c:3581:12: error: invalid storage class for function ‘dtCompare’ /usr/local/src/linux-git/fs/jfs/jfs_dtree.c:3657:12: error: invalid storage class for function ‘ciCompare’ /usr/local/src/linux-git/fs/jfs/jfs_dtree.c:3765:12: error: invalid storage class for function ‘ciGetLeafPrefixKey’ /usr/local/src/linux-git/fs/jfs/jfs_dtree.c:3832:13: error: invalid storage class for function ‘dtGetKey’ /usr/local/src/linux-git/fs/jfs/jfs_dtree.c:3896:13: error: invalid storage class for function ‘dtInsertEntry’ /usr/local/src/linux-git/fs/jfs/jfs_dtree.c:4054:13: error: invalid storage class for function ‘dtMoveEntry’ /usr/local/src/linux-git/fs/jfs/jfs_dtree.c:4255:13: error: invalid storage class for function ‘dtDeleteEntry’ /usr/local/src/linux-git/fs/jfs/jfs_dtree.c:4350:13: error: invalid storage class for function ‘dtTruncateEntry’ /usr/local/src/linux-git/fs/jfs/jfs_dtree.c:4430:13: error: invalid storage class for function ‘dtLinelockFreelist’ /usr/local/src/linux-git/fs/jfs/jfs_dtree.c:4565:1: error: expected declaration or statement at end of input /usr/local/src/linux-git/fs/jfs/jfs_dtree.c: At top level: /usr/local/src/linux-git/fs/jfs/jfs_dtree.c:152:12: warning: ‘dtSplitUp’ used but never defined [enabled by default] /usr/local/src/linux-git/fs/jfs/jfs_dtree.c:155:12: warning: ‘dtSplitPage’ used but never defined [enabled by default] /usr/local/src/linux-git/fs/jfs/jfs_dtree.c:158:12: warning: ‘dtExtendPage’ used but never defined [enabled by default] /usr/local/src/linux-git/fs/jfs/jfs_dtree.c:161:12: warning: ‘dtSplitRoot’ used but never defined [enabled by default] /usr/local/src/linux-git/fs/jfs/jfs_dtree.c:164:12: warning: ‘dtDeleteUp’ used but never defined [enabled by default] /usr/local/src/linux-git/fs/jfs/jfs_dtree.c:167:12: warning: ‘dtRelink’ used but never defined [enabled by default] /usr/local/src/linux-git/fs/jfs/jfs_dtree.c:169:12: warning: ‘dtReadFirst’ used but never defined [enabled by default] /usr/local/src/linux-git/fs/jfs/jfs_dtree.c:171:12: warning: ‘dtReadNext’ used but never defined [enabled by default] /usr/local/src/linux-git/fs/jfs/jfs_dtree.c:174:12: warning: ‘dtCompare’ used but never defined [enabled by default] /usr/local/src/linux-git/fs/jfs/jfs_dtree.c:176:12: warning: ‘ciCompare’ used but never defined [enabled by default] /usr/local/src/linux-git/fs/jfs/jfs_dtree.c:179:13: warning: ‘dtGetKey’ used but never defined [enabled by default] /usr/local/src/linux-git/fs/jfs/jfs_dtree.c:182:12: warning: ‘ciGetLeafPrefixKey’ used but never defined [enabled by default] /usr/local/src/linux-git/fs/jfs/jfs_dtree.c:185:13: warning: ‘dtInsertEntry’ used but never defined [enabled by default] /usr/local/src/linux-git/fs/jfs/jfs_dtree.c:188:13: warning: ‘dtMoveEntry’ used but never defined [enabled by default] /usr/local/src/linux-git/fs/jfs/jfs_dtree.c:192:13: warning: ‘dtDeleteEntry’ used but never defined [enabled by default] /usr/local/src/linux-git/fs/jfs/jfs_dtree.c:194:13: warning: ‘dtTruncateEntry’ used but never defined [enabled by default] /usr/local/src/linux-git/fs/jfs/jfs_dtree.c:196:13: warning: ‘dtLinelockFreelist’ used but never defined [enabled by default] make[7]: *** [fs/jfs/jfs_dtree.o] Error 1 make[7]: *** Waiting for unfinished jobs.... CC drivers/acpi/acpica/utmutex.o CC drivers/acpi/acpica/utobject.o make[6]: *** [fs/jfs] Error 2 make[5]: *** [fs] Error 2 make[5]: *** Waiting for unfinished jobs.... I'll run mrproper and try again... Christian.
On Wed, 14 Aug 2013 at 21:29, Christian Kujau wrote: > On Wed, 14 Aug 2013 at 22:54, Dave Kleikamp wrote: > > It looks like the problem is that jfs was using a cookie value of 2 for > > a real directory entry, where NFSv4 expect 2 to represent "..". This > > patch has so far only been lightly tested. > > Hm, a first compile of 3.11-rc5 errors out with: > > CC [M] fs/jfs/jfs_dtree.o > /usr/local/src/linux-git/fs/jfs/jfs_dtree.c: In function ‘add_index’: > /usr/local/src/linux-git/fs/jfs/jfs_dtree.c:493:13: error: invalid storage class for function ‘free_index’ [...] > > I'll run mrproper and try again... This did not help, but adding a closing bracket did, in fs/jfs/jfs_dtree.c:354 if (jfs_ip->next_index < 3) { jfs_ip->next_index = 3; } -----^ This compiled and booted and now I can run find(1) over that whole NFS share, without any "readdir loop" messages and with unique inode numbers, yay! Tested-by: Christian Kujau <lists@nerdbynature.de> Thanks! Christian.
On Wed, Aug 14, 2013 at 10:54:31PM -0500, Dave Kleikamp wrote: > For the sake of those not watching > https://bugzilla.kernel.org/show_bug.cgi?id=60737 > > It looks like the problem is that jfs was using a cookie value of 2 for > a real directory entry, where NFSv4 expect 2 to represent "..". This > patch has so far only been lightly tested. > > NFSv4 reserves cookie values 0, 1 and 2 for a rewind, and the "." and ".." > entries. jfs was using 0 and 1 for "." and "..", but 2 for a regular entry. > This patch makes jfs conform by using 1 and 2 for "." and ".." and fixes > any regular entry using the value 2. Oh, I'd forgotten that. From rfc 5661: For some file system environments, the directory entries "." and ".." have special meaning, and in other environments, they do not. If the server supports these special entries within a directory, they SHOULD NOT be returned to the client as part of the READDIR response. To enable some client environments, the cookie values of zero, 1, and 2 are to be considered reserved. Note that the UNIX client will use these values when combining the server's response and local representations to enable a fully formed UNIX directory presentation to the application. OK! --b. > > Signed-off-by: Dave Kleikamp <dave.kleikamp@oracle.com> > > diff --git a/fs/jfs/jfs_dtree.c b/fs/jfs/jfs_dtree.c > index 8743ba9..93466e8 100644 > --- a/fs/jfs/jfs_dtree.c > +++ b/fs/jfs/jfs_dtree.c > @@ -349,11 +349,8 @@ static u32 add_index(tid_t tid, struct inode *ip, s64 bn, int slot) > > ASSERT(DO_INDEX(ip)); > > - if (jfs_ip->next_index < 2) { > - jfs_warn("add_index: next_index = %d. Resetting!", > - jfs_ip->next_index); > - jfs_ip->next_index = 2; > - } > + if (jfs_ip->next_index < 3) { > + jfs_ip->next_index = 3; > > index = jfs_ip->next_index++; > > @@ -2864,7 +2861,7 @@ void dtInitRoot(tid_t tid, struct inode *ip, u32 idotdot) > } else > ip->i_size = 1; > > - jfs_ip->next_index = 2; > + jfs_ip->next_index = 3; > } else > ip->i_size = IDATASIZE; > > @@ -2951,7 +2948,7 @@ static void add_missing_indices(struct inode *inode, s64 bn) > for (i = 0; i < p->header.nextindex; i++) { > d = (struct ldtentry *) &p->slot[stbl[i]]; > index = le32_to_cpu(d->index); > - if ((index < 2) || (index >= JFS_IP(inode)->next_index)) { > + if ((index < 3) || (index >= JFS_IP(inode)->next_index)) { > d->index = cpu_to_le32(add_index(tid, inode, bn, i)); > if (dtlck->index >= dtlck->maxcnt) > dtlck = (struct dt_lock *) txLinelock(dtlck); > @@ -3031,7 +3028,7 @@ int jfs_readdir(struct file *file, struct dir_context *ctx) > struct jfs_dirent *jfs_dirent; > int jfs_dirents; > int overflow, fix_page, page_fixed = 0; > - static int unique_pos = 2; /* If we can't fix broken index */ > + static int unique_pos = 3; /* If we can't fix broken index */ > > if (ctx->pos == DIREND) > return 0; > @@ -3039,15 +3036,16 @@ int jfs_readdir(struct file *file, struct dir_context *ctx) > if (DO_INDEX(ip)) { > /* > * persistent index is stored in directory entries. > - * Special cases: 0 = . > - * 1 = .. > + * Special cases: 0 = rewind > + * 1 = . > + * 2 = .. > * -1 = End of directory > */ > do_index = 1; > > dir_index = (u32) ctx->pos; > > - if (dir_index > 1) { > + if (dir_index > 2) { > struct dir_table_slot dirtab_slot; > > if (dtEmpty(ip) || > @@ -3090,18 +3088,18 @@ int jfs_readdir(struct file *file, struct dir_context *ctx) > return 0; > } > } else { > - if (dir_index == 0) { > + if (dir_index < 2) { > /* > * self "." > */ > - ctx->pos = 0; > + ctx->pos = 1; > if (!dir_emit(ctx, ".", 1, ip->i_ino, DT_DIR)) > return 0; > } > /* > * parent ".." > */ > - ctx->pos = 1; > + ctx->pos = 2; > if (!dir_emit(ctx, "..", 2, PARENT(ip), DT_DIR)) > return 0; > > @@ -3122,22 +3120,24 @@ int jfs_readdir(struct file *file, struct dir_context *ctx) > /* > * Legacy filesystem - OS/2 & Linux JFS < 0.3.6 > * > - * pn = index = 0: First entry "." > - * pn = 0; index = 1: Second entry ".." > + * pn = 0; index = 1: First entry "." > + * pn = 0; index = 2: Second entry ".." > * pn > 0: Real entries, pn=1 -> leftmost page > * pn = index = -1: No more entries > */ > dtpos = ctx->pos; > - if (dtpos == 0) { > + if (dtpos < 2) { > + ctx->pos = 1; > /* build "." entry */ > if (!dir_emit(ctx, ".", 1, ip->i_ino, DT_DIR)) > return 0; > - dtoffset->index = 1; > + dtoffset->index = 2; > ctx->pos = dtpos; > } > > if (dtoffset->pn == 0) { > - if (dtoffset->index == 1) { > + if (dtoffset->index == 2) { > + ctx->pos = 2; > /* build ".." entry */ > if (!dir_emit(ctx, "..", 2, PARENT(ip), DT_DIR)) > return 0; > @@ -3210,8 +3210,12 @@ int jfs_readdir(struct file *file, struct dir_context *ctx) > * directory index for the lost+found > * directory. Rather than let it go, > * we can try to fix it. > + * > + * Additionally, a value of 2 used to be > + * valid, but it didn't work well with > + * NFSv4, so if found, we need to change it > */ > - if ((jfs_dirent->position < 2) || > + if ((jfs_dirent->position < 3) || > (jfs_dirent->position >= > JFS_IP(ip)->next_index)) { > if (!page_fixed && !isReadOnly(ip)) { -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/15/2013 02:09 AM, Christian Kujau wrote: > On Wed, 14 Aug 2013 at 21:29, Christian Kujau wrote: > >> On Wed, 14 Aug 2013 at 22:54, Dave Kleikamp wrote: >>> It looks like the problem is that jfs was using a cookie value of 2 for >>> a real directory entry, where NFSv4 expect 2 to represent "..". This >>> patch has so far only been lightly tested. >> >> Hm, a first compile of 3.11-rc5 errors out with: >> >> CC [M] fs/jfs/jfs_dtree.o >> /usr/local/src/linux-git/fs/jfs/jfs_dtree.c: In function ‘add_index’: >> /usr/local/src/linux-git/fs/jfs/jfs_dtree.c:493:13: error: invalid storage class for function ‘free_index’ > [...] >> >> I'll run mrproper and try again... > > This did not help, but adding a closing bracket did, in fs/jfs/jfs_dtree.c:354 > > if (jfs_ip->next_index < 3) { > jfs_ip->next_index = 3; > } > -----^ > > This compiled and booted and now I can run find(1) over that whole NFS > share, without any "readdir loop" messages and with unique inode numbers, > yay! My bad. That's what happens when you clean up the patch after you test it. I intended to remove the opening bracket when I removed a warning. > Tested-by: Christian Kujau <lists@nerdbynature.de> Thanks. After sleeping on it, I'm contemplating a simpler patch. I'll keep you up to date. > > Thanks! > Christian. > -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/jfs/jfs_dtree.c b/fs/jfs/jfs_dtree.c index 8743ba9..93466e8 100644 --- a/fs/jfs/jfs_dtree.c +++ b/fs/jfs/jfs_dtree.c @@ -349,11 +349,8 @@ static u32 add_index(tid_t tid, struct inode *ip, s64 bn, int slot) ASSERT(DO_INDEX(ip)); - if (jfs_ip->next_index < 2) { - jfs_warn("add_index: next_index = %d. Resetting!", - jfs_ip->next_index); - jfs_ip->next_index = 2; - } + if (jfs_ip->next_index < 3) { + jfs_ip->next_index = 3; index = jfs_ip->next_index++; @@ -2864,7 +2861,7 @@ void dtInitRoot(tid_t tid, struct inode *ip, u32 idotdot) } else ip->i_size = 1; - jfs_ip->next_index = 2; + jfs_ip->next_index = 3; } else ip->i_size = IDATASIZE; @@ -2951,7 +2948,7 @@ static void add_missing_indices(struct inode *inode, s64 bn) for (i = 0; i < p->header.nextindex; i++) { d = (struct ldtentry *) &p->slot[stbl[i]]; index = le32_to_cpu(d->index); - if ((index < 2) || (index >= JFS_IP(inode)->next_index)) { + if ((index < 3) || (index >= JFS_IP(inode)->next_index)) { d->index = cpu_to_le32(add_index(tid, inode, bn, i)); if (dtlck->index >= dtlck->maxcnt) dtlck = (struct dt_lock *) txLinelock(dtlck); @@ -3031,7 +3028,7 @@ int jfs_readdir(struct file *file, struct dir_context *ctx) struct jfs_dirent *jfs_dirent; int jfs_dirents; int overflow, fix_page, page_fixed = 0; - static int unique_pos = 2; /* If we can't fix broken index */ + static int unique_pos = 3; /* If we can't fix broken index */ if (ctx->pos == DIREND) return 0; @@ -3039,15 +3036,16 @@ int jfs_readdir(struct file *file, struct dir_context *ctx) if (DO_INDEX(ip)) { /* * persistent index is stored in directory entries. - * Special cases: 0 = . - * 1 = .. + * Special cases: 0 = rewind + * 1 = . + * 2 = .. * -1 = End of directory */ do_index = 1; dir_index = (u32) ctx->pos; - if (dir_index > 1) { + if (dir_index > 2) { struct dir_table_slot dirtab_slot; if (dtEmpty(ip) || @@ -3090,18 +3088,18 @@ int jfs_readdir(struct file *file, struct dir_context *ctx) return 0; } } else { - if (dir_index == 0) { + if (dir_index < 2) { /* * self "." */ - ctx->pos = 0; + ctx->pos = 1; if (!dir_emit(ctx, ".", 1, ip->i_ino, DT_DIR)) return 0; } /* * parent ".." */ - ctx->pos = 1; + ctx->pos = 2; if (!dir_emit(ctx, "..", 2, PARENT(ip), DT_DIR)) return 0; @@ -3122,22 +3120,24 @@ int jfs_readdir(struct file *file, struct dir_context *ctx) /* * Legacy filesystem - OS/2 & Linux JFS < 0.3.6 * - * pn = index = 0: First entry "." - * pn = 0; index = 1: Second entry ".." + * pn = 0; index = 1: First entry "." + * pn = 0; index = 2: Second entry ".." * pn > 0: Real entries, pn=1 -> leftmost page * pn = index = -1: No more entries */ dtpos = ctx->pos; - if (dtpos == 0) { + if (dtpos < 2) { + ctx->pos = 1; /* build "." entry */ if (!dir_emit(ctx, ".", 1, ip->i_ino, DT_DIR)) return 0; - dtoffset->index = 1; + dtoffset->index = 2; ctx->pos = dtpos; } if (dtoffset->pn == 0) { - if (dtoffset->index == 1) { + if (dtoffset->index == 2) { + ctx->pos = 2; /* build ".." entry */ if (!dir_emit(ctx, "..", 2, PARENT(ip), DT_DIR)) return 0; @@ -3210,8 +3210,12 @@ int jfs_readdir(struct file *file, struct dir_context *ctx) * directory index for the lost+found * directory. Rather than let it go, * we can try to fix it. + * + * Additionally, a value of 2 used to be + * valid, but it didn't work well with + * NFSv4, so if found, we need to change it */ - if ((jfs_dirent->position < 2) || + if ((jfs_dirent->position < 3) || (jfs_dirent->position >= JFS_IP(ip)->next_index)) { if (!page_fixed && !isReadOnly(ip)) {
For the sake of those not watching https://bugzilla.kernel.org/show_bug.cgi?id=60737 It looks like the problem is that jfs was using a cookie value of 2 for a real directory entry, where NFSv4 expect 2 to represent "..". This patch has so far only been lightly tested. NFSv4 reserves cookie values 0, 1 and 2 for a rewind, and the "." and ".." entries. jfs was using 0 and 1 for "." and "..", but 2 for a regular entry. This patch makes jfs conform by using 1 and 2 for "." and ".." and fixes any regular entry using the value 2. Signed-off-by: Dave Kleikamp <dave.kleikamp@oracle.com> -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html