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.
>> + 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;
}
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
