On Fri, Aug 24, 2012 at 08:12:49AM +0200, Otto Moerbeek wrote: > On Thu, Aug 23, 2012 at 01:33:37PM -0700, Philip Guenther wrote: > > > On Thu, Aug 23, 2012 at 12:32 PM, Jan Stary <h...@stare.cz> wrote: > > > cp.c says: > > > > > > /* > > > * mastercmp -- > > > * The comparison function for the copy order. The order is to copy > > > * non-directory files before directory files. The reason for this > > > * is because files tend to be in the same cylinder group as their > > > * parent directory, whereas directories tend not to be. Copying the > > > * files first reduces seeking. > > > */ > > ... > > > Doesn't the code actually do the opposite? > > > If 'a' is a directory, it comes first? > > > > Yep. Nice catch. > > > > > > > Should that be FTS_F? > > > > I think it's correct to do the test against FTS_D, it just should > > return them in the other direction to do what the comment says. > > > > What I don't know is whether the rule described is actually still > > considered a good and useful thing still. Otto? > > > > > > Philip Guenther > > netbsd removed the sorting in 1.52: > > "Remove fts sorting. It was originally put there to copy files > before directories since files (usually) are in the same cylinder > group and subdirectories aren't. However, this mostly changed with > the new ffs dirpref algorithm in 2001." > > I'd have to check which dirpref() we use. And do some tests. > > That said, the code might not agree with the comment, but as long as files > stick > together as a group I do not think it matters much. > > -Otto
We used the same "new" dirpref() as freebsd and netbsd. Actually, they copied gluk@'s code from us ;-) Netbsd removed sorting citing peformance gains, freebsd has some pr (http://www.FreeBSD.org/cgi/query-pr.cgi?pr=53475)) requesting that, but they still have it. Limited testing here shows no significant difference. I'm all for removing it. Userland should not guess about filesystem layout policies. The filesystem might not even be ffs and modern (SSD) disks do not have any simple relation between blocks and seek times anyway. So here's a diff. -Otto Index: cp.c =================================================================== RCS file: /cvs/src/bin/cp/cp.c,v retrieving revision 1.34 diff -u -p -r1.34 cp.c --- cp.c 4 Nov 2007 02:01:57 -0000 1.34 +++ cp.c 24 Aug 2012 08:55:12 -0000 @@ -77,7 +77,6 @@ mode_t myumask; enum op { FILE_TO_FILE, FILE_TO_DIR, DIR_TO_DNE }; int copy(char *[], enum op, int); -int mastercmp(const FTSENT **, const FTSENT **); char *find_last_component(char *); int @@ -260,7 +259,7 @@ copy(char *argv[], enum op type, int fts char *p, *target_mid; base = 0; - if ((ftsp = fts_open(argv, fts_options, mastercmp)) == NULL) + if ((ftsp = fts_open(argv, fts_options, NULL)) == NULL) err(1, NULL); for (rval = 0; (curr = fts_read(ftsp)) != NULL;) { switch (curr->fts_info) { @@ -445,30 +444,4 @@ copy(char *argv[], enum op type, int fts err(1, "fts_read"); (void)fts_close(ftsp); return (rval); -} - -/* - * mastercmp -- - * The comparison function for the copy order. The order is to copy - * non-directory files before directory files. The reason for this - * is because files tend to be in the same cylinder group as their - * parent directory, whereas directories tend not to be. Copying the - * files first reduces seeking. - */ -int -mastercmp(const FTSENT **a, const FTSENT **b) -{ - int a_info, b_info; - - a_info = (*a)->fts_info; - if (a_info == FTS_ERR || a_info == FTS_NS || a_info == FTS_DNR) - return (0); - b_info = (*b)->fts_info; - if (b_info == FTS_ERR || b_info == FTS_NS || b_info == FTS_DNR) - return (0); - if (a_info == FTS_D) - return (-1); - if (b_info == FTS_D) - return (1); - return (0); }
