Re: patch for 2.0.31 NFS -- need testers!

Dr. Werner Fink (werner@suse.de)
Thu, 14 Aug 1997 13:15:53 +0200


> > Maybe it has been pointed out to you allready but in the nfs patch you
> > forgot the patch on include/linux/fs_nfs.h. Fortunatly it wasn't hard
> > to figure out. Btw I am not expiriencing any NFS problems but I will
> > build 2.0.31pre5 with it anyway.
>
> Leon,
> Thanks, I forgot to extract the diffs for the include file. Corrected
> patch is attached for anyone who hasn't already fixed it.
>
> Regards,
> Bill

I will test this patch. Something similar should go to 2.0.31 to
avoid this seldom but nasty nfs oops.

For testers out there: you need an idle system an a virgin mounted
home nfs partition. Then ... maybe ... you will see such an oops
or better some of Bill's debug messages :-)

Werner

> --------------819AA874ED05D067BD01131F
> Content-Type: text/plain; charset=us-ascii; name="nfs_30-patch"
> Content-Transfer-Encoding: 7bit
> Content-Disposition: inline; filename="nfs_30-patch"
>
> --- linux-2.0.31/fs/nfs/inode.c.old Sat Nov 30 05:21:21 1996
> +++ linux-2.0.31/fs/nfs/inode.c Wed Aug 13 09:02:16 1997
> @@ -73,12 +73,34 @@
> NFS_CACHEINV(inode);
> }
>
> +/*
> + * Defer silly rename cleanup until after clearing inode to avoid races.
> + */
> static void nfs_put_inode(struct inode * inode)
> {
> - if (NFS_RENAMED_DIR(inode))
> - nfs_sillyrename_cleanup(inode);
> - if (inode->i_pipe)
> + struct inode * dir = NFS_RENAMED_DIR(inode);
> + long ino = inode->i_ino;
> +
> + if (inode->i_count > 1) {
> + printk("nfs_put_inode: still in use, count=%d\n",
> + inode->i_count);
> + }
> +
> + /*
> + * Always clear FIFOs, deleted files, and silly renamed files.
> + */
> + if (inode->i_pipe || !inode->i_nlink || dir)
> clear_inode(inode);
> +
> + /*
> + * Defer cleanup until the inode has been cleared to avoid races,
> + * but bump the count temporarily so the inode can't be reused.
> + */
> + if (dir) {
> + inode->i_count++;
> + nfs_sillyrename_cleanup(dir, ino);
> + inode->i_count--;
> + }
> }
>
> void nfs_put_super(struct super_block *sb)
> @@ -112,9 +134,7 @@
> MOD_INC_USE_COUNT;
> if (!data) {
> printk("nfs_read_super: missing data argument\n");
> - sb->s_dev = 0;
> - MOD_DEC_USE_COUNT;
> - return NULL;
> + goto exit_fail;
> }
> fd = data->fd;
> if (data->version != NFS_MOUNT_VERSION) {
> @@ -123,15 +143,11 @@
> }
> if (fd >= NR_OPEN || !(filp = current->files->fd[fd])) {
> printk("nfs_read_super: invalid file descriptor\n");
> - sb->s_dev = 0;
> - MOD_DEC_USE_COUNT;
> - return NULL;
> + goto exit_fail;
> }
> if (!S_ISSOCK(filp->f_inode->i_mode)) {
> printk("nfs_read_super: not a socket\n");
> - sb->s_dev = 0;
> - MOD_DEC_USE_COUNT;
> - return NULL;
> + goto exit_fail;
> }
> filp->f_count++;
> lock_super(sb);
> @@ -169,8 +185,7 @@
> if (data->addr.sin_addr.s_addr == INADDR_ANY) { /* No address passed */
> if (((struct sockaddr_in *)(&server->toaddr))->sin_addr.s_addr == INADDR_ANY) {
> printk("NFS: Error passed unconnected socket and no address\n") ;
> - MOD_DEC_USE_COUNT;
> - return NULL ;
> + goto exit_unlock;
> } else {
> /* Need access to socket internals JSP */
> struct socket *sock;
> @@ -191,19 +206,25 @@
>
> if ((server->rsock = rpc_makesock(filp)) == NULL) {
> printk("NFS: cannot create RPC socket.\n");
> - MOD_DEC_USE_COUNT;
> - return NULL;
> + goto exit_unlock;
> }
>
> sb->u.nfs_sb.s_root = data->root;
> unlock_super(sb);
> if (!(sb->s_mounted = nfs_fhget(sb, &data->root, NULL))) {
> - sb->s_dev = 0;
> printk("nfs_read_super: get root inode failed\n");
> - MOD_DEC_USE_COUNT;
> - return NULL;
> + goto exit_filp;
> }
> return sb;
> +
> +exit_unlock:
> + unlock_super(sb);
> +exit_filp:
> + filp->f_count--;
> +exit_fail:
> + sb->s_dev = 0;
> + MOD_DEC_USE_COUNT;
> + return NULL;
> }
>
> void nfs_statfs(struct super_block *sb, struct statfs *buf, int bufsiz)
> --- linux-2.0.31/fs/nfs/dir.c.old Sun Apr 27 15:54:43 1997
> +++ linux-2.0.31/fs/nfs/dir.c Wed Aug 13 08:45:56 1997
> @@ -516,7 +516,7 @@
> static int nfs_sillyrename(struct inode *dir, const char *name, int len)
> {
> struct inode *inode;
> - char silly[16];
> + char silly[30];
> int slen, ret;
>
> dir->i_count++;
> @@ -544,18 +544,21 @@
> return ret;
> }
>
> -void nfs_sillyrename_cleanup(struct inode *inode)
> +/*
> + * When this routine is called, we know for certain the inode was
> + * given a silly name ... remove the file with no further checks.
> + */
> +void nfs_sillyrename_cleanup(struct inode*dir, long ino)
> {
> - struct inode *dir = NFS_RENAMED_DIR(inode);
> - char silly[14];
> - int error, slen;
> + char silly[30];
> + int error;
>
> - slen = sprintf(silly, ".nfs%ld", inode->i_ino);
> - if ((error = nfs_unlink(dir, silly, slen)) < 0) {
> - printk("NFS silly_rename cleanup failed (err = %d)\n",
> - -error);
> - }
> - NFS_RENAMED_DIR(inode) = NULL;
> + sprintf(silly, ".nfs%ld", ino);
> + nfs_lookup_cache_remove(dir, NULL, silly);
> + error = nfs_proc_remove(NFS_SERVER(dir), NFS_FH(dir), silly);
> + if (error < 0)
> + printk("NFS silly_rename cleanup failed (err = %d)\n", -error);
> + iput(dir);
> }
>
> static int nfs_unlink(struct inode *dir, const char *name, int len)
> @@ -682,7 +685,8 @@
>
> void nfs_refresh_inode(struct inode *inode, struct nfs_fattr *fattr)
> {
> - int was_empty;
> + struct inode_operations *old_ops;
> + umode_t old_mode;
>
> if (!inode || !fattr) {
> printk("nfs_refresh_inode: inode or fattr is NULL\n");
> @@ -692,7 +696,10 @@
> printk("nfs_refresh_inode: inode number mismatch\n");
> return;
> }
> - was_empty = (inode->i_mode == 0);
> +
> + old_ops = inode->i_op;
> + old_mode = inode->i_mode;
> +
> inode->i_mode = fattr->mode;
> inode->i_nlink = fattr->nlink;
> inode->i_uid = fattr->uid;
> @@ -723,10 +730,23 @@
> else if (S_ISBLK(inode->i_mode))
> inode->i_op = &blkdev_inode_operations;
> else if (S_ISFIFO(inode->i_mode)) {
> - if (was_empty)
> + /*
> + * Make sure FIFO initialization is done just once
> + */
> + if (old_mode == 0 && !inode->i_pipe)
> init_fifo(inode);
> } else
> inode->i_op = NULL;
> + /*
> + * Check whether the i_op pointer changed ... we don't want
> + * pre-tested i_op entries to become invalid or NULL.
> + */
> + if (old_ops && inode->i_op != old_ops) {
> + printk("nfs_refresh_inode: mode changed i_op, old=%x new=%x\n",
> + old_mode, inode->i_mode);
> + /* For now, just complain */
> + /* inode->i_op = &inode_error_operations; */
> + }
> nfs_lookup_cache_refresh(inode, fattr);
> }
>
> --- linux-2.0.31/fs/nfs/nfsiod.c.old Sat Jun 29 05:00:46 1996
> +++ linux-2.0.31/fs/nfs/nfsiod.c Mon Aug 11 17:58:58 1997
> @@ -87,6 +87,7 @@
> nfsiod(void)
> {
> struct nfsiod_req request, *req = &request;
> + struct nfsiod_req *next, **prev;
> int result;
>
> dprintk("BIO: nfsiod %d starting\n", current->pid);
> @@ -101,10 +102,26 @@
> dprintk("BIO: nfsiod %d waiting\n", current->pid);
> interruptible_sleep_on(&req->rq_wait);
>
> + /*
> + * Make sure our request isn't still in the free list ...
> + * we might have been awakened by a signal.
> + */
> + prev = &free_list;
> + while ((next = *prev) != NULL) {
> + if (next == &request) {
> + printk("nfsiod: request still in free list!\n");
> + *prev = next->rq_next;
> + break;
> + }
> + prev = &next->rq_next;
> + }
> +
> if (current->signal & ~current->blocked)
> break;
> - if (!req->rq_rpcreq.rq_slot)
> + if (!req->rq_rpcreq.rq_slot) {
> + printk("nfsiod: no slot specified, sleeping again\n");
> continue;
> + }
> dprintk("BIO: nfsiod %d woken up; calling nfs_rpc_doio.\n",
> current->pid);
> active++;
> --- linux-2.0.31/include/linux/nfs_fs.h.old Sun Apr 27 16:22:13 1997
> +++ linux-2.0.31/include/linux/nfs_fs.h Tue Aug 12 12:00:49 1997
> @@ -129,7 +129,7 @@
> /* linux/fs/nfs/dir.c */
>
> extern struct inode_operations nfs_dir_inode_operations;
> -extern void nfs_sillyrename_cleanup(struct inode *);
> +extern void nfs_sillyrename_cleanup(struct inode *, long);
> extern void nfs_kfree_cache(void);
>
> /* linux/fs/nfs/symlink.c */
>
> --------------819AA874ED05D067BD01131F--
>