]>
Commit | Line | Data |
---|---|---|
6a930a95 BS |
1 | From: Jeff Mahoney <jeffm@suse.com> |
2 | Subject: reiserfs: make per-inode xattr locking more fine grained | |
3 | ||
4 | The per-inode locking can be made more fine-grained to surround just the | |
5 | interaction with the filesystem itself. This really only applies to protecting | |
6 | reads during a write, since concurrent writes are barred with inode->i_mutex | |
7 | at the vfs level. | |
8 | ||
9 | Signed-off-by: Jeff Mahoney <jeffm@suse.com> | |
10 | ||
11 | -- | |
12 | fs/reiserfs/xattr.c | 114 +++++++++++++++++++---------------------- | |
13 | fs/reiserfs/xattr_acl.c | 7 -- | |
14 | include/linux/reiserfs_fs_i.h | 2 | |
15 | include/linux/reiserfs_xattr.h | 22 ------- | |
16 | 4 files changed, 55 insertions(+), 90 deletions(-) | |
17 | ||
18 | --- a/fs/reiserfs/xattr_acl.c | |
19 | +++ b/fs/reiserfs/xattr_acl.c | |
20 | @@ -418,9 +418,7 @@ int reiserfs_cache_default_acl(struct in | |
21 | int ret = 0; | |
22 | if (reiserfs_posixacl(inode->i_sb) && !IS_PRIVATE(inode)) { | |
23 | struct posix_acl *acl; | |
24 | - reiserfs_read_lock_xattr_i(inode); | |
25 | acl = reiserfs_get_acl(inode, ACL_TYPE_DEFAULT); | |
26 | - reiserfs_read_unlock_xattr_i(inode); | |
27 | ret = (acl && !IS_ERR(acl)); | |
28 | if (ret) | |
29 | posix_acl_release(acl); | |
30 | @@ -452,11 +450,8 @@ int reiserfs_acl_chmod(struct inode *ino | |
31 | if (!clone) | |
32 | return -ENOMEM; | |
33 | error = posix_acl_chmod_masq(clone, inode->i_mode); | |
34 | - if (!error) { | |
35 | - reiserfs_write_lock_xattr_i(inode); | |
36 | + if (!error) | |
37 | error = reiserfs_set_acl(inode, ACL_TYPE_ACCESS, clone); | |
38 | - reiserfs_write_unlock_xattr_i(inode); | |
39 | - } | |
40 | posix_acl_release(clone); | |
41 | return error; | |
42 | } | |
43 | --- a/fs/reiserfs/xattr.c | |
44 | +++ b/fs/reiserfs/xattr.c | |
45 | @@ -29,10 +29,8 @@ | |
46 | * to the inode so that unnecessary lookups are avoided. | |
47 | * | |
48 | * Locking works like so: | |
49 | - * The xattr root (/.reiserfs_priv/xattrs) is protected by its i_mutex. | |
50 | - * The xattr dir (/.reiserfs_priv/xattrs/<oid>.<gen>) is protected by | |
51 | - * inode->xattr_sem. | |
52 | - * The xattrs themselves are likewise protected by the xattr_sem. | |
53 | + * Directory components (xattr root, xattr dir) are protectd by their i_mutex. | |
54 | + * The xattrs themselves are protected by the xattr_sem. | |
55 | */ | |
56 | ||
57 | #include <linux/reiserfs_fs.h> | |
58 | @@ -55,6 +53,8 @@ | |
59 | #define PRIVROOT_NAME ".reiserfs_priv" | |
60 | #define XAROOT_NAME "xattrs" | |
61 | ||
62 | +static struct reiserfs_xattr_handler *find_xattr_handler_prefix(const char *); | |
63 | + | |
64 | /* Helpers for inode ops. We do this so that we don't have all the VFS | |
65 | * overhead and also for proper i_mutex annotation. | |
66 | * dir->i_mutex must be held for all of them. */ | |
67 | @@ -339,12 +339,14 @@ int xattr_readdir(struct inode *inode, f | |
68 | return res; | |
69 | } | |
70 | ||
71 | +/* expects xadir->d_inode->i_mutex to be locked */ | |
72 | static int | |
73 | __reiserfs_xattr_del(struct dentry *xadir, const char *name, int namelen) | |
74 | { | |
75 | struct dentry *dentry; | |
76 | struct inode *dir = xadir->d_inode; | |
77 | int err = 0; | |
78 | + struct reiserfs_xattr_handler *xah; | |
79 | ||
80 | dentry = lookup_one_len(name, xadir, namelen); | |
81 | if (IS_ERR(dentry)) { | |
82 | @@ -372,6 +374,14 @@ __reiserfs_xattr_del(struct dentry *xadi | |
83 | return -EIO; | |
84 | } | |
85 | ||
86 | + /* Deletion pre-operation */ | |
87 | + xah = find_xattr_handler_prefix(name); | |
88 | + if (xah && xah->del) { | |
89 | + err = xah->del(dentry->d_inode, name); | |
90 | + if (err) | |
91 | + goto out; | |
92 | + } | |
93 | + | |
94 | err = xattr_unlink(dir, dentry); | |
95 | ||
96 | out_file: | |
97 | @@ -398,7 +408,7 @@ reiserfs_delete_xattrs_filler(void *buf, | |
98 | /* This is called w/ inode->i_mutex downed */ | |
99 | int reiserfs_delete_xattrs(struct inode *inode) | |
100 | { | |
101 | - int err = 0; | |
102 | + int err = -ENODATA; | |
103 | struct dentry *dir, *root; | |
104 | struct reiserfs_transaction_handle th; | |
105 | int blocks = JOURNAL_PER_BALANCE_CNT * 2 + 2 + | |
106 | @@ -414,14 +424,19 @@ int reiserfs_delete_xattrs(struct inode | |
107 | goto out; | |
108 | } else if (!dir->d_inode) { | |
109 | dput(dir); | |
110 | - return 0; | |
111 | + goto out; | |
112 | } | |
113 | ||
114 | mutex_lock_nested(&dir->d_inode->i_mutex, I_MUTEX_XATTR); | |
115 | err = xattr_readdir(dir->d_inode, reiserfs_delete_xattrs_filler, dir); | |
116 | mutex_unlock(&dir->d_inode->i_mutex); | |
117 | - if (err) | |
118 | - goto out_dir; | |
119 | + if (err) { | |
120 | + dput(dir); | |
121 | + goto out; | |
122 | + } | |
123 | + | |
124 | + root = dget(dir->d_parent); | |
125 | + dput(dir); | |
126 | ||
127 | /* We start a transaction here to avoid a ABBA situation | |
128 | * between the xattr root's i_mutex and the journal lock. | |
129 | @@ -435,19 +450,14 @@ int reiserfs_delete_xattrs(struct inode | |
130 | err = journal_begin(&th, inode->i_sb, blocks); | |
131 | if (!err) { | |
132 | int jerror; | |
133 | - root = dget(dir->d_parent); | |
134 | mutex_lock_nested(&root->d_inode->i_mutex, I_MUTEX_XATTR); | |
135 | err = xattr_rmdir(root->d_inode, dir); | |
136 | jerror = journal_end(&th, inode->i_sb, blocks); | |
137 | mutex_unlock(&root->d_inode->i_mutex); | |
138 | - dput(root); | |
139 | - | |
140 | err = jerror ?: err; | |
141 | } | |
142 | ||
143 | -out_dir: | |
144 | - dput(dir); | |
145 | - | |
146 | + dput(root); | |
147 | out: | |
148 | if (!err) | |
149 | REISERFS_I(inode)->i_flags = | |
150 | @@ -484,7 +494,7 @@ reiserfs_chown_xattrs_filler(void *buf, | |
151 | ||
152 | if (!S_ISDIR(xafile->d_inode->i_mode)) { | |
153 | mutex_lock_nested(&xafile->d_inode->i_mutex, I_MUTEX_CHILD); | |
154 | - err = notify_change(xafile, attrs); | |
155 | + err = reiserfs_setattr(xafile, attrs); | |
156 | mutex_unlock(&xafile->d_inode->i_mutex); | |
157 | } | |
158 | dput(xafile); | |
159 | @@ -520,13 +530,16 @@ int reiserfs_chown_xattrs(struct inode * | |
160 | err = xattr_readdir(dir->d_inode, reiserfs_chown_xattrs_filler, &buf); | |
161 | ||
162 | if (!err) | |
163 | - err = notify_change(dir, attrs); | |
164 | + err = reiserfs_setattr(dir, attrs); | |
165 | mutex_unlock(&dir->d_inode->i_mutex); | |
166 | ||
167 | attrs->ia_valid = ia_valid; | |
168 | out_dir: | |
169 | dput(dir); | |
170 | out: | |
171 | + if (err) | |
172 | + reiserfs_warning(inode->i_sb, "jdm-20007", | |
173 | + "Couldn't chown all xattrs (%d)\n", err); | |
174 | return err; | |
175 | } | |
176 | ||
177 | @@ -635,9 +648,8 @@ reiserfs_xattr_set(struct inode *inode, | |
178 | if (get_inode_sd_version(inode) == STAT_DATA_V1) | |
179 | return -EOPNOTSUPP; | |
180 | ||
181 | - /* Empty xattrs are ok, they're just empty files, no hash */ | |
182 | - if (buffer && buffer_size) | |
183 | - xahash = xattr_hash(buffer, buffer_size); | |
184 | + if (!buffer) | |
185 | + return reiserfs_xattr_del(inode, name); | |
186 | ||
187 | dentry = get_xa_file_dentry(inode, name, flags); | |
188 | if (IS_ERR(dentry)) { | |
189 | @@ -645,13 +657,19 @@ reiserfs_xattr_set(struct inode *inode, | |
190 | goto out; | |
191 | } | |
192 | ||
193 | + down_write(&REISERFS_I(inode)->i_xattr_sem); | |
194 | + | |
195 | + xahash = xattr_hash(buffer, buffer_size); | |
196 | REISERFS_I(inode)->i_flags |= i_has_xattr_dir; | |
197 | ||
198 | /* Resize it so we're ok to write there */ | |
199 | newattrs.ia_size = buffer_size; | |
200 | + newattrs.ia_ctime = current_fs_time(inode->i_sb); | |
201 | newattrs.ia_valid = ATTR_SIZE | ATTR_CTIME; | |
202 | mutex_lock_nested(&dentry->d_inode->i_mutex, I_MUTEX_XATTR); | |
203 | - err = notify_change(dentry, &newattrs); | |
204 | + down_write(&dentry->d_inode->i_alloc_sem); | |
205 | + err = reiserfs_setattr(dentry, &newattrs); | |
206 | + up_write(&dentry->d_inode->i_alloc_sem); | |
207 | mutex_unlock(&dentry->d_inode->i_mutex); | |
208 | if (err) | |
209 | goto out_filp; | |
210 | @@ -712,6 +730,7 @@ reiserfs_xattr_set(struct inode *inode, | |
211 | } | |
212 | ||
213 | out_filp: | |
214 | + up_write(&REISERFS_I(inode)->i_xattr_sem); | |
215 | dput(dentry); | |
216 | ||
217 | out: | |
218 | @@ -747,10 +766,7 @@ reiserfs_xattr_get(const struct inode *i | |
219 | goto out; | |
220 | } | |
221 | ||
222 | - /* protect against concurrent access. xattrs are backed by | |
223 | - * regular files, but they're not regular files. The updates | |
224 | - * must be atomic from the perspective of the user. */ | |
225 | - mutex_lock_nested(&dentry->d_inode->i_mutex, I_MUTEX_XATTR); | |
226 | + down_read(&REISERFS_I(inode)->i_xattr_sem); | |
227 | ||
228 | isize = i_size_read(dentry->d_inode); | |
229 | REISERFS_I(inode)->i_flags |= i_has_xattr_dir; | |
230 | @@ -758,12 +774,12 @@ reiserfs_xattr_get(const struct inode *i | |
231 | /* Just return the size needed */ | |
232 | if (buffer == NULL) { | |
233 | err = isize - sizeof(struct reiserfs_xattr_header); | |
234 | - goto out_dput; | |
235 | + goto out_unlock; | |
236 | } | |
237 | ||
238 | if (buffer_size < isize - sizeof(struct reiserfs_xattr_header)) { | |
239 | err = -ERANGE; | |
240 | - goto out_dput; | |
241 | + goto out_unlock; | |
242 | } | |
243 | ||
244 | while (file_pos < isize) { | |
245 | @@ -778,7 +794,7 @@ reiserfs_xattr_get(const struct inode *i | |
246 | page = reiserfs_get_page(dentry->d_inode, file_pos); | |
247 | if (IS_ERR(page)) { | |
248 | err = PTR_ERR(page); | |
249 | - goto out_dput; | |
250 | + goto out_unlock; | |
251 | } | |
252 | ||
253 | lock_page(page); | |
254 | @@ -797,7 +813,7 @@ reiserfs_xattr_get(const struct inode *i | |
255 | "associated with %k", name, | |
256 | INODE_PKEY(inode)); | |
257 | err = -EIO; | |
258 | - goto out_dput; | |
259 | + goto out_unlock; | |
260 | } | |
261 | hash = le32_to_cpu(rxh->h_hash); | |
262 | } | |
263 | @@ -818,8 +834,8 @@ reiserfs_xattr_get(const struct inode *i | |
264 | err = -EIO; | |
265 | } | |
266 | ||
267 | -out_dput: | |
268 | - mutex_unlock(&dentry->d_inode->i_mutex); | |
269 | +out_unlock: | |
270 | + up_read(&REISERFS_I(inode)->i_xattr_sem); | |
271 | dput(dentry); | |
272 | ||
273 | out: | |
274 | @@ -852,8 +868,6 @@ int reiserfs_xattr_del(struct inode *ino | |
275 | } | |
276 | ||
277 | /* Actual operations that are exported to VFS-land */ | |
278 | - | |
279 | -static struct reiserfs_xattr_handler *find_xattr_handler_prefix(const char *); | |
280 | /* | |
281 | * Inode operation getxattr() | |
282 | */ | |
283 | @@ -868,9 +882,7 @@ reiserfs_getxattr(struct dentry * dentry | |
284 | get_inode_sd_version(dentry->d_inode) == STAT_DATA_V1) | |
285 | return -EOPNOTSUPP; | |
286 | ||
287 | - reiserfs_read_lock_xattr_i(dentry->d_inode); | |
288 | err = xah->get(dentry->d_inode, name, buffer, size); | |
289 | - reiserfs_read_unlock_xattr_i(dentry->d_inode); | |
290 | return err; | |
291 | } | |
292 | ||
293 | @@ -890,9 +902,7 @@ reiserfs_setxattr(struct dentry *dentry, | |
294 | get_inode_sd_version(dentry->d_inode) == STAT_DATA_V1) | |
295 | return -EOPNOTSUPP; | |
296 | ||
297 | - reiserfs_write_lock_xattr_i(dentry->d_inode); | |
298 | err = xah->set(dentry->d_inode, name, value, size, flags); | |
299 | - reiserfs_write_unlock_xattr_i(dentry->d_inode); | |
300 | return err; | |
301 | } | |
302 | ||
303 | @@ -910,21 +920,11 @@ int reiserfs_removexattr(struct dentry * | |
304 | get_inode_sd_version(dentry->d_inode) == STAT_DATA_V1) | |
305 | return -EOPNOTSUPP; | |
306 | ||
307 | - reiserfs_write_lock_xattr_i(dentry->d_inode); | |
308 | - /* Deletion pre-operation */ | |
309 | - if (xah->del) { | |
310 | - err = xah->del(dentry->d_inode, name); | |
311 | - if (err) | |
312 | - goto out; | |
313 | - } | |
314 | - | |
315 | err = reiserfs_xattr_del(dentry->d_inode, name); | |
316 | ||
317 | dentry->d_inode->i_ctime = CURRENT_TIME_SEC; | |
318 | mark_inode_dirty(dentry->d_inode); | |
319 | ||
320 | - out: | |
321 | - reiserfs_write_unlock_xattr_i(dentry->d_inode); | |
322 | return err; | |
323 | } | |
324 | ||
325 | @@ -986,7 +986,6 @@ ssize_t reiserfs_listxattr(struct dentry | |
326 | get_inode_sd_version(dentry->d_inode) == STAT_DATA_V1) | |
327 | return -EOPNOTSUPP; | |
328 | ||
329 | - reiserfs_read_lock_xattr_i(dentry->d_inode); | |
330 | dir = open_xa_dir(dentry->d_inode, XATTR_REPLACE); | |
331 | if (IS_ERR(dir)) { | |
332 | err = PTR_ERR(dir); | |
333 | @@ -1005,19 +1004,16 @@ ssize_t reiserfs_listxattr(struct dentry | |
334 | mutex_lock_nested(&dir->d_inode->i_mutex, I_MUTEX_XATTR); | |
335 | err = xattr_readdir(dir->d_inode, reiserfs_listxattr_filler, &buf); | |
336 | mutex_unlock(&dir->d_inode->i_mutex); | |
337 | - if (err) | |
338 | - goto out_dir; | |
339 | ||
340 | - if (buf.r_pos > buf.r_size && buffer != NULL) | |
341 | - err = -ERANGE; | |
342 | - else | |
343 | - err = buf.r_pos; | |
344 | + if (!err) { | |
345 | + if (buf.r_pos > buf.r_size && buffer != NULL) | |
346 | + err = -ERANGE; | |
347 | + else | |
348 | + err = buf.r_pos; | |
349 | + } | |
350 | ||
351 | - out_dir: | |
352 | dput(dir); | |
353 | - | |
354 | - out: | |
355 | - reiserfs_read_unlock_xattr_i(dentry->d_inode); | |
356 | +out: | |
357 | return err; | |
358 | } | |
359 | ||
360 | @@ -1115,12 +1111,8 @@ static int reiserfs_check_acl(struct ino | |
361 | struct posix_acl *acl; | |
362 | int error = -EAGAIN; /* do regular unix permission checks by default */ | |
363 | ||
364 | - reiserfs_read_lock_xattr_i(inode); | |
365 | - | |
366 | acl = reiserfs_get_acl(inode, ACL_TYPE_ACCESS); | |
367 | ||
368 | - reiserfs_read_unlock_xattr_i(inode); | |
369 | - | |
370 | if (acl) { | |
371 | if (!IS_ERR(acl)) { | |
372 | error = posix_acl_permission(inode, acl, mask); | |
373 | --- a/include/linux/reiserfs_fs_i.h | |
374 | +++ b/include/linux/reiserfs_fs_i.h | |
375 | @@ -59,7 +59,7 @@ struct reiserfs_inode_info { | |
376 | struct posix_acl *i_acl_default; | |
377 | #endif | |
378 | #ifdef CONFIG_REISERFS_FS_XATTR | |
379 | - struct rw_semaphore xattr_sem; | |
380 | + struct rw_semaphore i_xattr_sem; | |
381 | #endif | |
382 | struct inode vfs_inode; | |
383 | }; | |
384 | --- a/include/linux/reiserfs_xattr.h | |
385 | +++ b/include/linux/reiserfs_xattr.h | |
386 | @@ -67,24 +67,6 @@ extern struct reiserfs_xattr_handler use | |
387 | extern struct reiserfs_xattr_handler trusted_handler; | |
388 | extern struct reiserfs_xattr_handler security_handler; | |
389 | ||
390 | -static inline void reiserfs_write_lock_xattr_i(struct inode *inode) | |
391 | -{ | |
392 | - down_write(&REISERFS_I(inode)->i_xattr_sem); | |
393 | -} | |
394 | -static inline void reiserfs_write_unlock_xattr_i(struct inode *inode) | |
395 | -{ | |
396 | - up_write(&REISERFS_I(inode)->i_xattr_sem); | |
397 | -} | |
398 | -static inline void reiserfs_read_lock_xattr_i(struct inode *inode) | |
399 | -{ | |
400 | - down_read(&REISERFS_I(inode)->i_xattr_sem); | |
401 | -} | |
402 | - | |
403 | -static inline void reiserfs_read_unlock_xattr_i(struct inode *inode) | |
404 | -{ | |
405 | - up_read(&REISERFS_I(inode)->i_xattr_sem); | |
406 | -} | |
407 | - | |
408 | static inline void reiserfs_init_xattr_rwsem(struct inode *inode) | |
409 | { | |
410 | init_rwsem(&REISERFS_I(inode)->i_xattr_sem); | |
411 | @@ -96,10 +78,6 @@ static inline void reiserfs_init_xattr_r | |
412 | #define reiserfs_setxattr NULL | |
413 | #define reiserfs_listxattr NULL | |
414 | #define reiserfs_removexattr NULL | |
415 | -#define reiserfs_write_lock_xattrs(sb) do {;} while(0) | |
416 | -#define reiserfs_write_unlock_xattrs(sb) do {;} while(0) | |
417 | -#define reiserfs_read_lock_xattrs(sb) | |
418 | -#define reiserfs_read_unlock_xattrs(sb) | |
419 | ||
420 | #define reiserfs_permission NULL | |
421 |