From owner-freebsd-arch@FreeBSD.ORG Sun Mar 3 13:02:55 2013 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 583D6980 for ; Sun, 3 Mar 2013 13:02:55 +0000 (UTC) (envelope-from christoph.mallon@gmx.de) Received: from mout.gmx.net (mout.gmx.net [212.227.15.19]) by mx1.freebsd.org (Postfix) with ESMTP id E83116A5 for ; Sun, 3 Mar 2013 13:02:54 +0000 (UTC) Received: from mailout-de.gmx.net ([10.1.76.31]) by mrigmx.server.lan (mrigmx001) with ESMTP (Nemesis) id 0LjP2f-1UnwYR0gn9-00dTHf for ; Sun, 03 Mar 2013 14:02:48 +0100 Received: (qmail invoked by alias); 03 Mar 2013 13:02:48 -0000 Received: from p5B131D04.dip.t-dialin.net (EHLO rotluchs.lokal) [91.19.29.4] by mail.gmx.net (mp031) with SMTP; 03 Mar 2013 14:02:48 +0100 X-Authenticated: #1673122 X-Provags-ID: V01U2FsdGVkX1+xUA0TN5Sx2Juq/hdValqeutidDUHtDmcOKVkig+ d8Yw6vnSMh/gcx Message-ID: <513349F6.4010309@gmx.de> Date: Sun, 03 Mar 2013 14:02:46 +0100 From: Christoph Mallon User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:17.0) Gecko/20130222 Thunderbird/17.0.3 MIME-Version: 1.0 To: freebsd-arch@freebsd.org Subject: Re: Proposal: Unify printing the function name in panic messages() References: <51141E33.4080103@gmx.de> In-Reply-To: <51141E33.4080103@gmx.de> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Y-GMX-Trusted: 0 Cc: Kirk McKusick X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 03 Mar 2013 13:02:55 -0000 Hi, here's an updated patch series, which replaces all panic() calls, except for the places, which have their own stuff, like sys/boot/: http://tron.homeunix.org/zeug/FreeBSD/panic/ For now, PANIC() prints the function name. This is the most common location information used by panic() users. It can easily be changed in sys/systm.h, where the macro PANIC() is defined. I added the option PANIC_LOCATION. I would like to enable it by default, so it has to be deactivated explicitly. How can this be achieved? Please test this stuff. Christoph From owner-freebsd-arch@FreeBSD.ORG Mon Mar 4 21:55:21 2013 Return-Path: Delivered-To: arch@freebsd.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id A8989DC7 for ; Mon, 4 Mar 2013 21:55:21 +0000 (UTC) (envelope-from simon@qxnitro.org) Received: from mail-oa0-f42.google.com (mail-oa0-f42.google.com [209.85.219.42]) by mx1.freebsd.org (Postfix) with ESMTP id 742CD1788 for ; Mon, 4 Mar 2013 21:55:21 +0000 (UTC) Received: by mail-oa0-f42.google.com with SMTP id i18so9960142oag.15 for ; Mon, 04 Mar 2013 13:55:20 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=qxnitro.org; s=google; h=mime-version:x-received:x-originating-ip:in-reply-to:references :date:message-id:subject:from:to:cc:content-type; bh=/kfpx5oomg5t5OMQBFBydR6s7cqlA9czhIAY6m9BYnQ=; b=DpaPKUVh2FMxunVUKqmOD02EGAEVlAFitPhp58jtR5Jp+Mq8lZFz6/ivsMYQAl+Ai8 Gmai39iTYfdIRAWJnM6seeGtK11qoWYDkWeMC8KLpDC53vMHzwo442iLCXBP8ETHu6Of lrZIq3ZuWSfHzbfzMlIXs9FIvvFa5fj5sRv98= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=mime-version:x-received:x-originating-ip:in-reply-to:references :date:message-id:subject:from:to:cc:content-type:x-gm-message-state; bh=/kfpx5oomg5t5OMQBFBydR6s7cqlA9czhIAY6m9BYnQ=; b=acKjxksesW1SjmamqENxGk5vzcVLAGyqUr3K+oCmWcCLbsoHIoLAqQCC1nnUvDQZt8 ZG6nUudF4OCrx4g9HqxPPcsno2nvi9ski/z14zeQDqykn4Ay7yQoLiCxpGSH3l3JmdE5 81z0OXiCEXWPU0XbUZ9zv2GYRT5wyT4NR3IX0qsBkHAuFIuiwR2rvoO9egs0Bvkrj+av nyuHhltrQgJx0GfMyVXc4pYK7U02h/QuExeTXmrgA8c85gn7L1O3t3OP05BejR3a+8or p7Ul5ARYpq4oOoiCY2PRnT7GJvQQAPpUBlisGrrv2G7yH4+8tqf5LbcQy2uFWtWCfrOr ArRg== MIME-Version: 1.0 X-Received: by 10.60.12.170 with SMTP id z10mr17444058oeb.122.1362434120579; Mon, 04 Mar 2013 13:55:20 -0800 (PST) Received: by 10.76.99.113 with HTTP; Mon, 4 Mar 2013 13:55:20 -0800 (PST) X-Originating-IP: [2.138.23.213] Received: by 10.76.99.113 with HTTP; Mon, 4 Mar 2013 13:55:20 -0800 (PST) In-Reply-To: <20130302165239.GB27078@ithaqua.etoilebsd.net> References: <20130302165239.GB27078@ithaqua.etoilebsd.net> Date: Mon, 4 Mar 2013 21:55:20 +0000 Message-ID: Subject: Re: Import libyaml into base From: "Simon L. B. Nielsen" To: Baptiste Daroussin X-Gm-Message-State: ALoCoQkUP7dT5EV2PjeqrwcirLU6X6Uc263vGBPi148/R2eXa+Cilb2MzBTfdZJwQgxSzZiTZbvx Content-Type: text/plain; charset=UTF-8 X-Content-Filtered-By: Mailman/MimeDel 2.1.14 Cc: arch@freebsd.org, current@freebsd.org X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 04 Mar 2013 21:55:21 -0000 On 2 Mar 2013 16:52, "Baptiste Daroussin" wrote: > I want to import libyaml into base as libbsdyml so that no ports will use it > like we do for expat. > > I need it for the pkg bootstrap, so it it can parse pkg.conf. > > I know that some of the bhyve people will also be glad to use it. > > libyaml is MIT licensed, it is stable: no abi/api revolution in it for a while. > > Does anyone have an objection? I don't but please add a stub manual page telling people not to use it outside base. Simon From owner-freebsd-arch@FreeBSD.ORG Mon Mar 4 22:03:49 2013 Return-Path: Delivered-To: arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id C46DDFDE; Mon, 4 Mar 2013 22:03:49 +0000 (UTC) (envelope-from baptiste.daroussin@gmail.com) Received: from mail-ea0-x236.google.com (mail-ea0-x236.google.com [IPv6:2a00:1450:4013:c01::236]) by mx1.freebsd.org (Postfix) with ESMTP id 3C10717D9; Mon, 4 Mar 2013 22:03:49 +0000 (UTC) Received: by mail-ea0-f182.google.com with SMTP id a12so983338eaa.13 for ; Mon, 04 Mar 2013 14:03:48 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=x-received:sender:date:from:to:cc:subject:message-id:references :mime-version:content-type:content-disposition:in-reply-to :user-agent; bh=/tdOunfV3I71bBqtEY1WuzYyif73mZjcrE0FmqUIZ4s=; b=zX4MHB4HZwOEzXMmU4ecIxCzgn7yump3dU0LYsdDCn6mPQsmmoiIX5Dy4y2z/rRz46 WMhGc6zDSqGfeZ0STeAjfD7KksQSUSioZFLGtENBhH8g7GtEel8XY3ixOQ49opMRiJ+3 i+w+httmi0t44P9uJj7MtU2F1/R9TUgPPAj4fX0QbboMcaJF7EDzphhgAeCA+Nw8/oEY Po/g6Djs1z7M1rtHBNO1U+2PG2yMSFbcwzlfG5RafRj0xIt4sW0q9/Pdy8qZuPbRiiza 9yyg7KX8sF0v6hQS3IwPboQld3kaJzu2y6nFst1vTERCVMQ/o8qn019cTDStnX6c4Wpq dM9Q== X-Received: by 10.14.193.134 with SMTP id k6mr63533221een.37.1362434628216; Mon, 04 Mar 2013 14:03:48 -0800 (PST) Received: from ithaqua.etoilebsd.net (ithaqua.etoilebsd.net. [37.59.37.188]) by mx.google.com with ESMTPS id 3sm34185380eej.6.2013.03.04.14.03.46 (version=TLSv1 cipher=RC4-SHA bits=128/128); Mon, 04 Mar 2013 14:03:46 -0800 (PST) Sender: Baptiste Daroussin Date: Mon, 4 Mar 2013 23:03:44 +0100 From: Baptiste Daroussin To: "Simon L. B. Nielsen" Subject: Re: Import libyaml into base Message-ID: <20130304220344.GN64570@ithaqua.etoilebsd.net> References: <20130302165239.GB27078@ithaqua.etoilebsd.net> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="FX+Db2fp7WJhXKrW" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Cc: arch@freebsd.org, current@freebsd.org X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 04 Mar 2013 22:03:49 -0000 --FX+Db2fp7WJhXKrW Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Mar 04, 2013 at 09:55:20PM +0000, Simon L. B. Nielsen wrote: > On 2 Mar 2013 16:52, "Baptiste Daroussin" wrote: > > I want to import libyaml into base as libbsdyml so that no ports will u= se > it > > like we do for expat. > > > > I need it for the pkg bootstrap, so it it can parse pkg.conf. > > > > I know that some of the bhyve people will also be glad to use it. > > > > libyaml is MIT licensed, it is stable: no abi/api revolution in it for a > while. > > > > Does anyone have an objection? >=20 > I don't but please add a stub manual page telling people not to use it > outside base. >=20 > Simon Sure I was planning to losely do the same as for libexpat. The only user ou= tside base will be pkgng, but it is a bit special ;) regards, Bapt --FX+Db2fp7WJhXKrW Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.19 (FreeBSD) iEYEARECAAYFAlE1GkAACgkQ8kTtMUmk6EwXlACgvDODc6SxB9YUzFGshiqOO+Bl 8OcAn0r2HCKPUWkNj/xrbz7YyHfPcqPL =eGQp -----END PGP SIGNATURE----- --FX+Db2fp7WJhXKrW-- From owner-freebsd-arch@FreeBSD.ORG Tue Mar 5 00:56:17 2013 Return-Path: Delivered-To: arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id B4B25452 for ; Tue, 5 Mar 2013 00:56:17 +0000 (UTC) (envelope-from kabaev@gmail.com) Received: from mail-qe0-f51.google.com (mail-qe0-f51.google.com [209.85.128.51]) by mx1.freebsd.org (Postfix) with ESMTP id 519CB6B9 for ; Tue, 5 Mar 2013 00:56:16 +0000 (UTC) Received: by mail-qe0-f51.google.com with SMTP id nd7so4330448qeb.38 for ; Mon, 04 Mar 2013 16:56:16 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=x-received:date:from:to:subject:message-id:in-reply-to:references :x-mailer:mime-version:content-type; bh=xhm9d/I4glFIAY9uKcqVBx5j2QnP/8rppx/xmhu6OhM=; b=Ogaz+hrc6Y6zGY4glGtqovy43g5TYrHXtCg5IgL75rFAkuIWILX/1GfK1KFmpKTZIa nFidSgHYAaQowxQ+u/AzRnhaEZOHB6ixKYhEHwpZB/X5t9X0LKnSYCz0z7WYmwXPuYRG 0vAD5ZwLVFnUZXWAyjHdF3xFZK9Fqxrbe1rBGiPIbfvRfmD5fMEKXfE2yVznMk97XNCe ZahKlbCx6fGaUlSz476Rsgj3cYczAcLOiZF0/T32xcdaRvOsNKKStzZ6HgXWqrOhJoFJ DBZkUxxgi0k7/BYCsLJCEv8UN7MZnAFAB6iCFKoauoHLkkaB9dTZEtYW1usoDiyOFaTZ zxbQ== X-Received: by 10.229.77.24 with SMTP id e24mr7875177qck.50.1362444976334; Mon, 04 Mar 2013 16:56:16 -0800 (PST) Received: from kan.dyndns.org (c-24-63-226-98.hsd1.ma.comcast.net. [24.63.226.98]) by mx.google.com with ESMTPS id hr1sm39627564qeb.3.2013.03.04.16.56.14 (version=SSLv3 cipher=RC4-SHA bits=128/128); Mon, 04 Mar 2013 16:56:15 -0800 (PST) Date: Mon, 4 Mar 2013 19:56:07 -0500 From: Alexander Kabaev To: arch@freebsd.org Subject: Re: Import libyaml into base Message-ID: <20130304195607.4823f548@kan.dyndns.org> In-Reply-To: References: <20130302165239.GB27078@ithaqua.etoilebsd.net> X-Mailer: Claws Mail 3.9.0 (GTK+ 2.24.6; amd64-portbld-freebsd10.0) Mime-Version: 1.0 Content-Type: multipart/signed; micalg=PGP-SHA1; boundary="Sig_/V_DM/a=cpF+xesvF/QyV_PN"; protocol="application/pgp-signature" X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 05 Mar 2013 00:56:17 -0000 --Sig_/V_DM/a=cpF+xesvF/QyV_PN Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable On Mon, 4 Mar 2013 21:55:20 +0000 "Simon L. B. Nielsen" wrote: > On 2 Mar 2013 16:52, "Baptiste Daroussin" wrote: > > I want to import libyaml into base as libbsdyml so that no ports > > will use > it > > like we do for expat. > > > > I need it for the pkg bootstrap, so it it can parse pkg.conf. > > > > I know that some of the bhyve people will also be glad to use it. > > > > libyaml is MIT licensed, it is stable: no abi/api revolution in it > > for a > while. > > > > Does anyone have an objection? >=20 > I don't but please add a stub manual page telling people not to use it > outside base. >=20 > Simon We dealt with this in the past by renaming the library that goes into the base to be something else than its upstream variant, witness libbsdxml. Should this be done to libyaml-in-base as well? --=20 Alexander Kabaev --Sig_/V_DM/a=cpF+xesvF/QyV_PN Content-Type: application/pgp-signature; name=signature.asc Content-Disposition: attachment; filename=signature.asc -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.19 (FreeBSD) iD8DBQFRNUKuQ6z1jMm+XZYRAjE8AKCkBelkYs/1LqovV7UfnoUUqvhq+wCgyV0k Dg2hpbIRpUKqA+TW26JKfCU= =cE4X -----END PGP SIGNATURE----- --Sig_/V_DM/a=cpF+xesvF/QyV_PN-- From owner-freebsd-arch@FreeBSD.ORG Tue Mar 5 02:17:14 2013 Return-Path: Delivered-To: arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 3710CE19 for ; Tue, 5 Mar 2013 02:17:14 +0000 (UTC) (envelope-from lists@eitanadler.com) Received: from mail-wg0-f52.google.com (mail-wg0-f52.google.com [74.125.82.52]) by mx1.freebsd.org (Postfix) with ESMTP id CAA1E8C4 for ; Tue, 5 Mar 2013 02:17:13 +0000 (UTC) Received: by mail-wg0-f52.google.com with SMTP id 12so4807850wgh.31 for ; Mon, 04 Mar 2013 18:17:06 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=eitanadler.com; s=0xdeadbeef; h=x-received:mime-version:in-reply-to:references:from:date:message-id :subject:to:cc:content-type; bh=Rme2MNBrQqUKdewyC2g7HGgNi+Kmqi1esoYPX1XRSNM=; b=QpuWKUX5SnIg9LpsOJKq3GZQL+oMrK9dIqTVVnCbAmV7led0rnpZWqhAMef5T78+sS ssw1Zh10HlCJSI/XbEILEGmhf0QcBiCSuDNQyFoRRfLhv2YvLv/UHmdlWaFEIC1B54px slecaONQ6cIpOzAvy4nLDthrJmHyOAbTJqOH4= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-received:mime-version:in-reply-to:references:from:date:message-id :subject:to:cc:content-type:x-gm-message-state; bh=Rme2MNBrQqUKdewyC2g7HGgNi+Kmqi1esoYPX1XRSNM=; b=IDnKv0CkGgTxkjnYqxuHUUbkooqgybG+4fyZPhxtPL2+7Cz/clgdIqP58axtPW8S3O Oze5yE4a5WLcelKyJPUuwFcWLddruTZugq2YzNT0tSdNDxMXyf43imEmGcazhWjak5e5 uRBFIwZK/1E+9K694HuJTOF6kCmphqd95dXrazlhb4pwCuyD5MwI6PZXLqfN37mLshhj NXIcQiLICKviSzVt4hfofkF+/UbTdXYz8PBH+RaRg/lTtJ7ymNOzHJRC8KNTOxSZ3YG8 nuPvaSbLx9beabu9pVz4Dr/KCJGBitFDAWYJi+o4zLDYd/Y126Tfmz46AJjF/72GSaFr 3wjw== X-Received: by 10.194.119.200 with SMTP id kw8mr35749698wjb.31.1362449826891; Mon, 04 Mar 2013 18:17:06 -0800 (PST) MIME-Version: 1.0 Received: by 10.194.44.130 with HTTP; Mon, 4 Mar 2013 18:16:36 -0800 (PST) In-Reply-To: <20130304195607.4823f548@kan.dyndns.org> References: <20130302165239.GB27078@ithaqua.etoilebsd.net> <20130304195607.4823f548@kan.dyndns.org> From: Eitan Adler Date: Mon, 4 Mar 2013 21:16:36 -0500 Message-ID: Subject: Re: Import libyaml into base To: Alexander Kabaev Content-Type: text/plain; charset=UTF-8 X-Gm-Message-State: ALoCoQniGFLi9AFYqWrTlVIsn/W3YssvxSaFbdPCNPU/fMXgkTt3RrXyaeeYE4P9QTuWI4IzSrTX Cc: arch@freebsd.org X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 05 Mar 2013 02:17:14 -0000 On 4 March 2013 19:56, Alexander Kabaev wrote: > We dealt with this in the past by renaming the library that goes into > the base to be something else than its upstream variant, witness > libbsdxml. Should this be done to libyaml-in-base as well? This was done as "libbsdyml" -- Eitan Adler From owner-freebsd-arch@FreeBSD.ORG Tue Mar 5 08:00:58 2013 Return-Path: Delivered-To: arch@freebsd.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id 150DED99; Tue, 5 Mar 2013 08:00:58 +0000 (UTC) (envelope-from luigi@onelab2.iet.unipi.it) Received: from onelab2.iet.unipi.it (onelab2.iet.unipi.it [131.114.59.238]) by mx1.freebsd.org (Postfix) with ESMTP id CFEE5807; Tue, 5 Mar 2013 08:00:57 +0000 (UTC) Received: by onelab2.iet.unipi.it (Postfix, from userid 275) id 8C1D27300A; Tue, 5 Mar 2013 09:01:34 +0100 (CET) Date: Tue, 5 Mar 2013 09:01:34 +0100 From: Luigi Rizzo To: arch@freebsd.org, Davide Italiano , Alexander Motin Subject: tickless design guidelines Message-ID: <20130305080134.GC13187@onelab2.iet.unipi.it> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.5.20 (2009-06-14) X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 05 Mar 2013 08:00:58 -0000 Hi, i would like a bit of advice on how to get coarse time estimates (such as "no need to do anything unless at least X msec have elapsed") in a tickless kernel. In the past (and I mean a distant one, i got this habit back in 2.x), rather than calling get*time(), i used to look at "ticks" to see if it had changed, and only fetch the actual time accordingly. This worked well with HZ=1000 or above, when my required accuracy a few milliseconds. With tickless kernels, i am not sure anymore how often ticks gets updated. I may need to do this tests quite frequently (e.g. up to a million times per second), so I'd rather have some lightweight function. Any suggestion ? thanks luigi From owner-freebsd-arch@FreeBSD.ORG Tue Mar 5 08:37:41 2013 Return-Path: Delivered-To: arch@freebsd.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id 6D5F972A for ; Tue, 5 Mar 2013 08:37:41 +0000 (UTC) (envelope-from luigi@onelab2.iet.unipi.it) Received: from onelab2.iet.unipi.it (onelab2.iet.unipi.it [131.114.59.238]) by mx1.freebsd.org (Postfix) with ESMTP id 25B10957 for ; Tue, 5 Mar 2013 08:37:40 +0000 (UTC) Received: by onelab2.iet.unipi.it (Postfix, from userid 275) id 5C3597300A; Tue, 5 Mar 2013 09:38:17 +0100 (CET) Date: Tue, 5 Mar 2013 09:38:17 +0100 From: Luigi Rizzo To: arch@freebsd.org Subject: revising sys/conf/files* dependencies Message-ID: <20130305083817.GD13187@onelab2.iet.unipi.it> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.5.20 (2009-06-14) X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 05 Mar 2013 08:37:41 -0000 Short Summary: I would like to revise sys/conf/files* and fix many erroneous usages of some/file.c optional foo_dev bar_bus changing them into one of the following some/file.c optional foo_dev # link error if bar_bus is missing some/file.cxi optional foo_dev | bar_bus # logical OR ---------- Full description: I always thought (wrongly) that a line of the form some/file.c optional foo bar baz # 1 in sys/conf/files* meant that file.c is compiled in if _any_ of the options is specified in the kernel config. But i was wrong, the above means that _all_ options are require, and the correct syntax for alternative options is some/file.c optional foo | bar | baz # 2 I believe that i am not alone in this misunderstanding, and that most entries in sys/conf/files* use form #1 in a wrong way, e.g.: dev/hptiop/hptiop.c optional hptiop scbus dev/iscsi/initiator/iscsi.c optional iscsi_initiator scbus dev/mfi/mfi_cam.c optional mfip scbus pci/viapm.c optional viapm pci pci/intpm.c optional intpm pci pci/if_rl.c optional rl pci (there are many many more) In all these cases, if you forget the scbus or pci in the kernel config, the driver is not compiled in but you only detect it at compile time. I'd rather be notified of the error at kernel link time. So, as said in the summary, I'd like to modify these and similar lines so that the error notification comes early; normally this is achieved by removing the bus name. Probably the only case where the "AND" form makes sense is when two "device ..." entries in the kernel config also need to bring in some additional files. Examples (perhaps) are drivers which support multiple buses, such as dev/an/if_an.c optional an dev/an/if_an_isa.c optional an isa dev/an/if_an_pccard.c optional an pccard dev/an/if_an_pci.c optional an pci but this does not seem the main usage. It would be really good if we could force dependencies, e.g. "device da" implies "device scbus" but there is no good way to specify this in sys/conf/files* without horribly cluttering the entries for a bus with all the devices that use it. Probably one could extract this information from the MODULE_DEPEND() macros within the source tree, but i am unclear on what is the most efficient way to process the information without having to change config(8) -- which being written in C is a bit error-prone. comments ? cheers luigi From owner-freebsd-arch@FreeBSD.ORG Tue Mar 5 08:41:23 2013 Return-Path: Delivered-To: arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id E108B927; Tue, 5 Mar 2013 08:41:23 +0000 (UTC) (envelope-from mavbsd@gmail.com) Received: from mail-la0-x232.google.com (mail-la0-x232.google.com [IPv6:2a00:1450:4010:c03::232]) by mx1.freebsd.org (Postfix) with ESMTP id 464C298A; Tue, 5 Mar 2013 08:41:23 +0000 (UTC) Received: by mail-la0-f50.google.com with SMTP id ec20so5926879lab.9 for ; Tue, 05 Mar 2013 00:41:21 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=x-received:sender:message-id:date:from:user-agent:mime-version:to :cc:subject:references:in-reply-to:content-type :content-transfer-encoding; bh=27T89Kx2jq1t+783vkzb6QFAAVGLrwrnqkT2HcWY/Qs=; b=tax2eehFmqF2oVEsTcmP0vN8aVjQXE+hE2yZwRufkGUXXG7Y/TZe5qa25YZaOP2r9A B4fltVNDjesbfd3EqwS8KQ38KMSwVxafqa/NcdmtYQzFLRiMSuigv4/aUiJzQG2QJn89 A+t6LwlDhs2wQw3yCH6BAyaQhXa9ZX0mWUXVvjPbXKnzoEvat8/1GaBLFO1hPAniD0/1 dzekBhX5g5Ra5qBnuMpkToPLW1cYB4GPn04AOGynBs+RhkfjfCZvxgFhdCbNOxoJSHB1 YUr8FzcVmIaEjqurf3KPS61JWaQJJhGPo8MGWyQVMwjIJmWTzamBOpTCF0RUm2zuDM7J xTPA== X-Received: by 10.112.28.41 with SMTP id y9mr5635575lbg.133.1362472881436; Tue, 05 Mar 2013 00:41:21 -0800 (PST) Received: from mavbook.mavhome.dp.ua (mavhome.mavhome.dp.ua. [213.227.240.37]) by mx.google.com with ESMTPS id q9sm8265986lbz.3.2013.03.05.00.41.18 (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Tue, 05 Mar 2013 00:41:20 -0800 (PST) Sender: Alexander Motin Message-ID: <5135AFAD.70408@FreeBSD.org> Date: Tue, 05 Mar 2013 10:41:17 +0200 From: Alexander Motin User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:17.0) Gecko/20130125 Thunderbird/17.0.2 MIME-Version: 1.0 To: Luigi Rizzo Subject: Re: tickless design guidelines References: <20130305080134.GC13187@onelab2.iet.unipi.it> In-Reply-To: <20130305080134.GC13187@onelab2.iet.unipi.it> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: Davide Italiano , arch@freebsd.org X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 05 Mar 2013 08:41:23 -0000 On 05.03.2013 10:01, Luigi Rizzo wrote: > Hi, > i would like a bit of advice on how to get coarse time estimates > (such as "no need to do anything unless at least X msec have elapsed") > in a tickless kernel. > > In the past (and I mean a distant one, i got this habit back in 2.x), > rather than calling get*time(), i used to look at "ticks" to > see if it had changed, and only fetch the actual time accordingly. > > This worked well with HZ=1000 or above, when my required accuracy > a few milliseconds. With tickless kernels, i am not sure anymore > how often ticks gets updated. > > I may need to do this tests quite frequently (e.g. up to a million > times per second), so I'd rather have some lightweight function. At this point each active CPU still executes hardclock() with HZ rate as before, skipping only ticks when CPU is idle. As result, both ticks and getbinuptime() are working as before. First -- giving resolution of 1/HZ, second -- min(1/HZ, 1ms). The only place where it is unreliable to use them is hardware interrupt handlers (not interrupt threads), as if they fire during a long idle period timers may not be updated yet. Another way that may be better then polling is to let callout(9) manage time events. You can specify desired time and precision, and depending on precision it will use either binuptime(), or getbinuptime(), or fast custom way equivalent to ticks. Recent tests show that on x86 with LAPIC timer and TSC timecounter present code can handle up to 600K event per second from user-level on one core. From kernel I think rate could be even higher. -- Alexander Motin From owner-freebsd-arch@FreeBSD.ORG Tue Mar 5 09:06:59 2013 Return-Path: Delivered-To: arch@freebsd.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id 1F4DE128; Tue, 5 Mar 2013 09:06:59 +0000 (UTC) (envelope-from luigi@onelab2.iet.unipi.it) Received: from onelab2.iet.unipi.it (onelab2.iet.unipi.it [131.114.59.238]) by mx1.freebsd.org (Postfix) with ESMTP id D4865A7E; Tue, 5 Mar 2013 09:06:58 +0000 (UTC) Received: by onelab2.iet.unipi.it (Postfix, from userid 275) id 788457300A; Tue, 5 Mar 2013 10:07:35 +0100 (CET) Date: Tue, 5 Mar 2013 10:07:35 +0100 From: Luigi Rizzo To: Alexander Motin Subject: Re: tickless design guidelines Message-ID: <20130305090735.GB18221@onelab2.iet.unipi.it> References: <20130305080134.GC13187@onelab2.iet.unipi.it> <5135AFAD.70408@FreeBSD.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <5135AFAD.70408@FreeBSD.org> User-Agent: Mutt/1.5.20 (2009-06-14) Cc: Davide Italiano , arch@freebsd.org X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 05 Mar 2013 09:06:59 -0000 On Tue, Mar 05, 2013 at 10:41:17AM +0200, Alexander Motin wrote: > On 05.03.2013 10:01, Luigi Rizzo wrote: > > Hi, > > i would like a bit of advice on how to get coarse time estimates > > (such as "no need to do anything unless at least X msec have elapsed") > > in a tickless kernel. > > > > In the past (and I mean a distant one, i got this habit back in 2.x), > > rather than calling get*time(), i used to look at "ticks" to > > see if it had changed, and only fetch the actual time accordingly. > > > > This worked well with HZ=1000 or above, when my required accuracy > > a few milliseconds. With tickless kernels, i am not sure anymore > > how often ticks gets updated. > > > > I may need to do this tests quite frequently (e.g. up to a million > > times per second), so I'd rather have some lightweight function. > > At this point each active CPU still executes hardclock() with HZ rate as > before, skipping only ticks when CPU is idle. As result, both ticks and > getbinuptime() are working as before. First -- giving resolution of > 1/HZ, second -- min(1/HZ, 1ms). The only place where it is unreliable to > use them is hardware interrupt handlers (not interrupt threads), as if > they fire during a long idle period timers may not be updated yet. excellent - can you add the above paragraph to share/man/man9/microtime.9 so it is clearly documented ? Also i wonder if it may make sense to add a feature so that whenever we get an interrupt and a fast and suitable timecounter is available, some system-wide bintime is updated. So getbinuptime() could just use this counter, becoming extremely cheap (perhaps it is already, i am not sure) and in the long term, as CPUs with fixed frequency TSC become ubiquitous, we would get higher resolution as the interrupt load increases. > Another way that may be better then polling is to let callout(9) manage > time events. You can specify desired time and precision, and depending > on precision it will use either binuptime(), or getbinuptime(), or fast > custom way equivalent to ticks. Recent tests show that on x86 with LAPIC > timer and TSC timecounter present code can handle up to 600K event per > second from user-level on one core. From kernel I think rate could be > even higher. I will keep the suggestion in mind although this is not my current use case; right now i need to get some coarse timestamps on incoming packets to implement features such as "codel" (where i need to detect if a packet has been queued for at least ~5ms, but do not care about microsecond resolution or absolute timestamps). thanks luigi > -- > Alexander Motin From owner-freebsd-arch@FreeBSD.ORG Tue Mar 5 09:22:02 2013 Return-Path: Delivered-To: arch@freebsd.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id C0391ABE; Tue, 5 Mar 2013 09:22:02 +0000 (UTC) (envelope-from mavbsd@gmail.com) Received: from mail-la0-x230.google.com (mail-la0-x230.google.com [IPv6:2a00:1450:4010:c03::230]) by mx1.freebsd.org (Postfix) with ESMTP id E6881D50; Tue, 5 Mar 2013 09:22:01 +0000 (UTC) Received: by mail-la0-f48.google.com with SMTP id fq13so5953668lab.35 for ; Tue, 05 Mar 2013 01:22:00 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=x-received:sender:message-id:date:from:user-agent:mime-version:to :cc:subject:references:in-reply-to:content-type :content-transfer-encoding; bh=q7SL+gQX2m2dS5aMqlhn52ZSx8c51ttNydnBpFiRxwg=; b=Im5kRioHWRvaFPpI2XeFiI6OeekQxEzmmpzSxsJfOwdgUqxdb6GaAUW+TuQAfEpd/Y 5gPuIN8X/XLmsd9zY0ayxD5uz7aoHa5RPb0jwwRO6brc/+pivfYk/P3yE27rkAckKyP6 AJclmrbyMOfgnwGW6pRIJDMg1Uad45JKEQeyqF/0CaVYKwKAXLRXuhV9bez5SzytQalT eHXBZDmjmR4A3mf1/CZX9sDtKsf1XHIbBXCuWP7oW2u8T4JV0hjUHNgpSvLxAgy3zgl1 HCvYXz0AtnA31tKgUk46PjmKMFqzFSa6z5/OZkojYKbQXTORP7RsqdUarEUgguGt/riU J/Hg== X-Received: by 10.112.98.227 with SMTP id el3mr5452790lbb.131.1362475320802; Tue, 05 Mar 2013 01:22:00 -0800 (PST) Received: from mavbook.mavhome.dp.ua (mavhome.mavhome.dp.ua. [213.227.240.37]) by mx.google.com with ESMTPS id q9sm8313676lbz.3.2013.03.05.01.21.58 (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Tue, 05 Mar 2013 01:21:59 -0800 (PST) Sender: Alexander Motin Message-ID: <5135B934.5060202@FreeBSD.org> Date: Tue, 05 Mar 2013 11:21:56 +0200 From: Alexander Motin User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:17.0) Gecko/20130125 Thunderbird/17.0.2 MIME-Version: 1.0 To: Luigi Rizzo Subject: Re: tickless design guidelines References: <20130305080134.GC13187@onelab2.iet.unipi.it> <5135AFAD.70408@FreeBSD.org> <20130305090735.GB18221@onelab2.iet.unipi.it> In-Reply-To: <20130305090735.GB18221@onelab2.iet.unipi.it> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: Davide Italiano , arch@freebsd.org X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 05 Mar 2013 09:22:02 -0000 On 05.03.2013 11:07, Luigi Rizzo wrote: > Also i wonder if it may make sense to add a feature so that whenever > we get an interrupt and a fast and suitable timecounter is available, > some system-wide bintime is updated. > > So getbinuptime() could just use this counter, becoming extremely > cheap (perhaps it is already, i am not sure) and in the long term, > as CPUs with fixed frequency TSC become ubiquitous, > we would get higher resolution as the interrupt load increases. Each time timer interrupt fires, or CPU timers "switched" between idle and active modes present time is fetched. The only question is how to store it somewhere safely. It was difficult for 96/128-bit struct bintime. It is easier for 64-bit sbintime_t, but still not all archs are able to do it atomically. The mechanism used for getbinuptime() now may not scale well to the very high update rates. -- Alexander Motin From owner-freebsd-arch@FreeBSD.ORG Tue Mar 5 09:48:28 2013 Return-Path: Delivered-To: arch@freebsd.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id 3027DADA for ; Tue, 5 Mar 2013 09:48:28 +0000 (UTC) (envelope-from des@des.no) Received: from smtp.des.no (smtp.des.no [194.63.250.102]) by mx1.freebsd.org (Postfix) with ESMTP id EC5BFF6 for ; Tue, 5 Mar 2013 09:48:27 +0000 (UTC) Received: from ds4.des.no (smtp.des.no [194.63.250.102]) by smtp-int.des.no (Postfix) with ESMTP id 2E32E8A42; Tue, 5 Mar 2013 09:48:21 +0000 (UTC) Received: by ds4.des.no (Postfix, from userid 1001) id EB5DD9017; Tue, 5 Mar 2013 10:48:20 +0100 (CET) From: =?utf-8?Q?Dag-Erling_Sm=C3=B8rgrav?= To: Luigi Rizzo Subject: Re: revising sys/conf/files* dependencies References: <20130305083817.GD13187@onelab2.iet.unipi.it> Date: Tue, 05 Mar 2013 10:48:20 +0100 In-Reply-To: <20130305083817.GD13187@onelab2.iet.unipi.it> (Luigi Rizzo's message of "Tue, 5 Mar 2013 09:38:17 +0100") Message-ID: <86hakqtbx7.fsf@ds4.des.no> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.2 (berkeley-unix) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Cc: arch@freebsd.org X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 05 Mar 2013 09:48:28 -0000 Luigi Rizzo writes: > In all these cases, if you forget the scbus or pci in the kernel > config, the driver is not compiled in but you only detect it at > compile time. I'd rather be notified of the error at kernel link time. I would rather be notified by config(8): "iscsi_initiator requires scbus, please edit your kernel configuration and re-run config(8)" and perhaps a command-line option that makes it automatically resolve dependencies instead of complaining about them. DES --=20 Dag-Erling Sm=C3=B8rgrav - des@des.no From owner-freebsd-arch@FreeBSD.ORG Tue Mar 5 10:43:50 2013 Return-Path: Delivered-To: arch@FreeBSD.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id 08F098C7; Tue, 5 Mar 2013 10:43:50 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from fallbackmx07.syd.optusnet.com.au (fallbackmx07.syd.optusnet.com.au [211.29.132.9]) by mx1.freebsd.org (Postfix) with ESMTP id 241C2622; Tue, 5 Mar 2013 10:43:48 +0000 (UTC) Received: from mail17.syd.optusnet.com.au (mail17.syd.optusnet.com.au [211.29.132.198]) by fallbackmx07.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id r25AhfEo002465; Tue, 5 Mar 2013 21:43:41 +1100 Received: from c211-30-173-106.carlnfd1.nsw.optusnet.com.au (c211-30-173-106.carlnfd1.nsw.optusnet.com.au [211.30.173.106]) by mail17.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id r25AhUe1023287 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Tue, 5 Mar 2013 21:43:32 +1100 Date: Tue, 5 Mar 2013 21:43:30 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Alexander Motin Subject: Re: tickless design guidelines In-Reply-To: <5135B934.5060202@FreeBSD.org> Message-ID: <20130305204500.T1059@besplex.bde.org> References: <20130305080134.GC13187@onelab2.iet.unipi.it> <5135AFAD.70408@FreeBSD.org> <20130305090735.GB18221@onelab2.iet.unipi.it> <5135B934.5060202@FreeBSD.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.0 cv=DdhPMYRW c=1 sm=1 a=kj9zAlcOel0A:10 a=PO7r1zJSAAAA:8 a=JzwRw_2MAAAA:8 a=ZGdlnkk9SXAA:10 a=u4QguocfA062RjAEVIMA:9 a=CjuIK1q_8ugA:10 a=fZR5MC7dhzonfZNR:21 a=1N-mQgnlXDl8t-hT:21 a=TEtd8y5WR3g2ypngnwZWYw==:117 Cc: Davide Italiano , arch@FreeBSD.org X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 05 Mar 2013 10:43:50 -0000 On Tue, 5 Mar 2013, Alexander Motin wrote: > On 05.03.2013 11:07, Luigi Rizzo wrote: >> Also i wonder if it may make sense to add a feature so that whenever >> we get an interrupt and a fast and suitable timecounter is available, >> some system-wide bintime is updated. bintime is intentionally updated as little as possible, since updating it is an expensive operation that would need even more expensive locking if it is done often. Note that get*time() is fundamentally broken, since the timestamps that it returns are incoherent with timestamps returned by non-get*time(). I used to sync get*time() with non-get*time() on every call to the latter. E.g., on every call to bintime(), time_second, timehands->th_microtime and timehands->th_nanotime are updated with the time just read by binuptime(). getbintime(), etc., use the possibly-updated values. This makes getbintime(), etc., more accurate if calls to them are mixed with calls to bintime(), but this is a side effect. The sync is done for correctness. It ensures that the time is monotonic when read using any mixture of get/non-get*time() APIs. File timestamps written using the value of time_second or getnanotime() are noticebly broken when compared with the real time read using gettimeofday() without this.Z. But this is slow, and phk didn't like it. I only did it for the !SMP cases and dropped it after a few years. Your suggestion is similar. Updating the system-wide bintime requires knowing the current bintime, and nothing much more or less than a very recent read of a hardware timecounter gives that. The difficulty is atomically updating the whole timecounter state with values derived from that. >> So getbinuptime() could just use this counter, becoming extremely >> cheap (perhaps it is already, i am not sure) and in the long term, >> as CPUs with fixed frequency TSC become ubiquitous, >> we would get higher resolution as the interrupt load increases. getbinuptime() is already extremely cheap, but imprecise and incoherent. If you assume a fixed frequency TSC, then you can just always use bintime(). Unfortunately, the same CPUs that have a fixed frequency TSC also have slow-to-read TSCs. The TSC read time increased from ~9 cycles on AthlonXP and Athlon64 to 42 on Phenom, and it core2 is even slower. 9 cycles were almost in the noise, so getbintime() was only slightly faster than bintime() on Athlon64. > Each time timer interrupt fires, or CPU timers "switched" between idle > and active modes present time is fetched. The only question is how to > store it somewhere safely. It was difficult for 96/128-bit struct > bintime. It is easier for 64-bit sbintime_t, but still not all archs are > able to do it atomically. The mechanism used for getbinuptime() now may > not scale well to the very high update rates. getbinuptime() with high update rates would reduce to a bad version of binuptime(). Actually, getbinuptime() would probably be unchanged. The updates would be in binuptime(). You could update only every 10 or 100 usec to get not so high update rates but not coherency. Actually^2, timekeeping is unimportant for timeouts. Thus you could use sloppy non-atomic updates for everything and barely notice losing the races for this. To get semi-coherent and more accurate (when the races aren't lost) in sbinuptime(), just record a delta-time on every hardware timecounter call. Arrange to call hardware timecounters fairly often, and add the delta time in sbinuptime(). Better not touch getbinuptime(), etc. Let them remain mostly race-free and incoherent. Here is my old syncing code: @ diff -c2 src/sys/kern/kern_tc.c~ src/sys/kern/kern_tc.c @ *** src/sys/kern/kern_tc.c~ Fri Mar 5 17:03:07 2004 @ --- src/sys/kern/kern_tc.c Fri Mar 5 17:03:08 2004 @ *************** @ *** 172,175 **** @ --- 224,254 ---- @ } @ @ + #define SYNC_TIME @ + @ + #if defined(SYNC_TIME) && !defined(SMP) @ + static void @ + sync_time(long sec, long nsec, long usec) nanotime() and microtime() call here with the time that they just read. I don't bother syncing full bintimes, since this was mainly intended to fix file timestamps. @ + { @ + register_t saveintr; @ + @ + saveintr = intr_disable(); Locking for !SMP only. @ + if (time_second < sec) @ + time_second = sec; @ + if (timehands->th_microtime.tv_sec < sec || @ + (timehands->th_microtime.tv_sec == sec && @ + timehands->th_microtime.tv_usec < usec)) { @ + timehands->th_microtime.tv_sec = sec; @ + timehands->th_microtime.tv_usec = usec; @ + } @ + if (timehands->th_nanotime.tv_sec < sec || @ + (timehands->th_nanotime.tv_sec == sec && @ + timehands->th_nanotime.tv_nsec < nsec)) { @ + timehands->th_nanotime.tv_sec = sec; @ + timehands->th_nanotime.tv_nsec = nsec; @ + } The secondary values are now coherent for use in getnanotime(), etc. sbintime() support could set the final value here instead of a delta. The reason to use a delta is that it fits in 32 bits so is easy to access atomically, so that the write here doesn't need much locking. Oops, the locking here is inadequate even for !SMP -- the updates here may be non-monotonic since the lock is not around the whole timcounter-read...update sequence. @ + intr_restore(saveintr); @ + } @ + #endif /* SYNC_TIME && !SMP */ @ + @ void @ bintime(struct bintime *bt) @ *************** @ *** 189,192 **** @ --- 268,274 ---- @ bintime(&bt); @ bintime2timespec(&bt, tsp); @ + #if defined(SYNC_TIME) && !defined(SMP) @ + sync_time(tsp->tv_sec, tsp->tv_nsec, tsp->tv_nsec / 1000); @ + #endif @ } @ Any call to bintime() syncs time_second, and the timespec and timeval in the timehands, but not the full bintime. So for example, getnanotime() benefits from bintime() being called, but getbintime() remains incoherent with bintime(). Bruce From owner-freebsd-arch@FreeBSD.ORG Tue Mar 5 12:19:01 2013 Return-Path: Delivered-To: arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 49040CB4 for ; Tue, 5 Mar 2013 12:19:01 +0000 (UTC) (envelope-from luigi@onelab2.iet.unipi.it) Received: from onelab2.iet.unipi.it (onelab2.iet.unipi.it [131.114.59.238]) by mx1.freebsd.org (Postfix) with ESMTP id 0FAFAA87 for ; Tue, 5 Mar 2013 12:19:00 +0000 (UTC) Received: by onelab2.iet.unipi.it (Postfix, from userid 275) id 47ADC7300A; Tue, 5 Mar 2013 13:19:37 +0100 (CET) Date: Tue, 5 Mar 2013 13:19:37 +0100 From: Luigi Rizzo To: Dag-Erling Sm??rgrav Subject: Re: revising sys/conf/files* dependencies Message-ID: <20130305121937.GA29075@onelab2.iet.unipi.it> References: <20130305083817.GD13187@onelab2.iet.unipi.it> <86hakqtbx7.fsf@ds4.des.no> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <86hakqtbx7.fsf@ds4.des.no> User-Agent: Mutt/1.5.20 (2009-06-14) Cc: arch@freebsd.org X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 05 Mar 2013 12:19:01 -0000 On Tue, Mar 05, 2013 at 10:48:20AM +0100, Dag-Erling Sm??rgrav wrote: > Luigi Rizzo writes: > > In all these cases, if you forget the scbus or pci in the kernel > > config, the driver is not compiled in but you only detect it at > > compile time. I'd rather be notified of the error at kernel link time. > > I would rather be notified by config(8): > > "iscsi_initiator requires scbus, please edit your kernel configuration > and re-run config(8)" > > and perhaps a command-line option that makes it automatically resolve > dependencies instead of complaining about them. right now the dependency is often, not always, just a MODULE_DEPEND() in some modules, and/or a comment in LINT (or options). Before defining the tool we should probably try to figure out what is the authoritative place with dependency information, and compile documentation and auto-dependencies starting from that. cheers luigi From owner-freebsd-arch@FreeBSD.ORG Tue Mar 5 15:15:35 2013 Return-Path: Delivered-To: arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 2C2BFD28 for ; Tue, 5 Mar 2013 15:15:35 +0000 (UTC) (envelope-from imp@bsdimp.com) Received: from mail-ia0-x235.google.com (mail-ia0-x235.google.com [IPv6:2607:f8b0:4001:c02::235]) by mx1.freebsd.org (Postfix) with ESMTP id D2AD27E8 for ; Tue, 5 Mar 2013 15:15:34 +0000 (UTC) Received: by mail-ia0-f181.google.com with SMTP id w33so6125475iag.26 for ; Tue, 05 Mar 2013 07:15:34 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-received:sender:subject:mime-version:content-type:from :in-reply-to:date:cc:content-transfer-encoding:message-id:references :to:x-mailer:x-gm-message-state; bh=2R2iy9VFpJlsj//chxxvJUHDCvgLhy71RJ8JX3yP4zg=; b=OLSw1n3hpe7go5cK97evjZmAPGI3Nyzx3Cbh12wn99TGfwXaOHBT0p7FXJkiXPvYAw IDFYb5IXhF6ZA+/p7PYJ7oY7WTVcmFtPBnnPQdRHAM7ySEVNRFlxnApeZJFaqN3H600C /lR8ch9gkqcfeUOmY6KsxflE/eZTSjAjtKKufBod34p9pRc4yi5APl0/JUi/igxBlcK8 /tMUMTAiuHr5f8f+XTBQ5MZ2STiL0wrO0HLfraVdR9SAsNU+PiQK8iCkEtrvdPGY57MM +Fc5iEvAJOpaWSienihVnsPE4ylePuQQE73+Mn2rYlbojZzIKFOvnKYnEP+aaXZGi2aC zX0Q== X-Received: by 10.50.88.199 with SMTP id bi7mr6481723igb.70.1362496534344; Tue, 05 Mar 2013 07:15:34 -0800 (PST) Received: from 53.imp.bsdimp.com (50-78-194-198-static.hfc.comcastbusiness.net. [50.78.194.198]) by mx.google.com with ESMTPS id s8sm17840429igs.0.2013.03.05.07.15.32 (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Tue, 05 Mar 2013 07:15:33 -0800 (PST) Sender: Warner Losh Subject: Re: revising sys/conf/files* dependencies Mime-Version: 1.0 (Apple Message framework v1085) Content-Type: text/plain; charset=us-ascii From: Warner Losh In-Reply-To: <20130305083817.GD13187@onelab2.iet.unipi.it> Date: Tue, 5 Mar 2013 08:15:30 -0700 Content-Transfer-Encoding: quoted-printable Message-Id: <112844CF-69C3-49A3-8581-8EF2A7DA8E8A@bsdimp.com> References: <20130305083817.GD13187@onelab2.iet.unipi.it> To: Luigi Rizzo X-Mailer: Apple Mail (2.1085) X-Gm-Message-State: ALoCoQkDdOwPwOwTcjElMmEJE6KOAldaZgC2MFePnswKlBWZCQEWWSSP2vqEsVAGiV1PqzWU2fZl Cc: arch@freebsd.org X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 05 Mar 2013 15:15:35 -0000 On Mar 5, 2013, at 1:38 AM, Luigi Rizzo wrote: > Short Summary: >=20 > I would like to revise sys/conf/files* and fix many erroneous usages = of >=20 > some/file.c optional foo_dev bar_bus >=20 > changing them into one of the following >=20 > some/file.c optional foo_dev # link error if bar_bus = is missing > some/file.cxi optional foo_dev | bar_bus # logical OR >=20 > ---------- > Full description: >=20 > I always thought (wrongly) that a line of the form >=20 > some/file.c optional foo bar baz # 1 >=20 > in sys/conf/files* meant that file.c is compiled in if _any_ of the > options is specified in the kernel config. But i was wrong, the > above means that _all_ options are require, and the correct syntax > for alternative options is >=20 > some/file.c optional foo | bar | baz # 2 >=20 > I believe that i am not alone in this misunderstanding, and that > most entries in sys/conf/files* use form #1 in a wrong way, e.g.: >=20 > dev/hptiop/hptiop.c optional hptiop scbus > dev/iscsi/initiator/iscsi.c optional iscsi_initiator scbus > dev/mfi/mfi_cam.c optional mfip scbus > pci/viapm.c optional viapm pci > pci/intpm.c optional intpm pci > pci/if_rl.c optional rl pci > (there are many many more) >=20 > In all these cases, if you forget the scbus or pci in the kernel > config, the driver is not compiled in but you only detect it at > compile time. I'd rather be notified of the error at kernel link time. I'd say those are correct: don't compile if_rl unless you have both rl = and pci in the kernel.... There's no misunderstanding here: people know = what it means. > So, as said in the summary, I'd like to modify these and similar > lines so that the error notification comes early; normally > this is achieved by removing the bus name. That might be OK. The original intent for this form was to allow one to = remove all pci drivers from the kernel by just removing the device pci = line from the config file. This is at best a dubious feature, experience = has shown. I'm worried that "fixing" it will produce a worse problem = than the one you are fixing. > Probably the only case where the "AND" form makes sense is when two > "device ..." entries in the kernel config also need to bring in > some additional files. Examples (perhaps) are drivers which support > multiple buses, such as >=20 > dev/an/if_an.c optional an > dev/an/if_an_isa.c optional an isa > dev/an/if_an_pccard.c optional an pccard > dev/an/if_an_pci.c optional an pci >=20 > but this does not seem the main usage. Yea. One problem with your suggestion though. Some busses, like pccard, = are totally loadable. you won't get an error if you don't have pccard in = the kernel, but if you stick a an card into the system it won't probe = either. In at least the case of pccard, that's intentional so that you = can load/unload pccard at runtime if you were debugging it :) > It would be really good if we could force dependencies, e.g. >=20 > "device da" implies "device scbus" >=20 > but there is no good way to specify this in sys/conf/files* without > horribly cluttering the entries for a bus with all the devices > that use it. Yes. > Probably one could extract this information from the MODULE_DEPEND() = macros Probably not. there are many conventions, but they are violated often = enough to not be automateable. > within the source tree, but i am unclear on what is the most > efficient way to process the information without having to change > config(8) -- which being written in C is a bit error-prone. Well, hacking config is error prone not because it is written in 'C', = but because it is in a style that people stopped writing in sometime in = the late 1980's because it was error-prone. > comments ? >=20 > cheers > luigi > _______________________________________________ > freebsd-arch@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-arch > To unsubscribe, send any mail to = "freebsd-arch-unsubscribe@freebsd.org" From owner-freebsd-arch@FreeBSD.ORG Tue Mar 5 21:19:21 2013 Return-Path: Delivered-To: arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id D3FF88DD for ; Tue, 5 Mar 2013 21:19:21 +0000 (UTC) (envelope-from luigi@onelab2.iet.unipi.it) Received: from onelab2.iet.unipi.it (onelab2.iet.unipi.it [131.114.59.238]) by mx1.freebsd.org (Postfix) with ESMTP id 9A3A4DF5 for ; Tue, 5 Mar 2013 21:19:21 +0000 (UTC) Received: by onelab2.iet.unipi.it (Postfix, from userid 275) id BC3717300A; Tue, 5 Mar 2013 22:19:53 +0100 (CET) Date: Tue, 5 Mar 2013 22:19:53 +0100 From: Luigi Rizzo To: Warner Losh Subject: Re: revising sys/conf/files* dependencies Message-ID: <20130305211953.GA51357@onelab2.iet.unipi.it> References: <20130305083817.GD13187@onelab2.iet.unipi.it> <112844CF-69C3-49A3-8581-8EF2A7DA8E8A@bsdimp.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <112844CF-69C3-49A3-8581-8EF2A7DA8E8A@bsdimp.com> User-Agent: Mutt/1.5.20 (2009-06-14) Cc: arch@freebsd.org X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 05 Mar 2013 21:19:21 -0000 On Tue, Mar 05, 2013 at 08:15:30AM -0700, Warner Losh wrote: > > On Mar 5, 2013, at 1:38 AM, Luigi Rizzo wrote: > > > Short Summary: > > > > I would like to revise sys/conf/files* and fix many erroneous usages of > > > > some/file.c optional foo_dev bar_bus > > > > changing them into one of the following > > > > some/file.c optional foo_dev # link error if bar_bus is missing > > some/file.cxi optional foo_dev | bar_bus # logical OR > > > > ---------- > > Full description: > > > > I always thought (wrongly) that a line of the form > > > > some/file.c optional foo bar baz # 1 > > > > in sys/conf/files* meant that file.c is compiled in if _any_ of the > > options is specified in the kernel config. But i was wrong, the > > above means that _all_ options are require, and the correct syntax > > for alternative options is > > > > some/file.c optional foo | bar | baz # 2 > > > > I believe that i am not alone in this misunderstanding, and that > > most entries in sys/conf/files* use form #1 in a wrong way, e.g.: > > > > dev/hptiop/hptiop.c optional hptiop scbus > > dev/iscsi/initiator/iscsi.c optional iscsi_initiator scbus > > dev/mfi/mfi_cam.c optional mfip scbus > > pci/viapm.c optional viapm pci > > pci/intpm.c optional intpm pci > > pci/if_rl.c optional rl pci > > (there are many many more) > > > > In all these cases, if you forget the scbus or pci in the kernel > > config, the driver is not compiled in but you only detect it at > > compile time. I'd rather be notified of the error at kernel link time. ^^^^^^^^^^^^ i meant "run time", and this is the main problem: if you forget the bus in the kernel config, the build will silently discard the entry, but you will only realize it when you actually run the kernel. Yet, having "rl" alone is surely an error, and it should be flagged as such at build time. > > I'd say those are correct: don't compile if_rl unless you have both rl and pci in the kernel.... There's no misunderstanding here: people know what it means. > > So, as said in the summary, I'd like to modify these and similar > > lines so that the error notification comes early; normally > > this is achieved by removing the bus name. > > That might be OK. The original intent for this form was to allow one to remove all pci drivers from the kernel by just removing the device pci line from the config file. This is at best a dubious feature, experience has shown. I'm worried that "fixing" it will produce a worse problem than the one you are fixing. if that is the intent, at the very least we should explain it in the file so it is clear why "pci" and "scbus" are handled differently. But apart form the convenience for developers (and given we have "include", this can be easily overcome), the "fix" i propose should only point out broken config files. cheers luigi From owner-freebsd-arch@FreeBSD.ORG Tue Mar 5 21:33:47 2013 Return-Path: Delivered-To: arch@freebsd.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id A6137BC2 for ; Tue, 5 Mar 2013 21:33:47 +0000 (UTC) (envelope-from imp@bsdimp.com) Received: from mail-ob0-x235.google.com (mail-ob0-x235.google.com [IPv6:2607:f8b0:4003:c01::235]) by mx1.freebsd.org (Postfix) with ESMTP id 45F27E63 for ; Tue, 5 Mar 2013 21:33:47 +0000 (UTC) Received: by mail-ob0-f181.google.com with SMTP id ni5so2963186obc.40 for ; Tue, 05 Mar 2013 13:33:47 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-received:sender:subject:mime-version:content-type:from :in-reply-to:date:cc:content-transfer-encoding:message-id:references :to:x-mailer:x-gm-message-state; bh=LQ95MY+gfIJnmAGt9OunrZJxie/NwbsUYbvnkZHgvUE=; b=OG+hRsDd0Ph7o75hp/EcOs4f4fMNQicC4Gnq8e6aJsRQBaTl7qhTUF1Po5r1y0FCtc gnEuxKG2DWEoBiKZSEm+c6mrr/hi2NHfFr/AWHfNfve1ApOmUI5pXnKUE7wsdJ37EN3U QuLiTuoqubiEb3zdaLeqi33/ddqg0Iw8FGgANDS+CjL4zGHs9Oax+9W+MZlIMXWuAavW oMnNc7ojf6S6AqvxP9qodqyOQ9/kBIknjBJNx3AMeoW4GKlVin0qTeN4x7xW5lVrH+C8 hmFgVdRkq/DJv/4cVTq3ugqlJSX8ScciI6E59uKhQ+KZqLzwODizVABVG2glRkp88GKf 0Jhg== X-Received: by 10.60.10.34 with SMTP id f2mr20183140oeb.104.1362519226836; Tue, 05 Mar 2013 13:33:46 -0800 (PST) Received: from fusionlt2834a.int.fusionio.com ([209.117.142.2]) by mx.google.com with ESMTPS id d10sm35952157oeh.7.2013.03.05.13.33.43 (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Tue, 05 Mar 2013 13:33:44 -0800 (PST) Sender: Warner Losh Subject: Re: revising sys/conf/files* dependencies Mime-Version: 1.0 (Apple Message framework v1085) Content-Type: text/plain; charset=us-ascii From: Warner Losh In-Reply-To: <20130305211953.GA51357@onelab2.iet.unipi.it> Date: Tue, 5 Mar 2013 14:33:41 -0700 Content-Transfer-Encoding: quoted-printable Message-Id: <6A93AA58-713D-4C25-A512-6927E27C5DE1@bsdimp.com> References: <20130305083817.GD13187@onelab2.iet.unipi.it> <112844CF-69C3-49A3-8581-8EF2A7DA8E8A@bsdimp.com> <20130305211953.GA51357@onelab2.iet.unipi.it> To: Luigi Rizzo X-Mailer: Apple Mail (2.1085) X-Gm-Message-State: ALoCoQkJc3BhFtqtUS/QWSE0Jx3X95aJW8j0kQ988CvNmAnAj+XimTy6/jZOiO4OJ2Ux1Y6PFdw8 Cc: arch@freebsd.org X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 05 Mar 2013 21:33:47 -0000 On Mar 5, 2013, at 2:19 PM, Luigi Rizzo wrote: > On Tue, Mar 05, 2013 at 08:15:30AM -0700, Warner Losh wrote: >>=20 >> On Mar 5, 2013, at 1:38 AM, Luigi Rizzo wrote: >>=20 >>> Short Summary: >>>=20 >>> I would like to revise sys/conf/files* and fix many erroneous usages = of >>>=20 >>> some/file.c optional foo_dev bar_bus >>>=20 >>> changing them into one of the following >>>=20 >>> some/file.c optional foo_dev # link error if bar_bus = is missing >>> some/file.cxi optional foo_dev | bar_bus # logical OR >>>=20 >>> ---------- >>> Full description: >>>=20 >>> I always thought (wrongly) that a line of the form >>>=20 >>> some/file.c optional foo bar baz # 1 >>>=20 >>> in sys/conf/files* meant that file.c is compiled in if _any_ of the >>> options is specified in the kernel config. But i was wrong, the >>> above means that _all_ options are require, and the correct syntax >>> for alternative options is >>>=20 >>> some/file.c optional foo | bar | baz # 2 >>>=20 >>> I believe that i am not alone in this misunderstanding, and that >>> most entries in sys/conf/files* use form #1 in a wrong way, e.g.: >>>=20 >>> dev/hptiop/hptiop.c optional hptiop scbus >>> dev/iscsi/initiator/iscsi.c optional iscsi_initiator scbus >>> dev/mfi/mfi_cam.c optional mfip scbus >>> pci/viapm.c optional viapm pci >>> pci/intpm.c optional intpm pci >>> pci/if_rl.c optional rl pci >>> (there are many many more) >>>=20 >>> In all these cases, if you forget the scbus or pci in the kernel >>> config, the driver is not compiled in but you only detect it at >>> compile time. I'd rather be notified of the error at kernel link = time. > ^^^^^^^^^^^^ i meant "run time", and this is the main problem: > if you forget the bus in the kernel config, the build will silently > discard the entry, but you will only realize it when you actually > run the kernel. Yet, having "rl" alone is surely an error, and it > should be flagged as such at build time. Yea... You are wanting dependency checking that does not exist today, = and for which the meta-data does not exist today. Like I said, the intent of the original feature was to disable large = classes of things quickly. Nobody really does that, so that feature can = go... It is likely half broken these days. >> I'd say those are correct: don't compile if_rl unless you have both = rl and pci in the kernel.... There's no misunderstanding here: people = know what it means. >=20 >>> So, as said in the summary, I'd like to modify these and similar >>> lines so that the error notification comes early; normally >>> this is achieved by removing the bus name. >>=20 >> That might be OK. The original intent for this form was to allow one = to remove all pci drivers from the kernel by just removing the device = pci line from the config file. This is at best a dubious feature, = experience has shown. I'm worried that "fixing" it will produce a worse = problem than the one you are fixing. >=20 > if that is the intent, at the very least we should explain it > in the file so it is clear why "pci" and "scbus" are handled > differently. But apart form the convenience for developers > (and given we have "include", this can be easily overcome), > the "fix" i propose should only point out broken config files. They aren't handled differently. You've always had to have all the = dependencies to make things work. The fix you propose moves the problem around, and won't solve the = problem for those devices that have multiple attachments. Since that has = gone rare, that might be an OK problem exchange. However, it would be better to have required dependencies be codified = somewhere in the files.*. Not having that, your solution sucks the least = to accomplish the goals that conflict with the original design that = happened to be a bad design point in the first place.... So yea, it's fine, but the real solution isn't to solve these problems = by hacking the current system, but rather by finishing the module system = that has always been totally half-assed. To finish the system, you = likely have to ditch config(8) as we know it. Warner From owner-freebsd-arch@FreeBSD.ORG Wed Mar 6 01:07:23 2013 Return-Path: Delivered-To: arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 80378677 for ; Wed, 6 Mar 2013 01:07:23 +0000 (UTC) (envelope-from marius@alchemy.franken.de) Received: from alchemy.franken.de (alchemy.franken.de [194.94.249.214]) by mx1.freebsd.org (Postfix) with ESMTP id 12E38871 for ; Wed, 6 Mar 2013 01:07:22 +0000 (UTC) Received: from alchemy.franken.de (localhost [127.0.0.1]) by alchemy.franken.de (8.14.6/8.14.6/ALCHEMY.FRANKEN.DE) with ESMTP id r2617K8L069532; Wed, 6 Mar 2013 02:07:20 +0100 (CET) (envelope-from marius@alchemy.franken.de) Received: (from marius@localhost) by alchemy.franken.de (8.14.6/8.14.6/Submit) id r2617KrT069531; Wed, 6 Mar 2013 02:07:20 +0100 (CET) (envelope-from marius) Date: Wed, 6 Mar 2013 02:07:20 +0100 From: Marius Strobl To: Warner Losh Subject: Re: revising sys/conf/files* dependencies Message-ID: <20130306010720.GA68018@alchemy.franken.de> References: <20130305083817.GD13187@onelab2.iet.unipi.it> <112844CF-69C3-49A3-8581-8EF2A7DA8E8A@bsdimp.com> <20130305211953.GA51357@onelab2.iet.unipi.it> <6A93AA58-713D-4C25-A512-6927E27C5DE1@bsdimp.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <6A93AA58-713D-4C25-A512-6927E27C5DE1@bsdimp.com> User-Agent: Mutt/1.5.21 (2010-09-15) Cc: arch@freebsd.org X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 06 Mar 2013 01:07:23 -0000 On Tue, Mar 05, 2013 at 02:33:41PM -0700, Warner Losh wrote: > > On Mar 5, 2013, at 2:19 PM, Luigi Rizzo wrote: > > > On Tue, Mar 05, 2013 at 08:15:30AM -0700, Warner Losh wrote: > >> > >> On Mar 5, 2013, at 1:38 AM, Luigi Rizzo wrote: > >> > >>> Short Summary: > >>> > >>> I would like to revise sys/conf/files* and fix many erroneous usages of > >>> > >>> some/file.c optional foo_dev bar_bus > >>> > >>> changing them into one of the following > >>> > >>> some/file.c optional foo_dev # link error if bar_bus is missing > >>> some/file.cxi optional foo_dev | bar_bus # logical OR > >>> > >>> ---------- > >>> Full description: > >>> > >>> I always thought (wrongly) that a line of the form > >>> > >>> some/file.c optional foo bar baz # 1 > >>> > >>> in sys/conf/files* meant that file.c is compiled in if _any_ of the > >>> options is specified in the kernel config. But i was wrong, the > >>> above means that _all_ options are require, and the correct syntax > >>> for alternative options is > >>> > >>> some/file.c optional foo | bar | baz # 2 > >>> > >>> I believe that i am not alone in this misunderstanding, and that > >>> most entries in sys/conf/files* use form #1 in a wrong way, e.g.: > >>> > >>> dev/hptiop/hptiop.c optional hptiop scbus > >>> dev/iscsi/initiator/iscsi.c optional iscsi_initiator scbus > >>> dev/mfi/mfi_cam.c optional mfip scbus > >>> pci/viapm.c optional viapm pci > >>> pci/intpm.c optional intpm pci > >>> pci/if_rl.c optional rl pci > >>> (there are many many more) > >>> > >>> In all these cases, if you forget the scbus or pci in the kernel > >>> config, the driver is not compiled in but you only detect it at > >>> compile time. I'd rather be notified of the error at kernel link time. > > ^^^^^^^^^^^^ i meant "run time", and this is the main problem: > > if you forget the bus in the kernel config, the build will silently > > discard the entry, but you will only realize it when you actually > > run the kernel. Yet, having "rl" alone is surely an error, and it > > should be flagged as such at build time. > > Yea... You are wanting dependency checking that does not exist today, and for which the meta-data does not exist today. > > Like I said, the intent of the original feature was to disable large classes of things quickly. Nobody really does that, so that feature can go... It is likely half broken these days. > Uhm, according to a quick test I'd say there definitely are drivers missing dependencies on pci in sys/conf/files, but it certainly seems way better than "half broken". > >> I'd say those are correct: don't compile if_rl unless you have both rl and pci in the kernel.... There's no misunderstanding here: people know what it means. > > > >>> So, as said in the summary, I'd like to modify these and similar > >>> lines so that the error notification comes early; normally > >>> this is achieved by removing the bus name. > >> > >> That might be OK. The original intent for this form was to allow one to remove all pci drivers from the kernel by just removing the device pci line from the config file. This is at best a dubious feature, experience has shown. I'm worried that "fixing" it will produce a worse problem than the one you are fixing. > > > > if that is the intent, at the very least we should explain it > > in the file so it is clear why "pci" and "scbus" are handled > > differently. But apart form the convenience for developers > > (and given we have "include", this can be easily overcome), > > the "fix" i propose should only point out broken config files. > > They aren't handled differently. You've always had to have all the dependencies to make things work. > > The fix you propose moves the problem around, and won't solve the problem for those devices that have multiple attachments. Since that has gone rare, that might be an OK problem exchange. > > However, it would be better to have required dependencies be codified somewhere in the files.*. Not having that, your solution sucks the least to accomplish the goals that conflict with the original design that happened to be a bad design point in the first place.... > > So yea, it's fine, but the real solution isn't to solve these problems by hacking the current system, but rather by finishing the module system that has always been totally half-assed. To finish the system, you likely have to ditch config(8) as we know it. > I'd say the proposed "fix" is only a bad kludge as it doesn't even solve the problem for drivers only having a PCI front-end. This is due to the fact that removing pci for these will only cause a link error iff these drivers use methods only compiled in when device pci is present, f.e. pci_enable_busmaster(9). However, one of the nice things about newbus is that the bus front-ends actually are rather bus-agnostic (and thus a single front-end may attach to different busses) and even using pci_get_{device,vendor}(9) doesn't cause a link error in case device pci isn't present. One example of such a PCI device driver compiling without device pci is sym(4). Yep, that example is a bit bad as sym_hipd.c actually misses the pci dependency in sys/conf/files. It's just the first driver I found and all "simple enough" drivers for PCI devices should fall in the same category. Marius From owner-freebsd-arch@FreeBSD.ORG Wed Mar 6 06:10:43 2013 Return-Path: Delivered-To: arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 3329F302 for ; Wed, 6 Mar 2013 06:10:43 +0000 (UTC) (envelope-from luigi@onelab2.iet.unipi.it) Received: from onelab2.iet.unipi.it (onelab2.iet.unipi.it [131.114.59.238]) by mx1.freebsd.org (Postfix) with ESMTP id 836852E5 for ; Wed, 6 Mar 2013 06:10:42 +0000 (UTC) Received: by onelab2.iet.unipi.it (Postfix, from userid 275) id F3A257300A; Wed, 6 Mar 2013 07:11:19 +0100 (CET) Date: Wed, 6 Mar 2013 07:11:19 +0100 From: Luigi Rizzo To: Marius Strobl Subject: Re: revising sys/conf/files* dependencies Message-ID: <20130306061119.GA77841@onelab2.iet.unipi.it> References: <20130305083817.GD13187@onelab2.iet.unipi.it> <112844CF-69C3-49A3-8581-8EF2A7DA8E8A@bsdimp.com> <20130305211953.GA51357@onelab2.iet.unipi.it> <6A93AA58-713D-4C25-A512-6927E27C5DE1@bsdimp.com> <20130306010720.GA68018@alchemy.franken.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20130306010720.GA68018@alchemy.franken.de> User-Agent: Mutt/1.5.20 (2009-06-14) Cc: arch@freebsd.org X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 06 Mar 2013 06:10:43 -0000 On Wed, Mar 06, 2013 at 02:07:20AM +0100, Marius Strobl wrote: > On Tue, Mar 05, 2013 at 02:33:41PM -0700, Warner Losh wrote: > > > > On Mar 5, 2013, at 2:19 PM, Luigi Rizzo wrote: > > > > > On Tue, Mar 05, 2013 at 08:15:30AM -0700, Warner Losh wrote: > > >> > > >> On Mar 5, 2013, at 1:38 AM, Luigi Rizzo wrote: > > >> > > >>> Short Summary: > > >>> > > >>> I would like to revise sys/conf/files* and fix many erroneous usages of > > >>> > > >>> some/file.c optional foo_dev bar_bus > > >>> > > >>> changing them into one of the following > > >>> > > >>> some/file.c optional foo_dev # link error if bar_bus is missing > > >>> some/file.cxi optional foo_dev | bar_bus # logical OR > > >>> > > >>> ---------- > > >>> Full description: > > >>> > > >>> I always thought (wrongly) that a line of the form > > >>> > > >>> some/file.c optional foo bar baz # 1 > > >>> > > >>> in sys/conf/files* meant that file.c is compiled in if _any_ of the > > >>> options is specified in the kernel config. But i was wrong, the > > >>> above means that _all_ options are require, and the correct syntax > > >>> for alternative options is > > >>> > > >>> some/file.c optional foo | bar | baz # 2 > > >>> > > >>> I believe that i am not alone in this misunderstanding, and that > > >>> most entries in sys/conf/files* use form #1 in a wrong way, e.g.: > > >>> > > >>> dev/hptiop/hptiop.c optional hptiop scbus > > >>> dev/iscsi/initiator/iscsi.c optional iscsi_initiator scbus > > >>> dev/mfi/mfi_cam.c optional mfip scbus > > >>> pci/viapm.c optional viapm pci > > >>> pci/intpm.c optional intpm pci > > >>> pci/if_rl.c optional rl pci > > >>> (there are many many more) > > >>> > > >>> In all these cases, if you forget the scbus or pci in the kernel > > >>> config, the driver is not compiled in but you only detect it at > > >>> compile time. I'd rather be notified of the error at kernel link time. > > > ^^^^^^^^^^^^ i meant "run time", and this is the main problem: > > > if you forget the bus in the kernel config, the build will silently > > > discard the entry, but you will only realize it when you actually > > > run the kernel. Yet, having "rl" alone is surely an error, and it > > > should be flagged as such at build time. > > > > Yea... You are wanting dependency checking that does not exist today, and for which the meta-data does not exist today. > > > > Like I said, the intent of the original feature was to disable large classes of things quickly. Nobody really does that, so that feature can go... It is likely half broken these days. > > > > Uhm, according to a quick test I'd say there definitely are drivers > missing dependencies on pci in sys/conf/files, but it certainly seems > way better than "half broken". Marius, just to clarify: sys/conf/files has no reasonable way to express dependencies. Take as an example the common case (more later) of a bus b with sources in sys/dev/b/... and devices d1, d2, ... dn with sources in sys/dev/d*/... All d* require b to be present in order to load. In the modules, you express this as MODULE_DEPEND(). Say you want to build a kernel with "device d1" but forget to include "device b". For sys/conf/files you have the following options: (1) silently ignore devices with missing dependencies. You will discover the misconfiguration at run time (or, you can consider this a feature to quickly remove all drivers that depend on a given bus) sys/dev/b/b_foo.c optional b sys/dev/b/b_bar.c optional b ... sys/dev/d1/d1_foo.c optional d1 b sys/dev/d1/d1_bar.c optional d1 b ... sys/dev/dn/dn_foo.c optional dn b sys/dev/dn/dn_bar_b.c optional dn b ... (2) omit the dependency, causing an error at link time sys/dev/b/b_foo.c optional b sys/dev/b/b_bar.c optional b ... sys/dev/d1/d1_foo.c optional d1 sys/dev/d1/d1_bar.c optional d1 ... sys/dev/dn/dn_foo.c optional dn sys/dev/dn/dn_bar_b.c optional dn ... (3) enforce the dependency sys/dev/b/b_foo.c optional b | d1 | d2 | ... | dn sys/dev/b/b_bar.c optional b | d1 | d2 | ... | dn ... sys/dev/d1/d1_foo.c optional d1 sys/dev/d1/d1_bar.c optional d1 ... sys/dev/dn/dn_foo.c optional dn sys/dev/dn/dn_bar_b.c optional dn ... Right now, some buses (e.g. pci) use #1; others (e.g. scbus) use #2; very few use #3 (which as you can see becomes unmanageable when the number of customers grows, or we have a chain of dependencies). Warner gave an explanation why #1 can be useful. Personally i prefer #2 so i can be notified sooner if my kernel config file has a missing dependency. My feeling is that many people (including myself up to a few weeks ago) believe #1 behaves as #3. As Warren mentioned, a proper solution requires replacing config (8) with something that is better suited to the task; but i am not suggesting to do this now, I just wanted to get opinions on whether #2 is considered significantly more useful than #1 to deserve a change. > I'd say the proposed "fix" is only a bad kludge as it doesn't even > solve the problem for drivers only having a PCI front-end. This is > due to the fact that removing pci for these will only cause a link > error iff these drivers use methods only compiled in when device pci > is present, f.e. pci_enable_busmaster(9). However, one of the nice > things about newbus is that the bus front-ends actually are rather > bus-agnostic (and thus a single front-end may attach to different > busses) and even using pci_get_{device,vendor}(9) doesn't cause a > link error in case device pci isn't present. One example of such a > PCI device driver compiling without device pci is sym(4). Yep, > that example is a bit bad as sym_hipd.c actually misses the pci > dependency in sys/conf/files. It's just the first driver I found > and all "simple enough" drivers for PCI devices should fall in the > same category. as you say this does not seem to be a relevant example :) And it uses #2 so it already in the form that i prefer... cheers luigi From owner-freebsd-arch@FreeBSD.ORG Wed Mar 6 08:34:24 2013 Return-Path: Delivered-To: arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 08A53CDF for ; Wed, 6 Mar 2013 08:34:24 +0000 (UTC) (envelope-from marius@alchemy.franken.de) Received: from alchemy.franken.de (alchemy.franken.de [194.94.249.214]) by mx1.freebsd.org (Postfix) with ESMTP id 8D6D69E4 for ; Wed, 6 Mar 2013 08:34:23 +0000 (UTC) Received: from alchemy.franken.de (localhost [127.0.0.1]) by alchemy.franken.de (8.14.6/8.14.6/ALCHEMY.FRANKEN.DE) with ESMTP id r268YLCM071164; Wed, 6 Mar 2013 09:34:21 +0100 (CET) (envelope-from marius@alchemy.franken.de) Received: (from marius@localhost) by alchemy.franken.de (8.14.6/8.14.6/Submit) id r268YL2R071163; Wed, 6 Mar 2013 09:34:21 +0100 (CET) (envelope-from marius) Date: Wed, 6 Mar 2013 09:34:21 +0100 From: Marius Strobl To: Luigi Rizzo Subject: Re: revising sys/conf/files* dependencies Message-ID: <20130306083420.GJ955@alchemy.franken.de> References: <20130305083817.GD13187@onelab2.iet.unipi.it> <112844CF-69C3-49A3-8581-8EF2A7DA8E8A@bsdimp.com> <20130305211953.GA51357@onelab2.iet.unipi.it> <6A93AA58-713D-4C25-A512-6927E27C5DE1@bsdimp.com> <20130306010720.GA68018@alchemy.franken.de> <20130306061119.GA77841@onelab2.iet.unipi.it> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20130306061119.GA77841@onelab2.iet.unipi.it> User-Agent: Mutt/1.5.21 (2010-09-15) Cc: arch@freebsd.org X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 06 Mar 2013 08:34:24 -0000 On Wed, Mar 06, 2013 at 07:11:19AM +0100, Luigi Rizzo wrote: > On Wed, Mar 06, 2013 at 02:07:20AM +0100, Marius Strobl wrote: > > On Tue, Mar 05, 2013 at 02:33:41PM -0700, Warner Losh wrote: > > > > > > On Mar 5, 2013, at 2:19 PM, Luigi Rizzo wrote: > > > > > > > On Tue, Mar 05, 2013 at 08:15:30AM -0700, Warner Losh wrote: > > > >> > > > >> On Mar 5, 2013, at 1:38 AM, Luigi Rizzo wrote: > > > >> > > > >>> Short Summary: > > > >>> > > > >>> I would like to revise sys/conf/files* and fix many erroneous usages of > > > >>> > > > >>> some/file.c optional foo_dev bar_bus > > > >>> > > > >>> changing them into one of the following > > > >>> > > > >>> some/file.c optional foo_dev # link error if bar_bus is missing > > > >>> some/file.cxi optional foo_dev | bar_bus # logical OR > > > >>> > > > >>> ---------- > > > >>> Full description: > > > >>> > > > >>> I always thought (wrongly) that a line of the form > > > >>> > > > >>> some/file.c optional foo bar baz # 1 > > > >>> > > > >>> in sys/conf/files* meant that file.c is compiled in if _any_ of the > > > >>> options is specified in the kernel config. But i was wrong, the > > > >>> above means that _all_ options are require, and the correct syntax > > > >>> for alternative options is > > > >>> > > > >>> some/file.c optional foo | bar | baz # 2 > > > >>> > > > >>> I believe that i am not alone in this misunderstanding, and that > > > >>> most entries in sys/conf/files* use form #1 in a wrong way, e.g.: > > > >>> > > > >>> dev/hptiop/hptiop.c optional hptiop scbus > > > >>> dev/iscsi/initiator/iscsi.c optional iscsi_initiator scbus > > > >>> dev/mfi/mfi_cam.c optional mfip scbus > > > >>> pci/viapm.c optional viapm pci > > > >>> pci/intpm.c optional intpm pci > > > >>> pci/if_rl.c optional rl pci > > > >>> (there are many many more) > > > >>> > > > >>> In all these cases, if you forget the scbus or pci in the kernel > > > >>> config, the driver is not compiled in but you only detect it at > > > >>> compile time. I'd rather be notified of the error at kernel link time. > > > > ^^^^^^^^^^^^ i meant "run time", and this is the main problem: > > > > if you forget the bus in the kernel config, the build will silently > > > > discard the entry, but you will only realize it when you actually > > > > run the kernel. Yet, having "rl" alone is surely an error, and it > > > > should be flagged as such at build time. > > > > > > Yea... You are wanting dependency checking that does not exist today, and for which the meta-data does not exist today. > > > > > > Like I said, the intent of the original feature was to disable large classes of things quickly. Nobody really does that, so that feature can go... It is likely half broken these days. > > > > > > > Uhm, according to a quick test I'd say there definitely are drivers > > missing dependencies on pci in sys/conf/files, but it certainly seems > > way better than "half broken". > > Marius, just to clarify: > > sys/conf/files has no reasonable way to express dependencies. Correct, as Warner already said, but what you propose isn't an acceptable workaround. > > Take as an example the common case (more later) > of a bus b with sources in sys/dev/b/... > and devices d1, d2, ... dn with sources in sys/dev/d*/... > All d* require b to be present in order to load. > In the modules, you express this as MODULE_DEPEND(). > > Say you want to build a kernel with "device d1" but forget > to include "device b". > > For sys/conf/files you have the following options: > > (1) silently ignore devices with missing dependencies. > You will discover the misconfiguration at run time > (or, you can consider this a feature to quickly > remove all drivers that depend on a given bus) > > sys/dev/b/b_foo.c optional b > sys/dev/b/b_bar.c optional b > ... > sys/dev/d1/d1_foo.c optional d1 b > sys/dev/d1/d1_bar.c optional d1 b > ... > sys/dev/dn/dn_foo.c optional dn b > sys/dev/dn/dn_bar_b.c optional dn b > ... > > (2) omit the dependency, causing an error at link time > > sys/dev/b/b_foo.c optional b > sys/dev/b/b_bar.c optional b > ... > sys/dev/d1/d1_foo.c optional d1 > sys/dev/d1/d1_bar.c optional d1 > ... > sys/dev/dn/dn_foo.c optional dn > sys/dev/dn/dn_bar_b.c optional dn > ... > > > (3) enforce the dependency > > sys/dev/b/b_foo.c optional b | d1 | d2 | ... | dn > sys/dev/b/b_bar.c optional b | d1 | d2 | ... | dn > ... > sys/dev/d1/d1_foo.c optional d1 > sys/dev/d1/d1_bar.c optional d1 > ... > sys/dev/dn/dn_foo.c optional dn > sys/dev/dn/dn_bar_b.c optional dn > ... > > Right now, some buses (e.g. pci) use #1; others (e.g. scbus) use #2; > very few use #3 (which as you can see becomes unmanageable > when the number of customers grows, or we have a chain of > dependencies). > > Warner gave an explanation why #1 can be useful. > Personally i prefer #2 so i can be notified sooner if my kernel > config file has a missing dependency. > My feeling is that many people (including myself up to a few > weeks ago) believe #1 behaves as #3. Like Warner I believe that most developers are very well aware how #1 actually behaves. > > As Warren mentioned, a proper solution requires replacing config (8) > with something that is better suited to the task; > but i am not suggesting to do this now, I just wanted to get > opinions on whether #2 is considered significantly more useful than #1 > to deserve a change. > > > > I'd say the proposed "fix" is only a bad kludge as it doesn't even > > solve the problem for drivers only having a PCI front-end. This is > > due to the fact that removing pci for these will only cause a link > > error iff these drivers use methods only compiled in when device pci > > is present, f.e. pci_enable_busmaster(9). However, one of the nice > > things about newbus is that the bus front-ends actually are rather > > bus-agnostic (and thus a single front-end may attach to different > > busses) and even using pci_get_{device,vendor}(9) doesn't cause a > > link error in case device pci isn't present. One example of such a > > PCI device driver compiling without device pci is sym(4). Yep, > > that example is a bit bad as sym_hipd.c actually misses the pci > > dependency in sys/conf/files. It's just the first driver I found > > and all "simple enough" drivers for PCI devices should fall in the > > same category. > > as you say this does not seem to be a relevant example :) > And it uses #2 so it already in the form that i prefer... > Wrong, it's not of your category #2 as building sym_hipd.o statically into the kernel doesn't cause a link error when "device pci" is omitted - please give it a try. As such it is a very relevant example of PCI drivers that don't use any PCI specialties like things generated from pci_if.m or functions in sys/dev/pci/*.c. For hardware busses besides PCI it's even more common that general interfaces like that of newbus are sufficient and omitting the respective "device foobus" doesn't cause a link error when building a device driver attaching to that foobus into the kernel. What's actually not relevant in your examples is scbus. This is due to the fact that "device scbus" actually means CAM, which is neither a hardware bus (well, we also use scbus to express the physical link between HBA and target, but the main thing is that it drags in all of CAM) nor newbus'ified and as such provides it's own API and ABI. Thus any HBA driver is almost guaranteed to need to call a function of CAM only present when "device scbus" is also present. Marius Marius From owner-freebsd-arch@FreeBSD.ORG Wed Mar 6 08:40:44 2013 Return-Path: Delivered-To: arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 7418BF11 for ; Wed, 6 Mar 2013 08:40:44 +0000 (UTC) (envelope-from luigi@onelab2.iet.unipi.it) Received: from onelab2.iet.unipi.it (onelab2.iet.unipi.it [131.114.59.238]) by mx1.freebsd.org (Postfix) with ESMTP id 39353A22 for ; Wed, 6 Mar 2013 08:40:44 +0000 (UTC) Received: by onelab2.iet.unipi.it (Postfix, from userid 275) id CD7167300A; Wed, 6 Mar 2013 09:41:21 +0100 (CET) Date: Wed, 6 Mar 2013 09:41:21 +0100 From: Luigi Rizzo To: Marius Strobl Subject: Re: revising sys/conf/files* dependencies Message-ID: <20130306084121.GA85685@onelab2.iet.unipi.it> References: <20130305083817.GD13187@onelab2.iet.unipi.it> <112844CF-69C3-49A3-8581-8EF2A7DA8E8A@bsdimp.com> <20130305211953.GA51357@onelab2.iet.unipi.it> <6A93AA58-713D-4C25-A512-6927E27C5DE1@bsdimp.com> <20130306010720.GA68018@alchemy.franken.de> <20130306061119.GA77841@onelab2.iet.unipi.it> <20130306083420.GJ955@alchemy.franken.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20130306083420.GJ955@alchemy.franken.de> User-Agent: Mutt/1.5.20 (2009-06-14) Cc: arch@freebsd.org X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 06 Mar 2013 08:40:44 -0000 On Wed, Mar 06, 2013 at 09:34:21AM +0100, Marius Strobl wrote: > On Wed, Mar 06, 2013 at 07:11:19AM +0100, Luigi Rizzo wrote: > > On Wed, Mar 06, 2013 at 02:07:20AM +0100, Marius Strobl wrote: ... > > Marius, just to clarify: > > > > sys/conf/files has no reasonable way to express dependencies. > > Correct, as Warner already said, but what you propose isn't an > acceptable workaround. it wasn't my intention to propose a workaround, i only preferred a more useful (in my opinion) failure mode. But given that there is little feedback and no agreement i'll take back my proposal. thanks for the comments luigi From owner-freebsd-arch@FreeBSD.ORG Wed Mar 6 17:07:18 2013 Return-Path: Delivered-To: arch@freebsd.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id B2299866 for ; Wed, 6 Mar 2013 17:07:18 +0000 (UTC) (envelope-from imp@bsdimp.com) Received: from mail-oa0-f41.google.com (mail-oa0-f41.google.com [209.85.219.41]) by mx1.freebsd.org (Postfix) with ESMTP id 72112831 for ; Wed, 6 Mar 2013 17:07:18 +0000 (UTC) Received: by mail-oa0-f41.google.com with SMTP id i10so13089377oag.14 for ; Wed, 06 Mar 2013 09:07:17 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-received:sender:subject:mime-version:content-type:from :in-reply-to:date:cc:content-transfer-encoding:message-id:references :to:x-mailer:x-gm-message-state; bh=I6bd4oPZw78f9ZricNvF7yqGsI5N7XaCsqfEnSafaRw=; b=KssWorrN5OfBbsCA08lf0mMWwNidROWEfGj3F5oZYLfp+ZNWdda7S51DlT/x3CZSCT 1f+L2X3lyUjvV58PYcE3eUmlhnDKAV+n5FDHGwACMqHGcVnWysafjG6VymRP3bhWOOuS ivAznmusUXNmBvswg3J7yWDCDx7iNZ8gjIsrUayQJ8Y1XLxy1Xam9nXHzcaSOVaF5Cjl mkK53qB9w3g3jeRrz/ePf5npkyQLBPlBaMzAZdNrX1fM6ML1Cu9m5XPLuAtgThpQjQ9G 3y7a7YOVq3TaKBAX/MkFoXdZvGouQY/aNTGel3cPiFF9ZOjX9drLYynAJ0Xq4GLsJNQa dLTA== X-Received: by 10.182.98.44 with SMTP id ef12mr23372264obb.25.1362589637776; Wed, 06 Mar 2013 09:07:17 -0800 (PST) Received: from [10.30.101.53] ([209.117.142.2]) by mx.google.com with ESMTPS id t9sm38351388obk.13.2013.03.06.09.07.13 (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Wed, 06 Mar 2013 09:07:16 -0800 (PST) Sender: Warner Losh Subject: Re: revising sys/conf/files* dependencies Mime-Version: 1.0 (Apple Message framework v1085) Content-Type: text/plain; charset=us-ascii From: Warner Losh In-Reply-To: <20130306061119.GA77841@onelab2.iet.unipi.it> Date: Wed, 6 Mar 2013 10:07:10 -0700 Content-Transfer-Encoding: quoted-printable Message-Id: <9043EE12-24DA-4A7C-A067-D5813A082AA6@bsdimp.com> References: <20130305083817.GD13187@onelab2.iet.unipi.it> <112844CF-69C3-49A3-8581-8EF2A7DA8E8A@bsdimp.com> <20130305211953.GA51357@onelab2.iet.unipi.it> <6A93AA58-713D-4C25-A512-6927E27C5DE1@bsdimp.com> <20130306010720.GA68018@alchemy.franken.de> <20130306061119.GA77841@onelab2.iet.unipi.it> To: Luigi Rizzo X-Mailer: Apple Mail (2.1085) X-Gm-Message-State: ALoCoQksIzBh5o9yBHlSlMk4qk+OhQ2MRLub6loNsRE/x7uEdhbccDDSv9ncaZCmNu2OTLGFycpV Cc: arch@freebsd.org, Marius Strobl X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 06 Mar 2013 17:07:18 -0000 On Mar 5, 2013, at 11:11 PM, Luigi Rizzo wrote: > On Wed, Mar 06, 2013 at 02:07:20AM +0100, Marius Strobl wrote: >> On Tue, Mar 05, 2013 at 02:33:41PM -0700, Warner Losh wrote: >>>=20 >>> On Mar 5, 2013, at 2:19 PM, Luigi Rizzo wrote: >>>=20 >>>> On Tue, Mar 05, 2013 at 08:15:30AM -0700, Warner Losh wrote: >>>>>=20 >>>>> On Mar 5, 2013, at 1:38 AM, Luigi Rizzo wrote: >>>>>=20 >>>>>> Short Summary: >>>>>>=20 >>>>>> I would like to revise sys/conf/files* and fix many erroneous = usages of >>>>>>=20 >>>>>> some/file.c optional foo_dev bar_bus >>>>>>=20 >>>>>> changing them into one of the following >>>>>>=20 >>>>>> some/file.c optional foo_dev # link error if bar_bus = is missing >>>>>> some/file.cxi optional foo_dev | bar_bus # logical OR >>>>>>=20 >>>>>> ---------- >>>>>> Full description: >>>>>>=20 >>>>>> I always thought (wrongly) that a line of the form >>>>>>=20 >>>>>> some/file.c optional foo bar baz # 1 >>>>>>=20 >>>>>> in sys/conf/files* meant that file.c is compiled in if _any_ of = the >>>>>> options is specified in the kernel config. But i was wrong, the >>>>>> above means that _all_ options are require, and the correct = syntax >>>>>> for alternative options is >>>>>>=20 >>>>>> some/file.c optional foo | bar | baz # 2 >>>>>>=20 >>>>>> I believe that i am not alone in this misunderstanding, and that >>>>>> most entries in sys/conf/files* use form #1 in a wrong way, e.g.: >>>>>>=20 >>>>>> dev/hptiop/hptiop.c optional hptiop scbus >>>>>> dev/iscsi/initiator/iscsi.c optional iscsi_initiator scbus >>>>>> dev/mfi/mfi_cam.c optional mfip scbus >>>>>> pci/viapm.c optional viapm pci >>>>>> pci/intpm.c optional intpm pci >>>>>> pci/if_rl.c optional rl pci >>>>>> (there are many many more) >>>>>>=20 >>>>>> In all these cases, if you forget the scbus or pci in the kernel >>>>>> config, the driver is not compiled in but you only detect it at >>>>>> compile time. I'd rather be notified of the error at kernel link = time. >>>> ^^^^^^^^^^^^ i meant "run time", and this is the main problem: >>>> if you forget the bus in the kernel config, the build will silently >>>> discard the entry, but you will only realize it when you actually >>>> run the kernel. Yet, having "rl" alone is surely an error, and it >>>> should be flagged as such at build time. >>>=20 >>> Yea... You are wanting dependency checking that does not exist = today, and for which the meta-data does not exist today. >>>=20 >>> Like I said, the intent of the original feature was to disable large = classes of things quickly. Nobody really does that, so that feature can = go... It is likely half broken these days. >>>=20 >>=20 >> Uhm, according to a quick test I'd say there definitely are drivers >> missing dependencies on pci in sys/conf/files, but it certainly seems >> way better than "half broken". >=20 > Marius, just to clarify: >=20 > sys/conf/files has no reasonable way to express dependencies. >=20 > Take as an example the common case (more later) > of a bus b with sources in sys/dev/b/... > and devices d1, d2, ... dn with sources in sys/dev/d*/... > All d* require b to be present in order to load. > In the modules, you express this as MODULE_DEPEND(). >=20 > Say you want to build a kernel with "device d1" but forget > to include "device b". >=20 > For sys/conf/files you have the following options: >=20 > (1) silently ignore devices with missing dependencies. > You will discover the misconfiguration at run time > (or, you can consider this a feature to quickly > remove all drivers that depend on a given bus) >=20 > sys/dev/b/b_foo.c optional b > sys/dev/b/b_bar.c optional b > ... > sys/dev/d1/d1_foo.c optional d1 b > sys/dev/d1/d1_bar.c optional d1 b > ... > sys/dev/dn/dn_foo.c optional dn b > sys/dev/dn/dn_bar_b.c optional dn b > ... >=20 > (2) omit the dependency, causing an error at link time >=20 > sys/dev/b/b_foo.c optional b > sys/dev/b/b_bar.c optional b > ... > sys/dev/d1/d1_foo.c optional d1 > sys/dev/d1/d1_bar.c optional d1 > ... > sys/dev/dn/dn_foo.c optional dn > sys/dev/dn/dn_bar_b.c optional dn > ... >=20 >=20 > (3) enforce the dependency >=20 > sys/dev/b/b_foo.c optional b | d1 | d2 | ... | dn > sys/dev/b/b_bar.c optional b | d1 | d2 | ... | dn > ... > sys/dev/d1/d1_foo.c optional d1 > sys/dev/d1/d1_bar.c optional d1 > ... > sys/dev/dn/dn_foo.c optional dn > sys/dev/dn/dn_bar_b.c optional dn > ... >=20 > Right now, some buses (e.g. pci) use #1; others (e.g. scbus) use = #2; > very few use #3 (which as you can see becomes unmanageable > when the number of customers grows, or we have a chain of > dependencies). >=20 > Warner gave an explanation why #1 can be useful. > Personally i prefer #2 so i can be notified sooner if my kernel > config file has a missing dependency. > My feeling is that many people (including myself up to a few > weeks ago) believe #1 behaves as #3. >=20 > As Warren mentioned, a proper solution requires replacing config = (8) > with something that is better suited to the task; > but i am not suggesting to do this now, I just wanted to get > opinions on whether #2 is considered significantly more useful than = #1 > to deserve a change. Marius gets all that, I'm sure. >> I'd say the proposed "fix" is only a bad kludge as it doesn't even >> solve the problem for drivers only having a PCI front-end. This is >> due to the fact that removing pci for these will only cause a link >> error iff these drivers use methods only compiled in when device pci >> is present, f.e. pci_enable_busmaster(9). However, one of the nice >> things about newbus is that the bus front-ends actually are rather >> bus-agnostic (and thus a single front-end may attach to different >> busses) and even using pci_get_{device,vendor}(9) doesn't cause a >> link error in case device pci isn't present. One example of such a >> PCI device driver compiling without device pci is sym(4). Yep, >> that example is a bit bad as sym_hipd.c actually misses the pci >> dependency in sys/conf/files. It's just the first driver I found >> and all "simple enough" drivers for PCI devices should fall in the >> same category. >=20 > as you say this does not seem to be a relevant example :) > And it uses #2 so it already in the form that i prefer... The relevant point is that the dependencies in files.* are *LINK* = dependencies. They are not *MODULE* dependencies. The difference being = exactly what Marius is talking about: the former go through a link-time = symbol. The latter is dependent only on newbus interfaces. pccard = drivers are an example of the latter, while pci drivers are an example = of the former. Warner From owner-freebsd-arch@FreeBSD.ORG Thu Mar 7 00:05:40 2013 Return-Path: Delivered-To: arch@freebsd.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id 1726CE95; Thu, 7 Mar 2013 00:05:40 +0000 (UTC) (envelope-from ken@kdm.org) Received: from nargothrond.kdm.org (nargothrond.kdm.org [70.56.43.81]) by mx1.freebsd.org (Postfix) with ESMTP id 6BC7B2F0; Thu, 7 Mar 2013 00:05:39 +0000 (UTC) Received: from nargothrond.kdm.org (localhost [127.0.0.1]) by nargothrond.kdm.org (8.14.2/8.14.2) with ESMTP id r2705Xjb038999; Wed, 6 Mar 2013 17:05:33 -0700 (MST) (envelope-from ken@nargothrond.kdm.org) Received: (from ken@localhost) by nargothrond.kdm.org (8.14.2/8.14.2/Submit) id r2705XOL038998; Wed, 6 Mar 2013 17:05:33 -0700 (MST) (envelope-from ken) Date: Wed, 6 Mar 2013 17:05:33 -0700 From: "Kenneth D. Merry" To: fs@freebsd.org, arch@freebsd.org Subject: patches to add new stat(2) file flags Message-ID: <20130307000533.GA38950@nargothrond.kdm.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="u3/rZRmxL6MmkK24" Content-Disposition: inline User-Agent: Mutt/1.4.2i X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 07 Mar 2013 00:05:40 -0000 --u3/rZRmxL6MmkK24 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Hi folks, I have attached diffs against head for some additional stat(2) file flags. The primary purpose of these flags is to improve compatibility with CIFS, both from the client and the server side. There are more attributes supported by CIFS than the attached patch adds, but these are the attributes that ZFS supports natively. (So no changes in the on-disk format for ZFS at least.) These are also the flags that the Solaris CIFS server supports. I also implemented the flags in a way that is compatible with the way that they are implemented in the MacOS X sys/stat.h and the MacOS X version of our smbfs code. So you can, for instance, set the hidden flag with our version of smbfs just like you can in MacOS X: # ls -lao total 32 drwxr-xr-x 1 root wheel - 16384 Mar 6 15:53 . drwxr-xr-x 1 root wheel - 16384 Dec 31 1969 .. -rwxr-xr-x 1 root wheel - 0 Mar 6 15:53 foo # chflags hidden foo # ls -lao total 32 drwxr-xr-x 1 root wheel - 16384 Mar 6 15:53 . drwxr-xr-x 1 root wheel - 16384 Dec 31 1969 .. -rwxr-xr-x 1 root wheel hidden 0 Mar 6 15:53 foo But you can also set the system flag, which MacOS X doesn't yet support: # chflags system foo # ls -lao total 32 drwxr-xr-x 1 root wheel - 16384 Mar 6 15:53 . drwxr-xr-x 1 root wheel - 16384 Dec 31 1969 .. -rwxr-xr-x 1 root wheel system,hidden 0 Mar 6 15:53 foo There are other Windows/CIFS file attributes that would be good to implement, most notably the NOT_CONTENT_INDEXED flag, but I limited the list for now to avoid more invasive changes to ZFS in particular. (Since we would want to at least attempt to synchronize with Illumos on that.) My local commit message is below, it gives more details. Any objections to committing this? Change 659250 by kenm@ken.spectrabsd7 on 2013/02/27 15:02:04 Expand the use of stat(2) flags to allow storing some Windows/DOS and CIFS file attributes as BSD stat(2) flags. This work is intended to be compatible with ZFS, the Solaris CIFS server's interaction with ZFS, with MacOS X, and of course with Windows. The Windows attributes that are implemented were chosen based on the attributes that ZFS already supports. The flag values and names were chosen to be compatible with some existing flags in MacOS X. There are several new flags, and two existing flags (UF_IMMUTABLE and SF_ARCHIVED) has been implemented in a couple of new filesystems. Although it is not included in this change, __FreeBSD_version in sys/param.h should be bumped when this is committed to FreeBSD proper, since it is a system API change. The summary of the flags is as follows: UF_SYSTEM: Command line name: "system" or "usystem" ZFS name: XAT_SYSTEM, ZFS_SYSTEM Windows: FILE_ATTRIBUTE_SYSTEM This flag means that the file is used by the operating system. FreeBSD does not enforce any special handling when this flag is set. UF_SPARSE: Command line name: "sparse" or "usparse" ZFS name: XAT_SPARSE, ZFS_SPARSE Windows: FILE_ATTRIBUTE_SPARSE_FILE This flag means that the file is sparse. Although ZFS may modify this in some situations, there is not generally any special handling for this flag. UF_OFFLINE: Command line name: "offline" or "uoffline" ZFS name: XAT_OFFLINE, ZFS_OFFLINE Windows: FILE_ATTRIBUTE_OFFLINE This flag means that the file has been moved to offline storage. FreeBSD does not have any special handling for this flag. UF_REPARSE: Command line name: "reparse" or "ureparse" ZFS name: XAT_REPARSE, ZFS_REPARSE Windows: FILE_ATTRIBUTE_REPARSE_POINT This flag means that the file is a Windows reparse point. ZFS has special handling code for reparse points, but we don't currently have the other supporting infrastructure for them. UF_HIDDEN: Command line name: "hidden" or "uhidden" ZFS name: XAT_HIDDEN, ZFS_HIDDEN Windows: FILE_ATTRIBUTE_HIDDEN This flag means that the file may be excluded from a directory listing if the application honors it. FreeBSD has no special handling for this flag. The name and bit definition for UF_HIDDEN are identical to the definition in MacOS X. UF_IMMUTABLE: Command line name: "uchg", "uimmutable" ZFS name: XAT_READONLY, ZFS_READONLY Windows: FILE_ATTRIBUTE_READONLY This flag means that the file may not be modified. This is not a new flag, but where applicable it is mapped to the Windows readonly bit. ZFS and UFS now both support the flag and enforce it. The behavior of this flag is compatible with MacOS X. SF_ARCHIVED: Command line name: "arch", "archived" ZFS_NAME: XAT_ARCHIVE, ZFS_ARCHIVE Windows name: FILE_ATTRIBUTE_ARCHIVE The SF_ARCHIVED flag means that the file has been archived. The meaning is the inverse of the Windows FILE_ATTRIBUTE_ARCHIVE, and the ZFS XAT_ARCHIVE and ZFS_ARCHIVE attribute. Accordingly, we invert the internal flags in ZFS and SMBFS to get the value of SF_ARCHIVED. chflags.1: Document the new command line flag names (e.g. "system", "hidden") available to the user. strtofflags.c: Implement the mapping between the new command line flag names and new stat(2) flags. chflags.2: Document all of the new stat(2) flags, and explain the intended behavior in a little more detail. Explain how they map to Windows file attributes. Different filesystems behave differently with respect to flags, so warn the application developer to take care when using them. zfs_vnops.c: Add support for getting and setting the UF_IMMUTABLE, UF_SYSTEM, UF_HIDDEN, UF_REPARSE, UF_OFFLINE, UF_SPARSE and SF_ARCHIVED flags. All of these flags are implemented using attributes that ZFS already supports, so the on-disk format has not changed. ZFS currently doesn't allow setting the UF_REPARSE flag, and we don't really have the other infrastructure to support reparse points. As mentioned above, the ZFS semantics for the ARCHIVE flag are the same as in Windows, but are the inverse of the semantics for SF_ARCHIVED. Thus we clear SF_ARCHIVED when ZFS sets its archive attribute and vice versa. msdosfs_vnops.c: Add support for getting and setting UF_HIDDEN and UF_SYSTEM in MSDOSFS. It already supports SF_ARCHIVED. It currently supports the readonly bit with standard Unix permissions, but could be extended to support UF_IMMUTABLE. smbfs_node.c, smbfs_vnops.c: Add support for UF_HIDDEN, UF_SYSTEM, UF_IMMUTABLE and SF_ARCHIVED in SMBFS. This is similar to changes that Apple has made in their version of SMBFS (as of smb-583.8, posted on opensource.apple.com). The semantics should be the same as what Apple has done, but the implementation is different because they have made significant changes to the codebase. stat.h: Add definitions for UF_SYSTEM, UF_SPARSE, UF_OFFLINE, UF_REPARSE, and UF_HIDDEN. The definition of UF_HIDDEN is the same as the MacOS X definition. Add commented-out definitions of UF_COMPRESSED and UF_TRACKED. They are defined in MacOS X (as of 10.8.2), but we do not implement them (yet). ufs_vnops.c: Add support for getting and setting UF_HIDDEN, UF_SYSTEM, UF_SPARSE, UF_OFFLINE, and UF_REPARSE in UFS. These new flags are only stored, UFS does not take any action if the flag is set. Ken -- Kenneth Merry ken@FreeBSD.ORG --u3/rZRmxL6MmkK24 Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="file_flags_head.20130306.1.txt" --- //depot/users/kenm/FreeBSD-test3/bin/chflags/chflags.1 2013-03-04 17:51:12.000000000 -0700 +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/bin/chflags/chflags.1 2013-03-04 17:51:12.000000000 -0700 --- /tmp/tmp.49594.86 2013-03-06 16:42:43.000000000 -0700 +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/bin/chflags/chflags.1 2013-03-06 14:47:25.987128763 -0700 @@ -117,6 +117,16 @@ set the user immutable flag (owner or super-user only) .It Cm uunlnk , uunlink set the user undeletable flag (owner or super-user only) +.It Cm system , usystem +set the Windows system flag (owner or super-user only) +.It Cm sparse , usparse +set the sparse file attribute (owner or super-user only) +.It Cm offline , uoffline +set the offline file attribute (owner or super-user only) +.It Cm reparse , ureparse +set the Windows reparse point file attribute (owner or super-user only) +.It Cm hidden , uhidden +set the hidden file attribute (owner or super-user only) .El .Pp Putting the letters --- //depot/users/kenm/FreeBSD-test3/lib/libc/gen/strtofflags.c 2013-03-04 17:51:12.000000000 -0700 +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/lib/libc/gen/strtofflags.c 2013-03-04 17:51:12.000000000 -0700 --- /tmp/tmp.49594.178 2013-03-06 16:42:43.000000000 -0700 +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/lib/libc/gen/strtofflags.c 2013-03-06 15:09:32.842437917 -0700 @@ -68,7 +68,17 @@ { "nodump", 1, UF_NODUMP }, { "noopaque", 0, UF_OPAQUE }, { "nouunlnk", 0, UF_NOUNLINK }, - { "nouunlink", 0, UF_NOUNLINK } + { "nouunlink", 0, UF_NOUNLINK }, + { "nosystem", 0, UF_SYSTEM, }, + { "nousystem", 0, UF_SYSTEM, }, + { "nosparse", 0, UF_SPARSE, }, + { "nousparse", 0, UF_SPARSE, }, + { "nooffline", 0, UF_OFFLINE, }, + { "nouoffline", 0, UF_OFFLINE, }, + { "noreparse", 0, UF_REPARSE, }, + { "noureparse", 0, UF_REPARSE, }, + { "nohidden", 0, UF_HIDDEN, }, + { "nouhidden", 0, UF_HIDDEN, } }; #define nmappings (sizeof(mapping) / sizeof(mapping[0])) --- //depot/users/kenm/FreeBSD-test3/lib/libc/sys/chflags.2 2013-03-04 17:51:12.000000000 -0700 +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/lib/libc/sys/chflags.2 2013-03-04 17:51:12.000000000 -0700 --- /tmp/tmp.49594.257 2013-03-06 16:42:43.000000000 -0700 +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/lib/libc/sys/chflags.2 2013-03-06 14:49:39.357125717 -0700 @@ -75,16 +75,49 @@ Do not dump the file. .It Dv UF_IMMUTABLE The file may not be changed. +Filesystems may use this flag to maintain compatibility with the Windows and +CIFS FILE_ATTRIBUTE_READONLY attribute. .It Dv UF_APPEND The file may only be appended to. .It Dv UF_NOUNLINK The file may not be renamed or deleted. .It Dv UF_OPAQUE The directory is opaque when viewed through a union stack. +.It Dv UF_SYSTEM +The file has the Windows and CIFS FILE_ATTRIBUTE_SYSTEM attribute. +Filesystems in FreeBSD may store and display this flag, but do not provide +any special handling when it is set. +.It Dv UF_SPARSE +The file has the Windows FILE_ATTRIBUTE_SPARSE_FILE attribute. +This may also be used by a filesystem to indicate a sparse file. +.It Dv UF_OFFLINE +The file is offline, or has the Windows and CIFS FILE_ATTRIBUTE_OFFLINE +attribute. +Filesystems in FreeBSD store and display this flag, but do not provide any +special handling when it is set. +.It Dv UF_REPARSE +The file contains a Windows reparse point and has the Windows and CIFS +FILE_ATTRIBUTE_REPARSE_POINT attribute. +.It Dv UF_HIDDEN +The file may be hidden from directory listings at the application's +discretion. +The file has the Windows FILE_ATTRIBUTE_HIDDEN attribute. .It Dv SF_ARCHIVED -The file may be archived. +The file has been archived. +This flag means the opposite of the Windows and CIFS FILE_ATTRIBUTE_ARCHIVE +attribute. +That attribute means that the file should be archived, whereas +.Dv SF_ARCHIVED +means that the file has been archived. +Filesystems in FreeBSD may or may not have special handling for this flag. +For instance, ZFS tracks changes to files and will clear this bit when a +file is updated. +UFS only stores the flag, and relies on the application to change it when +needed. .It Dv SF_IMMUTABLE The file may not be changed. +This flag also indicates that the Windows ans CIFS FILE_ATTRIBUTE_READONLY +attribute is set. .It Dv SF_APPEND The file may only be appended to. .It Dv SF_NOUNLINK @@ -121,6 +154,13 @@ .Xr init 8 for details.) .Pp +The implementation of all flags is filesystem-dependent. +See the description of the +.Dv SF_ARCHIVED +flag above for one example of the differences in behavior. +Care should be exercised when writing applications to account for +support or lack of support of these flags in various filesystems. +.Pp The .Dv SF_SNAPSHOT flag is maintained by the system and cannot be toggled. --- //depot/users/kenm/FreeBSD-test3/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c 2013-03-04 17:51:12.000000000 -0700 +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c 2013-03-04 17:51:12.000000000 -0700 --- /tmp/tmp.49594.333 2013-03-06 16:42:43.000000000 -0700 +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c 2013-03-06 14:49:41.836130019 -0700 @@ -6057,23 +6057,49 @@ XVA_SET_REQ(&xvap, XAT_APPENDONLY); XVA_SET_REQ(&xvap, XAT_NOUNLINK); XVA_SET_REQ(&xvap, XAT_NODUMP); + XVA_SET_REQ(&xvap, XAT_READONLY); + XVA_SET_REQ(&xvap, XAT_ARCHIVE); + XVA_SET_REQ(&xvap, XAT_SYSTEM); + XVA_SET_REQ(&xvap, XAT_HIDDEN); + XVA_SET_REQ(&xvap, XAT_REPARSE); + XVA_SET_REQ(&xvap, XAT_OFFLINE); + XVA_SET_REQ(&xvap, XAT_SPARSE); + error = zfs_getattr(ap->a_vp, (vattr_t *)&xvap, 0, ap->a_cred, NULL); if (error != 0) return (error); /* Convert ZFS xattr into chflags. */ -#define FLAG_CHECK(fflag, xflag, xfield) do { \ - if (XVA_ISSET_RTN(&xvap, (xflag)) && (xfield) != 0) \ +#define FLAG_CHECK(fflag, xflag, xfield, invert) do { \ + if (XVA_ISSET_RTN(&xvap, (xflag)) && (xfield) != 0) { \ + if (!invert) \ + fflags |= (fflag); \ + } else if (invert) \ fflags |= (fflag); \ } while (0) FLAG_CHECK(SF_IMMUTABLE, XAT_IMMUTABLE, - xvap.xva_xoptattrs.xoa_immutable); + xvap.xva_xoptattrs.xoa_immutable, 0); FLAG_CHECK(SF_APPEND, XAT_APPENDONLY, - xvap.xva_xoptattrs.xoa_appendonly); + xvap.xva_xoptattrs.xoa_appendonly, 0); FLAG_CHECK(SF_NOUNLINK, XAT_NOUNLINK, - xvap.xva_xoptattrs.xoa_nounlink); + xvap.xva_xoptattrs.xoa_nounlink, 0); + FLAG_CHECK(SF_ARCHIVED, XAT_ARCHIVE, + xvap.xva_xoptattrs.xoa_archive, 1); FLAG_CHECK(UF_NODUMP, XAT_NODUMP, - xvap.xva_xoptattrs.xoa_nodump); + xvap.xva_xoptattrs.xoa_nodump, 0); + FLAG_CHECK(UF_IMMUTABLE, XAT_READONLY, + xvap.xva_xoptattrs.xoa_readonly, 0); + FLAG_CHECK(UF_SYSTEM, XAT_SYSTEM, + xvap.xva_xoptattrs.xoa_system, 0); + FLAG_CHECK(UF_HIDDEN, XAT_HIDDEN, + xvap.xva_xoptattrs.xoa_hidden, 0); + FLAG_CHECK(UF_REPARSE, XAT_REPARSE, + xvap.xva_xoptattrs.xoa_reparse, 0); + FLAG_CHECK(UF_OFFLINE, XAT_OFFLINE, + xvap.xva_xoptattrs.xoa_offline, 0); + FLAG_CHECK(UF_SPARSE, XAT_SPARSE, + xvap.xva_xoptattrs.xoa_sparse, 0); + #undef FLAG_CHECK *vap = xvap.xva_vattr; vap->va_flags = fflags; @@ -6111,7 +6137,16 @@ return (EOPNOTSUPP); fflags = vap->va_flags; - if ((fflags & ~(SF_IMMUTABLE|SF_APPEND|SF_NOUNLINK|UF_NODUMP)) != 0) + /* + * XXX KDM + * We need to figure out whether it makes sense to allow + * UF_REPARSE through, since we don't really have other + * facilities to handle reparse points and zfs_setattr() + * doesn't currently allow setting that attribute anyway. + */ + if ((fflags & ~(SF_IMMUTABLE|SF_APPEND|SF_ARCHIVED|SF_NOUNLINK| + UF_NODUMP|UF_IMMUTABLE|UF_SYSTEM|UF_HIDDEN|UF_REPARSE| + UF_OFFLINE|UF_SPARSE)) != 0) return (EOPNOTSUPP); /* * Unprivileged processes are not permitted to unset system @@ -6148,23 +6183,45 @@ } } -#define FLAG_CHANGE(fflag, zflag, xflag, xfield) do { \ - if (((fflags & (fflag)) && !(zflags & (zflag))) || \ - ((zflags & (zflag)) && !(fflags & (fflag)))) { \ - XVA_SET_REQ(&xvap, (xflag)); \ - (xfield) = ((fflags & (fflag)) != 0); \ +#define FLAG_CHANGE(fflag, zflag, xflag, xfield, invert) do { \ + if (!invert) { \ + if (((fflags & (fflag)) && !(zflags & (zflag))) || \ + ((zflags & (zflag)) && !(fflags & (fflag)))) { \ + XVA_SET_REQ(&xvap, (xflag)); \ + (xfield) = ((fflags & (fflag)) != 0); \ + } \ + } else { \ + if (((fflags & (fflag)) && (zflags & (zflag))) || \ + (!(zflags & (zflag)) && !(fflags & (fflag)))) { \ + XVA_SET_REQ(&xvap, (xflag)); \ + (xfield) = ((fflags & (fflag)) == 0); \ + } \ } \ } while (0) /* Convert chflags into ZFS-type flags. */ /* XXX: what about SF_SETTABLE?. */ FLAG_CHANGE(SF_IMMUTABLE, ZFS_IMMUTABLE, XAT_IMMUTABLE, - xvap.xva_xoptattrs.xoa_immutable); + xvap.xva_xoptattrs.xoa_immutable, 0); FLAG_CHANGE(SF_APPEND, ZFS_APPENDONLY, XAT_APPENDONLY, - xvap.xva_xoptattrs.xoa_appendonly); + xvap.xva_xoptattrs.xoa_appendonly, 0); FLAG_CHANGE(SF_NOUNLINK, ZFS_NOUNLINK, XAT_NOUNLINK, - xvap.xva_xoptattrs.xoa_nounlink); + xvap.xva_xoptattrs.xoa_nounlink, 0); + FLAG_CHANGE(SF_ARCHIVED, ZFS_ARCHIVE, XAT_ARCHIVE, + xvap.xva_xoptattrs.xoa_archive, 1); FLAG_CHANGE(UF_NODUMP, ZFS_NODUMP, XAT_NODUMP, - xvap.xva_xoptattrs.xoa_nodump); + xvap.xva_xoptattrs.xoa_nodump, 0); + FLAG_CHANGE(UF_IMMUTABLE, ZFS_READONLY, XAT_READONLY, + xvap.xva_xoptattrs.xoa_readonly, 0); + FLAG_CHANGE(UF_SYSTEM, ZFS_SYSTEM, XAT_SYSTEM, + xvap.xva_xoptattrs.xoa_system, 0); + FLAG_CHANGE(UF_HIDDEN, ZFS_HIDDEN, XAT_HIDDEN, + xvap.xva_xoptattrs.xoa_hidden, 0); + FLAG_CHANGE(UF_REPARSE, ZFS_REPARSE, XAT_REPARSE, + xvap.xva_xoptattrs.xoa_hidden, 0); + FLAG_CHANGE(UF_OFFLINE, ZFS_OFFLINE, XAT_OFFLINE, + xvap.xva_xoptattrs.xoa_offline, 0); + FLAG_CHANGE(UF_SPARSE, ZFS_SPARSE, XAT_SPARSE, + xvap.xva_xoptattrs.xoa_sparse, 0); #undef FLAG_CHANGE } return (zfs_setattr(vp, (vattr_t *)&xvap, 0, cred, NULL)); --- //depot/users/kenm/FreeBSD-test3/sys/fs/msdosfs/msdosfs_vnops.c 2013-03-04 17:51:12.000000000 -0700 +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/sys/fs/msdosfs/msdosfs_vnops.c 2013-03-04 17:51:12.000000000 -0700 --- /tmp/tmp.49594.370 2013-03-06 16:42:43.000000000 -0700 +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/sys/fs/msdosfs/msdosfs_vnops.c 2013-03-06 14:49:47.179130318 -0700 @@ -345,8 +345,17 @@ vap->va_birthtime.tv_nsec = 0; } vap->va_flags = 0; + /* + * The DOS Archive attribute means that a file needs to be + * archived. The BSD SF_ARCHIVED attribute means that a file has + * been archived. Thus the inversion here. + */ if ((dep->de_Attributes & ATTR_ARCHIVE) == 0) vap->va_flags |= SF_ARCHIVED; + if (dep->de_Attributes & ATTR_HIDDEN) + vap->va_flags |= UF_HIDDEN; + if (dep->de_Attributes & ATTR_SYSTEM) + vap->va_flags |= UF_SYSTEM; vap->va_gen = 0; vap->va_blocksize = pmp->pm_bpcluster; vap->va_bytes = @@ -420,12 +429,21 @@ if (error) return (error); } - if (vap->va_flags & ~SF_ARCHIVED) + if (vap->va_flags & ~(SF_ARCHIVED|UF_HIDDEN|UF_SYSTEM)) return EOPNOTSUPP; if (vap->va_flags & SF_ARCHIVED) dep->de_Attributes &= ~ATTR_ARCHIVE; else if (!(dep->de_Attributes & ATTR_DIRECTORY)) dep->de_Attributes |= ATTR_ARCHIVE; + if (vap->va_flags & UF_HIDDEN) + dep->de_Attributes |= ATTR_HIDDEN; + else + dep->de_Attributes &= ~ATTR_HIDDEN; + if (vap->va_flags & UF_SYSTEM) + dep->de_Attributes |= ATTR_SYSTEM; + else + dep->de_Attributes &= ~ATTR_SYSTEM; + dep->de_flag |= DE_MODIFIED; } --- //depot/users/kenm/FreeBSD-test3/sys/fs/smbfs/smbfs_node.c 2013-03-04 17:51:12.000000000 -0700 +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/sys/fs/smbfs/smbfs_node.c 2013-03-04 17:51:12.000000000 -0700 --- /tmp/tmp.49594.417 2013-03-06 16:42:43.000000000 -0700 +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/sys/fs/smbfs/smbfs_node.c 2013-03-06 14:49:49.571128296 -0700 @@ -370,10 +370,13 @@ if (diff > 2) /* XXX should be configurable */ return ENOENT; va->va_type = vp->v_type; /* vnode type (for create) */ + va->va_flags = 0; /* flags defined for file */ if (vp->v_type == VREG) { va->va_mode = smp->sm_file_mode; /* files access mode and type */ - if (np->n_dosattr & SMB_FA_RDONLY) + if (np->n_dosattr & SMB_FA_RDONLY) { va->va_mode &= ~(S_IWUSR|S_IWGRP|S_IWOTH); + va->va_flags |= UF_IMMUTABLE; + } } else if (vp->v_type == VDIR) { va->va_mode = smp->sm_dir_mode; /* files access mode and type */ } else @@ -390,7 +393,17 @@ va->va_mtime = np->n_mtime; va->va_atime = va->va_ctime = va->va_mtime; /* time file changed */ va->va_gen = VNOVAL; /* generation number of file */ - va->va_flags = 0; /* flags defined for file */ + if (np->n_dosattr & SMB_FA_HIDDEN) + va->va_flags |= UF_HIDDEN; + if (np->n_dosattr & SMB_FA_SYSTEM) + va->va_flags |= UF_SYSTEM; + /* + * The meaning of the SF_ARCHIVED flag is the inverse of the CIFS + * archive flag. SF_ARCHIVED means that the file has been archived. + * SMB_FA_ARCHIVED means that the file needs to be archived. + */ + if ((vp->v_type != VDIR) && ((np->n_dosattr & SMB_FA_ARCHIVE) == 0)) + va->va_flags |= SF_ARCHIVED; va->va_rdev = NODEV; /* device the special file represents */ va->va_bytes = va->va_size; /* bytes of disk space held by file */ va->va_filerev = 0; /* file modification number */ --- //depot/users/kenm/FreeBSD-test3/sys/fs/smbfs/smbfs_vnops.c 2013-03-04 17:51:12.000000000 -0700 +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/sys/fs/smbfs/smbfs_vnops.c 2013-03-04 17:51:12.000000000 -0700 --- /tmp/tmp.49594.493 2013-03-06 16:42:43.000000000 -0700 +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/sys/fs/smbfs/smbfs_vnops.c 2013-03-06 15:05:41.942127128 -0700 @@ -305,16 +305,30 @@ int old_n_dosattr; SMBVDEBUG("\n"); - if (vap->va_flags != VNOVAL) - return EOPNOTSUPP; isreadonly = (vp->v_mount->mnt_flag & MNT_RDONLY); /* * Disallow write attempts if the filesystem is mounted read-only. */ if ((vap->va_uid != (uid_t)VNOVAL || vap->va_gid != (gid_t)VNOVAL || vap->va_atime.tv_sec != VNOVAL || vap->va_mtime.tv_sec != VNOVAL || - vap->va_mode != (mode_t)VNOVAL) && isreadonly) + vap->va_mode != (mode_t)VNOVAL || vap->va_flags != VNOVAL) && + isreadonly) return EROFS; + + /* + * We only support setting five flags. Don't allow setting others. + * + * We map both SF_IMMUTABLE and UF_IMMUTABLE to SMB_FA_RDONLY for + * setting attributes. This is compatible with the MacOS X version + * of this code. SMB_FA_RDONLY translates only to UF_IMMUTABLE + * when getting attributes. + */ + if (vap->va_flags != VNOVAL) { + if (vap->va_flags & ~(UF_IMMUTABLE|UF_HIDDEN|UF_SYSTEM| + SF_ARCHIVED|SF_IMMUTABLE)) + return EINVAL; + } + scred = smbfs_malloc_scred(); smb_makescred(scred, td, ap->a_cred); if (vap->va_size != VNOVAL) { @@ -353,12 +367,47 @@ goto out; } } - if (vap->va_mode != (mode_t)VNOVAL) { + if ((vap->va_flags != VNOVAL) || (vap->va_mode != (mode_t)VNOVAL)) { old_n_dosattr = np->n_dosattr; - if (vap->va_mode & S_IWUSR) - np->n_dosattr &= ~SMB_FA_RDONLY; - else - np->n_dosattr |= SMB_FA_RDONLY; + + if (vap->va_mode != (mode_t)VNOVAL) { + if (vap->va_mode & S_IWUSR) + np->n_dosattr &= ~SMB_FA_RDONLY; + else + np->n_dosattr |= SMB_FA_RDONLY; + } + + if (vap->va_flags != VNOVAL) { + if (vap->va_flags & UF_HIDDEN) + np->n_dosattr |= SMB_FA_HIDDEN; + else + np->n_dosattr &= ~SMB_FA_HIDDEN; + + if (vap->va_flags & UF_SYSTEM) + np->n_dosattr |= SMB_FA_SYSTEM; + else + np->n_dosattr &= ~SMB_FA_SYSTEM; + + if (vap->va_flags & SF_ARCHIVED) + np->n_dosattr &= ~SMB_FA_ARCHIVE; + else + np->n_dosattr |= SMB_FA_ARCHIVE; + + /* + * We only support setting the immutable / readonly + * bit for regular files. According to comments in + * the MacOS X version of this code, supporting the + * readonly bit on directories doesn't do the same + * thing in Windows as in Unix. + */ + if (vp->v_type == VREG) { + if (vap->va_flags & (UF_IMMUTABLE|SF_IMMUTABLE)) + np->n_dosattr |= SMB_FA_RDONLY; + else + np->n_dosattr &= ~SMB_FA_RDONLY; + } + } + if (np->n_dosattr != old_n_dosattr) { error = smbfs_smb_setpattr(np, np->n_dosattr, NULL, scred); if (error) --- //depot/users/kenm/FreeBSD-test3/sys/sys/stat.h 2013-03-04 17:51:12.000000000 -0700 +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/sys/sys/stat.h 2013-03-04 17:51:12.000000000 -0700 --- /tmp/tmp.49594.563 2013-03-06 16:42:43.000000000 -0700 +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/sys/sys/stat.h 2013-03-06 15:05:45.936126193 -0700 @@ -268,6 +268,22 @@ #define UF_OPAQUE 0x00000008 /* directory is opaque wrt. union */ #define UF_NOUNLINK 0x00000010 /* file may not be removed or renamed */ /* + * These two bits are defined in MacOS X. They are not currently used in + * FreeBSD. + */ +#if 0 +#define UF_COMPRESSED 0x00000020 /* file is compressed */ +#define UF_TRACKED 0x00000040 /* renames and deletes are tracked */ +#endif + +#define UF_SYSTEM 0x00000080 /* Windows system file bit */ +#define UF_SPARSE 0x00000100 /* sparse file */ +#define UF_OFFLINE 0x00000200 /* file is offline */ +#define UF_REPARSE 0x00000400 /* Windows reparse point file bit */ +/* This is the same as the MacOS X definition of UF_HIDDEN */ +#define UF_HIDDEN 0x00008000 /* file is hidden */ + +/* * Super-user changeable flags. */ #define SF_SETTABLE 0xffff0000 /* mask of superuser changeable flags */ --- //depot/users/kenm/FreeBSD-test3/sys/ufs/ufs/ufs_vnops.c 2013-03-04 17:51:12.000000000 -0700 +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/sys/ufs/ufs/ufs_vnops.c 2013-03-04 17:51:12.000000000 -0700 --- /tmp/tmp.49594.581 2013-03-06 16:42:43.000000000 -0700 +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/sys/ufs/ufs/ufs_vnops.c 2013-03-06 15:06:27.004132409 -0700 @@ -529,8 +529,9 @@ } if (vap->va_flags != VNOVAL) { if ((vap->va_flags & ~(UF_NODUMP | UF_IMMUTABLE | UF_APPEND | - UF_OPAQUE | UF_NOUNLINK | SF_ARCHIVED | SF_IMMUTABLE | - SF_APPEND | SF_NOUNLINK | SF_SNAPSHOT)) != 0) + UF_OPAQUE | UF_NOUNLINK | UF_HIDDEN | UF_SYSTEM | + UF_SPARSE | UF_OFFLINE | UF_REPARSE | SF_ARCHIVED | + SF_IMMUTABLE | SF_APPEND | SF_NOUNLINK | SF_SNAPSHOT)) != 0) return (EOPNOTSUPP); if (vp->v_mount->mnt_flag & MNT_RDONLY) return (EROFS); --u3/rZRmxL6MmkK24-- From owner-freebsd-arch@FreeBSD.ORG Thu Mar 7 11:22:03 2013 Return-Path: Delivered-To: arch@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 48D68647; Thu, 7 Mar 2013 11:22:03 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail110.syd.optusnet.com.au (mail110.syd.optusnet.com.au [211.29.132.97]) by mx1.freebsd.org (Postfix) with ESMTP id E9A08237; Thu, 7 Mar 2013 11:22:02 +0000 (UTC) Received: from c211-30-173-106.carlnfd1.nsw.optusnet.com.au (c211-30-173-106.carlnfd1.nsw.optusnet.com.au [211.30.173.106]) by mail110.syd.optusnet.com.au (Postfix) with ESMTPS id DD323780A45; Thu, 7 Mar 2013 22:21:51 +1100 (EST) Date: Thu, 7 Mar 2013 22:21:38 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: "Kenneth D. Merry" Subject: Re: patches to add new stat(2) file flags In-Reply-To: <20130307000533.GA38950@nargothrond.kdm.org> Message-ID: <20130307214649.X981@besplex.bde.org> References: <20130307000533.GA38950@nargothrond.kdm.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.0 cv=bNdOu4CZ c=1 sm=1 a=n2O7wv11oSwA:10 a=kj9zAlcOel0A:10 a=PO7r1zJSAAAA:8 a=JzwRw_2MAAAA:8 a=YOiZBDKP_E4A:10 a=vDvlQP4Unmb3WPkJY9MA:9 a=CjuIK1q_8ugA:10 a=TEtd8y5WR3g2ypngnwZWYw==:117 Cc: arch@FreeBSD.org, fs@FreeBSD.org X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 07 Mar 2013 11:22:03 -0000 On Wed, 6 Mar 2013, Kenneth D. Merry wrote: > I have attached diffs against head for some additional stat(2) file flags. > > The primary purpose of these flags is to improve compatibility with CIFS, > both from the client and the server side. > ... > UF_IMMUTABLE: Command line name: "uchg", "uimmutable" > ZFS name: XAT_READONLY, ZFS_READONLY > Windows: FILE_ATTRIBUTE_READONLY > > This flag means that the file may not be modified. > This is not a new flag, but where applicable it is > mapped to the Windows readonly bit. ZFS and UFS > now both support the flag and enforce it. > > The behavior of this flag is compatible with MacOS X. This is incompatible with mapping the DOS read-only attribute to the non-writeable file permission in msdosfs. msdosfs does this mainly to get at least one useful file permission, but the semantics are subtly different from all of file permissions, UF_IMMUTABLE and SF_IMMUTABLE. I think it should be a new flag. > SF_ARCHIVED: Command line name: "arch", "archived" > ZFS_NAME: XAT_ARCHIVE, ZFS_ARCHIVE > Windows name: FILE_ATTRIBUTE_ARCHIVE > > The SF_ARCHIVED flag means that the file has been > archived. The meaning is the inverse of the Windows > FILE_ATTRIBUTE_ARCHIVE, and the ZFS XAT_ARCHIVE and > ZFS_ARCHIVE attribute. Accordingly, we invert the > internal flags in ZFS and SMBFS to get the value of > SF_ARCHIVED. msdosfs inverts this too. Another problem with it is that (I think) changing it is not a privileged operation in WinDOS, so it should be mapped to UF_ARCHIVE and not reversed. Since you are adding lots of flags, you could easily fix this. You made all the new flags UF_*. Even removal of SF_ARCHIVED wouldn't be missed, since ffs doesn't maintain it and no FreeBSD utility in /usr/src supports it (except to change and print it). > msdosfs_vnops.c: Add support for getting and setting > UF_HIDDEN and UF_SYSTEM in MSDOSFS. There is also a PR that maps the SYSTEM attribute to SF_IMMUTABLE. That would be useful but I think it is not orthogonal enough, and SF_IMMUTABLE is probably too strict. Mapping it to UF_SYSTEM ... > It already supports SF_ARCHIVED. It > currently supports the readonly bit with > standard Unix permissions, but could be > extended to support UF_IMMUTABLE. ... and not mapping READONLY leaves no mapping to an immutable bit. If you map READONLY to a new flag, it can still be used to give a certain amount of immutability that is weaker than [US]F_IMMUTABLE. Under FreeBSD, the immutable flags prevent changing anything, but under WinXP, I didn't notice the READONLY attribute preventing Cygwin (on ntfs and FAT32 file systems) changing things like acls and times, but that might be because Cygwin works around what it considers excessive security. Bruce From owner-freebsd-arch@FreeBSD.ORG Thu Mar 7 13:37:25 2013 Return-Path: Delivered-To: arch@FreeBSD.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id 6E213489; Thu, 7 Mar 2013 13:37:25 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail03.syd.optusnet.com.au (mail03.syd.optusnet.com.au [211.29.132.184]) by mx1.freebsd.org (Postfix) with ESMTP id C45F9A37; Thu, 7 Mar 2013 13:37:24 +0000 (UTC) Received: from c211-30-173-106.carlnfd1.nsw.optusnet.com.au (c211-30-173-106.carlnfd1.nsw.optusnet.com.au [211.30.173.106]) by mail03.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id r27DbFZ3029843 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Fri, 8 Mar 2013 00:37:15 +1100 Date: Fri, 8 Mar 2013 00:37:15 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: "Kenneth D. Merry" Subject: Re: patches to add new stat(2) file flags In-Reply-To: <20130307000533.GA38950@nargothrond.kdm.org> Message-ID: <20130307222553.P981@besplex.bde.org> References: <20130307000533.GA38950@nargothrond.kdm.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.0 cv=DdhPMYRW c=1 sm=1 a=n2O7wv11oSwA:10 a=kj9zAlcOel0A:10 a=PO7r1zJSAAAA:8 a=JzwRw_2MAAAA:8 a=YOiZBDKP_E4A:10 a=-GnrcQN2Q_ltR23H6AcA:9 a=CjuIK1q_8ugA:10 a=TEtd8y5WR3g2ypngnwZWYw==:117 Cc: arch@FreeBSD.org, fs@FreeBSD.org X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 07 Mar 2013 13:37:25 -0000 On Wed, 6 Mar 2013, Kenneth D. Merry wrote: > I have attached diffs against head for some additional stat(2) file flags. > > The primary purpose of these flags is to improve compatibility with CIFS, > both from the client and the server side. > ... I missed looking at the diffs in my previous reply. % --- //depot/users/kenm/FreeBSD-test3/bin/chflags/chflags.1 2013-03-04 17:51:12.000000000 -0700 % +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/bin/chflags/chflags.1 2013-03-04 17:51:12.000000000 -0700 % --- /tmp/tmp.49594.86 2013-03-06 16:42:43.000000000 -0700 % +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/bin/chflags/chflags.1 2013-03-06 14:47:25.987128763 -0700 % @@ -117,6 +117,16 @@ % set the user immutable flag (owner or super-user only) % .It Cm uunlnk , uunlink % set the user undeletable flag (owner or super-user only) % +.It Cm system , usystem % +set the Windows system flag (owner or super-user only) This begins unsorting of the list. It's not just a Windows flag, since it also works in DOS. "Owner or" is too strict for msdosfs, since files can only have a single owner so it is controlling access using groups is needed. I use owner root and group msdosfs for msdosfs mounts. This works for normal operations like open/read/write, but fails for most attributes including file flags. msdosfs doesn't support many attributes but this change is supposed to add support for 3 new file flags so it would be good if it didn't restrict the support to root. % +.It Cm sparse , usparse % +set the sparse file attribute (owner or super-user only) % +.It Cm offline , uoffline % +set the offline file attribute (owner or super-user only) % +.It Cm reparse , ureparse % +set the Windows reparse point file attribute (owner or super-user only) % +.It Cm hidden , uhidden % +set the hidden file attribute (owner or super-user only) The additions at the end are also internally unsorted. Previously only "opaque" and "nodump" were unsorted. They are UF flags sorted with the SF flags, and "no" in "nodump" is not ignored for the purposes of sorting. Not having "u" in the old and new UF flag names messes up the sort order (unless you add futher confusion by ignoring "u" for the purposes of sorting) and makes it harder to add SF variants of the flags. % .El % .Pp % Putting the letters % --- //depot/users/kenm/FreeBSD-test3/lib/libc/gen/strtofflags.c 2013-03-04 17:51:12.000000000 -0700 % +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/lib/libc/gen/strtofflags.c 2013-03-04 17:51:12.000000000 -0700 % --- /tmp/tmp.49594.178 2013-03-06 16:42:43.000000000 -0700 % +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/lib/libc/gen/strtofflags.c 2013-03-06 15:09:32.842437917 -0700 % @@ -68,7 +68,17 @@ % { "nodump", 1, UF_NODUMP }, % { "noopaque", 0, UF_OPAQUE }, % { "nouunlnk", 0, UF_NOUNLINK }, % - { "nouunlink", 0, UF_NOUNLINK } % + { "nouunlink", 0, UF_NOUNLINK }, % + { "nosystem", 0, UF_SYSTEM, }, % + { "nousystem", 0, UF_SYSTEM, }, % + { "nosparse", 0, UF_SPARSE, }, % + { "nousparse", 0, UF_SPARSE, }, % + { "nooffline", 0, UF_OFFLINE, }, % + { "nouoffline", 0, UF_OFFLINE, }, % + { "noreparse", 0, UF_REPARSE, }, % + { "noureparse", 0, UF_REPARSE, }, % + { "nohidden", 0, UF_HIDDEN, }, % + { "nouhidden", 0, UF_HIDDEN, } This is totally disordered too. The old table was sorted except for "nosnapshot". Another bug is that "nosnapshot" is supported here (so chflags(1) can show it), but is not documented in chflags(1). % }; % #define nmappings (sizeof(mapping) / sizeof(mapping[0])) % % --- //depot/users/kenm/FreeBSD-test3/lib/libc/sys/chflags.2 2013-03-04 17:51:12.000000000 -0700 % +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/lib/libc/sys/chflags.2 2013-03-04 17:51:12.000000000 -0700 % --- /tmp/tmp.49594.257 2013-03-06 16:42:43.000000000 -0700 % +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/lib/libc/sys/chflags.2 2013-03-06 14:49:39.357125717 -0700 % @@ -75,16 +75,49 @@ % Do not dump the file. % .It Dv UF_IMMUTABLE % The file may not be changed. % +Filesystems may use this flag to maintain compatibility with the Windows and % +CIFS FILE_ATTRIBUTE_READONLY attribute. % .It Dv UF_APPEND % The file may only be appended to. This was already totally disordered. It even has UF's before SF's, although SF's sort before UF's both lexically and logically, though not in sys/stat.h or in bits. The disorder here was apparently copied from the implementation (sys/stat.h, which is in the order of the bits, which is historical for binary compatibility) and not cleaned up like chflags(1) and strttoflags(3). % .It Dv UF_NOUNLINK % The file may not be renamed or deleted. % .It Dv UF_OPAQUE % The directory is opaque when viewed through a union stack. % +.It Dv UF_SYSTEM % +The file has the Windows and CIFS FILE_ATTRIBUTE_SYSTEM attribute. % +Filesystems in FreeBSD may store and display this flag, but do not provide % +any special handling when it is set. More disordered than before... % +.It Dv UF_HIDDEN % +The file may be hidden from directory listings at the application's % +discretion. % +The file has the Windows FILE_ATTRIBUTE_HIDDEN attribute. Not just Windows... % .It Dv SF_ARCHIVED % -The file may be archived. % +The file has been archived. % +This flag means the opposite of the Windows and CIFS FILE_ATTRIBUTE_ARCHIVE % +attribute. % +That attribute means that the file should be archived, whereas % +.Dv SF_ARCHIVED % +means that the file has been archived. WinXP uses the poor wording "avialable for archiving". This is better. % +Filesystems in FreeBSD may or may not have special handling for this flag. % +For instance, ZFS tracks changes to files and will clear this bit when a % +file is updated. % +UFS only stores the flag, and relies on the application to change it when % +needed. I think that is useless, since changing it is needed whenever the file changes, and applications can do that (short of running as daemons and watching for changes). WinXP seems to not set the flag when attributes change. I think that is a bug, and FreeBSD msdosfs does set it when attributes are changed (except for the SF_ARCHIVED attribute itself, and atimes when the atimes are set automatically). The FreeBSD behaviour is like setting the ctime for any change and is bug for bug compatible in doing this even for null changes. FreeBSD doesn't have many attributes to set, but you just added some more. I think this should be controlled by a mount option so that bug for bug compatibility with WinDOS is possible. % .It Dv SF_IMMUTABLE % The file may not be changed. % +This flag also indicates that the Windows ans CIFS FILE_ATTRIBUTE_READONLY % +attribute is set. s/ans/and/ You only mentioned UF_IMMUTABLE in your general description. Mapping READONLY to both gives even more confusing semantics, and mapping it to SF_IMMUTABLE seems too strict since for example it prevents using FreeBSD to manage the flag at high securelevels. % --- //depot/users/kenm/FreeBSD-test3/sys/fs/msdosfs/msdosfs_vnops.c 2013-03-04 17:51:12.000000000 -0700 % +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/sys/fs/msdosfs/msdosfs_vnops.c 2013-03-04 17:51:12.000000000 -0700 % --- /tmp/tmp.49594.370 2013-03-06 16:42:43.000000000 -0700 % +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/sys/fs/msdosfs/msdosfs_vnops.c 2013-03-06 14:49:47.179130318 -0700 % @@ -345,8 +345,17 @@ % vap->va_birthtime.tv_nsec = 0; % } % vap->va_flags = 0; % + /* % + * The DOS Archive attribute means that a file needs to be % + * archived. The BSD SF_ARCHIVED attribute means that a file has % + * been archived. Thus the inversion here. % + */ No need to document it again. It goes without saying that ARCHIVE != ARCHIVED. % if ((dep->de_Attributes & ATTR_ARCHIVE) == 0) % vap->va_flags |= SF_ARCHIVED; % + if (dep->de_Attributes & ATTR_HIDDEN) % + vap->va_flags |= UF_HIDDEN; % + if (dep->de_Attributes & ATTR_SYSTEM) % + vap->va_flags |= UF_SYSTEM; % vap->va_gen = 0; % vap->va_blocksize = pmp->pm_bpcluster; % vap->va_bytes = % @@ -420,12 +429,21 @@ % if (error) % return (error); % } The permissions check before this is delicate and was broken and is more broken now. It is still short-circuit to handle setting the single flag that used to be supported, and is slightly broken for that: - unprivileged user asks to set ARCHIVE by passing !SF_ARCHIVED. We allow that, although this may toggle the flag and normal semantics for SF flags is to not allow toggling. - unprivileged user asks to clear ARCHIVE by passing SF_ARCHIVED. We don't allow that. But we should allow preserving ARCHIVE if it is already clear. The bug wasn't very important when only 1 flag was supported. Now it prevents unprivileged users managing the new UF flags if ARCHIVE is clear. Fortunately, this is the unusual case. Anyway, unprivileged users can set ARCHIVE by doing some other operation. Even the chflags() operation should set ARCHIVE and thus allow further chflags()'s that now keep ARCHIVE set. Except it is very confusing if a chflags() asks for ARCHIVE to be clear. This request might be just to try to preserve the current setting and not want it if other things are changed, or it might be to purposely clear it. Changing it from set to clear should still be privileged. See the more complicated permissions check in ffs. It would be safest to duplicate most of it, to get different permissions checking for the SF and UF flags. Then decide if we want to keep allowing setting ARCHIVE without privilege. % - if (vap->va_flags & ~SF_ARCHIVED) % + if (vap->va_flags & ~(SF_ARCHIVED|UF_HIDDEN|UF_SYSTEM)) Style bugs (missing spaces around binary operator '|'). These style bugs are common in atribute handling for other fs's but were not here. % return EOPNOTSUPP; % if (vap->va_flags & SF_ARCHIVED) % dep->de_Attributes &= ~ATTR_ARCHIVE; % else if (!(dep->de_Attributes & ATTR_DIRECTORY)) % dep->de_Attributes |= ATTR_ARCHIVE; The comment before this says that we ignore attmps to set ATTR_ARCHIVED for directories. However, it is out of date. WinXP allows setting it and all the new flags for directories, and so do we. % + if (vap->va_flags & UF_HIDDEN) % + dep->de_Attributes |= ATTR_HIDDEN; % + else % + dep->de_Attributes &= ~ATTR_HIDDEN; % + if (vap->va_flags & UF_SYSTEM) % + dep->de_Attributes |= ATTR_SYSTEM; % + else % + dep->de_Attributes &= ~ATTR_SYSTEM; I thought you were adding ATTR_READONLY -> UF_IMMUTABLE here. The WinXP attrib command (at least on a FAT32 fs) doesn't allow setting or clearing ARCHIVE (even if it is already set or clear) if any of HIDDEN, READONLY or SYSTEM is already set and remains set after the command. Thus the HRS attributes act a bit like immutable flags, but subtly differently. (ffs has the quite different and worse behaviour of allowing chflags() irrespective of immutable flags being set before or after, provided there is enough privilege to change the immutable flags.) Anyway, they should all give some aspects of immutability. % + Style bug (extra blank line). % dep->de_flag |= DE_MODIFIED; % } % % --- //depot/users/kenm/FreeBSD-test3/sys/sys/stat.h 2013-03-04 17:51:12.000000000 -0700 % +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/sys/sys/stat.h 2013-03-04 17:51:12.000000000 -0700 % --- /tmp/tmp.49594.563 2013-03-06 16:42:43.000000000 -0700 % +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/sys/sys/stat.h 2013-03-06 15:05:45.936126193 -0700 % @@ -268,6 +268,22 @@ % #define UF_OPAQUE 0x00000008 /* directory is opaque wrt. union */ % #define UF_NOUNLINK 0x00000010 /* file may not be removed or renamed */ Old style bugs: inconsistent space instead of tab after #define for the 2 newer definitions. % /* % + * These two bits are defined in MacOS X. They are not currently used in % + * FreeBSD. % + */ % +#if 0 % +#define UF_COMPRESSED 0x00000020 /* file is compressed */ % +#define UF_TRACKED 0x00000040 /* renames and deletes are tracked */ % +#endif % + % +#define UF_SYSTEM 0x00000080 /* Windows system file bit */ % +#define UF_SPARSE 0x00000100 /* sparse file */ % +#define UF_OFFLINE 0x00000200 /* file is offline */ % +#define UF_REPARSE 0x00000400 /* Windows reparse point file bit */ % +/* This is the same as the MacOS X definition of UF_HIDDEN */ % +#define UF_HIDDEN 0x00008000 /* file is hidden */ New style bugs: random spaces/tabs after #define. 1 main comment not punctuated. % + % +/* % * Super-user changeable flags. % */ % #define SF_SETTABLE 0xffff0000 /* mask of superuser changeable flags */ % --- //depot/users/kenm/FreeBSD-test3/sys/ufs/ufs/ufs_vnops.c 2013-03-04 17:51:12.000000000 -0700 % +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/sys/ufs/ufs/ufs_vnops.c 2013-03-04 17:51:12.000000000 -0700 % --- /tmp/tmp.49594.581 2013-03-06 16:42:43.000000000 -0700 % +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/sys/ufs/ufs/ufs_vnops.c 2013-03-06 15:06:27.004132409 -0700 % @@ -529,8 +529,9 @@ % } % if (vap->va_flags != VNOVAL) { % if ((vap->va_flags & ~(UF_NODUMP | UF_IMMUTABLE | UF_APPEND | % - UF_OPAQUE | UF_NOUNLINK | SF_ARCHIVED | SF_IMMUTABLE | % - SF_APPEND | SF_NOUNLINK | SF_SNAPSHOT)) != 0) % + UF_OPAQUE | UF_NOUNLINK | UF_HIDDEN | UF_SYSTEM | % + UF_SPARSE | UF_OFFLINE | UF_REPARSE | SF_ARCHIVED | % + SF_IMMUTABLE | SF_APPEND | SF_NOUNLINK | SF_SNAPSHOT)) != 0) % return (EOPNOTSUPP); % if (vp->v_mount->mnt_flag & MNT_RDONLY) % return (EROFS); The random order is harder to read with more flags. I think unsupported flags should just be unsupported -- most or all of the new flags and SF_ARCHIVED too. Most need system support to work. Without system support you can only copy them from other fs's and then lose them when utilities like tar don't unsderstand them. Bruce From owner-freebsd-arch@FreeBSD.ORG Fri Mar 8 23:21:58 2013 Return-Path: Delivered-To: arch@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 8C80FC7B; Fri, 8 Mar 2013 23:21:58 +0000 (UTC) (envelope-from ken@kdm.org) Received: from nargothrond.kdm.org (nargothrond.kdm.org [70.56.43.81]) by mx1.freebsd.org (Postfix) with ESMTP id 5B64CF12; Fri, 8 Mar 2013 23:21:57 +0000 (UTC) Received: from nargothrond.kdm.org (localhost [127.0.0.1]) by nargothrond.kdm.org (8.14.2/8.14.2) with ESMTP id r28NLuMG049746; Fri, 8 Mar 2013 16:21:56 -0700 (MST) (envelope-from ken@nargothrond.kdm.org) Received: (from ken@localhost) by nargothrond.kdm.org (8.14.2/8.14.2/Submit) id r28NLt1c049745; Fri, 8 Mar 2013 16:21:55 -0700 (MST) (envelope-from ken) Date: Fri, 8 Mar 2013 16:21:55 -0700 From: "Kenneth D. Merry" To: Bruce Evans Subject: Re: patches to add new stat(2) file flags Message-ID: <20130308232155.GA47062@nargothrond.kdm.org> References: <20130307000533.GA38950@nargothrond.kdm.org> <20130307222553.P981@besplex.bde.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="Q68bSM7Ycu6FN28Q" Content-Disposition: inline In-Reply-To: <20130307222553.P981@besplex.bde.org> User-Agent: Mutt/1.4.2i Cc: arch@FreeBSD.org, fs@FreeBSD.org X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 08 Mar 2013 23:21:58 -0000 --Q68bSM7Ycu6FN28Q Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Fri, Mar 08, 2013 at 00:37:15 +1100, Bruce Evans wrote: > On Wed, 6 Mar 2013, Kenneth D. Merry wrote: > > >I have attached diffs against head for some additional stat(2) file flags. > > > >The primary purpose of these flags is to improve compatibility with CIFS, > >both from the client and the server side. > >... > > I missed looking at the diffs in my previous reply. > > % --- //depot/users/kenm/FreeBSD-test3/bin/chflags/chflags.1 2013-03-04 > 17:51:12.000000000 -0700 > % +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/bin/chflags/chflags.1 > 2013-03-04 17:51:12.000000000 -0700 > % --- /tmp/tmp.49594.86 2013-03-06 16:42:43.000000000 -0700 > % +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/bin/chflags/chflags.1 > 2013-03-06 14:47:25.987128763 -0700 > % @@ -117,6 +117,16 @@ > % set the user immutable flag (owner or super-user only) > % .It Cm uunlnk , uunlink > % set the user undeletable flag (owner or super-user only) > % +.It Cm system , usystem > % +set the Windows system flag (owner or super-user only) > > This begins unsorting of the list. Fixed. > It's not just a Windows flag, since it also works in DOS. Fixed. > "Owner or" is too strict for msdosfs, since files can only have a > single owner so it is controlling access using groups is needed. I > use owner root and group msdosfs for msdosfs mounts. This works for > normal operations like open/read/write, but fails for most attributes > including file flags. msdosfs doesn't support many attributes but > this change is supposed to add support for 3 new file flags so it would > be good if it didn't restrict the support to root. I wasn't trying to change the existing security model for msdosfs, but if you've got a suggested patch to fix it I can add that in. > % +.It Cm sparse , usparse > % +set the sparse file attribute (owner or super-user only) > % +.It Cm offline , uoffline > % +set the offline file attribute (owner or super-user only) > % +.It Cm reparse , ureparse > % +set the Windows reparse point file attribute (owner or super-user only) > % +.It Cm hidden , uhidden > % +set the hidden file attribute (owner or super-user only) > > The additions at the end are also internally unsorted. Fixed. > Previously only "opaque" and "nodump" were unsorted. They are UF flags > sorted with the SF flags, and "no" in "nodump" is not ignored for the > purposes of sorting. > > Not having "u" in the old and new UF flag names messes up the sort order > (unless you add futher confusion by ignoring "u" for the purposes of > sorting) and makes it harder to add SF variants of the flags. They're now sorted in alphabetical order. > % .El > % .Pp > % Putting the letters > % --- //depot/users/kenm/FreeBSD-test3/lib/libc/gen/strtofflags.c > 2013-03-04 17:51:12.000000000 -0700 > % +++ > /usr/home/kenm/perforce4/kenm/FreeBSD-test3/lib/libc/gen/strtofflags.c > 2013-03-04 17:51:12.000000000 -0700 > % --- /tmp/tmp.49594.178 2013-03-06 16:42:43.000000000 -0700 > % +++ > /usr/home/kenm/perforce4/kenm/FreeBSD-test3/lib/libc/gen/strtofflags.c > 2013-03-06 15:09:32.842437917 -0700 > % @@ -68,7 +68,17 @@ > % { "nodump", 1, UF_NODUMP }, > % { "noopaque", 0, UF_OPAQUE }, > % { "nouunlnk", 0, UF_NOUNLINK }, > % - { "nouunlink", 0, UF_NOUNLINK } > % + { "nouunlink", 0, UF_NOUNLINK }, > % + { "nosystem", 0, UF_SYSTEM, }, > % + { "nousystem", 0, UF_SYSTEM, }, > % + { "nosparse", 0, UF_SPARSE, }, > % + { "nousparse", 0, UF_SPARSE, }, > % + { "nooffline", 0, UF_OFFLINE, }, > % + { "nouoffline", 0, UF_OFFLINE, }, > % + { "noreparse", 0, UF_REPARSE, }, > % + { "noureparse", 0, UF_REPARSE, }, > % + { "nohidden", 0, UF_HIDDEN, }, > % + { "nouhidden", 0, UF_HIDDEN, } > > This is totally disordered too. > > The old table was sorted except for "nosnapshot". Another bug is that > "nosnapshot" is supported here (so chflags(1) can show it), but is not > documented in chflags(1). Actually, I think the table was previously sorted by the stat(2) flag name, and UF_NOUNLINK appears to be the only one that was out of place. I have fixed that and my additions. > % }; > % #define nmappings (sizeof(mapping) / sizeof(mapping[0])) > % > % --- //depot/users/kenm/FreeBSD-test3/lib/libc/sys/chflags.2 2013-03-04 > 17:51:12.000000000 -0700 > % +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/lib/libc/sys/chflags.2 > 2013-03-04 17:51:12.000000000 -0700 > % --- /tmp/tmp.49594.257 2013-03-06 16:42:43.000000000 -0700 > % +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/lib/libc/sys/chflags.2 > 2013-03-06 14:49:39.357125717 -0700 > % @@ -75,16 +75,49 @@ > % Do not dump the file. > % .It Dv UF_IMMUTABLE > % The file may not be changed. > % +Filesystems may use this flag to maintain compatibility with the Windows > and > % +CIFS FILE_ATTRIBUTE_READONLY attribute. > % .It Dv UF_APPEND > % The file may only be appended to. > > This was already totally disordered. It even has UF's before SF's, although > SF's sort before UF's both lexically and logically, though not in sys/stat.h > or in bits. > > The disorder here was apparently copied from the implementation > (sys/stat.h, which is in the order of the bits, which is historical > for binary compatibility) and not cleaned up like chflags(1) and > strttoflags(3). Fixed. > % .It Dv UF_NOUNLINK > % The file may not be renamed or deleted. > % .It Dv UF_OPAQUE > % The directory is opaque when viewed through a union stack. > % +.It Dv UF_SYSTEM > % +The file has the Windows and CIFS FILE_ATTRIBUTE_SYSTEM attribute. > % +Filesystems in FreeBSD may store and display this flag, but do not > provide > % +any special handling when it is set. > > More disordered than before... > > % +.It Dv UF_HIDDEN > % +The file may be hidden from directory listings at the application's > % +discretion. > % +The file has the Windows FILE_ATTRIBUTE_HIDDEN attribute. > > Not just Windows... Fixed. > % .It Dv SF_ARCHIVED > % -The file may be archived. > % +The file has been archived. > % +This flag means the opposite of the Windows and CIFS > FILE_ATTRIBUTE_ARCHIVE > % +attribute. > % +That attribute means that the file should be archived, whereas > % +.Dv SF_ARCHIVED > % +means that the file has been archived. > > WinXP uses the poor wording "avialable for archiving". This is better. > > % +Filesystems in FreeBSD may or may not have special handling for this > flag. > % +For instance, ZFS tracks changes to files and will clear this bit when a > % +file is updated. > % +UFS only stores the flag, and relies on the application to change it when > % +needed. > > I think that is useless, since changing it is needed whenever the file > changes, and applications can do that (short of running as daemons and > watching for changes). Do you mean applications can't do that or can? > WinXP seems to not set the flag when attributes change. I think that is > a bug, and FreeBSD msdosfs does set it when attributes are changed (except > for the SF_ARCHIVED attribute itself, and atimes when the atimes are set > automatically). The FreeBSD behaviour is like setting the ctime for any > change and is bug for bug compatible in doing this even for null changes. > FreeBSD doesn't have many attributes to set, but you just added some more. > I think this should be controlled by a mount option so that bug for bug > compatibility with WinDOS is possible. > > % .It Dv SF_IMMUTABLE > % The file may not be changed. > % +This flag also indicates that the Windows ans CIFS > FILE_ATTRIBUTE_READONLY > % +attribute is set. > > s/ans/and/ > > You only mentioned UF_IMMUTABLE in your general description. Mapping > READONLY to both gives even more confusing semantics, and mapping it to > SF_IMMUTABLE seems too strict since for example it prevents using FreeBSD > to manage the flag at high securelevels. I agree. That man page change was left over from an earlier version of the code. I took it out. > % --- //depot/users/kenm/FreeBSD-test3/sys/fs/msdosfs/msdosfs_vnops.c > 2013-03-04 17:51:12.000000000 -0700 > % +++ > /usr/home/kenm/perforce4/kenm/FreeBSD-test3/sys/fs/msdosfs/msdosfs_vnops.c > 2013-03-04 17:51:12.000000000 -0700 > % --- /tmp/tmp.49594.370 2013-03-06 16:42:43.000000000 -0700 > % +++ > /usr/home/kenm/perforce4/kenm/FreeBSD-test3/sys/fs/msdosfs/msdosfs_vnops.c > 2013-03-06 14:49:47.179130318 -0700 > % @@ -345,8 +345,17 @@ > % vap->va_birthtime.tv_nsec = 0; > % } > % vap->va_flags = 0; > % + /* > % + * The DOS Archive attribute means that a file needs to be > % + * archived. The BSD SF_ARCHIVED attribute means that a file has > % + * been archived. Thus the inversion here. > % + */ > > No need to document it again. It goes without saying that ARCHIVE > != ARCHIVED. I disagree. It wasn't immediately obvious to me that SF_ARCHIVED was generally used as the inverse of the DOS Archived bit until I started digging into this. If this helps anyone figure that out more quickly, it's useful. > % if ((dep->de_Attributes & ATTR_ARCHIVE) == 0) > % vap->va_flags |= SF_ARCHIVED; > % + if (dep->de_Attributes & ATTR_HIDDEN) > % + vap->va_flags |= UF_HIDDEN; > % + if (dep->de_Attributes & ATTR_SYSTEM) > % + vap->va_flags |= UF_SYSTEM; > % vap->va_gen = 0; > % vap->va_blocksize = pmp->pm_bpcluster; > % vap->va_bytes = > % @@ -420,12 +429,21 @@ > % if (error) > % return (error); > % } > > The permissions check before this is delicate and was broken and is > more broken now. It is still short-circuit to handle setting the > single flag that used to be supported, and is slightly broken for that: > - unprivileged user asks to set ARCHIVE by passing !SF_ARCHIVED. We > allow that, although this may toggle the flag and normal semantics > for SF flags is to not allow toggling. > - unprivileged user asks to clear ARCHIVE by passing SF_ARCHIVED. We > don't allow that. But we should allow preserving ARCHIVE if it is > already clear. > The bug wasn't very important when only 1 flag was supported. Now it > prevents unprivileged users managing the new UF flags if ARCHIVE is > clear. Fortunately, this is the unusual case. Anyway, unprivileged > users can set ARCHIVE by doing some other operation. Even the chflags() > operation should set ARCHIVE and thus allow further chflags()'s that now > keep ARCHIVE set. Except it is very confusing if a chflags() asks for > ARCHIVE to be clear. This request might be just to try to preserve > the current setting and not want it if other things are changed, or > it might be to purposely clear it. Changing it from set to clear should > still be privileged. I changed it to allow setting or clearing SF_ARCHIVED. Now I can set or clear the flag as non-root: [root@doc-sd /testpool]# mount_msdosfs -u operator -g operator /dev/md0 /mnt [root@doc-sd /testpool]# cd /mnt [root@doc-sd /mnt]# ls -lao total 20 drwxr-xr-x 1 operator operator arch 16384 Jan 1 1980 . drwxr-xr-x 27 root wheel - 1024 Mar 8 17:05 .. -rwxr-xr-x 1 operator operator - 0 Mar 8 16:54 foo [root@doc-sd /mnt]# su operator [operator@doc-sd /mnt]$ chflags arch foo [operator@doc-sd /mnt]$ ls -lao total 20 drwxr-xr-x 1 operator operator arch 16384 Jan 1 1980 . drwxr-xr-x 27 root wheel - 1024 Mar 8 17:05 .. -rwxr-xr-x 1 operator operator arch 0 Mar 8 16:54 foo [operator@doc-sd /mnt]$ chflags 0 foo [operator@doc-sd /mnt]$ ls -lao total 20 drwxr-xr-x 1 operator operator arch 16384 Jan 1 1980 . drwxr-xr-x 27 root wheel - 1024 Mar 8 17:05 .. -rwxr-xr-x 1 operator operator - 0 Mar 8 16:54 foo > See the more complicated permissions check in ffs. It would be safest > to duplicate most of it, to get different permissions checking for the > SF and UF flags. Then decide if we want to keep allowing setting > ARCHIVE without privilege. I think we should allow getting and setting SF_ARCHIVED without special privileges. Given how it is generally used, I don't think it should be restricted to the super-user. Can you provide some code demonstrating how the permissions code should be changed in msdosfs? I don't know that much about that sort of thing, so I'll probably spend an inordinate amount of time stumbling through it. > % - if (vap->va_flags & ~SF_ARCHIVED) > % + if (vap->va_flags & ~(SF_ARCHIVED|UF_HIDDEN|UF_SYSTEM)) > > Style bugs (missing spaces around binary operator '|'). These style > bugs are common in atribute handling for other fs's but were not here. Fixed. > % return EOPNOTSUPP; > % if (vap->va_flags & SF_ARCHIVED) > % dep->de_Attributes &= ~ATTR_ARCHIVE; > % else if (!(dep->de_Attributes & ATTR_DIRECTORY)) > % dep->de_Attributes |= ATTR_ARCHIVE; > > The comment before this says that we ignore attmps to set ATTR_ARCHIVED > for directories. However, it is out of date. WinXP allows setting it > and all the new flags for directories, and so do we. Do you mean we allow setting it in UFS, or where? Obviously the code above won't set it on a directory. > % + if (vap->va_flags & UF_HIDDEN) > % + dep->de_Attributes |= ATTR_HIDDEN; > % + else > % + dep->de_Attributes &= ~ATTR_HIDDEN; > % + if (vap->va_flags & UF_SYSTEM) > % + dep->de_Attributes |= ATTR_SYSTEM; > % + else > % + dep->de_Attributes &= ~ATTR_SYSTEM; > > I thought you were adding ATTR_READONLY -> UF_IMMUTABLE here. No, I said in the commit message that it could be extended to map ATTR_READONLY to UF_IMMUTABLE, not that I actually did that. I didn't make the change because of the existing code to map readonly to standard Unix permissions. > The WinXP attrib command (at least on a FAT32 fs) doesn't allow setting > or clearing ARCHIVE (even if it is already set or clear) if any of > HIDDEN, READONLY or SYSTEM is already set and remains set after the > command. Thus the HRS attributes act a bit like immutable flags, but > subtly differently. (ffs has the quite different and worse behaviour > of allowing chflags() irrespective of immutable flags being set before > or after, provided there is enough privilege to change the immutable > flags.) Anyway, they should all give some aspects of immutability. We could do that for msdosfs, but why add more things for the user to trip over given how the filesystem is typically used? Most people probably use it for USB thumb drives these days. Or perahps on a dual boot system to access their Windows partition. > % + > > Style bug (extra blank line). Fixed. > % dep->de_flag |= DE_MODIFIED; > % } > % > % --- //depot/users/kenm/FreeBSD-test3/sys/sys/stat.h 2013-03-04 > 17:51:12.000000000 -0700 > % +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/sys/sys/stat.h 2013-03-04 > 17:51:12.000000000 -0700 > % --- /tmp/tmp.49594.563 2013-03-06 16:42:43.000000000 -0700 > % +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/sys/sys/stat.h 2013-03-06 > 15:05:45.936126193 -0700 > % @@ -268,6 +268,22 @@ > % #define UF_OPAQUE 0x00000008 /* directory is opaque wrt. union */ > % #define UF_NOUNLINK 0x00000010 /* file may not be removed or > renamed */ > > Old style bugs: inconsistent space instead of tab after #define for the 2 > newer definitions. Fixed. > % /* > % + * These two bits are defined in MacOS X. They are not currently used in > % + * FreeBSD. > % + */ > % +#if 0 > % +#define UF_COMPRESSED 0x00000020 /* file is compressed */ > % +#define UF_TRACKED 0x00000040 /* renames and deletes are > tracked */ > % +#endif > % + > % +#define UF_SYSTEM 0x00000080 /* Windows system file bit */ > % +#define UF_SPARSE 0x00000100 /* sparse file */ > % +#define UF_OFFLINE 0x00000200 /* file is offline */ > % +#define UF_REPARSE 0x00000400 /* Windows reparse point file bit */ > % +/* This is the same as the MacOS X definition of UF_HIDDEN */ > % +#define UF_HIDDEN 0x00008000 /* file is hidden */ > > New style bugs: random spaces/tabs after #define. 1 main comment not > punctuated. Fixed. > % + > % +/* > % * Super-user changeable flags. > % */ > % #define SF_SETTABLE 0xffff0000 /* mask of superuser > changeable flags */ > % --- //depot/users/kenm/FreeBSD-test3/sys/ufs/ufs/ufs_vnops.c 2013-03-04 > 17:51:12.000000000 -0700 > % +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/sys/ufs/ufs/ufs_vnops.c > 2013-03-04 17:51:12.000000000 -0700 > % --- /tmp/tmp.49594.581 2013-03-06 16:42:43.000000000 -0700 > % +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/sys/ufs/ufs/ufs_vnops.c > 2013-03-06 15:06:27.004132409 -0700 > % @@ -529,8 +529,9 @@ > % } > % if (vap->va_flags != VNOVAL) { > % if ((vap->va_flags & ~(UF_NODUMP | UF_IMMUTABLE | UF_APPEND | > % - UF_OPAQUE | UF_NOUNLINK | SF_ARCHIVED | SF_IMMUTABLE | > % - SF_APPEND | SF_NOUNLINK | SF_SNAPSHOT)) != 0) > % + UF_OPAQUE | UF_NOUNLINK | UF_HIDDEN | UF_SYSTEM | > % + UF_SPARSE | UF_OFFLINE | UF_REPARSE | SF_ARCHIVED | > % + SF_IMMUTABLE | SF_APPEND | SF_NOUNLINK | SF_SNAPSHOT)) > != 0) > % return (EOPNOTSUPP); > % if (vp->v_mount->mnt_flag & MNT_RDONLY) > % return (EROFS); > > The random order is harder to read with more flags. Fixed. > I think unsupported flags should just be unsupported -- most or all of the > new flags and SF_ARCHIVED too. Most need system support to work. Without > system support you can only copy them from other fs's and then lose them > when utilities like tar don't unsderstand them. The primary reason I added these flags to UFS is to support CIFS servers. I have modifications to Likewise to support storing DOS/Windows/CIFS attributes as file flags, and we could do the same for Samba. My eventual intent is to also add support to our NFS code to get/set the attributes that are allowed in the NFS 4.1 standard (hidden, system and archive are the ones I could find). So in this case, the filesystem role for some of these really is just to store it for someone else. That is the way that ZFS handles attributes like hidden and system. It doesn't do anything with them, they're just used by the Solaris CIFS server. Ken -- Kenneth Merry ken@FreeBSD.ORG --Q68bSM7Ycu6FN28Q Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="file_flags_head.20130308.1.txt" *** src/bin/chflags/chflags.1.orig --- src/bin/chflags/chflags.1 *************** *** 101,120 **** .Bl -tag -offset indent -width ".Cm opaque" .It Cm arch , archived set the archived flag (super-user only) .It Cm opaque set the opaque flag (owner or super-user only) - .It Cm nodump - set the nodump flag (owner or super-user only) .It Cm sappnd , sappend set the system append-only flag (super-user only) .It Cm schg , schange , simmutable set the system immutable flag (super-user only) .It Cm sunlnk , sunlink set the system undeletable flag (super-user only) .It Cm uappnd , uappend set the user append-only flag (owner or super-user only) .It Cm uchg , uchange , uimmutable set the user immutable flag (owner or super-user only) .It Cm uunlnk , uunlink set the user undeletable flag (owner or super-user only) .El --- 101,132 ---- .Bl -tag -offset indent -width ".Cm opaque" .It Cm arch , archived set the archived flag (super-user only) + .It Cm nodump + set the nodump flag (owner or super-user only) .It Cm opaque set the opaque flag (owner or super-user only) .It Cm sappnd , sappend set the system append-only flag (super-user only) .It Cm schg , schange , simmutable set the system immutable flag (super-user only) + .It Cm snapshot + set the snapshot flag (most filesystems do not allow changing this flag) .It Cm sunlnk , sunlink set the system undeletable flag (super-user only) .It Cm uappnd , uappend set the user append-only flag (owner or super-user only) .It Cm uchg , uchange , uimmutable set the user immutable flag (owner or super-user only) + .It Cm uhidden , hidden + set the hidden file attribute (owner or super-user only) + .It Cm uoffline , offline + set the offline file attribute (owner or super-user only) + .It Cm usparse , sparse + set the sparse file attribute (owner or super-user only) + .It Cm usystem , system + set the DOS and Windows system flag (owner or super-user only) + .It Cm ureparse , reparse + set the Windows reparse point file attribute (owner or super-user only) .It Cm uunlnk , uunlink set the user undeletable flag (owner or super-user only) .El *** src/lib/libc/gen/strtofflags.c.orig --- src/lib/libc/gen/strtofflags.c *************** *** 62,74 **** #endif { "nouappnd", 0, UF_APPEND }, { "nouappend", 0, UF_APPEND }, { "nouchg", 0, UF_IMMUTABLE }, { "nouchange", 0, UF_IMMUTABLE }, { "nouimmutable", 0, UF_IMMUTABLE }, { "nodump", 1, UF_NODUMP }, { "noopaque", 0, UF_OPAQUE }, ! { "nouunlnk", 0, UF_NOUNLINK }, ! { "nouunlink", 0, UF_NOUNLINK } }; #define nmappings (sizeof(mapping) / sizeof(mapping[0])) --- 62,84 ---- #endif { "nouappnd", 0, UF_APPEND }, { "nouappend", 0, UF_APPEND }, + { "nohidden", 0, UF_HIDDEN, }, + { "nouhidden", 0, UF_HIDDEN, }, { "nouchg", 0, UF_IMMUTABLE }, { "nouchange", 0, UF_IMMUTABLE }, { "nouimmutable", 0, UF_IMMUTABLE }, { "nodump", 1, UF_NODUMP }, + { "nouunlnk", 0, UF_NOUNLINK }, + { "nouunlink", 0, UF_NOUNLINK }, + { "nooffline", 0, UF_OFFLINE, }, + { "nouoffline", 0, UF_OFFLINE, }, { "noopaque", 0, UF_OPAQUE }, ! { "noreparse", 0, UF_REPARSE, }, ! { "noureparse", 0, UF_REPARSE, }, ! { "nosparse", 0, UF_SPARSE, }, ! { "nousparse", 0, UF_SPARSE, }, ! { "nosystem", 0, UF_SYSTEM, }, ! { "nousystem", 0, UF_SYSTEM, } }; #define nmappings (sizeof(mapping) / sizeof(mapping[0])) *** src/lib/libc/sys/chflags.2.orig --- src/lib/libc/sys/chflags.2 *************** *** 71,96 **** the following values .Pp .Bl -tag -width ".Dv SF_IMMUTABLE" -compact -offset indent ! .It Dv UF_NODUMP ! Do not dump the file. ! .It Dv UF_IMMUTABLE ! The file may not be changed. ! .It Dv UF_APPEND The file may only be appended to. - .It Dv UF_NOUNLINK - The file may not be renamed or deleted. - .It Dv UF_OPAQUE - The directory is opaque when viewed through a union stack. .It Dv SF_ARCHIVED ! The file may be archived. .It Dv SF_IMMUTABLE The file may not be changed. - .It Dv SF_APPEND - The file may only be appended to. .It Dv SF_NOUNLINK The file may not be renamed or deleted. .It Dv SF_SNAPSHOT The file is a snapshot file. .El .Pp If one of --- 71,127 ---- the following values .Pp .Bl -tag -width ".Dv SF_IMMUTABLE" -compact -offset indent ! .It Dv SF_APPEND The file may only be appended to. .It Dv SF_ARCHIVED ! The file has been archived. ! This flag means the opposite of the Windows and CIFS FILE_ATTRIBUTE_ARCHIVE ! attribute. ! That attribute means that the file should be archived, whereas ! .Dv SF_ARCHIVED ! means that the file has been archived. ! Filesystems in FreeBSD may or may not have special handling for this flag. ! For instance, ZFS tracks changes to files and will clear this bit when a ! file is updated. ! UFS only stores the flag, and relies on the application to change it when ! needed. .It Dv SF_IMMUTABLE The file may not be changed. .It Dv SF_NOUNLINK The file may not be renamed or deleted. .It Dv SF_SNAPSHOT The file is a snapshot file. + .It Dv UF_APPEND + The file may only be appended to. + .It Dv UF_HIDDEN + The file may be hidden from directory listings at the application's + discretion. + The file has the DOS and Windows FILE_ATTRIBUTE_HIDDEN attribute. + .It Dv UF_IMMUTABLE + The file may not be changed. + Filesystems may use this flag to maintain compatibility with the Windows and + CIFS FILE_ATTRIBUTE_READONLY attribute. + .It Dv UF_NODUMP + Do not dump the file. + .It Dv UF_NOUNLINK + The file may not be renamed or deleted. + .It Dv UF_OFFLINE + The file is offline, or has the Windows and CIFS FILE_ATTRIBUTE_OFFLINE + attribute. + Filesystems in FreeBSD store and display this flag, but do not provide any + special handling when it is set. + .It Dv UF_OPAQUE + The directory is opaque when viewed through a union stack. + .It Dv UF_REPARSE + The file contains a Windows reparse point and has the Windows and CIFS + FILE_ATTRIBUTE_REPARSE_POINT attribute. + .It Dv UF_SPARSE + The file has the Windows FILE_ATTRIBUTE_SPARSE_FILE attribute. + This may also be used by a filesystem to indicate a sparse file. + .It Dv UF_SYSTEM + The file has the Windows and CIFS FILE_ATTRIBUTE_SYSTEM attribute. + Filesystems in FreeBSD may store and display this flag, but do not provide + any special handling when it is set. .El .Pp If one of *************** *** 121,126 **** --- 152,164 ---- .Xr init 8 for details.) .Pp + The implementation of all flags is filesystem-dependent. + See the description of the + .Dv SF_ARCHIVED + flag above for one example of the differences in behavior. + Care should be exercised when writing applications to account for + support or lack of support of these flags in various filesystems. + .Pp The .Dv SF_SNAPSHOT flag is maintained by the system and cannot be toggled. *** src/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c.orig --- src/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c *************** *** 6057,6079 **** XVA_SET_REQ(&xvap, XAT_APPENDONLY); XVA_SET_REQ(&xvap, XAT_NOUNLINK); XVA_SET_REQ(&xvap, XAT_NODUMP); error = zfs_getattr(ap->a_vp, (vattr_t *)&xvap, 0, ap->a_cred, NULL); if (error != 0) return (error); /* Convert ZFS xattr into chflags. */ ! #define FLAG_CHECK(fflag, xflag, xfield) do { \ ! if (XVA_ISSET_RTN(&xvap, (xflag)) && (xfield) != 0) \ fflags |= (fflag); \ } while (0) FLAG_CHECK(SF_IMMUTABLE, XAT_IMMUTABLE, ! xvap.xva_xoptattrs.xoa_immutable); FLAG_CHECK(SF_APPEND, XAT_APPENDONLY, ! xvap.xva_xoptattrs.xoa_appendonly); FLAG_CHECK(SF_NOUNLINK, XAT_NOUNLINK, ! xvap.xva_xoptattrs.xoa_nounlink); FLAG_CHECK(UF_NODUMP, XAT_NODUMP, ! xvap.xva_xoptattrs.xoa_nodump); #undef FLAG_CHECK *vap = xvap.xva_vattr; vap->va_flags = fflags; --- 6057,6105 ---- XVA_SET_REQ(&xvap, XAT_APPENDONLY); XVA_SET_REQ(&xvap, XAT_NOUNLINK); XVA_SET_REQ(&xvap, XAT_NODUMP); + XVA_SET_REQ(&xvap, XAT_READONLY); + XVA_SET_REQ(&xvap, XAT_ARCHIVE); + XVA_SET_REQ(&xvap, XAT_SYSTEM); + XVA_SET_REQ(&xvap, XAT_HIDDEN); + XVA_SET_REQ(&xvap, XAT_REPARSE); + XVA_SET_REQ(&xvap, XAT_OFFLINE); + XVA_SET_REQ(&xvap, XAT_SPARSE); + error = zfs_getattr(ap->a_vp, (vattr_t *)&xvap, 0, ap->a_cred, NULL); if (error != 0) return (error); /* Convert ZFS xattr into chflags. */ ! #define FLAG_CHECK(fflag, xflag, xfield, invert) do { \ ! if (XVA_ISSET_RTN(&xvap, (xflag)) && (xfield) != 0) { \ ! if (!invert) \ ! fflags |= (fflag); \ ! } else if (invert) \ fflags |= (fflag); \ } while (0) FLAG_CHECK(SF_IMMUTABLE, XAT_IMMUTABLE, ! xvap.xva_xoptattrs.xoa_immutable, 0); FLAG_CHECK(SF_APPEND, XAT_APPENDONLY, ! xvap.xva_xoptattrs.xoa_appendonly, 0); FLAG_CHECK(SF_NOUNLINK, XAT_NOUNLINK, ! xvap.xva_xoptattrs.xoa_nounlink, 0); ! FLAG_CHECK(SF_ARCHIVED, XAT_ARCHIVE, ! xvap.xva_xoptattrs.xoa_archive, 1); FLAG_CHECK(UF_NODUMP, XAT_NODUMP, ! xvap.xva_xoptattrs.xoa_nodump, 0); ! FLAG_CHECK(UF_IMMUTABLE, XAT_READONLY, ! xvap.xva_xoptattrs.xoa_readonly, 0); ! FLAG_CHECK(UF_SYSTEM, XAT_SYSTEM, ! xvap.xva_xoptattrs.xoa_system, 0); ! FLAG_CHECK(UF_HIDDEN, XAT_HIDDEN, ! xvap.xva_xoptattrs.xoa_hidden, 0); ! FLAG_CHECK(UF_REPARSE, XAT_REPARSE, ! xvap.xva_xoptattrs.xoa_reparse, 0); ! FLAG_CHECK(UF_OFFLINE, XAT_OFFLINE, ! xvap.xva_xoptattrs.xoa_offline, 0); ! FLAG_CHECK(UF_SPARSE, XAT_SPARSE, ! xvap.xva_xoptattrs.xoa_sparse, 0); ! #undef FLAG_CHECK *vap = xvap.xva_vattr; vap->va_flags = fflags; *************** *** 6111,6117 **** return (EOPNOTSUPP); fflags = vap->va_flags; ! if ((fflags & ~(SF_IMMUTABLE|SF_APPEND|SF_NOUNLINK|UF_NODUMP)) != 0) return (EOPNOTSUPP); /* * Unprivileged processes are not permitted to unset system --- 6137,6152 ---- return (EOPNOTSUPP); fflags = vap->va_flags; ! /* ! * XXX KDM ! * We need to figure out whether it makes sense to allow ! * UF_REPARSE through, since we don't really have other ! * facilities to handle reparse points and zfs_setattr() ! * doesn't currently allow setting that attribute anyway. ! */ ! if ((fflags & ~(SF_IMMUTABLE|SF_APPEND|SF_ARCHIVED|SF_NOUNLINK| ! UF_NODUMP|UF_IMMUTABLE|UF_SYSTEM|UF_HIDDEN|UF_REPARSE| ! UF_OFFLINE|UF_SPARSE)) != 0) return (EOPNOTSUPP); /* * Unprivileged processes are not permitted to unset system *************** *** 6148,6170 **** } } ! #define FLAG_CHANGE(fflag, zflag, xflag, xfield) do { \ ! if (((fflags & (fflag)) && !(zflags & (zflag))) || \ ! ((zflags & (zflag)) && !(fflags & (fflag)))) { \ ! XVA_SET_REQ(&xvap, (xflag)); \ ! (xfield) = ((fflags & (fflag)) != 0); \ } \ } while (0) /* Convert chflags into ZFS-type flags. */ /* XXX: what about SF_SETTABLE?. */ FLAG_CHANGE(SF_IMMUTABLE, ZFS_IMMUTABLE, XAT_IMMUTABLE, ! xvap.xva_xoptattrs.xoa_immutable); FLAG_CHANGE(SF_APPEND, ZFS_APPENDONLY, XAT_APPENDONLY, ! xvap.xva_xoptattrs.xoa_appendonly); FLAG_CHANGE(SF_NOUNLINK, ZFS_NOUNLINK, XAT_NOUNLINK, ! xvap.xva_xoptattrs.xoa_nounlink); FLAG_CHANGE(UF_NODUMP, ZFS_NODUMP, XAT_NODUMP, ! xvap.xva_xoptattrs.xoa_nodump); #undef FLAG_CHANGE } return (zfs_setattr(vp, (vattr_t *)&xvap, 0, cred, NULL)); --- 6183,6227 ---- } } ! #define FLAG_CHANGE(fflag, zflag, xflag, xfield, invert) do { \ ! if (!invert) { \ ! if (((fflags & (fflag)) && !(zflags & (zflag))) || \ ! ((zflags & (zflag)) && !(fflags & (fflag)))) { \ ! XVA_SET_REQ(&xvap, (xflag)); \ ! (xfield) = ((fflags & (fflag)) != 0); \ ! } \ ! } else { \ ! if (((fflags & (fflag)) && (zflags & (zflag))) || \ ! (!(zflags & (zflag)) && !(fflags & (fflag)))) { \ ! XVA_SET_REQ(&xvap, (xflag)); \ ! (xfield) = ((fflags & (fflag)) == 0); \ ! } \ } \ } while (0) /* Convert chflags into ZFS-type flags. */ /* XXX: what about SF_SETTABLE?. */ FLAG_CHANGE(SF_IMMUTABLE, ZFS_IMMUTABLE, XAT_IMMUTABLE, ! xvap.xva_xoptattrs.xoa_immutable, 0); FLAG_CHANGE(SF_APPEND, ZFS_APPENDONLY, XAT_APPENDONLY, ! xvap.xva_xoptattrs.xoa_appendonly, 0); FLAG_CHANGE(SF_NOUNLINK, ZFS_NOUNLINK, XAT_NOUNLINK, ! xvap.xva_xoptattrs.xoa_nounlink, 0); ! FLAG_CHANGE(SF_ARCHIVED, ZFS_ARCHIVE, XAT_ARCHIVE, ! xvap.xva_xoptattrs.xoa_archive, 1); FLAG_CHANGE(UF_NODUMP, ZFS_NODUMP, XAT_NODUMP, ! xvap.xva_xoptattrs.xoa_nodump, 0); ! FLAG_CHANGE(UF_IMMUTABLE, ZFS_READONLY, XAT_READONLY, ! xvap.xva_xoptattrs.xoa_readonly, 0); ! FLAG_CHANGE(UF_SYSTEM, ZFS_SYSTEM, XAT_SYSTEM, ! xvap.xva_xoptattrs.xoa_system, 0); ! FLAG_CHANGE(UF_HIDDEN, ZFS_HIDDEN, XAT_HIDDEN, ! xvap.xva_xoptattrs.xoa_hidden, 0); ! FLAG_CHANGE(UF_REPARSE, ZFS_REPARSE, XAT_REPARSE, ! xvap.xva_xoptattrs.xoa_hidden, 0); ! FLAG_CHANGE(UF_OFFLINE, ZFS_OFFLINE, XAT_OFFLINE, ! xvap.xva_xoptattrs.xoa_offline, 0); ! FLAG_CHANGE(UF_SPARSE, ZFS_SPARSE, XAT_SPARSE, ! xvap.xva_xoptattrs.xoa_sparse, 0); #undef FLAG_CHANGE } return (zfs_setattr(vp, (vattr_t *)&xvap, 0, cred, NULL)); *** src/sys/fs/msdosfs/msdosfs_vnops.c.orig --- src/sys/fs/msdosfs/msdosfs_vnops.c *************** *** 345,352 **** --- 345,361 ---- vap->va_birthtime.tv_nsec = 0; } vap->va_flags = 0; + /* + * The DOS Archive attribute means that a file needs to be + * archived. The BSD SF_ARCHIVED attribute means that a file has + * been archived. Thus the inversion here. + */ if ((dep->de_Attributes & ATTR_ARCHIVE) == 0) vap->va_flags |= SF_ARCHIVED; + if (dep->de_Attributes & ATTR_HIDDEN) + vap->va_flags |= UF_HIDDEN; + if (dep->de_Attributes & ATTR_SYSTEM) + vap->va_flags |= UF_SYSTEM; vap->va_gen = 0; vap->va_blocksize = pmp->pm_bpcluster; vap->va_bytes = *************** *** 415,431 **** * set ATTR_ARCHIVE for directories `cp -pr' from a more * sensible filesystem attempts it a lot. */ ! if (vap->va_flags & SF_SETTABLE) { error = priv_check_cred(cred, PRIV_VFS_SYSFLAGS, 0); if (error) return (error); } ! if (vap->va_flags & ~SF_ARCHIVED) return EOPNOTSUPP; if (vap->va_flags & SF_ARCHIVED) dep->de_Attributes &= ~ATTR_ARCHIVE; else if (!(dep->de_Attributes & ATTR_DIRECTORY)) dep->de_Attributes |= ATTR_ARCHIVE; dep->de_flag |= DE_MODIFIED; } --- 424,448 ---- * set ATTR_ARCHIVE for directories `cp -pr' from a more * sensible filesystem attempts it a lot. */ ! if (vap->va_flags & (SF_SETTABLE & ~(SF_ARCHIVED))) { error = priv_check_cred(cred, PRIV_VFS_SYSFLAGS, 0); if (error) return (error); } ! if (vap->va_flags & ~(SF_ARCHIVED | UF_HIDDEN | UF_SYSTEM)) return EOPNOTSUPP; if (vap->va_flags & SF_ARCHIVED) dep->de_Attributes &= ~ATTR_ARCHIVE; else if (!(dep->de_Attributes & ATTR_DIRECTORY)) dep->de_Attributes |= ATTR_ARCHIVE; + if (vap->va_flags & UF_HIDDEN) + dep->de_Attributes |= ATTR_HIDDEN; + else + dep->de_Attributes &= ~ATTR_HIDDEN; + if (vap->va_flags & UF_SYSTEM) + dep->de_Attributes |= ATTR_SYSTEM; + else + dep->de_Attributes &= ~ATTR_SYSTEM; dep->de_flag |= DE_MODIFIED; } *** src/sys/fs/smbfs/smbfs_node.c.orig --- src/sys/fs/smbfs/smbfs_node.c *************** *** 370,379 **** if (diff > 2) /* XXX should be configurable */ return ENOENT; va->va_type = vp->v_type; /* vnode type (for create) */ if (vp->v_type == VREG) { va->va_mode = smp->sm_file_mode; /* files access mode and type */ ! if (np->n_dosattr & SMB_FA_RDONLY) va->va_mode &= ~(S_IWUSR|S_IWGRP|S_IWOTH); } else if (vp->v_type == VDIR) { va->va_mode = smp->sm_dir_mode; /* files access mode and type */ } else --- 370,382 ---- if (diff > 2) /* XXX should be configurable */ return ENOENT; va->va_type = vp->v_type; /* vnode type (for create) */ + va->va_flags = 0; /* flags defined for file */ if (vp->v_type == VREG) { va->va_mode = smp->sm_file_mode; /* files access mode and type */ ! if (np->n_dosattr & SMB_FA_RDONLY) { va->va_mode &= ~(S_IWUSR|S_IWGRP|S_IWOTH); + va->va_flags |= UF_IMMUTABLE; + } } else if (vp->v_type == VDIR) { va->va_mode = smp->sm_dir_mode; /* files access mode and type */ } else *************** *** 390,396 **** va->va_mtime = np->n_mtime; va->va_atime = va->va_ctime = va->va_mtime; /* time file changed */ va->va_gen = VNOVAL; /* generation number of file */ ! va->va_flags = 0; /* flags defined for file */ va->va_rdev = NODEV; /* device the special file represents */ va->va_bytes = va->va_size; /* bytes of disk space held by file */ va->va_filerev = 0; /* file modification number */ --- 393,409 ---- va->va_mtime = np->n_mtime; va->va_atime = va->va_ctime = va->va_mtime; /* time file changed */ va->va_gen = VNOVAL; /* generation number of file */ ! if (np->n_dosattr & SMB_FA_HIDDEN) ! va->va_flags |= UF_HIDDEN; ! if (np->n_dosattr & SMB_FA_SYSTEM) ! va->va_flags |= UF_SYSTEM; ! /* ! * The meaning of the SF_ARCHIVED flag is the inverse of the CIFS ! * archive flag. SF_ARCHIVED means that the file has been archived. ! * SMB_FA_ARCHIVED means that the file needs to be archived. ! */ ! if ((vp->v_type != VDIR) && ((np->n_dosattr & SMB_FA_ARCHIVE) == 0)) ! va->va_flags |= SF_ARCHIVED; va->va_rdev = NODEV; /* device the special file represents */ va->va_bytes = va->va_size; /* bytes of disk space held by file */ va->va_filerev = 0; /* file modification number */ *** src/sys/fs/smbfs/smbfs_vnops.c.orig --- src/sys/fs/smbfs/smbfs_vnops.c *************** *** 305,320 **** int old_n_dosattr; SMBVDEBUG("\n"); - if (vap->va_flags != VNOVAL) - return EOPNOTSUPP; isreadonly = (vp->v_mount->mnt_flag & MNT_RDONLY); /* * Disallow write attempts if the filesystem is mounted read-only. */ if ((vap->va_uid != (uid_t)VNOVAL || vap->va_gid != (gid_t)VNOVAL || vap->va_atime.tv_sec != VNOVAL || vap->va_mtime.tv_sec != VNOVAL || ! vap->va_mode != (mode_t)VNOVAL) && isreadonly) return EROFS; scred = smbfs_malloc_scred(); smb_makescred(scred, td, ap->a_cred); if (vap->va_size != VNOVAL) { --- 305,334 ---- int old_n_dosattr; SMBVDEBUG("\n"); isreadonly = (vp->v_mount->mnt_flag & MNT_RDONLY); /* * Disallow write attempts if the filesystem is mounted read-only. */ if ((vap->va_uid != (uid_t)VNOVAL || vap->va_gid != (gid_t)VNOVAL || vap->va_atime.tv_sec != VNOVAL || vap->va_mtime.tv_sec != VNOVAL || ! vap->va_mode != (mode_t)VNOVAL || vap->va_flags != VNOVAL) && ! isreadonly) return EROFS; + + /* + * We only support setting five flags. Don't allow setting others. + * + * We map both SF_IMMUTABLE and UF_IMMUTABLE to SMB_FA_RDONLY for + * setting attributes. This is compatible with the MacOS X version + * of this code. SMB_FA_RDONLY translates only to UF_IMMUTABLE + * when getting attributes. + */ + if (vap->va_flags != VNOVAL) { + if (vap->va_flags & ~(UF_IMMUTABLE|UF_HIDDEN|UF_SYSTEM| + SF_ARCHIVED|SF_IMMUTABLE)) + return EINVAL; + } + scred = smbfs_malloc_scred(); smb_makescred(scred, td, ap->a_cred); if (vap->va_size != VNOVAL) { *************** *** 353,364 **** goto out; } } ! if (vap->va_mode != (mode_t)VNOVAL) { old_n_dosattr = np->n_dosattr; ! if (vap->va_mode & S_IWUSR) ! np->n_dosattr &= ~SMB_FA_RDONLY; ! else ! np->n_dosattr |= SMB_FA_RDONLY; if (np->n_dosattr != old_n_dosattr) { error = smbfs_smb_setpattr(np, np->n_dosattr, NULL, scred); if (error) --- 367,413 ---- goto out; } } ! if ((vap->va_flags != VNOVAL) || (vap->va_mode != (mode_t)VNOVAL)) { old_n_dosattr = np->n_dosattr; ! ! if (vap->va_mode != (mode_t)VNOVAL) { ! if (vap->va_mode & S_IWUSR) ! np->n_dosattr &= ~SMB_FA_RDONLY; ! else ! np->n_dosattr |= SMB_FA_RDONLY; ! } ! ! if (vap->va_flags != VNOVAL) { ! if (vap->va_flags & UF_HIDDEN) ! np->n_dosattr |= SMB_FA_HIDDEN; ! else ! np->n_dosattr &= ~SMB_FA_HIDDEN; ! ! if (vap->va_flags & UF_SYSTEM) ! np->n_dosattr |= SMB_FA_SYSTEM; ! else ! np->n_dosattr &= ~SMB_FA_SYSTEM; ! ! if (vap->va_flags & SF_ARCHIVED) ! np->n_dosattr &= ~SMB_FA_ARCHIVE; ! else ! np->n_dosattr |= SMB_FA_ARCHIVE; ! ! /* ! * We only support setting the immutable / readonly ! * bit for regular files. According to comments in ! * the MacOS X version of this code, supporting the ! * readonly bit on directories doesn't do the same ! * thing in Windows as in Unix. ! */ ! if (vp->v_type == VREG) { ! if (vap->va_flags & (UF_IMMUTABLE|SF_IMMUTABLE)) ! np->n_dosattr |= SMB_FA_RDONLY; ! else ! np->n_dosattr &= ~SMB_FA_RDONLY; ! } ! } ! if (np->n_dosattr != old_n_dosattr) { error = smbfs_smb_setpattr(np, np->n_dosattr, NULL, scred); if (error) *** src/sys/sys/stat.h.orig --- src/sys/sys/stat.h *************** *** 265,272 **** #define UF_NODUMP 0x00000001 /* do not dump file */ #define UF_IMMUTABLE 0x00000002 /* file may not be changed */ #define UF_APPEND 0x00000004 /* writes to file may only append */ ! #define UF_OPAQUE 0x00000008 /* directory is opaque wrt. union */ ! #define UF_NOUNLINK 0x00000010 /* file may not be removed or renamed */ /* * Super-user changeable flags. */ --- 265,288 ---- #define UF_NODUMP 0x00000001 /* do not dump file */ #define UF_IMMUTABLE 0x00000002 /* file may not be changed */ #define UF_APPEND 0x00000004 /* writes to file may only append */ ! #define UF_OPAQUE 0x00000008 /* directory is opaque wrt. union */ ! #define UF_NOUNLINK 0x00000010 /* file may not be removed or renamed */ ! /* ! * These two bits are defined in MacOS X. They are not currently used in ! * FreeBSD. ! */ ! #if 0 ! #define UF_COMPRESSED 0x00000020 /* file is compressed */ ! #define UF_TRACKED 0x00000040 /* renames and deletes are tracked */ ! #endif ! ! #define UF_SYSTEM 0x00000080 /* Windows system file bit */ ! #define UF_SPARSE 0x00000100 /* sparse file */ ! #define UF_OFFLINE 0x00000200 /* file is offline */ ! #define UF_REPARSE 0x00000400 /* Windows reparse point file bit */ ! /* This is the same as the MacOS X definition of UF_HIDDEN. */ ! #define UF_HIDDEN 0x00008000 /* file is hidden */ ! /* * Super-user changeable flags. */ *** src/sys/ufs/ufs/ufs_vnops.c.orig --- src/sys/ufs/ufs/ufs_vnops.c *************** *** 528,536 **** return (EINVAL); } if (vap->va_flags != VNOVAL) { ! if ((vap->va_flags & ~(UF_NODUMP | UF_IMMUTABLE | UF_APPEND | ! UF_OPAQUE | UF_NOUNLINK | SF_ARCHIVED | SF_IMMUTABLE | ! SF_APPEND | SF_NOUNLINK | SF_SNAPSHOT)) != 0) return (EOPNOTSUPP); if (vp->v_mount->mnt_flag & MNT_RDONLY) return (EROFS); --- 528,537 ---- return (EINVAL); } if (vap->va_flags != VNOVAL) { ! if ((vap->va_flags & ~(SF_APPEND | SF_ARCHIVED | SF_IMMUTABLE | ! SF_NOUNLINK | SF_SNAPSHOT | UF_APPEND | UF_HIDDEN | ! UF_IMMUTABLE | UF_NODUMP | UF_NOUNLINK | UF_OFFLINE | ! UF_OPAQUE | UF_REPARSE | UF_SPARSE | UF_SYSTEM)) != 0) return (EOPNOTSUPP); if (vp->v_mount->mnt_flag & MNT_RDONLY) return (EROFS); --Q68bSM7Ycu6FN28Q-- From owner-freebsd-arch@FreeBSD.ORG Sat Mar 9 00:12:53 2013 Return-Path: Delivered-To: arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 82CC47D7; Sat, 9 Mar 2013 00:12:53 +0000 (UTC) (envelope-from rmacklem@uoguelph.ca) Received: from esa-annu.net.uoguelph.ca (esa-annu.mail.uoguelph.ca [131.104.91.36]) by mx1.freebsd.org (Postfix) with ESMTP id C7BC715D; Sat, 9 Mar 2013 00:12:52 +0000 (UTC) X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: AqIEANx9OlGDaFvO/2dsb2JhbAA9BoglvDaBc3SCLAEBAQMBAQEBIAQnIAsFFhgCAg0ZAikBCRgBDQYIBwQBHASHbAYMqWGSNYEjjDgMcQEzB4ItgRMDiHGKHYEHgj6BHo9UgyhPgQU1 X-IronPort-AV: E=Sophos;i="4.84,810,1355115600"; d="scan'208";a="17831967" Received: from erie.cs.uoguelph.ca (HELO zcs3.mail.uoguelph.ca) ([131.104.91.206]) by esa-annu.net.uoguelph.ca with ESMTP; 08 Mar 2013 19:11:44 -0500 Received: from zcs3.mail.uoguelph.ca (localhost.localdomain [127.0.0.1]) by zcs3.mail.uoguelph.ca (Postfix) with ESMTP id 7CBF8B402B; Fri, 8 Mar 2013 19:11:44 -0500 (EST) Date: Fri, 8 Mar 2013 19:11:44 -0500 (EST) From: Rick Macklem To: "Kenneth D. Merry" Message-ID: <37540270.3721223.1362787904494.JavaMail.root@erie.cs.uoguelph.ca> In-Reply-To: <20130308232155.GA47062@nargothrond.kdm.org> Subject: Re: patches to add new stat(2) file flags MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Originating-IP: [172.17.91.202] X-Mailer: Zimbra 6.0.10_GA_2692 (ZimbraWebClient - FF3.0 (Win)/6.0.10_GA_2692) Cc: arch@FreeBSD.org, fs@FreeBSD.org X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 09 Mar 2013 00:12:53 -0000 Kenneth D. Merry wrote: > On Fri, Mar 08, 2013 at 00:37:15 +1100, Bruce Evans wrote: > > On Wed, 6 Mar 2013, Kenneth D. Merry wrote: > > > > >I have attached diffs against head for some additional stat(2) file > > >flags. > > > > > >The primary purpose of these flags is to improve compatibility with > > >CIFS, > > >both from the client and the server side. > > >... > > > > I missed looking at the diffs in my previous reply. > > > > % --- //depot/users/kenm/FreeBSD-test3/bin/chflags/chflags.1 > > 2013-03-04 > > 17:51:12.000000000 -0700 > > % +++ > > /usr/home/kenm/perforce4/kenm/FreeBSD-test3/bin/chflags/chflags.1 > > 2013-03-04 17:51:12.000000000 -0700 > > % --- /tmp/tmp.49594.86 2013-03-06 16:42:43.000000000 -0700 > > % +++ > > /usr/home/kenm/perforce4/kenm/FreeBSD-test3/bin/chflags/chflags.1 > > 2013-03-06 14:47:25.987128763 -0700 > > % @@ -117,6 +117,16 @@ > > % set the user immutable flag (owner or super-user only) > > % .It Cm uunlnk , uunlink > > % set the user undeletable flag (owner or super-user only) > > % +.It Cm system , usystem > > % +set the Windows system flag (owner or super-user only) > > > > This begins unsorting of the list. > > Fixed. > > > It's not just a Windows flag, since it also works in DOS. > > Fixed. > > > "Owner or" is too strict for msdosfs, since files can only have a > > single owner so it is controlling access using groups is needed. I > > use owner root and group msdosfs for msdosfs mounts. This works for > > normal operations like open/read/write, but fails for most > > attributes > > including file flags. msdosfs doesn't support many attributes but > > this change is supposed to add support for 3 new file flags so it > > would > > be good if it didn't restrict the support to root. > > I wasn't trying to change the existing security model for msdosfs, but > if > you've got a suggested patch to fix it I can add that in. > > > % +.It Cm sparse , usparse > > % +set the sparse file attribute (owner or super-user only) > > % +.It Cm offline , uoffline > > % +set the offline file attribute (owner or super-user only) > > % +.It Cm reparse , ureparse > > % +set the Windows reparse point file attribute (owner or super-user > > only) > > % +.It Cm hidden , uhidden > > % +set the hidden file attribute (owner or super-user only) > > > > The additions at the end are also internally unsorted. > > Fixed. > > > Previously only "opaque" and "nodump" were unsorted. They are UF > > flags > > sorted with the SF flags, and "no" in "nodump" is not ignored for > > the > > purposes of sorting. > > > > Not having "u" in the old and new UF flag names messes up the sort > > order > > (unless you add futher confusion by ignoring "u" for the purposes of > > sorting) and makes it harder to add SF variants of the flags. > > They're now sorted in alphabetical order. > > > % .El > > % .Pp > > % Putting the letters > > % --- //depot/users/kenm/FreeBSD-test3/lib/libc/gen/strtofflags.c > > 2013-03-04 17:51:12.000000000 -0700 > > % +++ > > /usr/home/kenm/perforce4/kenm/FreeBSD-test3/lib/libc/gen/strtofflags.c > > 2013-03-04 17:51:12.000000000 -0700 > > % --- /tmp/tmp.49594.178 2013-03-06 16:42:43.000000000 -0700 > > % +++ > > /usr/home/kenm/perforce4/kenm/FreeBSD-test3/lib/libc/gen/strtofflags.c > > 2013-03-06 15:09:32.842437917 -0700 > > % @@ -68,7 +68,17 @@ > > % { "nodump", 1, UF_NODUMP }, > > % { "noopaque", 0, UF_OPAQUE }, > > % { "nouunlnk", 0, UF_NOUNLINK }, > > % - { "nouunlink", 0, UF_NOUNLINK } > > % + { "nouunlink", 0, UF_NOUNLINK }, > > % + { "nosystem", 0, UF_SYSTEM, }, > > % + { "nousystem", 0, UF_SYSTEM, }, > > % + { "nosparse", 0, UF_SPARSE, }, > > % + { "nousparse", 0, UF_SPARSE, }, > > % + { "nooffline", 0, UF_OFFLINE, }, > > % + { "nouoffline", 0, UF_OFFLINE, }, > > % + { "noreparse", 0, UF_REPARSE, }, > > % + { "noureparse", 0, UF_REPARSE, }, > > % + { "nohidden", 0, UF_HIDDEN, }, > > % + { "nouhidden", 0, UF_HIDDEN, } > > > > This is totally disordered too. > > > > The old table was sorted except for "nosnapshot". Another bug is > > that > > "nosnapshot" is supported here (so chflags(1) can show it), but is > > not > > documented in chflags(1). > > Actually, I think the table was previously sorted by the stat(2) flag > name, > and UF_NOUNLINK appears to be the only one that was out of place. I > have > fixed that and my additions. > > > % }; > > % #define nmappings (sizeof(mapping) / sizeof(mapping[0])) > > % > > % --- //depot/users/kenm/FreeBSD-test3/lib/libc/sys/chflags.2 > > 2013-03-04 > > 17:51:12.000000000 -0700 > > % +++ > > /usr/home/kenm/perforce4/kenm/FreeBSD-test3/lib/libc/sys/chflags.2 > > 2013-03-04 17:51:12.000000000 -0700 > > % --- /tmp/tmp.49594.257 2013-03-06 16:42:43.000000000 -0700 > > % +++ > > /usr/home/kenm/perforce4/kenm/FreeBSD-test3/lib/libc/sys/chflags.2 > > 2013-03-06 14:49:39.357125717 -0700 > > % @@ -75,16 +75,49 @@ > > % Do not dump the file. > > % .It Dv UF_IMMUTABLE > > % The file may not be changed. > > % +Filesystems may use this flag to maintain compatibility with the > > Windows > > and > > % +CIFS FILE_ATTRIBUTE_READONLY attribute. > > % .It Dv UF_APPEND > > % The file may only be appended to. > > > > This was already totally disordered. It even has UF's before SF's, > > although > > SF's sort before UF's both lexically and logically, though not in > > sys/stat.h > > or in bits. > > > > The disorder here was apparently copied from the implementation > > (sys/stat.h, which is in the order of the bits, which is historical > > for binary compatibility) and not cleaned up like chflags(1) and > > strttoflags(3). > > Fixed. > > > % .It Dv UF_NOUNLINK > > % The file may not be renamed or deleted. > > % .It Dv UF_OPAQUE > > % The directory is opaque when viewed through a union stack. > > % +.It Dv UF_SYSTEM > > % +The file has the Windows and CIFS FILE_ATTRIBUTE_SYSTEM > > attribute. > > % +Filesystems in FreeBSD may store and display this flag, but do > > not > > provide > > % +any special handling when it is set. > > > > More disordered than before... > > > > % +.It Dv UF_HIDDEN > > % +The file may be hidden from directory listings at the > > application's > > % +discretion. > > % +The file has the Windows FILE_ATTRIBUTE_HIDDEN attribute. > > > > Not just Windows... > > Fixed. > > > % .It Dv SF_ARCHIVED > > % -The file may be archived. > > % +The file has been archived. > > % +This flag means the opposite of the Windows and CIFS > > FILE_ATTRIBUTE_ARCHIVE > > % +attribute. > > % +That attribute means that the file should be archived, whereas > > % +.Dv SF_ARCHIVED > > % +means that the file has been archived. > > > > WinXP uses the poor wording "avialable for archiving". This is > > better. > > > > % +Filesystems in FreeBSD may or may not have special handling for > > this > > flag. > > % +For instance, ZFS tracks changes to files and will clear this bit > > when a > > % +file is updated. > > % +UFS only stores the flag, and relies on the application to change > > it when > > % +needed. > > > > I think that is useless, since changing it is needed whenever the > > file > > changes, and applications can do that (short of running as daemons > > and > > watching for changes). > > Do you mean applications can't do that or can? > > > WinXP seems to not set the flag when attributes change. I think that > > is > > a bug, and FreeBSD msdosfs does set it when attributes are changed > > (except > > for the SF_ARCHIVED attribute itself, and atimes when the atimes are > > set > > automatically). The FreeBSD behaviour is like setting the ctime for > > any > > change and is bug for bug compatible in doing this even for null > > changes. > > FreeBSD doesn't have many attributes to set, but you just added some > > more. > > I think this should be controlled by a mount option so that bug for > > bug > > compatibility with WinDOS is possible. > > > > % .It Dv SF_IMMUTABLE > > % The file may not be changed. > > % +This flag also indicates that the Windows ans CIFS > > FILE_ATTRIBUTE_READONLY > > % +attribute is set. > > > > s/ans/and/ > > > > You only mentioned UF_IMMUTABLE in your general description. Mapping > > READONLY to both gives even more confusing semantics, and mapping it > > to > > SF_IMMUTABLE seems too strict since for example it prevents using > > FreeBSD > > to manage the flag at high securelevels. > > I agree. That man page change was left over from an earlier version of > the > code. I took it out. > > > % --- > > //depot/users/kenm/FreeBSD-test3/sys/fs/msdosfs/msdosfs_vnops.c > > 2013-03-04 17:51:12.000000000 -0700 > > % +++ > > /usr/home/kenm/perforce4/kenm/FreeBSD-test3/sys/fs/msdosfs/msdosfs_vnops.c > > 2013-03-04 17:51:12.000000000 -0700 > > % --- /tmp/tmp.49594.370 2013-03-06 16:42:43.000000000 -0700 > > % +++ > > /usr/home/kenm/perforce4/kenm/FreeBSD-test3/sys/fs/msdosfs/msdosfs_vnops.c > > 2013-03-06 14:49:47.179130318 -0700 > > % @@ -345,8 +345,17 @@ > > % vap->va_birthtime.tv_nsec = 0; > > % } > > % vap->va_flags = 0; > > % + /* > > % + * The DOS Archive attribute means that a file needs to be > > % + * archived. The BSD SF_ARCHIVED attribute means that a file has > > % + * been archived. Thus the inversion here. > > % + */ > > > > No need to document it again. It goes without saying that ARCHIVE > > != ARCHIVED. > > I disagree. It wasn't immediately obvious to me that SF_ARCHIVED was > generally used as the inverse of the DOS Archived bit until I started > digging into this. If this helps anyone figure that out more quickly, > it's > useful. > > > % if ((dep->de_Attributes & ATTR_ARCHIVE) == 0) > > % vap->va_flags |= SF_ARCHIVED; > > % + if (dep->de_Attributes & ATTR_HIDDEN) > > % + vap->va_flags |= UF_HIDDEN; > > % + if (dep->de_Attributes & ATTR_SYSTEM) > > % + vap->va_flags |= UF_SYSTEM; > > % vap->va_gen = 0; > > % vap->va_blocksize = pmp->pm_bpcluster; > > % vap->va_bytes = > > % @@ -420,12 +429,21 @@ > > % if (error) > > % return (error); > > % } > > > > The permissions check before this is delicate and was broken and is > > more broken now. It is still short-circuit to handle setting the > > single flag that used to be supported, and is slightly broken for > > that: > > - unprivileged user asks to set ARCHIVE by passing !SF_ARCHIVED. We > > allow that, although this may toggle the flag and normal semantics > > for SF flags is to not allow toggling. > > - unprivileged user asks to clear ARCHIVE by passing SF_ARCHIVED. We > > don't allow that. But we should allow preserving ARCHIVE if it is > > already clear. > > The bug wasn't very important when only 1 flag was supported. Now it > > prevents unprivileged users managing the new UF flags if ARCHIVE is > > clear. Fortunately, this is the unusual case. Anyway, unprivileged > > users can set ARCHIVE by doing some other operation. Even the > > chflags() > > operation should set ARCHIVE and thus allow further chflags()'s that > > now > > keep ARCHIVE set. Except it is very confusing if a chflags() asks > > for > > ARCHIVE to be clear. This request might be just to try to preserve > > the current setting and not want it if other things are changed, or > > it might be to purposely clear it. Changing it from set to clear > > should > > still be privileged. > > I changed it to allow setting or clearing SF_ARCHIVED. Now I can set > or > clear the flag as non-root: > > [root@doc-sd /testpool]# mount_msdosfs -u operator -g operator > /dev/md0 /mnt > [root@doc-sd /testpool]# cd /mnt > [root@doc-sd /mnt]# ls -lao > total 20 > drwxr-xr-x 1 operator operator arch 16384 Jan 1 1980 . > drwxr-xr-x 27 root wheel - 1024 Mar 8 17:05 .. > -rwxr-xr-x 1 operator operator - 0 Mar 8 16:54 foo > [root@doc-sd /mnt]# su operator > [operator@doc-sd /mnt]$ chflags arch foo > [operator@doc-sd /mnt]$ ls -lao > total 20 > drwxr-xr-x 1 operator operator arch 16384 Jan 1 1980 . > drwxr-xr-x 27 root wheel - 1024 Mar 8 17:05 .. > -rwxr-xr-x 1 operator operator arch 0 Mar 8 16:54 foo > [operator@doc-sd /mnt]$ chflags 0 foo > [operator@doc-sd /mnt]$ ls -lao > total 20 > drwxr-xr-x 1 operator operator arch 16384 Jan 1 1980 . > drwxr-xr-x 27 root wheel - 1024 Mar 8 17:05 .. > -rwxr-xr-x 1 operator operator - 0 Mar 8 16:54 foo > > > See the more complicated permissions check in ffs. It would be > > safest > > to duplicate most of it, to get different permissions checking for > > the > > SF and UF flags. Then decide if we want to keep allowing setting > > ARCHIVE without privilege. > > I think we should allow getting and setting SF_ARCHIVED without > special > privileges. Given how it is generally used, I don't think it should be > restricted to the super-user. > > Can you provide some code demonstrating how the permissions code > should > be changed in msdosfs? I don't know that much about that sort of > thing, > so I'll probably spend an inordinate amount of time stumbling > through it. > > > % - if (vap->va_flags & ~SF_ARCHIVED) > > % + if (vap->va_flags & ~(SF_ARCHIVED|UF_HIDDEN|UF_SYSTEM)) > > > > Style bugs (missing spaces around binary operator '|'). These style > > bugs are common in atribute handling for other fs's but were not > > here. > > Fixed. > > > % return EOPNOTSUPP; > > % if (vap->va_flags & SF_ARCHIVED) > > % dep->de_Attributes &= ~ATTR_ARCHIVE; > > % else if (!(dep->de_Attributes & ATTR_DIRECTORY)) > > % dep->de_Attributes |= ATTR_ARCHIVE; > > > > The comment before this says that we ignore attmps to set > > ATTR_ARCHIVED > > for directories. However, it is out of date. WinXP allows setting it > > and all the new flags for directories, and so do we. > > Do you mean we allow setting it in UFS, or where? Obviously the code > above > won't set it on a directory. > > > % + if (vap->va_flags & UF_HIDDEN) > > % + dep->de_Attributes |= ATTR_HIDDEN; > > % + else > > % + dep->de_Attributes &= ~ATTR_HIDDEN; > > % + if (vap->va_flags & UF_SYSTEM) > > % + dep->de_Attributes |= ATTR_SYSTEM; > > % + else > > % + dep->de_Attributes &= ~ATTR_SYSTEM; > > > > I thought you were adding ATTR_READONLY -> UF_IMMUTABLE here. > > No, I said in the commit message that it could be extended to map > ATTR_READONLY to UF_IMMUTABLE, not that I actually did that. > > I didn't make the change because of the existing code to map readonly > to > standard Unix permissions. > > > The WinXP attrib command (at least on a FAT32 fs) doesn't allow > > setting > > or clearing ARCHIVE (even if it is already set or clear) if any of > > HIDDEN, READONLY or SYSTEM is already set and remains set after the > > command. Thus the HRS attributes act a bit like immutable flags, but > > subtly differently. (ffs has the quite different and worse behaviour > > of allowing chflags() irrespective of immutable flags being set > > before > > or after, provided there is enough privilege to change the immutable > > flags.) Anyway, they should all give some aspects of immutability. > > We could do that for msdosfs, but why add more things for the user to > trip > over given how the filesystem is typically used? Most people probably > use it for USB thumb drives these days. Or perahps on a dual boot > system > to access their Windows partition. > > > % + > > > > Style bug (extra blank line). > > Fixed. > > > % dep->de_flag |= DE_MODIFIED; > > % } > > % > > % --- //depot/users/kenm/FreeBSD-test3/sys/sys/stat.h 2013-03-04 > > 17:51:12.000000000 -0700 > > % +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/sys/sys/stat.h > > 2013-03-04 > > 17:51:12.000000000 -0700 > > % --- /tmp/tmp.49594.563 2013-03-06 16:42:43.000000000 -0700 > > % +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/sys/sys/stat.h > > 2013-03-06 > > 15:05:45.936126193 -0700 > > % @@ -268,6 +268,22 @@ > > % #define UF_OPAQUE 0x00000008 /* directory is opaque wrt. union */ > > % #define UF_NOUNLINK 0x00000010 /* file may not be removed or > > renamed */ > > > > Old style bugs: inconsistent space instead of tab after #define for > > the 2 > > newer definitions. > > Fixed. > > > % /* > > % + * These two bits are defined in MacOS X. They are not currently > > used in > > % + * FreeBSD. > > % + */ > > % +#if 0 > > % +#define UF_COMPRESSED 0x00000020 /* file is compressed */ > > % +#define UF_TRACKED 0x00000040 /* renames and deletes are > > tracked */ > > % +#endif > > % + > > % +#define UF_SYSTEM 0x00000080 /* Windows system file bit */ > > % +#define UF_SPARSE 0x00000100 /* sparse file */ > > % +#define UF_OFFLINE 0x00000200 /* file is offline */ > > % +#define UF_REPARSE 0x00000400 /* Windows reparse point file bit > > */ > > % +/* This is the same as the MacOS X definition of UF_HIDDEN */ > > % +#define UF_HIDDEN 0x00008000 /* file is hidden */ > > > > New style bugs: random spaces/tabs after #define. 1 main comment not > > punctuated. > > Fixed. > > > % + > > % +/* > > % * Super-user changeable flags. > > % */ > > % #define SF_SETTABLE 0xffff0000 /* mask of superuser > > changeable flags */ > > % --- //depot/users/kenm/FreeBSD-test3/sys/ufs/ufs/ufs_vnops.c > > 2013-03-04 > > 17:51:12.000000000 -0700 > > % +++ > > /usr/home/kenm/perforce4/kenm/FreeBSD-test3/sys/ufs/ufs/ufs_vnops.c > > 2013-03-04 17:51:12.000000000 -0700 > > % --- /tmp/tmp.49594.581 2013-03-06 16:42:43.000000000 -0700 > > % +++ > > /usr/home/kenm/perforce4/kenm/FreeBSD-test3/sys/ufs/ufs/ufs_vnops.c > > 2013-03-06 15:06:27.004132409 -0700 > > % @@ -529,8 +529,9 @@ > > % } > > % if (vap->va_flags != VNOVAL) { > > % if ((vap->va_flags & ~(UF_NODUMP | UF_IMMUTABLE | UF_APPEND | > > % - UF_OPAQUE | UF_NOUNLINK | SF_ARCHIVED | SF_IMMUTABLE | > > % - SF_APPEND | SF_NOUNLINK | SF_SNAPSHOT)) != 0) > > % + UF_OPAQUE | UF_NOUNLINK | UF_HIDDEN | UF_SYSTEM | > > % + UF_SPARSE | UF_OFFLINE | UF_REPARSE | SF_ARCHIVED | > > % + SF_IMMUTABLE | SF_APPEND | SF_NOUNLINK | SF_SNAPSHOT)) > > != 0) > > % return (EOPNOTSUPP); > > % if (vp->v_mount->mnt_flag & MNT_RDONLY) > > % return (EROFS); > > > > The random order is harder to read with more flags. > > Fixed. > > > I think unsupported flags should just be unsupported -- most or all > > of the > > new flags and SF_ARCHIVED too. Most need system support to work. > > Without > > system support you can only copy them from other fs's and then lose > > them > > when utilities like tar don't unsderstand them. > > The primary reason I added these flags to UFS is to support CIFS > servers. > I have modifications to Likewise to support storing DOS/Windows/CIFS > attributes as file flags, and we could do the same for Samba. > > My eventual intent is to also add support to our NFS code to get/set > the > attributes that are allowed in the NFS 4.1 standard (hidden, system > and > archive are the ones I could find). So in this case, the filesystem > role > for some of these really is just to store it for someone else. > Just fyi, they are supported by NFSv4.0 as well, so there is no need to wait for the NFSv4.1 server code (which I have just started to work on). If you'd like, I can easily do this in April (or later), if/when your patch is committed. Have fun with it, rick > That is the way that ZFS handles attributes like hidden and system. It > doesn't do anything with them, they're just used by the Solaris CIFS > server. > > Ken > -- > Kenneth Merry > ken@FreeBSD.ORG > > _______________________________________________ > freebsd-arch@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-arch > To unsubscribe, send any mail to > "freebsd-arch-unsubscribe@freebsd.org"