On Mon, Jan 06, 2025 at 07:16:17PM -0700, Jens Axboe wrote:
> On 1/6/25 7:11 PM, Darrick J. Wong wrote:
> >> +void
> >> +read_dontcache_f(opnum_t opno, long r)
> >> +{
> >> + int e;
> >> + pathname_t f;
> >> + int fd;
> >> + int64_t lr;
> >> + off64_t off;
> >> + struct stat64 stb;
> >> + int v;
> >> + char st[1024];
> >> + struct iovec iov;
> >> + int flags;
> >> +
> >> + init_pathname(&f);
> >> + if (!get_fname(FT_REGFILE, r, &f, NULL, NULL, &v)) {
> >> + if (v)
> >> + printf("%d/%lld: read - no filename\n", procid, opno);
> >> + free_pathname(&f);
> >> + return;
> >> + }
> >> + fd = open_path(&f, O_RDONLY);
> >> + e = fd < 0 ? errno : 0;
> >> + check_cwd();
> >> + if (fd < 0) {
> >> + if (v)
> >> + printf("%d/%lld: read - open %s failed %d\n",
> >> + procid, opno, f.path, e);
> >> + free_pathname(&f);
> >> + return;
> >> + }
> >> + if (fstat64(fd, &stb) < 0) {
> >> + if (v)
> >> + printf("%d/%lld: read - fstat64 %s failed %d\n",
> >> + procid, opno, f.path, errno);
> >> + free_pathname(&f);
> >> + close(fd);
> >> + return;
> >> + }
> >> + inode_info(st, sizeof(st), &stb, v);
> >> + if (stb.st_size == 0) {
> >> + if (v)
> >> + printf("%d/%lld: read - %s%s zero size\n", procid, opno,
> >> + f.path, st);
> >> + free_pathname(&f);
> >> + close(fd);
> >> + return;
> >> + }
> >> + lr = ((int64_t)random() << 32) + random();
> >> + off = (off64_t)(lr % stb.st_size);
> >> + iov.iov_len = (random() % FILELEN_MAX) + 1;
> >> + iov.iov_base = malloc(iov.iov_len);
> >
> > Should there be a check for null iov_base after the allocation?
>
> Nothing else in fsstress seems to bother with malloc() failures, which
> at least on Linux, is probably fair game.
lol ok.
> >> + flags = have_rwf_dontcache ? RWF_DONTCACHE : 0;
> >> + e = preadv2(fd, &iov, 1, off, flags) < 0 ? errno : 0;
> >> + if (have_rwf_dontcache && e == EOPNOTSUPP)
> >
> > ...and should this set have_rwf_dontcache = 0?
>
> I don't think so? If we get EOPNOTSUPP and we don't have dontcache, then
> it's a fatal condition. fsstress defaults to it being available, so we
> may very well run into EOPNOTSUPP and then just do a regular read or
> write for that case. We could probably do:
>
> if (have_rwf_dontcache && e == EOPNOTSUPP) {
> have_rwf_dontcache = 0;
> e = preadv2(fd, &iov, 1, off, 0) < 0 ? errno : 0;
> }
Yep, that's exactly what I was thinking. :)
--D
> here and on the write side, at least then we won't repeatedly try
> RWF_DONTCACHE if we hit EOPNOTSUPP. But in terms of logic, it should be
> correct as-is.
>
> --
> Jens Axboe
>
