On 01/31, Linus Torvalds wrote:
>
> On Fri, 31 Jan 2025 at 01:50, K Prateek Nayak <kprateek.nayak@xxxxxxx> wrote:
> >
> > On my 3rd Generation EPYC system (2 x 64C/128T), I see that on reverting
> > the changes on the above mentioned commit, sched-messaging sees a
> > regression up until the 8 group case which contains 320 tasks, however
> > with 16 groups (640 tasks), the revert helps with performance.
>
> I suspect that the extra wakeups just end up perturbing timing, and
> then you just randomly get better performance on that particular
> test-case and machine.
>
> I'm not sure this is worth worrying about, unless there's a real load
> somewhere that shows this regression.
Well yes, but the problem is that people seem to believe that hackbench
is the "real" workload, even in the "overloaded" case...
And if we do care about performance... Could you look at the trivial patch
at the end? I don't think {a,c,m}time make any sense in the !fifo case, but
as you explained before they are visible to fstat() so we probably shouldn't
remove file_accessed/file_update_time unconditionally.
This patch does help if I change hackbench to uses pipe2(O_NOATIME) instead
of pipe(). And in fact it helps even in the simplest case:
static char buf[17 * 4096];
static struct timeval TW, TR;
int wr(int fd, int size)
{
int c, r;
struct timeval t0, t1;
gettimeofday(&t0, NULL);
for (c = 0; (r = write(fd, buf, size)) > 0; c += r);
gettimeofday(&t1, NULL);
timeradd(&TW, &t1, &TW);
timersub(&TW, &t0, &TW);
return c;
}
int rd(int fd, int size)
{
int c, r;
struct timeval t0, t1;
gettimeofday(&t0, NULL);
for (c = 0; (r = read(fd, buf, size)) > 0; c += r);
gettimeofday(&t1, NULL);
timeradd(&TR, &t1, &TR);
timersub(&TR, &t0, &TR);
return c;
}
int main(int argc, const char *argv[])
{
int fd[2], nb = 1, noat, loop, size;
assert(argc == 4);
noat = atoi(argv[1]) ? O_NOATIME : 0;
loop = atoi(argv[2]);
size = atoi(argv[3]);
assert(pipe2(fd, noat) == 0);
assert(ioctl(fd[0], FIONBIO, &nb) == 0);
assert(ioctl(fd[1], FIONBIO, &nb) == 0);
assert(size <= sizeof(buf));
while (loop--)
assert(wr(fd[1], size) == rd(fd[0], size));
printf("TW = %lu.%03lu\n", TW.tv_sec, TW.tv_usec/1000);
printf("TR = %lu.%03lu\n", TR.tv_sec, TR.tv_usec/1000);
return 0;
}
Now,
/# for i in 1 2 3; do /host/tmp/test 0 10000 100; done
TW = 7.692
TR = 5.704
TW = 7.930
TR = 5.858
TW = 7.685
TR = 5.697
/#
/# for i in 1 2 3; do /host/tmp/test 1 10000 100; done
TW = 6.432
TR = 4.533
TW = 6.612
TR = 4.638
TW = 6.409
TR = 4.523
Oleg.
---
diff --git a/fs/pipe.c b/fs/pipe.c
index a3f5fd7256e9..14b2c0f8b616 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -1122,6 +1122,9 @@ int create_pipe_files(struct file **res, int flags)
}
}
+ if (flags & O_NOATIME)
+ inode->i_flags |= S_NOCMTIME;
+
f = alloc_file_pseudo(inode, pipe_mnt, "",
O_WRONLY | (flags & (O_NONBLOCK | O_DIRECT)),
&pipefifo_fops);
@@ -1134,7 +1137,7 @@ int create_pipe_files(struct file **res, int flags)
f->private_data = inode->i_pipe;
f->f_pipe = 0;
- res[0] = alloc_file_clone(f, O_RDONLY | (flags & O_NONBLOCK),
+ res[0] = alloc_file_clone(f, O_RDONLY | (flags & (O_NONBLOCK | O_NOATIME)),
&pipefifo_fops);
if (IS_ERR(res[0])) {
put_pipe_info(inode, inode->i_pipe);
@@ -1154,7 +1157,7 @@ static int __do_pipe_flags(int *fd, struct file **files, int flags)
int error;
int fdw, fdr;
- if (flags & ~(O_CLOEXEC | O_NONBLOCK | O_DIRECT | O_NOTIFICATION_PIPE))
+ if (flags & ~(O_CLOEXEC | O_NONBLOCK | O_DIRECT | O_NOATIME | O_NOTIFICATION_PIPE))
return -EINVAL;
error = create_pipe_files(files, flags);
