Re: [RFC][PATCH 0/4] Enhanced file stat system call

[NeilBrown]

On Fri, Nov 18 2016, Andreas Dilger wrote:

> [ Unknown signature status ]
>
>> On Nov 17, 2016, at 1:00 PM, J. Bruce Fields <bfields@xxxxxxxxxxxx> wrote:
>> 
>> On Thu, Nov 17, 2016 at 04:45:45PM +0000, David Howells wrote:
>>> One Thousand Gnomes <gnomes@xxxxxxxxxxxxxxxxxxx> 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: signature.asc
Description: PGP signature