From owner-svn-src-all@FreeBSD.ORG Sun Mar 29 18:42:44 2015 Return-Path: Delivered-To: svn-src-all@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 56AC488A; Sun, 29 Mar 2015 18:42:44 +0000 (UTC) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id D2552802; Sun, 29 Mar 2015 18:42:43 +0000 (UTC) Received: from tom.home (kostik@localhost [127.0.0.1]) by kib.kiev.ua (8.14.9/8.14.9) with ESMTP id t2TIgcIo036561 (version=TLSv1/SSLv3 cipher=DHE-RSA-CAMELLIA256-SHA bits=256 verify=NO); Sun, 29 Mar 2015 21:42:38 +0300 (EEST) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.9.2 kib.kiev.ua t2TIgcIo036561 Received: (from kostik@localhost) by tom.home (8.14.9/8.14.9/Submit) id t2TIgcmZ036560; Sun, 29 Mar 2015 21:42:38 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Sun, 29 Mar 2015 21:42:38 +0300 From: Konstantin Belousov To: Jilles Tjoelker Subject: Re: svn commit: r280308 - head/sys/fs/devfs Message-ID: <20150329184238.GB2379@kib.kiev.ua> References: <20150322162507.GD2379@kib.kiev.ua> <201503221825.t2MIP7jv096531@gw.catspoiler.org> <20150329175137.GD95224@stack.nl> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150329175137.GD95224@stack.nl> User-Agent: Mutt/1.5.23 (2014-03-12) X-Spam-Status: No, score=-2.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FREEMAIL_FROM,NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=3.4.0 X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on tom.home Cc: src-committers@FreeBSD.org, Don Lewis , delphij@FreeBSD.org, brde@optusnet.com.au, svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 29 Mar 2015 18:42:44 -0000 On Sun, Mar 29, 2015 at 07:51:37PM +0200, Jilles Tjoelker wrote: > On Sun, Mar 22, 2015 at 11:25:07AM -0700, Don Lewis wrote: > > It's not totally worthless. I think the mtime on tty devices is used to > > calculate the idle time that is printed by the w command. We just don't > > need nanosecond accuracy for that. > > Hmm. The idle time on tty devices breaking is a clear POLA violation, > but I'm not sure what's the best way to fix it. By the way, w uses atime > and who -u uses mtime. > > Some options are: > > * Bypass vfs_timestamp() and hard-code second accuracy in devfs; > futimens() will still set timestamps to the nanosecond, even with > UTIME_NOW. A timestamp update after UTIME_NOW setting may set back the > timestamp to an older value. > > * Make vfs.timestamp_precision filesystem-specific. Since this does not > affect futimens() with explicit timestamps, it will not cause strange > situations with cp -p. > > * Restrict file times on devfs to seconds. > > * Somehow add a special case for TTY devices so they will always keep > timestamps. > > * Somehow add a special case for "performance-critical" devices so they > will not keep timestamps. > > * Change the default for vfs.timestamp_precision to 1, which still > provides non-zero nanoseconds (so much of the same bugs) but is not so > slow. The file server people won't like this though. > > My proposal for delayed updates as in UFS clearly does not work for TTY > idle times, so there is no point in that. > Interesting complication with the devfs timestamp update is that devfs_read_f() and devfs_write_f() do not lock the vnode. So whatever update method is used, stat(2) on devfs might return inconsistent value, since tv_src/tv_nsec cannot be updated or read by single op, without locking. That said, we could either: - ignore the race. Then the patch below might be good enough. - Trim the precision of futimens() for devfs to always set tv_nsec to 0. Then, the race can be handled satisfactory with atomics. - Add a lock (IMO this does not make sense). diff --git a/sys/fs/devfs/devfs_vnops.c b/sys/fs/devfs/devfs_vnops.c index 941f92c..e199d52 100644 --- a/sys/fs/devfs/devfs_vnops.c +++ b/sys/fs/devfs/devfs_vnops.c @@ -83,8 +83,24 @@ MTX_SYSINIT(cdevpriv_mtx, &cdevpriv_mtx, "cdevpriv lock", MTX_DEF); SYSCTL_DECL(_vfs_devfs); static int devfs_dotimes; -SYSCTL_INT(_vfs_devfs, OID_AUTO, dotimes, CTLFLAG_RW, - &devfs_dotimes, 0, "Update timestamps on DEVFS"); +SYSCTL_INT(_vfs_devfs, OID_AUTO, dotimes, CTLFLAG_RW, &devfs_dotimes, 0, + "Update timestamps on DEVFS with default precision"); + +static void +devfs_timestamp(struct timespec *tsp) +{ + time_t ts; + + if (devfs_dotimes) { + vfs_timestamp(tsp); + } else { + ts = time_second; + if (tsp->tv_sec < ts) { + tsp->tv_sec = ts; + tsp->tv_nsec = 0; + } + } +} static int devfs_fp_check(struct file *fp, struct cdev **devp, struct cdevsw **dswp, @@ -1228,9 +1244,8 @@ devfs_read_f(struct file *fp, struct uio *uio, struct ucred *cred, foffset_lock_uio(fp, uio, flags | FOF_NOLOCK); error = dsw->d_read(dev, uio, ioflag); - if (devfs_dotimes && - (uio->uio_resid != resid || (error == 0 && resid != 0))) - vfs_timestamp(&dev->si_atime); + if (uio->uio_resid != resid || (error == 0 && resid != 0)) + devfs_timestamp(&dev->si_atime); td->td_fpop = fpop; dev_relthread(dev, ref); @@ -1708,9 +1723,8 @@ devfs_write_f(struct file *fp, struct uio *uio, struct ucred *cred, resid = uio->uio_resid; error = dsw->d_write(dev, uio, ioflag); - if (devfs_dotimes && - (uio->uio_resid != resid || (error == 0 && resid != 0))) { - vfs_timestamp(&dev->si_ctime); + if (uio->uio_resid != resid || (error == 0 && resid != 0)) { + devfs_timestamp(&dev->si_ctime); dev->si_mtime = dev->si_ctime; } td->td_fpop = fpop;