linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Eric Biggers <ebiggers3@gmail.com>
To: David Howells <dhowells@redhat.com>
Cc: Christoph Hellwig <hch@infradead.org>,
	mtk.manpages@gmail.com,
	"Darrick J. Wong" <darrick.wong@oracle.com>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	xfs <linux-xfs@vger.kernel.org>
Subject: Re: statx manpage
Date: Thu, 9 Mar 2017 21:01:56 -0800	[thread overview]
Message-ID: <20170310050156.GA11794@zzz> (raw)
In-Reply-To: <17957.1488966060@warthog.procyon.org.uk>

On Wed, Mar 08, 2017 at 09:41:00AM +0000, David Howells wrote:
> > 
> > This example is confusing because chmod() doesn't change st_uid, and chown()
> > doesn't ordinarily change st_mode.
> 
> Doesn't chown() clear the S_ISUID and S_ISGID flags?
> 

It can, but it's not a straightforward example.

> 
> No, it's not a bug, exactly.  It's a design decision.  It might be worth
> adding an AT_STAT_LOCK flag to synchronise against attribute setting, but it
> will cost you something in terms of performance - and even then, over a
> network filesystem it might not achieve anything.
> 

Well, regardless of whether people want to fix it or not, I do think it's a bug.
Currently even the update of a timestamp is non-atomic, since that involves
updating both tv_sec and tv_nsec.  Therefore, stat() might return the new tv_sec
along with the old tv_nsec, or vice versa.  This can cause some very strange
behavior, such as two successively observed timestamps going backwards in time.

I expect that historically people haven't complained enough for this to be
worthwhile to fix.  But in theory I think it could be fixed with little
performance loss by protecting all the stat data with a sequence count, similar
to how reads of i_size are made atomic on 32-bit platforms by using
i_size_seqcount.

> 
> Good point.  Should I reject AT_EMPTY_PATH, AT_NO_AUTOMOUNT and
> AT_SYMLINK_NOFOLLOW if pathname is NULL, I wonder?
> 

It could be argued either way, but I was thinking those particular flags should
just be ignored, as they have known meanings but simply aren't relevant.  It
could also cause confusion for

	statx(dfd, "", AT_EMPTY_PATH, 0, buffer)

to succeed but for

	statx(dfd, NULL, AT_EMPTY_PATH, 0, buffer)

to fail (for example).

- Eric

  reply	other threads:[~2017-03-10  5:02 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-07  0:06 [PATCH] xfs: report crtime and attribute flags to statx Darrick J. Wong
2017-03-07  5:01 ` Christoph Hellwig
2017-03-07 17:23   ` Darrick J. Wong
2017-03-07 17:22 ` statx manpage David Howells
2017-03-07 18:02   ` Darrick J. Wong
2017-03-07 18:39   ` David Howells
2017-03-07 18:44     ` Darrick J. Wong
2017-03-07 18:55     ` David Howells
2017-03-07 21:44   ` Andreas Dilger
2017-03-07 22:55   ` Eric Biggers
2017-03-08  3:45   ` Eryu Guan
2017-03-08  9:24   ` David Howells
2017-03-08 15:26     ` Jeff Layton
2017-03-20 16:01       ` Matthew Wilcox
2017-03-22 10:55         ` Jeff Layton
2017-03-08  9:41   ` David Howells
2017-03-10  5:01     ` Eric Biggers [this message]
2017-03-09  6:46   ` David Howells
2017-03-09  6:59     ` Eryu Guan
2017-03-09  6:59     ` Darrick J. Wong
2017-03-09 14:01       ` Christoph Hellwig
2017-03-09  7:45     ` David Howells
2017-03-24 20:53   ` Eric Biggers
2017-03-27  0:46     ` Andreas Dilger
2017-03-27 15:40       ` Darrick J. Wong
2017-03-27 16:25       ` David Howells
2017-03-27 16:46         ` Christoph Hellwig
2017-03-27 19:04           ` Jeff Layton
2017-03-28 19:39             ` Andreas Dilger
2017-03-28 20:22               ` Jeff Layton
2017-03-31 15:49             ` Christoph Hellwig
2017-03-27 18:57         ` David Howells
2017-03-28  0:13         ` [PATCH] Add initial batch of statx() LTP tests David Howells
2017-03-28  6:28           ` Christoph Hellwig
2017-03-28  8:23           ` David Howells
2017-03-27  9:55   ` statx manpage David Howells
2017-03-31 15:56   ` Christoph Hellwig
2017-03-31 16:43   ` 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=20170310050156.GA11794@zzz \
    --to=ebiggers3@gmail.com \
    --cc=darrick.wong@oracle.com \
    --cc=dhowells@redhat.com \
    --cc=hch@infradead.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=mtk.manpages@gmail.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).