diff options
author | Jan Harkes <jaharkes@cs.cmu.edu> | 2007-07-21 07:37:26 -0400 |
---|---|---|
committer | Linus Torvalds <torvalds@woody.linux-foundation.org> | 2007-07-21 20:49:14 -0400 |
commit | d3fec424b23c47686efcf3f2004c3f1c1cee4d9c (patch) | |
tree | ffad3cf369d35e85fca9a340347eea09caa53ff3 | |
parent | b50731732f926d6c49fd0724616a7344c31cd5cf (diff) |
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>
-rw-r--r-- | fs/coda/dir.c | 1 | ||||
-rw-r--r-- | fs/coda/file.c | 65 | ||||
-rw-r--r-- | fs/coda/upcall.c | 49 | ||||
-rw-r--r-- | include/linux/coda_linux.h | 1 | ||||
-rw-r--r-- | include/linux/coda_psdev.h | 3 |
5 files changed, 7 insertions, 112 deletions
diff --git a/fs/coda/dir.c b/fs/coda/dir.c index 8e61236abf4a..f89ff083079b 100644 --- a/fs/coda/dir.c +++ b/fs/coda/dir.c | |||
@@ -86,7 +86,6 @@ const struct file_operations coda_dir_operations = { | |||
86 | .read = generic_read_dir, | 86 | .read = generic_read_dir, |
87 | .readdir = coda_readdir, | 87 | .readdir = coda_readdir, |
88 | .open = coda_open, | 88 | .open = coda_open, |
89 | .flush = coda_flush, | ||
90 | .release = coda_release, | 89 | .release = coda_release, |
91 | .fsync = coda_fsync, | 90 | .fsync = coda_fsync, |
92 | }; | 91 | }; |
diff --git a/fs/coda/file.c b/fs/coda/file.c index 7594962604c2..29137ff3ca67 100644 --- a/fs/coda/file.c +++ b/fs/coda/file.c | |||
@@ -25,10 +25,6 @@ | |||
25 | 25 | ||
26 | #include "coda_int.h" | 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 | static ssize_t | 28 | static ssize_t |
33 | coda_file_read(struct file *coda_file, char __user *buf, size_t count, loff_t *ppos) | 29 | coda_file_read(struct file *coda_file, char __user *buf, size_t count, loff_t *ppos) |
34 | { | 30 | { |
@@ -163,47 +159,6 @@ int coda_open(struct inode *coda_inode, struct file *coda_file) | |||
163 | return 0; | 159 | return 0; |
164 | } | 160 | } |
165 | 161 | ||
166 | int coda_flush(struct file *coda_file, fl_owner_t id) | ||
167 | { | ||
168 | unsigned short flags = coda_file->f_flags & ~O_EXCL; | ||
169 | unsigned short coda_flags = coda_flags_to_cflags(flags); | ||
170 | struct coda_file_info *cfi; | ||
171 | struct inode *coda_inode; | ||
172 | int err = 0, fcnt; | ||
173 | |||
174 | lock_kernel(); | ||
175 | |||
176 | /* last close semantics */ | ||
177 | fcnt = file_count(coda_file); | ||
178 | if (fcnt > 1) | ||
179 | goto out; | ||
180 | |||
181 | /* No need to make an upcall when we have not made any modifications | ||
182 | * to the file */ | ||
183 | if ((coda_file->f_flags & O_ACCMODE) == O_RDONLY) | ||
184 | goto out; | ||
185 | |||
186 | if (use_coda_close) | ||
187 | goto out; | ||
188 | |||
189 | cfi = CODA_FTOC(coda_file); | ||
190 | BUG_ON(!cfi || cfi->cfi_magic != CODA_MAGIC); | ||
191 | |||
192 | coda_inode = coda_file->f_path.dentry->d_inode; | ||
193 | |||
194 | err = venus_store(coda_inode->i_sb, coda_i2f(coda_inode), coda_flags, | ||
195 | coda_file->f_uid); | ||
196 | |||
197 | if (err == -EOPNOTSUPP) { | ||
198 | use_coda_close = 1; | ||
199 | err = 0; | ||
200 | } | ||
201 | |||
202 | out: | ||
203 | unlock_kernel(); | ||
204 | return err; | ||
205 | } | ||
206 | |||
207 | int coda_release(struct inode *coda_inode, struct file *coda_file) | 162 | int coda_release(struct inode *coda_inode, struct file *coda_file) |
208 | { | 163 | { |
209 | unsigned short flags = (coda_file->f_flags) & (~O_EXCL); | 164 | unsigned short flags = (coda_file->f_flags) & (~O_EXCL); |
@@ -215,21 +170,11 @@ int coda_release(struct inode *coda_inode, struct file *coda_file) | |||
215 | 170 | ||
216 | lock_kernel(); | 171 | lock_kernel(); |
217 | 172 | ||
218 | if (!use_coda_close) { | ||
219 | err = venus_release(coda_inode->i_sb, coda_i2f(coda_inode), | ||
220 | coda_flags); | ||
221 | if (err == -EOPNOTSUPP) { | ||
222 | use_coda_close = 1; | ||
223 | err = 0; | ||
224 | } | ||
225 | } | ||
226 | |||
227 | cfi = CODA_FTOC(coda_file); | 173 | cfi = CODA_FTOC(coda_file); |
228 | BUG_ON(!cfi || cfi->cfi_magic != CODA_MAGIC); | 174 | BUG_ON(!cfi || cfi->cfi_magic != CODA_MAGIC); |
229 | 175 | ||
230 | if (use_coda_close) | 176 | err = venus_close(coda_inode->i_sb, coda_i2f(coda_inode), |
231 | err = venus_close(coda_inode->i_sb, coda_i2f(coda_inode), | 177 | coda_flags, coda_file->f_uid); |
232 | coda_flags, coda_file->f_uid); | ||
233 | 178 | ||
234 | host_inode = cfi->cfi_container->f_path.dentry->d_inode; | 179 | host_inode = cfi->cfi_container->f_path.dentry->d_inode; |
235 | cii = ITOC(coda_inode); | 180 | cii = ITOC(coda_inode); |
@@ -246,7 +191,10 @@ int coda_release(struct inode *coda_inode, struct file *coda_file) | |||
246 | coda_file->private_data = NULL; | 191 | coda_file->private_data = NULL; |
247 | 192 | ||
248 | unlock_kernel(); | 193 | unlock_kernel(); |
249 | return err; | 194 | |
195 | /* VFS fput ignores the return value from file_operations->release, so | ||
196 | * there is no use returning an error here */ | ||
197 | return 0; | ||
250 | } | 198 | } |
251 | 199 | ||
252 | int coda_fsync(struct file *coda_file, struct dentry *coda_dentry, int datasync) | 200 | int coda_fsync(struct file *coda_file, struct dentry *coda_dentry, int datasync) |
@@ -288,7 +236,6 @@ const struct file_operations coda_file_operations = { | |||
288 | .write = coda_file_write, | 236 | .write = coda_file_write, |
289 | .mmap = coda_file_mmap, | 237 | .mmap = coda_file_mmap, |
290 | .open = coda_open, | 238 | .open = coda_open, |
291 | .flush = coda_flush, | ||
292 | .release = coda_release, | 239 | .release = coda_release, |
293 | .fsync = coda_fsync, | 240 | .fsync = coda_fsync, |
294 | .splice_read = coda_file_splice_read, | 241 | .splice_read = coda_file_splice_read, |
diff --git a/fs/coda/upcall.c b/fs/coda/upcall.c index cd561d2e90b0..cdb4c07a7870 100644 --- a/fs/coda/upcall.c +++ b/fs/coda/upcall.c | |||
@@ -160,55 +160,8 @@ int venus_lookup(struct super_block *sb, struct CodaFid *fid, | |||
160 | return error; | 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 | int venus_close(struct super_block *sb, struct CodaFid *fid, int flags, | 163 | int venus_close(struct super_block *sb, struct CodaFid *fid, int flags, |
211 | vuid_t uid) | 164 | vuid_t uid) |
212 | { | 165 | { |
213 | union inputArgs *inp; | 166 | union inputArgs *inp; |
214 | union outputArgs *outp; | 167 | union outputArgs *outp; |
diff --git a/include/linux/coda_linux.h b/include/linux/coda_linux.h index c4079b403e9e..1c47a34aa794 100644 --- a/include/linux/coda_linux.h +++ b/include/linux/coda_linux.h | |||
@@ -36,7 +36,6 @@ extern const struct file_operations coda_ioctl_operations; | |||
36 | 36 | ||
37 | /* operations shared over more than one file */ | 37 | /* operations shared over more than one file */ |
38 | int coda_open(struct inode *i, struct file *f); | 38 | int coda_open(struct inode *i, struct file *f); |
39 | int coda_flush(struct file *f, fl_owner_t id); | ||
40 | int coda_release(struct inode *i, struct file *f); | 39 | int coda_release(struct inode *i, struct file *f); |
41 | int coda_permission(struct inode *inode, int mask, struct nameidata *nd); | 40 | int coda_permission(struct inode *inode, int mask, struct nameidata *nd); |
42 | int coda_revalidate_inode(struct dentry *); | 41 | int coda_revalidate_inode(struct dentry *); |
diff --git a/include/linux/coda_psdev.h b/include/linux/coda_psdev.h index aa8f454b3b77..07ae8f846055 100644 --- a/include/linux/coda_psdev.h +++ b/include/linux/coda_psdev.h | |||
@@ -33,9 +33,6 @@ int venus_setattr(struct super_block *, struct CodaFid *, struct coda_vattr *); | |||
33 | int venus_lookup(struct super_block *sb, struct CodaFid *fid, | 33 | int venus_lookup(struct super_block *sb, struct CodaFid *fid, |
34 | const char *name, int length, int *type, | 34 | const char *name, int length, int *type, |
35 | struct CodaFid *resfid); | 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 | int venus_close(struct super_block *sb, struct CodaFid *fid, int flags, | 36 | int venus_close(struct super_block *sb, struct CodaFid *fid, int flags, |
40 | vuid_t uid); | 37 | vuid_t uid); |
41 | int venus_open(struct super_block *sb, struct CodaFid *fid, int flags, | 38 | int venus_open(struct super_block *sb, struct CodaFid *fid, int flags, |