Re: cp(1) and mmap

Andriy Gapon wrote:
> on 28/05/2008 12:02 Harti Brandt said the following:
>> Hi all,
>>
>> it looks like there is no fallback in cp(1) when mmaping the source file
>> fails. I'm mounting SMB shares via smbnetfs (which in turn uses fuse)
>> and it seems not to support mmaping files. Shouldn't cp just fallback to
>> a normal read()/write() loop in this case?
> 
> I would think that it should.
> This topic was brought up several times, but no resolution so far.
> I think that I've even seen patches.
> 

I've not seen the patches, but the fix is trivial (see the attached 
patch).  If there are no objections, I can commit it.

I also think we should use mmap for larger files, mmapping and writing 
them out in several chunks.

Diomidis - dds_at_

Index: utils.c
===================================================================
RCS file: /home/ncvs/src/bin/cp/utils.c,v
retrieving revision 1.53
diff -u -r1.53 utils.c
--- utils.c	10 Mar 2008 19:58:41 -0000	1.53
+++ utils.c	29 May 2008 20:39:38 -0000
_at__at_ -137,41 +137,39 _at__at_
 		 * Mmap and write if less than 8M (the limit is so we don't totally
 		 * trash memory on big files.  This is really a minor hack, but it
 		 * wins some CPU back.
+		 * Some filesystems, such as smbnetfs, don't support mmap,
+		 * so this is a best-effort attempt.
 		 */
 #ifdef VM_AND_BUFFER_CACHE_SYNCHRONIZED
 		if (S_ISREG(fs->st_mode) && fs->st_size > 0 &&
-	    	fs->st_size <= 8 * 1048576) {
-			if ((p = mmap(NULL, (size_t)fs->st_size, PROT_READ,
-		    	MAP_SHARED, from_fd, (off_t)0)) == MAP_FAILED) {
+	    	    fs->st_size <= 8 * 1048576 &&
+		    (p = mmap(NULL, (size_t)fs->st_size, PROT_READ,
+		    	MAP_SHARED, from_fd, (off_t)0)) != MAP_FAILED) {
+			wtotal = 0;
+			for (bufp = p, wresid = fs->st_size; ;
+			bufp += wcount, wresid -= (size_t)wcount) {
+				wcount = write(to_fd, bufp, wresid);
+				if (wcount <= 0)
+					break;
+				wtotal += wcount;
+				if (info) {
+					info = 0;
+					(void)fprintf(stderr,
+					    "%s -> %s %3d%%\n",
+					    entp->fts_path, to.p_path,
+					    cp_pct(wtotal, fs->st_size));
+				}
+				if (wcount >= (ssize_t)wresid)
+					break;
+			}
+			if (wcount != (ssize_t)wresid) {
+				warn("%s", to.p_path);
+				rval = 1;
+			}
+			/* Some systems don't unmap on close(2). */
+			if (munmap(p, fs->st_size) < 0) {
 				warn("%s", entp->fts_path);
 				rval = 1;
-			} else {
-				wtotal = 0;
-				for (bufp = p, wresid = fs->st_size; ;
-			    	bufp += wcount, wresid -= (size_t)wcount) {
-					wcount = write(to_fd, bufp, wresid);
-					if (wcount <= 0)
-						break;
-					wtotal += wcount;
-					if (info) {
-						info = 0;
-						(void)fprintf(stderr,
-						    "%s -> %s %3d%%\n",
-						    entp->fts_path, to.p_path,
-						    cp_pct(wtotal, fs->st_size));
-					}
-					if (wcount >= (ssize_t)wresid)
-						break;
-				}
-				if (wcount != (ssize_t)wresid) {
-					warn("%s", to.p_path);
-					rval = 1;
-				}
-				/* Some systems don't unmap on close(2). */
-				if (munmap(p, fs->st_size) < 0) {
-					warn("%s", entp->fts_path);
-					rval = 1;
-				}
 			}
 		} else
 #endif