1 From: Jeff Mahoney <jeffm@suse.com>
2 Subject: reiserfs: use generic readdir for operations across all xattrs
4 The current reiserfs xattr implementation open codes reiserfs_readdir and
5 frees the path before calling the filldir function. Typically, the filldir
6 function is something that modifies the file system, such as a chown or
7 an inode deletion that also require reading of an inode associated with each
8 direntry. Since the file system is modified, the path retained becomes
9 invalid for the next run. In addition, it runs backwards in attempt to
12 This is clearly suboptimal from a code cleanliness perspective as well as
15 This patch implements a generic reiserfs_for_each_xattr that uses the generic
16 readdir and a specific filldir routine that simply populates an array of
17 dentries and then performs a specific operation on them. When all files have
18 been operated on, it then calls the operation on the directory itself.
20 The result is a noticable code reduction and better performance.
22 Signed-off-by: Jeff Mahoney <jeffm@suse.com>
25 fs/reiserfs/dir.c | 28 +--
26 fs/reiserfs/xattr.c | 402 ++++++++++++--------------------------------
27 include/linux/reiserfs_fs.h | 1
28 3 files changed, 131 insertions(+), 300 deletions(-)
30 --- a/fs/reiserfs/dir.c
31 +++ b/fs/reiserfs/dir.c
32 @@ -41,10 +41,10 @@ static int reiserfs_dir_fsync(struct fil
34 #define store_ih(where,what) copy_item_head (where, what)
37 -static int reiserfs_readdir(struct file *filp, void *dirent, filldir_t filldir)
38 +int reiserfs_readdir_dentry(struct dentry *dentry, void *dirent,
39 + filldir_t filldir, loff_t *pos)
41 - struct inode *inode = filp->f_path.dentry->d_inode;
42 + struct inode *inode = dentry->d_inode;
43 struct cpu_key pos_key; /* key of current position in the directory (key of directory entry) */
44 INITIALIZE_PATH(path_to_entry);
45 struct buffer_head *bh;
46 @@ -64,13 +64,9 @@ static int reiserfs_readdir(struct file
48 /* form key for search the next directory entry using f_pos field of
50 - make_cpu_key(&pos_key, inode,
51 - (filp->f_pos) ? (filp->f_pos) : DOT_OFFSET, TYPE_DIRENTRY,
53 + make_cpu_key(&pos_key, inode, *pos ?: DOT_OFFSET, TYPE_DIRENTRY, 3);
54 next_pos = cpu_key_k_offset(&pos_key);
56 - /* reiserfs_warning (inode->i_sb, "reiserfs_readdir 1: f_pos = %Ld", filp->f_pos); */
58 path_to_entry.reada = PATH_READA;
61 @@ -144,7 +140,7 @@ static int reiserfs_readdir(struct file
62 /* Ignore the .reiserfs_priv entry */
63 if (reiserfs_xattrs(inode->i_sb) &&
64 !old_format_only(inode->i_sb) &&
65 - filp->f_path.dentry == inode->i_sb->s_root &&
66 + dentry == inode->i_sb->s_root &&
67 REISERFS_SB(inode->i_sb)->priv_root &&
68 REISERFS_SB(inode->i_sb)->priv_root->d_inode
69 && deh_objectid(deh) ==
70 @@ -156,7 +152,7 @@ static int reiserfs_readdir(struct file
73 d_off = deh_offset(deh);
74 - filp->f_pos = d_off;
76 d_ino = deh_objectid(deh);
78 local_buf = small_buf;
79 @@ -223,15 +219,21 @@ static int reiserfs_readdir(struct file
84 - filp->f_pos = next_pos;
87 pathrelse(&path_to_entry);
88 reiserfs_check_path(&path_to_entry);
91 reiserfs_write_unlock(inode->i_sb);
95 +static int reiserfs_readdir(struct file *file, void *dirent, filldir_t filldir)
97 + struct dentry *dentry = file->f_path.dentry;
98 + return reiserfs_readdir_dentry(dentry, dirent, filldir, &file->f_pos);
101 /* compose directory item containing "." and ".." entries (entries are
102 not aligned to 4 byte boundary) */
103 /* the last four params are LE */
104 --- a/fs/reiserfs/xattr.c
105 +++ b/fs/reiserfs/xattr.c
106 @@ -167,218 +167,65 @@ static struct dentry *open_xa_dir(const
111 - * this is very similar to fs/reiserfs/dir.c:reiserfs_readdir, but
112 - * we need to drop the path before calling the filldir struct. That
113 - * would be a big performance hit to the non-xattr case, so I've copied
114 - * the whole thing for now. --clm
116 - * the big difference is that I go backwards through the directory,
117 - * and don't mess with f->f_pos, but the idea is the same. Do some
118 - * action on each and every entry in the directory.
120 - * we're called with i_mutex held, so there are no worries about the directory
121 - * changing underneath us.
123 -static int __xattr_readdir(struct inode *inode, void *dirent, filldir_t filldir)
125 - struct cpu_key pos_key; /* key of current position in the directory (key of directory entry) */
126 - INITIALIZE_PATH(path_to_entry);
127 - struct buffer_head *bh;
129 - struct item_head *ih, tmp_ih;
133 - char small_buf[32]; /* avoid kmalloc if we can */
134 - struct reiserfs_de_head *deh;
139 - struct reiserfs_dir_entry de;
141 - /* form key for search the next directory entry using f_pos field of
143 - next_pos = max_reiserfs_offset(inode);
147 - if (next_pos <= DOT_DOT_OFFSET)
149 - make_cpu_key(&pos_key, inode, next_pos, TYPE_DIRENTRY, 3);
152 - search_by_entry_key(inode->i_sb, &pos_key, &path_to_entry,
154 - if (search_res == IO_ERROR) {
155 - // FIXME: we could just skip part of directory which could
157 - pathrelse(&path_to_entry);
161 - if (search_res == NAME_NOT_FOUND)
164 - set_de_name_and_namelen(&de);
165 - entry_num = de.de_entry_num;
166 - deh = &(de.de_deh[entry_num]);
171 - if (!is_direntry_le_ih(ih)) {
172 - reiserfs_error(inode->i_sb, "jdm-20000",
173 - "not direntry %h", ih);
176 - copy_item_head(&tmp_ih, ih);
178 - /* we must have found item, that is item of this directory, */
179 - RFALSE(COMP_SHORT_KEYS(&(ih->ih_key), &pos_key),
180 - "vs-9000: found item %h does not match to dir we readdir %K",
183 - if (deh_offset(deh) <= DOT_DOT_OFFSET) {
187 - /* look for the previous entry in the directory */
188 - next_pos = deh_offset(deh) - 1;
190 - if (!de_visible(deh))
191 - /* it is hidden entry */
194 - d_reclen = entry_length(bh, ih, entry_num);
195 - d_name = B_I_DEH_ENTRY_FILE_NAME(bh, ih, deh);
196 - d_off = deh_offset(deh);
197 - d_ino = deh_objectid(deh);
199 - if (!d_name[d_reclen - 1])
200 - d_reclen = strlen(d_name);
202 - if (d_reclen > REISERFS_MAX_NAME(inode->i_sb->s_blocksize)) {
203 - /* too big to send back to VFS */
207 - /* Ignore the .reiserfs_priv entry */
208 - if (reiserfs_xattrs(inode->i_sb) &&
209 - !old_format_only(inode->i_sb) &&
210 - deh_objectid(deh) ==
211 - le32_to_cpu(INODE_PKEY
212 - (REISERFS_SB(inode->i_sb)->priv_root->d_inode)->
216 - if (d_reclen <= 32) {
217 - local_buf = small_buf;
219 - local_buf = kmalloc(d_reclen, GFP_NOFS);
221 - pathrelse(&path_to_entry);
224 - if (item_moved(&tmp_ih, &path_to_entry)) {
227 - /* sigh, must retry. Do this same offset again */
233 - // Note, that we copy name to user space via temporary
234 - // buffer (local_buf) because filldir will block if
235 - // user space buffer is swapped out. At that time
236 - // entry can move to somewhere else
237 - memcpy(local_buf, d_name, d_reclen);
239 - /* the filldir function might need to start transactions,
240 - * or do who knows what. Release the path now that we've
241 - * copied all the important stuff out of the deh
243 - pathrelse(&path_to_entry);
245 - if (filldir(dirent, local_buf, d_reclen, d_off, d_ino,
247 - if (local_buf != small_buf) {
252 - if (local_buf != small_buf) {
258 - pathrelse(&path_to_entry);
263 - * this could be done with dedicated readdir ops for the xattr files,
264 - * but I want to get something working asap
265 - * this is stolen from vfs_readdir
269 -int xattr_readdir(struct inode *inode, filldir_t filler, void *buf)
272 - if (!IS_DEADDIR(inode)) {
274 - res = __xattr_readdir(inode, buf, filler);
280 /* The following are side effects of other operations that aren't explicitly
281 * modifying extended attributes. This includes operations such as permissions
282 * or ownership changes, object deletions, etc. */
283 +struct reiserfs_dentry_buf {
284 + struct dentry *xadir;
286 + struct dentry *dentries[8];
290 -reiserfs_delete_xattrs_filler(void *buf, const char *name, int namelen,
291 - loff_t offset, u64 ino, unsigned int d_type)
292 +fill_with_dentries(void *buf, const char *name, int namelen, loff_t offset,
293 + u64 ino, unsigned int d_type)
295 - struct dentry *xadir = (struct dentry *)buf;
296 + struct reiserfs_dentry_buf *dbuf = buf;
297 struct dentry *dentry;
300 - dentry = lookup_one_len(name, xadir, namelen);
301 + if (dbuf->count == ARRAY_SIZE(dbuf->dentries))
304 + if (name[0] == '.' && (name[1] == '\0' ||
305 + (name[1] == '.' && name[2] == '\0')))
308 + dentry = lookup_one_len(name, dbuf->xadir, namelen);
309 if (IS_ERR(dentry)) {
310 - err = PTR_ERR(dentry);
312 + return PTR_ERR(dentry);
313 } else if (!dentry->d_inode) {
316 + /* A directory entry exists, but no file? */
317 + reiserfs_error(dentry->d_sb, "xattr-20003",
318 + "Corrupted directory: xattr %s listed but "
319 + "not found for file %s.\n",
320 + dentry->d_name.name, dbuf->xadir->d_name.name);
325 - /* Skip directories.. */
326 - if (S_ISDIR(dentry->d_inode->i_mode))
329 - err = xattr_unlink(xadir->d_inode, dentry);
333 + dbuf->dentries[dbuf->count++] = dentry;
340 +cleanup_dentry_buf(struct reiserfs_dentry_buf *buf)
343 + for (i = 0; i < buf->count; i++)
344 + if (buf->dentries[i])
345 + dput(buf->dentries[i]);
348 -/* This is called w/ inode->i_mutex downed */
349 -int reiserfs_delete_xattrs(struct inode *inode)
350 +static int reiserfs_for_each_xattr(struct inode *inode,
351 + int (*action)(struct dentry *, void *),
354 - int err = -ENODATA;
355 - struct dentry *dir, *root;
356 - struct reiserfs_transaction_handle th;
357 - int blocks = JOURNAL_PER_BALANCE_CNT * 2 + 2 +
358 - 4 * REISERFS_QUOTA_TRANS_BLOCKS(inode->i_sb);
359 + struct dentry *dir;
362 + struct reiserfs_dentry_buf buf = {
366 /* Skip out, an xattr has no xattrs associated with it */
367 if (IS_PRIVATE(inode) || get_inode_sd_version(inode) == STAT_DATA_V1)
368 @@ -389,117 +236,97 @@ int reiserfs_delete_xattrs(struct inode
371 } else if (!dir->d_inode) {
378 mutex_lock_nested(&dir->d_inode->i_mutex, I_MUTEX_XATTR);
379 - err = xattr_readdir(dir->d_inode, reiserfs_delete_xattrs_filler, dir);
380 - mutex_unlock(&dir->d_inode->i_mutex);
385 + err = reiserfs_readdir_dentry(dir, &buf, fill_with_dentries, &pos);
386 + while ((err == 0 || err == -ENOSPC) && buf.count) {
389 + for (i = 0; i < buf.count && buf.dentries[i]; i++) {
391 + struct dentry *dentry = buf.dentries[i];
393 + if (err == 0 && !S_ISDIR(dentry->d_inode->i_mode))
394 + lerr = action(dentry, data);
397 + buf.dentries[i] = NULL;
402 + err = reiserfs_readdir_dentry(dir, &buf,
403 + fill_with_dentries, &pos);
405 + mutex_unlock(&dir->d_inode->i_mutex);
407 - root = dget(dir->d_parent);
409 + /* Clean up after a failed readdir */
410 + cleanup_dentry_buf(&buf);
412 - /* We start a transaction here to avoid a ABBA situation
413 - * between the xattr root's i_mutex and the journal lock.
414 - * Inode creation will inherit an ACL, which requires a
415 - * lookup. The lookup locks the xattr root i_mutex with a
416 - * transaction open. Inode deletion takes teh xattr root
417 - * i_mutex to delete the directory and then starts a
418 - * transaction inside it. Boom. This doesn't incur much
419 - * additional overhead since the reiserfs_rmdir transaction
420 - * will just nest inside the outer transaction. */
421 - err = journal_begin(&th, inode->i_sb, blocks);
424 - mutex_lock_nested(&root->d_inode->i_mutex, I_MUTEX_XATTR);
425 - err = xattr_rmdir(root->d_inode, dir);
426 - jerror = journal_end(&th, inode->i_sb, blocks);
427 - mutex_unlock(&root->d_inode->i_mutex);
428 - err = jerror ?: err;
429 + /* We start a transaction here to avoid a ABBA situation
430 + * between the xattr root's i_mutex and the journal lock.
431 + * This doesn't incur much additional overhead since the
432 + * new transaction will just nest inside the
433 + * outer transaction. */
434 + int blocks = JOURNAL_PER_BALANCE_CNT * 2 + 2 +
435 + 4 * REISERFS_QUOTA_TRANS_BLOCKS(inode->i_sb);
436 + struct reiserfs_transaction_handle th;
437 + err = journal_begin(&th, inode->i_sb, blocks);
440 + mutex_lock_nested(&dir->d_parent->d_inode->i_mutex,
442 + err = action(dir, data);
443 + jerror = journal_end(&th, inode->i_sb, blocks);
444 + mutex_unlock(&dir->d_parent->d_inode->i_mutex);
445 + err = jerror ?: err;
454 - reiserfs_warning(inode->i_sb, "jdm-20004",
455 - "Couldn't remove all xattrs (%d)\n", err);
456 + /* -ENODATA isn't an error */
457 + if (err == -ENODATA)
462 -struct reiserfs_chown_buf {
463 - struct inode *inode;
464 - struct dentry *xadir;
465 - struct iattr *attrs;
468 -/* XXX: If there is a better way to do this, I'd love to hear about it */
470 -reiserfs_chown_xattrs_filler(void *buf, const char *name, int namelen,
471 - loff_t offset, u64 ino, unsigned int d_type)
472 +static int delete_one_xattr(struct dentry *dentry, void *data)
474 - struct reiserfs_chown_buf *chown_buf = (struct reiserfs_chown_buf *)buf;
475 - struct dentry *xafile, *xadir = chown_buf->xadir;
476 - struct iattr *attrs = chown_buf->attrs;
478 + struct inode *dir = dentry->d_parent->d_inode;
480 - xafile = lookup_one_len(name, xadir, namelen);
481 - if (IS_ERR(xafile))
482 - return PTR_ERR(xafile);
483 - else if (!xafile->d_inode) {
487 + /* This is the xattr dir, handle specially. */
488 + if (S_ISDIR(dentry->d_inode->i_mode))
489 + return xattr_rmdir(dir, dentry);
491 - if (!S_ISDIR(xafile->d_inode->i_mode)) {
492 - mutex_lock_nested(&xafile->d_inode->i_mutex, I_MUTEX_CHILD);
493 - err = reiserfs_setattr(xafile, attrs);
494 - mutex_unlock(&xafile->d_inode->i_mutex);
497 + return xattr_unlink(dir, dentry);
500 +static int chown_one_xattr(struct dentry *dentry, void *data)
502 + struct iattr *attrs = data;
503 + return reiserfs_setattr(dentry, attrs);
506 +/* No i_mutex, but the inode is unconnected. */
507 +int reiserfs_delete_xattrs(struct inode *inode)
509 + int err = reiserfs_for_each_xattr(inode, delete_one_xattr, NULL);
511 + reiserfs_warning(inode->i_sb, "jdm-20004",
512 + "Couldn't delete all xattrs (%d)\n", err);
516 +/* inode->i_mutex: down */
517 int reiserfs_chown_xattrs(struct inode *inode, struct iattr *attrs)
519 - struct dentry *dir;
521 - struct reiserfs_chown_buf buf;
522 - unsigned int ia_valid = attrs->ia_valid;
524 - /* Skip out, an xattr has no xattrs associated with it */
525 - if (IS_PRIVATE(inode) || get_inode_sd_version(inode) == STAT_DATA_V1)
528 - dir = open_xa_dir(inode, XATTR_REPLACE);
530 - if (PTR_ERR(dir) != -ENODATA)
531 - err = PTR_ERR(dir);
533 - } else if (!dir->d_inode)
536 - attrs->ia_valid &= (ATTR_UID | ATTR_GID | ATTR_CTIME);
541 - mutex_lock_nested(&dir->d_inode->i_mutex, I_MUTEX_XATTR);
542 - err = xattr_readdir(dir->d_inode, reiserfs_chown_xattrs_filler, &buf);
545 - err = reiserfs_setattr(dir, attrs);
546 - mutex_unlock(&dir->d_inode->i_mutex);
548 - attrs->ia_valid = ia_valid;
552 + int err = reiserfs_for_each_xattr(inode, chown_one_xattr, attrs);
554 reiserfs_warning(inode->i_sb, "jdm-20007",
555 "Couldn't chown all xattrs (%d)\n", err);
556 @@ -1004,6 +831,7 @@ ssize_t reiserfs_listxattr(struct dentry
561 struct listxattr_buf buf = {
562 .inode = dentry->d_inode,
564 @@ -1026,7 +854,7 @@ ssize_t reiserfs_listxattr(struct dentry
567 mutex_lock_nested(&dir->d_inode->i_mutex, I_MUTEX_XATTR);
568 - err = xattr_readdir(dir->d_inode, listxattr_filler, &buf);
569 + err = reiserfs_readdir_dentry(dir, &buf, listxattr_filler, &pos);
570 mutex_unlock(&dir->d_inode->i_mutex);
573 --- a/include/linux/reiserfs_fs.h
574 +++ b/include/linux/reiserfs_fs.h
575 @@ -1984,6 +1984,7 @@ extern const struct inode_operations rei
576 extern const struct inode_operations reiserfs_symlink_inode_operations;
577 extern const struct inode_operations reiserfs_special_inode_operations;
578 extern const struct file_operations reiserfs_dir_operations;
579 +int reiserfs_readdir_dentry(struct dentry *, void *, filldir_t, loff_t *);
581 /* tail_conversion.c */
582 int direct2indirect(struct reiserfs_transaction_handle *, struct inode *,