From owner-freebsd-hackers@FreeBSD.ORG Sat Feb 20 05:19:31 2010 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id D2EF7106566B for ; Sat, 20 Feb 2010 05:19:31 +0000 (UTC) (envelope-from kientzle@freebsd.org) Received: from monday.kientzle.com (kientzle.com [66.166.149.50]) by mx1.freebsd.org (Postfix) with ESMTP id 8DD5A8FC08 for ; Sat, 20 Feb 2010 05:19:31 +0000 (UTC) Received: (from root@localhost) by monday.kientzle.com (8.14.3/8.14.3) id o1K5JcXZ098490; Sat, 20 Feb 2010 05:19:38 GMT (envelope-from kientzle@freebsd.org) Received: from dark.x.kientzle.com (fw2.kientzle.com [10.123.1.2]) by kientzle.com with SMTP id 57cz5sipheprmhd9dxfqqwv7in; Sat, 20 Feb 2010 05:19:38 +0000 (UTC) (envelope-from kientzle@freebsd.org) Message-ID: <4B7F711E.6040402@freebsd.org> Date: Fri, 19 Feb 2010 21:20:30 -0800 From: Tim Kientzle User-Agent: Mozilla/5.0 (X11; U; FreeBSD i386; en-US; rv:1.8.1.21) Gecko/20090601 SeaMonkey/1.1.16 MIME-Version: 1.0 To: Juergen Lock References: <20100217215940.GA19713@triton8.kn-bremen.de> <4B7CE066.4030403@freebsd.org> <20100218183459.GA65508@triton8.kn-bremen.de> <7d6fde3d1002182212k3bdc866ckdc7c5f380b40f7d8@mail.gmail.com> <20100219181247.GA35702@triton8.kn-bremen.de> In-Reply-To: <20100219181247.GA35702@triton8.kn-bremen.de> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Garrett Cooper , freebsd-hackers@freebsd.org Subject: Re: "tar tfv /dev/cd0" speedup patch X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 20 Feb 2010 05:19:32 -0000 Juergen, I was looking at your Linux code here and thought the technique of trying lseek(SEEK_END) might work. Unfortunately, it doesn't: lseek(fd, 0, SEEK_END) gives zero for both /dev/sa0 (a tape drive) and /dev/cd0 (an optical drive). Are you sure it works on Linux? Tim P.S. Here's the program I used to test. Running it against regular files and various device nodes is quite illuminating. #include #include #include #include int main(int argc, char **argv) { int fd; if (argv[1] == NULL) { fprintf(stderr, "Need to specify a pathname.\n"); exit(1); } fd = open(argv[1], O_RDWR); printf("fd=%d\n", fd); printf("lseek(fd, 0, SEEK_SET)=%d\n", (int)lseek(fd, 0, SEEK_SET)); printf("lseek(fd, 10240, SEEK_SET)=%d\n", (int)lseek(fd, 10240, SEEK_SET)); printf("lseek(fd, 10240, SEEK_CUR)=%d\n", (int)lseek(fd, 10240, SEEK_CUR)); printf("lseek(fd, 0, SEEK_END)=%d\n", (int)lseek(fd, 0, SEEK_END)); printf("lseek(fd, 0, SEEK_SET)=%d\n", (int)lseek(fd, 0, SEEK_SET)); return (0); } Juergen Lock wrote: > On Thu, Feb 18, 2010 at 10:12:24PM -0800, Garrett Cooper wrote: >> On Thu, Feb 18, 2010 at 10:34 AM, Juergen Lock wrote: >>> On Wed, Feb 17, 2010 at 10:38:30PM -0800, Tim Kientzle wrote: >>>> Juergen Lock wrote: >>>>> ... since bsdtar/libarchive know iso9660 I just did the command in the >>>>> Subject. It worked, but it was sloow... :( Apparently it read all of >>>>> the disc without seeking. The following patch fixes this, is something >>>>> like this desired? If yes I could look how to do the same for Linux, >>>> Juergen, >>>> >>>> This is great! If you can figure out how to get this >>>> right, I would really appreciate it. If you have a >>>> tape drive handy, definitely test with that. My first >>>> attempts here actually broke reading from tape drives, >>>> which is why the current code is so conservative. >>>> >>> Hmm I can't test on a tape atm but if I look at the kernel, >>> http://fxr.watson.org/fxr/ident?i=DIOCGMEDIASIZE >>> DIOCGMEDIASIZE is only handled for geom, xen block devices, old CD drives >>> and pc98 floppies, and I'm pretty sure tapes don't use geom. :) >>> >>>> Minor style comments: >>>>> else if (S_ISCHR(st.st_mode) && >>>>> !ioctl(fd, DIOCGMEDIASIZE, &mediasize) && mediasize) { >>>> Please be explicit: S_ISCHR() && ioctl() == 0 && mediasize > 0 >>>> >>>>> archive_read_extract_set_skip_file(a, st.st_dev, st.st_ino); >>>> extract_skip_file isn't needed here; we don't read the >>>> contents of device nodes. >>>> >>>> Let me know as soon as you have something you're confident of. >>> Ok here is a new version of the patch with these things fixed and the >>> Linux case added: (Linux case not tested yet, and yes I did this on >>> stable/8.) >>> >>> Index: src/lib/libarchive/archive_read_open_filename.c >>> =================================================================== >>> RCS file: /home/scvs/src/lib/libarchive/archive_read_open_filename.c,v >>> retrieving revision 1.25.2.1 >>> diff -u -p -r1.25.2.1 archive_read_open_filename.c >>> --- src/lib/libarchive/archive_read_open_filename.c 3 Aug 2009 08:13:06 -0000 1.25.2.1 >>> +++ src/lib/libarchive/archive_read_open_filename.c 18 Feb 2010 18:14:16 -0000 >>> @@ -44,6 +44,10 @@ __FBSDID("$FreeBSD: src/lib/libarchive/a >>> #ifdef HAVE_UNISTD_H >>> #include >>> #endif >>> +#ifdef __FreeBSD__ >>> +#include >>> +#include >>> +#endif >>> >>> #include "archive.h" >>> >>> @@ -83,6 +87,9 @@ archive_read_open_filename(struct archiv >>> struct read_file_data *mine; >>> void *b; >>> int fd; >>> +#ifdef __FreeBSD__ >>> + off_t mediasize = 0; >>> +#endif >>> >>> archive_clear_error(a); >>> if (filename == NULL || filename[0] == '\0') { >>> @@ -143,6 +150,27 @@ archive_read_open_filename(struct archiv >>> */ >>> mine->can_skip = 1; >>> } >>> +#ifdef __FreeBSD__ >>> + /* >>> + * on FreeBSD if a device supports the DIOCGMEDIASIZE ioctl >>> + * it is a disk-like device and should be seekable. >>> + */ >>> + else if (S_ISCHR(st.st_mode) && >>> + ioctl(fd, DIOCGMEDIASIZE, &mediasize) == 0 && mediasize > 0) { >>> + mine->can_skip = 1; >>> + } >>> +#endif >>> +#ifdef __linux__ >> Is there any reason why a) you wouldn't check for other BSDs? b) you >> aren't doing something of the flavor... >> >> #if defined(__FreeBSD__) >> >> #elif defined(__linux__) >> >> #endif > > Okok, here's a new version of the patch that now also knows about > other BSDs and switched to #elif: (only NetBSD actually tested tho > since I had a VM lying around, more testing welcome!) > > (Also if anyone wants to switch this to autoconf-style HAVE_FOO > handling they are welcome too... :) > > Enjoy, > Juergen > > Index: src/lib/libarchive/archive_read_open_filename.c > =================================================================== > RCS file: /home/scvs/src/lib/libarchive/archive_read_open_filename.c,v > retrieving revision 1.25.2.1 > diff -u -p -u -p -r1.25.2.1 archive_read_open_filename.c > --- src/lib/libarchive/archive_read_open_filename.c 3 Aug 2009 08:13:06 -0000 1.25.2.1 > +++ src/lib/libarchive/archive_read_open_filename.c 19 Feb 2010 18:18:24 -0000 > @@ -44,6 +44,17 @@ __FBSDID("$FreeBSD: src/lib/libarchive/a > #ifdef HAVE_UNISTD_H > #include > #endif > +#if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) > +#include > +#include > +#elif defined(__NetBSD__) || defined(__OpenBSD__) > +#include > +#include > +#include > +#elif defined(__DragonFly__) > +#include > +#include > +#endif > > #include "archive.h" > > @@ -83,6 +94,13 @@ archive_read_open_filename(struct archiv > struct read_file_data *mine; > void *b; > int fd; > +#if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) > + off_t mediasize = 0; > +#elif defined(__NetBSD__) || defined(__OpenBSD__) > + struct disklabel dl; > +#elif defined(__DragonFly__) > + struct partinfo pi; > +#endif > > archive_clear_error(a); > if (filename == NULL || filename[0] == '\0') { > @@ -143,6 +161,45 @@ archive_read_open_filename(struct archiv > */ > mine->can_skip = 1; > } > +#if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) > + /* > + * on FreeBSD if a device supports the DIOCGMEDIASIZE ioctl > + * it is a disk-like device and should be seekable. > + */ > + else if (S_ISCHR(st.st_mode) && > + ioctl(fd, DIOCGMEDIASIZE, &mediasize) == 0 && mediasize > 0) { > + mine->can_skip = 1; > + } > +#elif defined(__NetBSD__) || defined(__OpenBSD__) > + /* > + * on Net/OpenBSD if a device supports the DIOCGDINFO ioctl > + * it is a disk-like device and should be seekable. > + */ > + else if ((S_ISCHR(st.st_mode) || S_ISBLK(st.st_mode)) && > + ioctl(fd, DIOCGDINFO, &dl) == 0 && > + dl.d_partitions[DISKPART(st.st_rdev)].p_size > 0) { > + mine->can_skip = 1; > + } > +#elif defined(__DragonFly__) > + /* > + * on DragonFly BSD if a device supports the DIOCGPART ioctl > + * it is a disk-like device and should be seekable. > + */ > + else if (S_ISCHR(st.st_mode) && > + ioctl(fd, DIOCGPART, &pi) == 0 && pi.media_size > 0) { > + mine->can_skip = 1; > + } > +#elif defined(__linux__) > + /* > + * on Linux just check whether its a block device and that > + * lseek works. (Tapes are character devices there.) > + */ > + else if (S_ISBLK(st.st_mode) && > + lseek(fd, 0, SEEK_CUR) == 0 && lseek(fd, 0, SEEK_SET) == 0 && > + lseek(fd, 0, SEEK_END) > 0 && lseek(fd, 0, SEEK_SET) == 0) { > + mine->can_skip = 1; > + } > +#endif > return (archive_read_open2(a, mine, > NULL, file_read, file_skip, file_close)); > } > >