From owner-freebsd-hackers@FreeBSD.ORG Fri Feb 19 06:12:25 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 67964106568F for ; Fri, 19 Feb 2010 06:12:25 +0000 (UTC) (envelope-from yanefbsd@gmail.com) Received: from mail-px0-f180.google.com (mail-px0-f180.google.com [209.85.216.180]) by mx1.freebsd.org (Postfix) with ESMTP id 336588FC19 for ; Fri, 19 Feb 2010 06:12:24 +0000 (UTC) Received: by pxi10 with SMTP id 10so2424432pxi.13 for ; Thu, 18 Feb 2010 22:12:24 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:in-reply-to:references :date:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=40MEnjACSLW6p933sbyvyR5qXy64OgSeRwHzgaq7Mj4=; b=JXLjI/YoMdbvjN8cp/wivNwyd1A3x70i4ET4K407lNfARY7fHIkkaLCktNN5kWz9L2 ecJ3ocPztbG8IxM8bKo024PXBm0o2YB2zK6YgdaLD9BDb+VsB4yEBGFeK1sASWN/1/U0 naFr8m9hgKt0o0R6LRwWgfS+x8+f1F5HusCYE= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; b=vrEpu2Z5D2ZRqPkn011yN0CEbfqPmyoJEjNX4SOPrT0hvJsNUO79OlXob7vH8wMzWP jVRMQOZz3KxWQW6ix3GuiVj6gA9Tz0iDeNj/5C3Z2eJgsX0whSsEsJcY2RNKJG5LzIWY fyp9x3Qi4H5HZKPTO9bY5xOWuHm1Or/92tUXw= MIME-Version: 1.0 Received: by 10.143.21.18 with SMTP id y18mr3557668wfi.58.1266559944549; Thu, 18 Feb 2010 22:12:24 -0800 (PST) In-Reply-To: <20100218183459.GA65508@triton8.kn-bremen.de> References: <20100217215940.GA19713@triton8.kn-bremen.de> <4B7CE066.4030403@freebsd.org> <20100218183459.GA65508@triton8.kn-bremen.de> Date: Thu, 18 Feb 2010 22:12:24 -0800 Message-ID: <7d6fde3d1002182212k3bdc866ckdc7c5f380b40f7d8@mail.gmail.com> From: Garrett Cooper To: Juergen Lock Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: freebsd-hackers@freebsd.org, Tim Kientzle 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: Fri, 19 Feb 2010 06:12:25 -0000 On Thu, Feb 18, 2010 at 10:34 AM, Juergen Lock wro= te: > On Wed, Feb 17, 2010 at 10:38:30PM -0800, Tim Kientzle wrote: >> Juergen Lock wrote: >> > >> > =A0... =A0since bsdtar/libarchive know iso9660 I just did the command = in the >> > Subject. =A0It worked, but it was sloow... :( =A0Apparently it read al= l of >> > the disc without seeking. =A0The following patch fixes this, is someth= ing >> > like this desired? =A0If yes I could look how to do the same for Linux= , >> >> Juergen, >> >> This is great! =A0If you can figure out how to get this >> right, I would really appreciate it. =A0If you have a >> tape drive handy, definitely test with that. =A0My 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, > =A0 =A0 =A0 =A0http://fxr.watson.org/fxr/ident?i=3DDIOCGMEDIASIZE > 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: >> > =A0else if (S_ISCHR(st.st_mode) && >> > =A0 =A0 !ioctl(fd, DIOCGMEDIASIZE, &mediasize) && mediasize) { >> >> Please be explicit: =A0S_ISCHR() && ioctl() =3D=3D 0 =A0&& mediasize > 0 >> >> > =A0 =A0 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. > > =A0Ok here is a new version of the patch with these things fixed and the > Linux case added: =A0(Linux case not tested yet, and yes I did this on > stable/8.) > > Index: src/lib/libarchive/archive_read_open_filename.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > 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 =A0 =A0 3 Aug 2009 08= :13:06 -0000 =A0 =A0 =A0 1.25.2.1 > +++ src/lib/libarchive/archive_read_open_filename.c =A0 =A0 18 Feb 2010 1= 8:14:16 -0000 > @@ -44,6 +44,10 @@ __FBSDID("$FreeBSD: src/lib/libarchive/a > =A0#ifdef HAVE_UNISTD_H > =A0#include > =A0#endif > +#ifdef __FreeBSD__ > +#include > +#include > +#endif > > =A0#include "archive.h" > > @@ -83,6 +87,9 @@ archive_read_open_filename(struct archiv > =A0 =A0 =A0 =A0struct read_file_data *mine; > =A0 =A0 =A0 =A0void *b; > =A0 =A0 =A0 =A0int fd; > +#ifdef __FreeBSD__ > + =A0 =A0 =A0 off_t mediasize =3D 0; > +#endif > > =A0 =A0 =A0 =A0archive_clear_error(a); > =A0 =A0 =A0 =A0if (filename =3D=3D NULL || filename[0] =3D=3D '\0') { > @@ -143,6 +150,27 @@ archive_read_open_filename(struct archiv > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 */ > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0mine->can_skip =3D 1; > =A0 =A0 =A0 =A0} > +#ifdef __FreeBSD__ > + =A0 =A0 =A0 /* > + =A0 =A0 =A0 =A0* on FreeBSD if a device supports the DIOCGMEDIASIZE ioc= tl > + =A0 =A0 =A0 =A0* it is a disk-like device and should be seekable. > + =A0 =A0 =A0 =A0*/ > + =A0 =A0 =A0 else if (S_ISCHR(st.st_mode) && > + =A0 =A0 =A0 =A0 =A0 ioctl(fd, DIOCGMEDIASIZE, &mediasize) =3D=3D 0 && m= ediasize > 0) { > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mine->can_skip =3D 1; > + =A0 =A0 =A0 } > +#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 > + =A0 =A0 =A0 /* > + =A0 =A0 =A0 =A0* on Linux just check whether its a block device and tha= t > + =A0 =A0 =A0 =A0* lseek works. =A0(Tapes are character devices there.) > + =A0 =A0 =A0 =A0*/ > + =A0 =A0 =A0 else if (S_ISBLK(st.st_mode) && > + =A0 =A0 =A0 =A0 =A0 lseek(fd, 0, SEEK_CUR) =3D=3D 0 && lseek(fd, 0, SEE= K_SET) =3D=3D 0 && > + =A0 =A0 =A0 =A0 =A0 lseek(fd, 0, SEEK_END) > 0 && lseek(fd, 0, SEEK_SET= ) =3D=3D 0) { > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mine->can_skip =3D 1; > + =A0 =A0 =A0 =A0} > +#endif > =A0 =A0 =A0 =A0return (archive_read_open2(a, mine, > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0NULL, file_read, file_skip, file_close)); > =A0} Cheers! -Garrett