From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Morton Subject: Re: [PATCH][RFC] inode time update funnies in ncpfs Date: Fri, 20 Aug 2004 22:33:57 -0700 Sender: linux-fsdevel-owner@vger.kernel.org Message-ID: <20040820223357.4868f8d8.akpm@osdl.org> References: <20040818144307.GA3514@lst.de> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: vandrove@vc.cvut.cz, linux-fsdevel@vger.kernel.org Return-path: Received: from fw.osdl.org ([65.172.181.6]:62865 "EHLO mail.osdl.org") by vger.kernel.org with ESMTP id S268852AbUHUFfn (ORCPT ); Sat, 21 Aug 2004 01:35:43 -0400 To: Christoph Hellwig In-Reply-To: <20040818144307.GA3514@lst.de> List-Id: linux-fsdevel.vger.kernel.org Christoph Hellwig wrote: > > ncfpfs seems to update inode times by hand everywhere instead of using > the proper helpers. This means: > > - the atime updates in mmap() and read() seems to miss various checks > upodate_atime or one of the wrappers does. Also it doesn't mark the > inode dirty. > - in write() you update mtime and _a_time instead of ctime as expected, > also the usual checks and optimizations are missing. > > In addition the fops contain some bogus checks like for a refular file > (but the fops are only used of ISREG files) and inode->i_sb although > that is guranteed to be non-zero. > Patch looks very correct to me - I'll merge it. > --- 1.13/fs/ncpfs/file.c 2004-06-04 06:03:33 +02:00 > +++ edited/fs/ncpfs/file.c 2004-08-18 16:16:27 +02:00 > @@ -115,11 +115,6 @@ > > if (!ncp_conn_valid(NCP_SERVER(inode))) > return -EIO; > - if (!S_ISREG(inode->i_mode)) { > - DPRINTK("ncp_file_read: read from non-file, mode %07o\n", > - inode->i_mode); > - return -EINVAL; > - } > > pos = *ppos; > > @@ -175,10 +170,8 @@ > > *ppos = pos; > > - if (!IS_RDONLY(inode)) { > - inode->i_atime = CURRENT_TIME; > - } > - > + file_accessed(file); > + > DPRINTK("ncp_file_read: exit %s/%s\n", > dentry->d_parent->d_name.name, dentry->d_name.name); > outrel: > @@ -201,11 +194,6 @@ > dentry->d_parent->d_name.name, dentry->d_name.name); > if (!ncp_conn_valid(NCP_SERVER(inode))) > return -EIO; > - if (!S_ISREG(inode->i_mode)) { > - DPRINTK("ncp_file_write: write to non-file, mode %07o\n", > - inode->i_mode); > - return -EINVAL; > - } > if ((ssize_t) count < 0) > return -EINVAL; > pos = *ppos; > @@ -273,8 +261,9 @@ > } > } > vfree(bouncebuffer); > - inode->i_mtime = inode->i_atime = CURRENT_TIME; > - > + > + inode_update_time(inode, 1); > + > *ppos = pos; > > if (pos > inode->i_size) { > ===== fs/ncpfs/mmap.c 1.8 vs edited ===== > --- 1.8/fs/ncpfs/mmap.c 2003-12-29 23:04:51 +01:00 > +++ edited/fs/ncpfs/mmap.c 2004-08-18 16:06:59 +02:00 > @@ -110,23 +110,19 @@ > > DPRINTK("ncp_mmap: called\n"); > > - if (!ncp_conn_valid(NCP_SERVER(inode))) { > + if (!ncp_conn_valid(NCP_SERVER(inode))) > return -EIO; > - } > + > /* only PAGE_COW or read-only supported now */ > if (vma->vm_flags & VM_SHARED) > return -EINVAL; > - if (!inode->i_sb || !S_ISREG(inode->i_mode)) > - return -EACCES; > /* we do not support files bigger than 4GB... We eventually > supports just 4GB... */ > if (((vma->vm_end - vma->vm_start) >> PAGE_SHIFT) + vma->vm_pgoff > > (1U << (32 - PAGE_SHIFT))) > return -EFBIG; > - if (!IS_RDONLY(inode)) { > - inode->i_atime = CURRENT_TIME; > - } > > vma->vm_ops = &ncp_file_mmap; > + file_accessed(file); > return 0; > } > - > To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html