All of lore.kernel.org
 help / color / mirror / Atom feed
From: NeilBrown <neilb@suse.com>
To: Andreas Dilger <adilger@dilger.ca>,
	"J. Bruce Fields" <bfields@fieldses.org>
Cc: David Howells <dhowells@redhat.com>,
	One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC][PATCH 0/4] Enhanced file stat system call
Date: Fri, 18 Nov 2016 15:29:57 +1100	[thread overview]
Message-ID: <87h975fm0q.fsf@notabene.neil.brown.name> (raw)
In-Reply-To: <3852DE26-42A6-4AFF-AA9C-08C4E9837511@dilger.ca>

[-- Attachment #1: Type: text/plain, Size: 4499 bytes --]

On Fri, Nov 18 2016, Andreas Dilger wrote:

> [ Unknown signature status ]
>
>> On Nov 17, 2016, at 1:00 PM, J. Bruce Fields <bfields@fieldses.org> wrote:
>> 
>> On Thu, Nov 17, 2016 at 04:45:45PM +0000, David Howells wrote:
>>> One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> wrote:
>>> 
>>>>> (2) Lightweight stat (AT_STATX_DONT_SYNC): Ask for just those details of
>>>>>     interest, and allow a network fs to approximate anything not of
>>>>>     interest, without going to the server.
>>>>> 
>>>>> (3) Heavyweight stat (AT_STATX_FORCE_SYNC): Force a network fs to flush
>>>>>     buffers and go to the server, even if it thinks its cached attributes
>>>>>     are up to date.
>>>> 
>>>> That seems an odd way to do it. Wouldn't it be cleaner and more flexible
>>>> to give a timestamp of the oldest time you consider acceptable (and
>>>> obviously passing 0 indicates whatever you have)
>>> 
>>> Perhaps, though adding 6-argument syscalls is apparently frowned upon.
>>> 
>>>>> Note that no lstat() equivalent is required as that can be implemented
>>>>> through statx() with atflag == 0.  There is also no fstat() equivalent as
>>>>> that can be implemented through statx() with filename == NULL and the
>>>>> relevant fd passed as dfd.
>>>> 
>>>> and dfd + a name gives you fstatat() ?
>>> 
>>> Yes.
>>> 
>>>> The cover note could be clearer on this.
>>> 
>>> Fixed.
>>> 
>>>> Should the fields really be split the way they are for times rather than
>>>> a struct for each one so you can write code generically to handle one of
>>>> those rather than having to have a 4 way switch statement all the time.
>>> 
>>> It depends.  Doing so leaves 16 bytes of hole in the structure.  I could
>>> ameliorate the wastage by using a union to overlay useful fields in the gaps,
>>> but that's pretty icky and might be compiler dependent.
>>> 
>>>> Another attribute that would be nice (but migt need some trivial device
>>>> layer tweaking) would be STATX_ATTR_VOLATILE for filesystems that will
>>>> probably evaporate on a reboot. That's useful information for tools like
>>>> installers and also for sanity checking things like backup paths.
>>> 
>>> There's a FILE_ATTRIBUTE_TEMPORARY that I could map for windows filesystems
>>> that could be used with this.
>>> 
>>>> Remote needs to have clear semantics: is ext4fs over nbd 'remote' for
>>>> example ?
>>> 
>>> Hmmm... Interesting question.  Probably should.  But you could be insane and
>>> RAID an nbd and a local disk.  Further, does NFS over a loopback device to
>>> nfsd on the same machine qualify as root?  What if that's exposing a local fs
>>> on NBD?  Perhaps I should drop 'REMOTE' for now.  It sounds like something
>>> that a GUI filemanager might find interesting, though.
>> 
>> Sorry, I haven't been paying attention, just popping up for this, but:
>> "shared" might be a more useful term than "remote".
>> 
>> A filesystem that may be mounted from more than one system is "shared".
>> Caching performance and semantics of such a filesystem are more
>> complicated since the filesystem may change out from under us.  This is
>> what makes e.g. the lightweight/heavyweight stat difference more
>> interesting in the shared case.
>> 
>> The filesystem should be able to make that shared/unshared distinction
>> without knowledge of the storage it's sitting on top of.
>> 
>> Answering your questions by that criterion:
>> 
>> 	- ext4/nbd: not shared
>> 	- nfs/lo: shared
>> 
>> But, it's fine with me to drop any features for now as long as we can
>> always add them later.
>
> Please, please, please, let's get the syscall and basic functionality
> landed first, and then nit-pick about extensions later.  This has been
> dragging on for _years_ and bike shedded to death.

I very much agree with this, but I think it will require dropping (not
replacing yet) things that do not have a well defined meaning, including

> 	STATX_ATTR_KERNEL_API		File is kernel API (eg: procfs/sysfs)
> 	STATX_ATTR_REMOTE		File is remote and needs network
> 	STATX_ATTR_FABRICATED		File was made up by fs

Without clear guidance on how the filesystem should choose to set these,
and how a program should interpret them, they are worse than noise.
I imagine each could possibly be useful, but without clear unambiguous
documentation, they aren't.
So just remove them for now, and consider adding them once the core
syscall has landed.

NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 800 bytes --]

  reply	other threads:[~2016-11-18  4:30 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-17 13:34 [RFC][PATCH 0/4] Enhanced file stat system call David Howells
2016-11-17 13:35 ` [PATCH 1/4] statx: Add a system call to make enhanced file info available David Howells
2016-11-17 18:39   ` Jeff Layton
2016-11-18  2:32     ` Andreas Dilger
2016-11-18  8:59     ` David Howells
2016-11-18  8:59       ` David Howells
2016-11-18  9:25       ` Andreas Dilger
2016-11-18  9:25         ` Andreas Dilger
2016-11-17 23:40   ` Dave Chinner
2016-11-18  3:28     ` Andreas Dilger
2016-11-18 22:07       ` Dave Chinner
2016-11-18 22:54       ` David Howells
2016-11-19 22:43         ` Dave Chinner
2016-11-21 14:30         ` One Thousand Gnomes
2016-11-21 20:43           ` Dave Chinner
2016-11-22 10:39         ` David Howells
2016-11-22 13:55           ` Jeff Layton
2016-11-22 20:58           ` Dave Chinner
2016-11-18  9:53     ` David Howells
2016-11-18  8:48   ` David Howells
2016-11-18 12:01     ` Jeff Layton
2016-11-18  9:36   ` David Howells
2016-11-18 17:17     ` Jeff Layton
2016-11-18 18:04     ` David Howells
2016-11-18 18:54       ` Jeff Layton
2016-11-18 19:08       ` David Howells
2016-11-18  9:43   ` David Howells
2016-11-18 21:41     ` Dave Chinner
2016-11-18 22:24     ` David Howells
2016-11-18 10:29   ` David Howells
2016-11-18 10:29     ` David Howells
2016-11-18 21:27     ` Dave Chinner
2016-11-18 21:48     ` David Howells
2016-11-18 21:48       ` David Howells
2016-11-18 22:17       ` Dave Chinner
2016-11-18 22:17         ` Dave Chinner
2016-11-19 10:21         ` Michael Kerrisk (man-pages)
2016-11-17 13:35 ` [PATCH 2/4] statx: Ext4: Return enhanced file attributes David Howells
2016-11-18  3:30   ` Andreas Dilger
2016-11-17 13:35 ` [PATCH 3/4] statx: NFS: " David Howells
2016-11-17 13:35 ` [PATCH 4/4] statx: AFS: " David Howells
2016-11-18  3:34   ` Andreas Dilger
2016-11-18  8:47   ` David Howells
2016-11-17 14:39 ` [RFC][PATCH 0/4] Enhanced file stat system call One Thousand Gnomes
2016-11-17 15:10 ` Michael Kerrisk
2016-11-17 16:33 ` David Howells
2016-11-17 16:45 ` David Howells
2016-11-17 20:00   ` J. Bruce Fields
2016-11-18  2:30     ` Andreas Dilger
2016-11-18  4:29       ` NeilBrown [this message]
2016-11-18 13:41   ` One Thousand Gnomes
2016-11-18 13:49   ` David Howells

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87h975fm0q.fsf@notabene.neil.brown.name \
    --to=neilb@suse.com \
    --cc=adilger@dilger.ca \
    --cc=bfields@fieldses.org \
    --cc=dhowells@redhat.com \
    --cc=gnomes@lxorguk.ukuu.org.uk \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.