From owner-freebsd-arch@FreeBSD.ORG Mon May 5 04:30:48 2003 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id D6FA237B401; Mon, 5 May 2003 04:30:48 -0700 (PDT) Received: from mailman.zeta.org.au (mailman.zeta.org.au [203.26.10.16]) by mx1.FreeBSD.org (Postfix) with ESMTP id 2946243F85; Mon, 5 May 2003 04:30:47 -0700 (PDT) (envelope-from bde@zeta.org.au) Received: from katana.zip.com.au (katana.zip.com.au [61.8.7.246]) by mailman.zeta.org.au (8.9.3p2/8.8.7) with ESMTP id VAA07088; Mon, 5 May 2003 21:30:41 +1000 Date: Mon, 5 May 2003 21:30:39 +1000 (EST) From: Bruce Evans X-X-Sender: bde@gamplex.bde.org To: Kirk McKusick In-Reply-To: <200305050617.h456HNTh017652@beastie.mckusick.com> Message-ID: <20030505205914.D8183@gamplex.bde.org> References: <200305050617.h456HNTh017652@beastie.mckusick.com> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII cc: arch@FreeBSD.org cc: Brian Buhrow cc: Jens Schweikhardt Subject: Re: Access times on executables (kern/25777) X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 05 May 2003 11:30:49 -0000 On Sun, 4 May 2003, Kirk McKusick wrote: > > From: Bruce Evans > > > > On Sun, 4 May 2003, Kirk McKusick wrote: > > > OK, so how about instead of use VOP_SETATTR, we just try to VOP_READ > > > one byte of data. It will run with the speed of read (and indeed since > > > we just mapped in the header above, the data should be in the cache). > > > It has the benefit of speed and not requiring the user to own the file. > > > It has the drawback of requiring that the file be readable though most > > > executables are set to be readable. > > > > It would work better than that. VOP_READ() works right here since the > > permissions checks are done at the vfs level at open(2) time, not on every > > VOP_READ() or read(2). > > > > I think VOP_SETATTR() should work similarly: do permissions checks at > > the vfs level using VOP_ACCESS() instead of in every file system, so > > that there is less duplicated code and the permissions checks don't > > get in the way when you don't want them. This change is now easy for > > at least setting times. In Lite2, ufs_setattr() had to dereference > It will work right on the local filesystem, but I think that it would > still be broken on NFS since NFS does permission checking on every RPC. I assume that "it" is VOP_ACCESS() here. Yes, we don't want to go near VOP_ACCESS() or the normal path of VOP_SETATTR(set_times) for remote file systems since the access checking would be very expensive. > Still, I think that VOP_READ is likely to be more efficient and work in > a higher percentage of the times (always on the local filesystem and > most of the time on NFS). Thus, I enclose my (new) proposed change below. > Index: sys/kern_exec.c > =================================================================== > RCS file: /usr/ncvs/src/sys/kern/kern_exec.c,v > retrieving revision 1.218 > diff -c -r1.218 kern_exec.c > *** kern_exec.c 1 Apr 2003 01:26:20 -0000 1.218 > --- kern_exec.c 5 May 2003 06:15:21 -0000 > *************** > *** 598,603 **** > --- 598,626 ---- > exec_setregs(td, imgp->entry_addr, > (u_long)(uintptr_t)stack_base, imgp->ps_strings); > > + /* > + * At this point, it counts as an access. > + * Ensure that atime gets updated if needed. > + */ > + if (!(textvp->v_mount->mnt_flag & MNT_NOATIME)) { Note that this check doesn't help for the most expensive case (nfs), since nfs doesn't implement the noatime mount flag so it never sets MNT_NOATIME. However, nfs doesn't actually implement setting of atimes either -- if the read goes to the server, then the server sets the atime (unless the file system on the server is mounted -noatime) and the change is eventually seen by the clent, but most reads on the client are from the buffer cache so they don't affect the atime on the server unless we tell it, and we never tell it directly. This seems to be required for reasonable efficiency. > + struct uio auio; > + struct iovec aiov; > + char ch; > + > + auio.uio_iov = &aiov; > + auio.uio_iovcnt = 1; > + aiov.iov_base = &ch; > + aiov.iov_len = 1; > + auio.uio_resid = 1; > + auio.uio_offset = 0; > + auio.uio_segflg = UIO_SYSSPACE; > + auio.uio_rw = UIO_READ; > + auio.uio_td = td; > + vn_lock(textvp, LK_EXCLUSIVE | LK_RETRY, td); > + (void) VOP_READ(textvp, &auio, 0, p->p_ucred); In the nfs clase, this read is very unlikely to do anything, because we read the header earlier so this read is very likely to be from the buffer cache. Then the previous read may have set the atime, but this one won't. > + VOP_UNLOCK(textvp, 0, td); > + } > + > done1: > /* > * Free any resources malloc'd earlier that we didn't use. > I think this version would be OK if it skipped remote file systems explicitly. Bruce