coreutils
[Top][All Lists]
Advanced

[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>




reply via email to

[Prev in Thread] Current Thread [Next in Thread]