Frank Holton
2013-Sep-17 02:21 UTC
[RFC] btrfs-progs: Add recursive defrag using -r option
I''m working on a patch to add the recursive option to the
btrfs filesystem defrag command. I''d like some feedback on how
the patch looks as written. I''ve added two helper functions, which
might need to be renamed, one to call the ioctl and one to actually handle the
recursion into the directory.
Let me know what you think.
-Frank
Added a recursive option that allows defrag to defragment
the directory and all files and directories below it.
Signed-off-by: Frank Holton <fholton@gmail.com>
---
cmds-filesystem.c | 174 +++++++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 154 insertions(+), 20 deletions(-)
diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index f41a72a..9a4b810 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -23,6 +23,9 @@
#include <uuid/uuid.h>
#include <ctype.h>
+#include <dirent.h>
+#include <fcntl.h>
+
#include "kerncompat.h"
#include "ctree.h"
#include "ioctl.h"
@@ -333,6 +336,7 @@ static const char * const cmd_defrag_usage[] = {
"Defragment a file or a directory",
"",
"-v be verbose",
+ "-r defragment directories and files recursively",
"-c[zlib,lzo] compress the file while defragmenting",
"-f flush data to disk immediately after defragmenting",
"-s start defragment only from byte onward",
@@ -341,6 +345,115 @@ static const char * const cmd_defrag_usage[] = {
NULL
};
+static int do_defrag(int fd, int fancy_ioctl,
+ struct btrfs_ioctl_defrag_range_args *range) {
+ int ret;
+ if (!fancy_ioctl) {
+ ret = ioctl(fd, BTRFS_IOC_DEFRAG, NULL);
+ } else {
+ ret = ioctl(fd, BTRFS_IOC_DEFRAG_RANGE, range);
+ }
+ return(ret);
+}
+
+static int walk_dir(char *name, int verbose, int fancy_ioctl, struct
+ btrfs_ioctl_defrag_range_args *range) {
+ int e;
+ int dir_fd;
+ int ret = 0;
+ DIR *dir;
+ char fn[FILENAME_MAX];
+ struct dirent *dent;
+ struct stat st;
+
+ int errors = 0;
+
+ int len = strlen(name);
+ if (len >= FILENAME_MAX - 1) {
+ fprintf(stderr, "ERROR: path name too long\n");
+ return 1;
+ }
+
+ strcpy(fn, name);
+ if (fn[len-1] != ''/'')
+ fn[len++] = ''/'';
+ fn[len] = 0;
+
+ if(!(dir = opendir(fn))) {
+ e = errno;
+ fprintf(stderr, "ERROR: cannot open directory %s - %s\n",
+ name, strerror(e));
+ return 1;
+ }
+
+ errno = 0;
+ ret = 0;
+ dir_fd = dirfd(dir);
+ ret = do_defrag(dir_fd, fancy_ioctl, range);
+ e = errno;
+ if (ret) {
+ fprintf(stderr, "ERROR: defrag failed on %s - %s\n",
+ fn, strerror(e));
+
+ /* directories can only be defragged as root... */
+ errors++;
+ }
+
+ while ((dent = readdir(dir)) != NULL) {
+ int fd = 0;
+ if (!strcmp(dent->d_name,".") || !strcmp(dent->d_name,
".."))
+ continue;
+
+ fn[len] = 0;
+ strncat(fn+len, dent->d_name, FILENAME_MAX - len);
+
+ if (lstat(fn, &st) == -1) {
+ fprintf(stderr,"ERROR: cannot stat %s\n", fn);
+ error++;
+ continue;
+ }
+
+ /* ignore symlinks ??*/
+ if (S_ISLNK(st.st_mode))
+ continue;
+
+ if(verbose)
+ printf("%s\n", fn);
+
+ /* directory entry */
+ if (S_ISDIR(st.st_mode)) {
+ ret = walk_dir(fn, verbose, fancy_ioctl, range);
+ errors += ret;
+ }
+ else {
+ fd = open(fn,O_RDWR);
+ e = errno;
+ if (fd < 0) {
+ fprintf(stderr, "ERROR: defrag failed on %s - %s\n",
+ fn, strerror(e));
+ error++;
+ continue;
+ }
+
+ ret = do_defrag(fd, fancy_ioctl, range);
+ e = errno;
+
+ if (ret) {
+ errors++;
+ fprintf(stderr, "ERROR: defrag failed on %s - %s\n",
+ fn, strerror(e));
+ error++;
+ }
+ close(fd);
+ }
+ }
+
+ close(dir_fd);
+ closedir(dir);
+
+ return(errors);
+}
+
static int cmd_defrag(int argc, char **argv)
{
int fd;
@@ -349,6 +462,7 @@ static int cmd_defrag(int argc, char **argv)
u64 len = (u64)-1;
u32 thresh = 0;
int i;
+ int recursive = 0;
int errors = 0;
int ret = 0;
int verbose = 0;
@@ -359,7 +473,7 @@ static int cmd_defrag(int argc, char **argv)
optind = 1;
while(1) {
- int c = getopt(argc, argv, "vc::fs:l:t:");
+ int c = getopt(argc, argv, "vrc::fs:l:t:");
if (c < 0)
break;
@@ -389,6 +503,9 @@ static int cmd_defrag(int argc, char **argv)
thresh = parse_size(optarg);
fancy_ioctl = 1;
break;
+ case ''r'':
+ recursive = 1;
+ break;
default:
usage(cmd_defrag_usage);
}
@@ -409,35 +526,52 @@ static int cmd_defrag(int argc, char **argv)
range.flags |= BTRFS_DEFRAG_RANGE_START_IO;
for (i = optind; i < argc; i++) {
- if (verbose)
- printf("%s\n", argv[i]);
+
fd = open_file_or_dir(argv[i]);
if (fd < 0) {
- fprintf(stderr, "failed to open %s\n", argv[i]);
+ fprintf(stderr, "ERROR: failed to open %s\n", argv[i]);
perror("open:");
errors++;
continue;
}
- if (!fancy_ioctl) {
- ret = ioctl(fd, BTRFS_IOC_DEFRAG, NULL);
- e=errno;
- } else {
- ret = ioctl(fd, BTRFS_IOC_DEFRAG_RANGE, &range);
- if (ret && errno == ENOTTY) {
- fprintf(stderr, "ERROR: defrag range ioctl not "
- "supported in this kernel, please try "
- "without any options.\n");
- errors++;
- close(fd);
- break;
+ if (recursive) {
+ struct stat st;
+ // check if this is a directory
+ fstat(fd, &st);
+
+ if (S_ISDIR(st.st_mode)) {
+ ret = walk_dir(argv[i], verbose, fancy_ioctl, &range );
+ e = errno;
+ errors += ret;
+ }
+ else {
+ if (verbose)
+ printf("%s\n", argv[i]);
+ ret = do_defrag(fd, fancy_ioctl, &range);
+ e = errno;
+ if (ret) {
+ fprintf(stderr, "ERROR: defrag failed on %s - %s\n",
+ argv[i], strerror(e));
+ errors++;
+ }
}
+ }
+ else {
+ if (verbose)
+ printf("%s\n", argv[i]);
+ ret = do_defrag(fd, fancy_ioctl, &range);
e = errno;
}
- if (ret) {
- fprintf(stderr, "ERROR: defrag failed on %s - %s\n",
- argv[i], strerror(e));
+
+ if (ret && errno == ENOTTY) {
+ fprintf(stderr, "ERROR: defrag range ioctl not "
+ "supported in this kernel, please try "
+ "without any options.\n");
errors++;
+ close(fd);
+ break;
}
+
close(fd);
}
if (verbose)
@@ -447,7 +581,7 @@ static int cmd_defrag(int argc, char **argv)
exit(1);
}
- return errors;
+ return 0;
}
static const char * const cmd_resize_usage[] = {
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Frank Holton posted on Mon, 16 Sep 2013 22:21:24 -0400 as excerpted:> I''m working on a patch to add the recursive option to the btrfs > filesystem defrag command. I''d like some feedback on how the patch looks > as written. I''ve added two helper functions, which might need to be > renamed, one to call the ioctl and one to actually handle the recursion > into the directory. > > Let me know what you think.Not being a dev I won''t comment on that angle, but this will sure help decomplicate general defragging from a sysadmin angle. Thanks. =:^) -- Duncan - List replies preferred. No HTML msgs. "Every nonfree program has a lord, a master -- and if you use the program, he is your master." Richard Stallman -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
David Sterba
2013-Sep-17 13:03 UTC
Re: [RFC] btrfs-progs: Add recursive defrag using -r option
On Mon, Sep 16, 2013 at 10:21:24PM -0400, Frank Holton wrote:> I''m working on a patch to add the recursive option to the > btrfs filesystem defrag command.Great!> I''d like some feedback on how the patch looks as written. I''ve added > two helper functions, which might need to be renamed, one to call the > ioctl and one to actually handle the recursion into the directory.Please use the ''ftw'' function from glibc http://man7.org/linux/man-pages/man3/ftw.3.html (it''s used in mkfs.c for example)> @@ -333,6 +336,7 @@ static const char * const cmd_defrag_usage[] = { > "Defragment a file or a directory", > "", > "-v be verbose", > + "-r defragment directories and files recursively",Directories do not get defragmented. Giving any directory to defragment will actually deframgent the subvolume fs_tree and then the shared extent tree. This is hidden in implementation and has confused people. For now, I suggest to recursively defrag only files, that''s IMHO the most frequent usecase. Deframgenting the subvol root or the extent tree are special cases and should be enabled only when requested, by a commandline option. This needs updating kernel. I can go into details later if needed. (I''ts possible to implement a per-directory defrag by extending btrfs_defrag_leaves with start/end key ranges that represent the directory items.) A few comments on the code follow, I hope you convert it to the ''ftw'' callback, this will remove half of the code from this patch, so I''d rather see the result first. Basically, the contents of the ftw callback is done inside this loop:> + while ((dent = readdir(dir)) != NULL) { > + int fd = 0; > + if (!strcmp(dent->d_name,".") || !strcmp(dent->d_name, "..")) > + continue; > + > + fn[len] = 0; > + strncat(fn+len, dent->d_name, FILENAME_MAX - len); > + > + if (lstat(fn, &st) == -1) { > + fprintf(stderr,"ERROR: cannot stat %s\n", fn); > + error++; > + continue; > + } > + > + /* ignore symlinks ??*/ > + if (S_ISLNK(st.st_mode)) > + continue; > + > + if(verbose) > + printf("%s\n", fn); > + > + /* directory entry */ > + if (S_ISDIR(st.st_mode)) { > + ret = walk_dir(fn, verbose, fancy_ioctl, range); > + errors += ret; > + } > + else { > + fd = open(fn,O_RDWR); > + e = errno; > + if (fd < 0) { > + fprintf(stderr, "ERROR: defrag failed on %s - %s\n", > + fn, strerror(e)); > + error++; > + continue; > + } > + > + ret = do_defrag(fd, fancy_ioctl, range); > + e = errno; > + > + if (ret) { > + errors++; > + fprintf(stderr, "ERROR: defrag failed on %s - %s\n", > + fn, strerror(e)); > + error++; > + } > + close(fd); > + } > + } > + > + close(dir_fd); > + closedir(dir); > + > + return(errors); > +}The command line argument handling is ok. Please get familiar with the coding style https://www.kernel.org/doc/Documentation/CodingStyle eg. spacing and { } placement. It''s not pointless to keep the style consistent, people who have to read the code appreciate if it looks the same all over the place. thanks, david -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html