Re: [PATCH] tests: fix FP in ls/stat-free-color.sh
On 20/04/2021 23:15, Bernhard Voelker wrote: This FP was seen on latest openSUSE - logfile attached. The patch fixes it. Okay to push? Jim also noticed that on Fedora 34. Oh so it's stating stdout. Could you change the "1" in your commit summary to STDOUT_FILENO as that makes the system behavior change more apparent to me at least. Also the test is now relaxed a bit as it's no longer really checking regular files. Could you instead try changing the first ls in the test to `ls -a` to ensure something is output? cheers, Pádraig
Re: [PATCH] ls: add --files0-from=FILE option
Hi Berny, On Wed, 21 Apr 2021, Bernhard Voelker wrote: shouldn't it use the 'argv-iter' gnulib module (like du.c and wc.c) instead of directly using the underlying ... +#include "readtokens0.h" I considered this, too! :) I think the short answer is that du and wc don't actually need to keep any information around from all the files they process except for the running totals, so "at scale" the argv-iter version for files0 processing allows for a constant (?) memory usage in du and wc regardless of the number of input items. But for ls, even if using argv-iter would result in one less copy of the filenames, the scaled memory requirements are still the same for sorting (fileinfo for each entry has to be kept till the end). [If anything, i would look more carefully into whether gobble_file in ls.c really needs to make a copy of each filename, rather than just using the command line arguments or names from files0 input directly.] So i ended up following the pattern closer to what's in sort.c. Note that argv-iter is not used in there either, perhaps for a similar reason (it's not going to bring the memory requirements down to O(1), the way it could for wc and du). The one case ls might possibly find an improvement with argv-iter is for unsorted, non-column output (so, specifically -U1), where the names are only needed once. But in that case there's no need to use the '--files0-from' option for global sort or summary info -- you could use 'xargs -0 ls -U1 ...' instead for identical output. It just didn't seem like there was a strong argument for it (same memory scaling regardless). And the other frank truth is (if you look at wc.c and du.c for examples), it seemed like adding argv_iterator processing would significantly complicate the code. Those were my thoughts anyway :) Have a good one, Carl
Re: [PATCH] ls: add --files0-from=FILE option
On Tue, 20 Apr 2021, p...@draigbrady.com wrote: > One can also implement this functionality with the DSU pattern like: > > nlargest=10 > find . -printf '%s\t%p\0' | > sort -z -k1,1n | tail -z -n"$nlargest" | cut -z -f2 | > xargs -r0 ls -lUd --color=auto -- > > Arguably that's more scalable as the sort operation will not > be restricted by available RAM, and will use temp storage. > > Also it's more robust as ls is the last step in the pipeline, > presenting the files to the user. If you wanted the largest 10 > with ls --files0-from then file names containing a newline > would mess up further processing by tail etc. > >> Similarly, say you would like to view / scroll through your extensive mp3 >> collection in chronological order (based on when you added the files to >> your collection). You can do it now with the new option: >> >> [music]$ find -name \*.mp3 -print0 | ls -lrth --files0-from=- > > I've used a https://www.pixelbeat.org/scripts/newest script > for a long time with similar find|xargs technique as I described above. > > In saying all of the above, I do agree though that for consistency > commands that need to process all arguments with global context, > should have a --files0-from option. > Currently that's du and wc for total counts, and sort(1) for sorting. > Since ls has sorting functionality, it should have this option too. Thanks Pádraig for the thoughtful reply! You bring up some good points, which for the sake of interesting discussion i'd like to follow up on also. (Maybe later this week...) Have a good day! Carl
Re: [PATCH] wc: Add AVX2 optimization when counting only lines
Hello,
On 2021-03-29 7:21 a.m., Pádraig Brady wrote:
On 28/03/2021 18:29, Kristoffer Brånemyr via GNU coreutils General
I wanted to practice some more using vector intrinsics, so I made a
small AVX2 optimization for wc -l. Depending on line length it is
about 2-5x faster than previous version. (Well, only looking at user
time it is much faster than that even.)
Excellent results.
I'll review this very soon.
I'm attaching the patch (copied from the Github's pull-request),
hopefully we can continue the discussion here on the mailing list.
-assaf
>From 462386ea5aad1b1673f7c1bc51983374aad325a8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Kristoffer=20Br=C3=A5nemyr?=
Date: Sat, 20 Feb 2021 12:27:17 +0100
Subject: [PATCH] wc: Add AVX2 optimization when counting only lines
---
configure.ac | 46 ++
po/POTFILES.in | 1 +
src/local.mk | 9 +++
src/wc.c | 162 -
src/wc_avx2.c | 115 +++
5 files changed, 290 insertions(+), 43 deletions(-)
create mode 100644 src/wc_avx2.c
diff --git a/configure.ac b/configure.ac
index 7fbecbf8d..8186b88f1 100644
--- a/configure.ac
+++ b/configure.ac
@@ -575,6 +575,52 @@ AM_CONDITIONAL([USE_PCLMUL_CRC32],
test "x$pclmul_intrinsic_exists" = "xyes"])
CFLAGS=$ac_save_CFLAGS
+AC_MSG_CHECKING([if __get_cpuid_count exists])
+AC_COMPILE_IFELSE(
+ [AC_LANG_SOURCE([[
+#include
+
+int main(void)
+{
+ unsigned int eax = 0, ebx = 0, ecx = 0, edx = 0;
+ __get_cpuid_count(7, 0, , , , );
+ return 1;
+}
+ ]])
+ ],[
+AC_MSG_RESULT([yes])
+get_cpuid_count_exists=yes
+ ],[
+AC_MSG_RESULT([no])
+ ])
+
+CFLAGS="-mavx2 $CFLAGS"
+AC_MSG_CHECKING([if avx2 intrinstics exists])
+AC_COMPILE_IFELSE(
+ [AC_LANG_SOURCE([[
+#include
+
+int main(void)
+{
+ __m256i a, b;
+ a = _mm256_sad_epu8(a, b);
+ return 1;
+}
+ ]])
+ ],[
+AC_MSG_RESULT([yes])
+AC_DEFINE([HAVE_AVX2_INTRINSIC], [1], [avx2 intrinsics exists])
+avx2_intrinsic_exists=yes
+ ],[
+AC_MSG_RESULT([no])
+ ])
+if test "x$get_cpuid_count_exists" = "xyes" && test "x$avx2_intrinsic_exists" = "xyes"; then
+ AC_DEFINE([USE_AVX2_WC_LINECOUNT], [1], [Counting lines with AVX2 enabled])
+fi
+AM_CONDITIONAL([USE_AVX2_WC_LINECOUNT], [test "x$get_cpuid_count_exists" = "xyes" && test "x$avx2_intrinsic_exists" = "xyes"])
+
+CFLAGS=$ac_save_CFLAGS
+
dnl Autogenerated by the 'gen-lists-of-programs.sh' auxiliary script.
diff --git a/po/POTFILES.in b/po/POTFILES.in
index b5f5bbff1..dc80762db 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -142,6 +142,7 @@ src/unlink.c
src/uptime.c
src/users.c
src/wc.c
+src/wc_avx2.c
src/who.c
src/whoami.c
src/yes.c
diff --git a/src/local.mk b/src/local.mk
index 8c8479a53..c6555dafb 100644
--- a/src/local.mk
+++ b/src/local.mk
@@ -427,6 +427,15 @@ src_basenc_CPPFLAGS = -DBASE_TYPE=42 $(AM_CPPFLAGS)
src_expand_SOURCES = src/expand.c src/expand-common.c
src_unexpand_SOURCES = src/unexpand.c src/expand-common.c
+src_wc_SOURCES = src/wc.c
+if USE_AVX2_WC_LINECOUNT
+noinst_LIBRARIES += src/libwc_avx2.a
+src_libwc_avx2_a_SOURCES = src/wc_avx2.c
+wc_avx2_ldadd = src/libwc_avx2.a
+src_wc_LDADD += $(wc_avx2_ldadd)
+src_libwc_avx2_a_CFLAGS = -mavx2 $(AM_CFLAGS)
+endif
+
# Ensure we don't link against libcoreutils.a as that lib is
# not compiled with -fPIC which causes issues on 64 bit at least
src_libstdbuf_so_LDADD = $(LIBINTL)
diff --git a/src/wc.c b/src/wc.c
index 5216db189..1ecec0d83 100644
--- a/src/wc.c
+++ b/src/wc.c
@@ -37,6 +37,9 @@
#include "safe-read.h"
#include "stat-size.h"
#include "xbinary-io.h"
+#ifdef USE_AVX2_WC_LINECOUNT
+#include
+#endif
#if !defined iswspace && !HAVE_ISWSPACE
# define iswspace(wc) \
@@ -53,6 +56,15 @@
/* Size of atomic reads. */
#define BUFFER_SIZE (16 * 1024)
+static
+bool wc_lines(const char *file, int fd, uintmax_t *lines_out, uintmax_t *bytes_out);
+#ifdef USE_AVX2_WC_LINECOUNT
+/* From wc_avx2.c */
+bool wc_lines_avx2(const char *file, int fd, uintmax_t *lines_out, uintmax_t *bytes_out);
+#endif
+bool (*wc_lines_p)(const char *file, int fd, uintmax_t *lines_out, uintmax_t *bytes_out) = wc_lines;
+
+
/* Cumulative number of lines, words, chars and bytes in all files so far.
max_line_length is the maximum over all files processed so far. */
static uintmax_t total_lines;
@@ -108,6 +120,41 @@ static struct option const longopts[] =
{NULL, 0, NULL, 0}
};
+#ifdef USE_AVX2_WC_LINECOUNT
+static bool
+avx2_supported(void)
+{
+ unsigned int eax = 0;
+ unsigned int ebx = 0;
+ unsigned int ecx = 0;
+ unsigned int edx = 0;
+
+ if (! __get_cpuid(1, , , , ))
+{
+ return false;
+}
+
+ if (! (ecx & bit_OSXSAVE))
+{
+ return false;
+}
+
+ eax = ebx = ecx = edx = 0;
+
+ if (! __get_cpuid_count(7, 0, , , , ))
+{
+ return
bug#47940: tests/tail-2/inotify-dir-recreate.sh FAILs for remote filesystems that passes is_local_dir_
Hi all, I am debugging a reproducible test failure in tests/tail-2/inotify-dir-recreate.sh for coreutils 8.32. Logs: https://gist.githubusercontent.com/dongcarl/d24bfe853cc5bd9402bba82c36513c07/raw/19b9a15c4b12edf601dd1504cdf4ec0ee0d9344c/inotify-dir-recreate.log Looking at the non-‘+’ lines, and the final compare diff, we can see that the “out” file only contained the string “inotify” and nothing else. From my understanding, it has to do with the fact that I’m running this test in a directory which is on overlayfs, which is considered “remote” by tail (see `fremote(int, const char*)`). The filesystem being “remote” should ordinarily trigger one of the 2 SKIP checks in this test, however, neither check triggered a SKIP. Check 1: is_local_dir_ At the top of inotify-dir-recreate.sh, we invoke is_local_dir_ to make sure that we’re in a “local” filesystem. However, is_local_dir_ invokes `df --local`, which does not use “fremote” to determine filesystem locality, but rather “read_file_system_list” from gnulib. The “me_remote” field of the returned “mount_entry" struct is then used to filter out entries which are non-local. However, in the case of overlayfs, the “me_remote” field will be false, and it will be considered “local” by `df --local` TL;DR: is_local_dir_ = “is possibly local”, but we really want to check if “is possibly remote” Check 2: Grepping for 'inotify (resources exhausted|cannot be used)' Here, we expect that if inotify cannot be used, tail will print an indication to stderr and we can skip this test. However, this message is not printed when inotify is disabled in the fast-failing codepath: https://github.com/coreutils/coreutils/blob/v8.32/src/tail.c#L2490-L2512 I think the easiest fix might be to have tail output ‘inotify cannot be used’ to stderr in the fast-failing disable case: https://github.com/coreutils/coreutils/blob/v8.32/src/tail.c#L2490-L2496. It seems like this bug has been encountered by others as well: 1. https://dnsglk.github.io/lfs/2018/06/28/lfs-coreutils-test-issue.html#inotify-dir-recreate 2. https://github.com/containers/podman/issues/5493#issuecomment-598851397 Carl Dong cont...@carldong.me "I fight for the users” P.S. I believe the comment line for `is_local_fs_type` in the generated `src/fs-is-local.h` needs to be fixed.
bug#47883: sort -o loses data when it crashes
On 4/18/21 10:46 AM, Peter van Dijk wrote: While the manual (but not the manpage) mentions the data loss, I think it would be great if sort did not have this problem at all, and I think the OpenGroup text also says it should not have this problem. I don't know of any 'sort' implementation that does not have the problem at all. For example, FreeBSD 'sort -o file file' can lose 'file' in some (rare) cases. The only portable way to avoid this problem in a shell script is to output to some other file first and make sure that worked, before attempting to replace the input file. Also, I don't see where the Open Group spec says what you're saying. On the contrary, the spec merely says that '-o output' should cause output to be sent to the output file. If there are multiple hard links to the output file, this suggests 'sort' should update the output file's contents without breaking any hard links. Admittedly the Open Group spec is a bit vague in this area, but I certainly don't see anything implying that GNU 'sort' does not conform to POSIX in this area. FreeBSD 'sort' has a problem, in that 'sort -o A B' preserves all hard links to A's file, but 'sort -o A A' does not because it breaks the link from A. That's confusing. Traditional Unix 'sort -o A' behaves the way GNU 'sort' does; it preserves all hard links to A's file. So there is a compatibility argument for doing things the way GNU 'sort' does them, even if that might lead to more data loss in rare cases.
