| Message ID | 1376769691.2544.4.camel@deadeye.wl.decadent.org.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 8/17/2013 2:01 PM, Ben Hutchings wrote: > On Thu, 2013-08-15 at 14:26 -0700, Christian Kujau wrote: >> On Thu, 15 Aug 2013 at 15:48, Dave Kleikamp wrote: >>> This patch replaces the one I posted yesterday. I like this better since >>> it doesn't require fixing existing on-disk cookies or skipping a >>> position in the in-inode index table. >> >> Thanks. Applied to 3.11-rc5 and tested, no more "readdir loop" messages >> and with unique inode numbers, great! >> >> Tested-by: Christian Kujau <lists@nerdbynature.de> > > Karl and Jonathan, could you test the attached backport to 3.2? > > (Instructions for rebuilding the Debian kernel package are at: > <http://kernel-handbook.alioth.debian.org/ch-common-tasks.html#s-common-official>) > > Ben. > When I read this the other day I thought It was a great idea, but when sleeping on it I have this gut feeling that somewhere there is a jfs_dirent->position--; missing but I don't have enough familiarity with the code to begin to guess where. Hopefully I am wrong. ben
On Sat, Aug 17, 2013 at 10:01:31PM +0200, Ben Hutchings wrote: > On Thu, 2013-08-15 at 14:26 -0700, Christian Kujau wrote: > > On Thu, 15 Aug 2013 at 15:48, Dave Kleikamp wrote: > > > This patch replaces the one I posted yesterday. I like this better since > > > it doesn't require fixing existing on-disk cookies or skipping a > > > position in the in-inode index table. > > > > Thanks. Applied to 3.11-rc5 and tested, no more "readdir loop" messages > > and with unique inode numbers, great! > > > > Tested-by: Christian Kujau <lists@nerdbynature.de> > > Karl and Jonathan, could you test the attached backport to 3.2? I finally managed to be able to schedule some downtime yesterday for one of the machines I've been seeing this issue with. So far since the reboot to this kernel (3.2.46-1 + the patch) I haven't seen a recurrence of the problem; will update if I see anything. J.
On Thu, Aug 29, 2013 at 03:48:03PM -0700, Jonathan McDowell wrote: > On Sat, Aug 17, 2013 at 10:01:31PM +0200, Ben Hutchings wrote: > > On Thu, 2013-08-15 at 14:26 -0700, Christian Kujau wrote: > > > On Thu, 15 Aug 2013 at 15:48, Dave Kleikamp wrote: > > > > This patch replaces the one I posted yesterday. I like this > > > > better since it doesn't require fixing existing on-disk cookies > > > > or skipping a position in the in-inode index table. > > > > > > Thanks. Applied to 3.11-rc5 and tested, no more "readdir loop" > > > messages and with unique inode numbers, great! > > > > > > Tested-by: Christian Kujau <lists@nerdbynature.de> > > > > Karl and Jonathan, could you test the attached backport to 3.2? > > I finally managed to be able to schedule some downtime yesterday for > one of the machines I've been seeing this issue with. So far since the > reboot to this kernel (3.2.46-1 + the patch) I haven't seen a > recurrence of the problem; will update if I see anything. I see the patch hit 3.11, but just to confirm the 3.2 backport on top of the Debian 3.2.46-1 kernel has seen no recurrence or issues in the past week (with a set of JFS filesystems that are fairly extensively used over NFS). J.
Date: Thu, 15 Aug 2013 15:48:39 -0500 From: Dave Kleikamp <dave.kleikamp@oracle.com> Subject: jfs: fix readdir cookie incompatibility with NFSv4 This patch replaces the one I posted yesterday. I like this better since it doesn't require fixing existing on-disk cookies or skipping a position in the in-inode index table. NFSv4 reserves readdir cookie values 0-2 for special entries (. and ..), but jfs allows a value of 2 for a non-special entry. This incompatibility can result in the nfs client reporting a readdir loop. This patch doesn't change the value stored internally, but adds one to the value exposed to the iterate method. Signed-off-by: Dave Kleikamp <dave.kleikamp@oracle.com> [bwh: Backported to 3.2: - Adjust context - s/ctx->pos/filp->f_pos/] Signed-off-by: Ben Hutchings <ben@decadent.org.uk> --- fs/jfs/jfs_dtree.c | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/fs/jfs/jfs_dtree.c b/fs/jfs/jfs_dtree.c index 8743ba9..0ec767e 100644 --- a/fs/jfs/jfs_dtree.c +++ b/fs/jfs/jfs_dtree.c @@ -3047,6 +3047,14 @@ int jfs_readdir(struct file *file, struct dir_context *ctx) dir_index = (u32) filp->f_pos; + /* + * NFSv4 reserves cookies 1 and 2 for . and .. so we add + * the value we return to the vfs is one greater than the + * one we use internally. + */ + if (dir_index) + dir_index--; + if (dir_index > 1) { struct dir_table_slot dirtab_slot; @@ -3086,7 +3094,7 @@ int jfs_readdir(struct file *file, struct dir_context *ctx) if (p->header.flag & BT_INTERNAL) { jfs_err("jfs_readdir: bad index table"); DT_PUTPAGE(mp); - filp->f_pos = -1; + filp->f_pos = DIREND; return 0; } } else { @@ -3094,15 +3102,15 @@ int jfs_readdir(struct file *file, struct dir_context *ctx) /* * self "." */ - filp->f_pos = 0; + filp->f_pos = 1; if (filldir(dirent, ".", 1, 0, ip->i_ino, DT_DIR)) return 0; } /* * parent ".." */ - filp->f_pos = 1; + filp->f_pos = 2; if (filldir(dirent, "..", 2, 1, PARENT(ip), DT_DIR)) return 0; @@ -3123,24 +3131,25 @@ 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 = filp->f_pos; - if (dtpos == 0) { + if (dtpos < 2) { /* build "." entry */ + filp->f_pos = 1; if (filldir(dirent, ".", 1, filp->f_pos, ip->i_ino, DT_DIR)) return 0; - dtoffset->index = 1; + dtoffset->index = 2; filp->f_pos = dtpos; } if (dtoffset->pn == 0) { - if (dtoffset->index == 1) { + if (dtoffset->index == 2) { /* build ".." entry */ if (filldir(dirent, "..", 2, filp->f_pos, @@ -3233,6 +3242,12 @@ int jfs_readdir(struct file *file, struct dir_context *ctx) } jfs_dirent->position = unique_pos++; } + /* + * We add 1 to the index because we may + * use a value of 2 internally, and NFSv4 + * doesn't like that. + */ + jfs_dirent->position++; } else { jfs_dirent->position = dtpos; len = min(d_namleft, DTLHDRDATALEN_LEGACY);