Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

coda: remove CODA_STORE/CODA_RELEASE upcalls

This is an variation on the patch sent by Christoph Hellwig which kills
file_count abuse by the Coda kernel module by moving the coda_flush
functionality into coda_release. However part of reason we were using the
coda_flush callback was to allow Coda to pass errors that occur during
writeback from the userspace cache manager back to close().

As Al Viro explained on linux-fsdevel, it is impossible to guarantee that
such errors can in fact be returned back to the caller. There are many
cases where the last reference to a file is not released by the close
system call and it is also impossible to pick some close as a 'last-close'
and delay it until all other references have been destroyed.

The CODA_STORE/CODA_RELEASE upcall combination is clearly a broken design,
and it is better to remove it completely.

Signed-off-by: Jan Harkes <jaharkes@cs.cmu.edu>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Al Viro <viro@ftp.linux.org.uk>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

authored by

Jan Harkes and committed by
Linus Torvalds
d3fec424 b5073173

+7 -112
-1
fs/coda/dir.c
··· 86 86 .read = generic_read_dir, 87 87 .readdir = coda_readdir, 88 88 .open = coda_open, 89 - .flush = coda_flush, 90 89 .release = coda_release, 91 90 .fsync = coda_fsync, 92 91 };
+6 -59
fs/coda/file.c
··· 25 25 26 26 #include "coda_int.h" 27 27 28 - /* if CODA_STORE fails with EOPNOTSUPP, venus clearly doesn't support 29 - * CODA_STORE/CODA_RELEASE and we fall back on using the CODA_CLOSE upcall */ 30 - static int use_coda_close; 31 - 32 28 static ssize_t 33 29 coda_file_read(struct file *coda_file, char __user *buf, size_t count, loff_t *ppos) 34 30 { ··· 159 163 return 0; 160 164 } 161 165 162 - int coda_flush(struct file *coda_file, fl_owner_t id) 163 - { 164 - unsigned short flags = coda_file->f_flags & ~O_EXCL; 165 - unsigned short coda_flags = coda_flags_to_cflags(flags); 166 - struct coda_file_info *cfi; 167 - struct inode *coda_inode; 168 - int err = 0, fcnt; 169 - 170 - lock_kernel(); 171 - 172 - /* last close semantics */ 173 - fcnt = file_count(coda_file); 174 - if (fcnt > 1) 175 - goto out; 176 - 177 - /* No need to make an upcall when we have not made any modifications 178 - * to the file */ 179 - if ((coda_file->f_flags & O_ACCMODE) == O_RDONLY) 180 - goto out; 181 - 182 - if (use_coda_close) 183 - goto out; 184 - 185 - cfi = CODA_FTOC(coda_file); 186 - BUG_ON(!cfi || cfi->cfi_magic != CODA_MAGIC); 187 - 188 - coda_inode = coda_file->f_path.dentry->d_inode; 189 - 190 - err = venus_store(coda_inode->i_sb, coda_i2f(coda_inode), coda_flags, 191 - coda_file->f_uid); 192 - 193 - if (err == -EOPNOTSUPP) { 194 - use_coda_close = 1; 195 - err = 0; 196 - } 197 - 198 - out: 199 - unlock_kernel(); 200 - return err; 201 - } 202 - 203 166 int coda_release(struct inode *coda_inode, struct file *coda_file) 204 167 { 205 168 unsigned short flags = (coda_file->f_flags) & (~O_EXCL); ··· 170 215 171 216 lock_kernel(); 172 217 173 - if (!use_coda_close) { 174 - err = venus_release(coda_inode->i_sb, coda_i2f(coda_inode), 175 - coda_flags); 176 - if (err == -EOPNOTSUPP) { 177 - use_coda_close = 1; 178 - err = 0; 179 - } 180 - } 181 - 182 218 cfi = CODA_FTOC(coda_file); 183 219 BUG_ON(!cfi || cfi->cfi_magic != CODA_MAGIC); 184 220 185 - if (use_coda_close) 186 - err = venus_close(coda_inode->i_sb, coda_i2f(coda_inode), 187 - coda_flags, coda_file->f_uid); 221 + err = venus_close(coda_inode->i_sb, coda_i2f(coda_inode), 222 + coda_flags, coda_file->f_uid); 188 223 189 224 host_inode = cfi->cfi_container->f_path.dentry->d_inode; 190 225 cii = ITOC(coda_inode); ··· 191 246 coda_file->private_data = NULL; 192 247 193 248 unlock_kernel(); 194 - return err; 249 + 250 + /* VFS fput ignores the return value from file_operations->release, so 251 + * there is no use returning an error here */ 252 + return 0; 195 253 } 196 254 197 255 int coda_fsync(struct file *coda_file, struct dentry *coda_dentry, int datasync) ··· 236 288 .write = coda_file_write, 237 289 .mmap = coda_file_mmap, 238 290 .open = coda_open, 239 - .flush = coda_flush, 240 291 .release = coda_release, 241 292 .fsync = coda_fsync, 242 293 .splice_read = coda_file_splice_read,
+1 -48
fs/coda/upcall.c
··· 160 160 return error; 161 161 } 162 162 163 - int venus_store(struct super_block *sb, struct CodaFid *fid, int flags, 164 - vuid_t uid) 165 - { 166 - union inputArgs *inp; 167 - union outputArgs *outp; 168 - int insize, outsize, error; 169 - #ifdef CONFIG_CODA_FS_OLD_API 170 - struct coda_cred cred = { 0, }; 171 - cred.cr_fsuid = uid; 172 - #endif 173 - 174 - insize = SIZE(store); 175 - UPARG(CODA_STORE); 176 - 177 - #ifdef CONFIG_CODA_FS_OLD_API 178 - memcpy(&(inp->ih.cred), &cred, sizeof(cred)); 179 - #else 180 - inp->ih.uid = uid; 181 - #endif 182 - 183 - inp->coda_store.VFid = *fid; 184 - inp->coda_store.flags = flags; 185 - 186 - error = coda_upcall(coda_vcp(sb), insize, &outsize, inp); 187 - 188 - CODA_FREE(inp, insize); 189 - return error; 190 - } 191 - 192 - int venus_release(struct super_block *sb, struct CodaFid *fid, int flags) 193 - { 194 - union inputArgs *inp; 195 - union outputArgs *outp; 196 - int insize, outsize, error; 197 - 198 - insize = SIZE(release); 199 - UPARG(CODA_RELEASE); 200 - 201 - inp->coda_release.VFid = *fid; 202 - inp->coda_release.flags = flags; 203 - 204 - error = coda_upcall(coda_vcp(sb), insize, &outsize, inp); 205 - 206 - CODA_FREE(inp, insize); 207 - return error; 208 - } 209 - 210 163 int venus_close(struct super_block *sb, struct CodaFid *fid, int flags, 211 - vuid_t uid) 164 + vuid_t uid) 212 165 { 213 166 union inputArgs *inp; 214 167 union outputArgs *outp;
-1
include/linux/coda_linux.h
··· 36 36 37 37 /* operations shared over more than one file */ 38 38 int coda_open(struct inode *i, struct file *f); 39 - int coda_flush(struct file *f, fl_owner_t id); 40 39 int coda_release(struct inode *i, struct file *f); 41 40 int coda_permission(struct inode *inode, int mask, struct nameidata *nd); 42 41 int coda_revalidate_inode(struct dentry *);
-3
include/linux/coda_psdev.h
··· 33 33 int venus_lookup(struct super_block *sb, struct CodaFid *fid, 34 34 const char *name, int length, int *type, 35 35 struct CodaFid *resfid); 36 - int venus_store(struct super_block *sb, struct CodaFid *fid, int flags, 37 - vuid_t uid); 38 - int venus_release(struct super_block *sb, struct CodaFid *fid, int flags); 39 36 int venus_close(struct super_block *sb, struct CodaFid *fid, int flags, 40 37 vuid_t uid); 41 38 int venus_open(struct super_block *sb, struct CodaFid *fid, int flags,