GNU bug report logs - #67490
[PATCH v2] tail: fix tailing sysfs files on large page kernels

Previous Next

Full log

Message #36 received at 67490-done <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Paul Eggert <eggert <at> cs.ucla.edu>,
 dann frazier <dann.frazier <at> canonical.com>, 67490-done <at> debbugs.gnu.org
Subject: Re: bug#67490: [PATCH v2] tail: fix tailing sysfs files on large page
 kernels
Date: Fri, 1 Dec 2023 23:23:19 +0000

On 01/12/2023 01:54, Paul Eggert wrote:
> On 11/30/23 12:11, Pádraig Brady wrote:
>> Though that will generally give 128K, which is good when processing all
>> of a file,
>> but perhaps overkill when processing just the last part of a file.
> 
> The 128 KiB number was computed as being better for apps like 'sed' that
> typically read all or most of the file. 'tail' sometimes behaves that
> way (e.g., 'tail -c +10') and so 'tail' should use 128 KiB in those
> cases. The simplest way to do that is for 'tail' to use 128 KiB all the
> time - that would cost little for uses like plain 'tail' and it could be
> a significant win for uses like 'tail -c +10'.

Yes I agree we should use io_blksize() in other routines in tail
where we may dump lots of a file. However in this (most common) case
the routine is dealing with the end of a regular file,
so it's probably best to somewhat minimize the amount of data read,
and more directly check the page_size which is issue at hand.
I've pushed the fix at https://github.com/coreutils/coreutils/commit/73d119f4f
where the adjustment (which also corresponds to what we do in wc) is:

  if (sb->st_size % page_size == 0)
    bufsize = MAX (BUFSIZ, page_size);

I'll follow up with another patch to address the performance aspect,
which uses io_blksize() where appropriate.

> (As an aside, the 128 KiB number was computed in 2014. These days 256
> KiB might be better if someone could take the time to measure....)

I periodically check with the documented script,
but I should update the comment when I do that.
I'll update the date in the comment now at least.
I quickly tested a few systems here,
which suggests 256KiB _may_ be a more appropriate default now.
More testing required.

Marking this as done.

thanks,
Pádraig

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.