[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 0/2] ls: use statx() when it's available
From: |
Jeff Layton |
Subject: |
Re: [PATCH v4 0/2] ls: use statx() when it's available |
Date: |
Thu, 31 Oct 2019 07:04:47 -0400 |
User-agent: |
Evolution 3.32.4 (3.32.4-1.fc30) |
On Thu, 2019-10-24 at 10:16 +0100, Pádraig Brady wrote:
> On 09/10/2019 22:23, Pádraig Brady wrote:
> > On 09/10/2019 11:14, Jeff Layton wrote:
> > > On Wed, 2019-10-09 at 10:19 +0100, Pádraig Brady wrote:
> > > > On 19/09/19 16:59, Jeff Layton wrote:
> > > > > v4:
> > > > > - set appropriate STATX_* bits for time_type, sort_type and
> > > > > print_block_size
> > > > >
> > > > > v3:
> > > > > - syntax cleanups. make syntax-check now passes
> > > > >
> > > > > v2:
> > > > > - add wrappers for stat_for_ino and fstat_for_ino, don't factor out
> > > > > loop
> > > > > detection
> > > > > - style cleanups
> > > >
> > > > Sorry for the delay in reviewing.
> > > >
> > > > This looks good, except for the usage
> > > > of AT_STATX_DONT_SYNC when retrieving inode info.
> > > > Sure that generally doesn't change, but that
> > > > would be file system dependent, and I have seen
> > > > file systems that populate inode with counters etc.
> > > > Anyway that sort of decision would be best done
> > > > in the kernel I think, where it would have the
> > > > info whether it needs to sync for STATX_INO or not.
> > > >
> > > > OK for me to push without the DONT_SYNC ?
> > > >
> > >
> > > Sure, that seems reasonable. Let me know if you need me to resend.
> >
> > Pushed without AT_STATX_DONT_SYNC.
> > Also added the new statx.h to noinst_HEADERS,
> > and used our _GL_ATTRIBUTE_PURE define rather than
> > the less portable __attribute__.
> >
> > https://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=a99ab26
>
> FYI I see this is causing issues with docker images (possibly due to seccomp).
> https://bugzilla.redhat.com/show_bug.cgi?id=1760300
> (There is a bug tracking the kernel issue but it's not publicly accessible.)
>
> From scanning the comments it seems that once statx() is called
> state is messed up, so that having ls fall back from statx() to stat()
> wouldn't help in this case. So the assumption that whatever libc providing
> statx() does appropriate fallback to stat() is probably still valid.
>
Yep, this seems like it's probably a seccomp bug. We were going to put
these patches into Fedora 31's coreutils, but reverted the patches until
this is resolved. Hopefully they'll nail down the problem soob.
--
Jeff Layton <address@hidden>