| Message ID | 20100130105501.GA22909@infradead.org |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
Christoph Hellwig wrote: > On Fri, Jan 29, 2010 at 02:05:46PM -0600, Eric Sandeen wrote: >> Testcase from Giel de Nijs <giel@vectorwise.com> >> on linux-ext4 list, ""Possible ext4 data corruption >> with large files and async I/O," on 29 Jan 2010 >> >> ext4 put byte offsets in a block offset u32 container >> in the endio struct, so 4g wrapped to 0 leading to >> data corruption when the unwritten extent did not >> get converted. > > There's various type messups in the test program that make it fail for oops, thanks for checking. > me on a 32-bit machine. The patch below fixes it up, but it seems like > we should rather add a variant of that code as aio_read/write commands > to xfs_io instead of adding a new test program. ok, that's probably better - again, though, it takes at least a release cycle before most folks can test it. But I guess that's not the end of the world. -Eric > Index: xfstests-dev/src/aio-write.c > =================================================================== > --- xfstests-dev.orig/src/aio-write.c 2010-01-30 10:42:24.000000000 +0000 > +++ xfstests-dev/src/aio-write.c 2010-01-30 10:45:30.000000000 +0000 > @@ -42,7 +42,7 @@ void usage(void) > /* > * Scale value by kilo, mega, or giga. > */ > -loff_t scale_by_kmg(long long value, char scale) > +long long scale_by_kmg(long long value, char scale) > { > switch (scale) { > case 'g': > @@ -69,7 +69,7 @@ switch (scale) { > int main(int argc, char ** argv) > { > char filename[PATH_MAX]; > - loff_t offset = 0; > + long long offset = 0; > size_t length = 0; > int seed = 0xFF; > int queue_depth = 8; > @@ -95,12 +95,12 @@ int main(int argc, char ** argv) > seed = (int)strtol(optarg, &endp, 0); > break; > case 'o': > - offset = strtol(optarg, &endp, 0); > - offset = scale_by_kmg((long long)offset, *endp); > + offset = strtoll(optarg, &endp, 0); > + offset = scale_by_kmg(offset, *endp); > break; > case 'l': > length = strtol(optarg, &endp, 0); > - length = scale_by_kmg((long long)length, *endp); > + length = scale_by_kmg(length, *endp); > break; > case 'v': > verbose++; > @@ -141,7 +141,8 @@ int main(int argc, char ** argv) > io_prep_pwrite(&iocb, fd, buf, length, offset); > iocblist[0] = &iocb; > if (verbose) > - printf("submitting write of %zd bytes at offset %zd\n", length, offset); > + printf("submitting write of %zd bytes at offset %lld\n", > + length, offset); > err = io_submit(io_ctx, 1, iocblist); > if (err < 0) { > printf("error submitting I/O requests: %s\n", strerror(-err)); > -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Jan 30, 2010 at 10:15:09AM -0600, Eric Sandeen wrote: > > me on a 32-bit machine. The patch below fixes it up, but it seems like > > we should rather add a variant of that code as aio_read/write commands > > to xfs_io instead of adding a new test program. > > ok, that's probably better - again, though, it takes at least a release > cycle before most folks can test it. But I guess that's not the end of > the world. Stupid question --- who uses xfs_io besides xfstests? Any chance we could consider dropping in some version of xfs_io into xfstests, or actually moving it into xfstests from xfsprogs if xfstests is the exclusive user of that program? I've been trying to get more people to use xfstests, since it would be good if more companies and more projects were using it --- and one of the things that makes it hard is all of the dependencies that it has. If there was some way we could gradually make xfstests more self-contained, it would certainly be nice. - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
tytso@mit.edu wrote: > On Sat, Jan 30, 2010 at 10:15:09AM -0600, Eric Sandeen wrote: >>> me on a 32-bit machine. The patch below fixes it up, but it seems like >>> we should rather add a variant of that code as aio_read/write commands >>> to xfs_io instead of adding a new test program. >> ok, that's probably better - again, though, it takes at least a release >> cycle before most folks can test it. But I guess that's not the end of >> the world. > > Stupid question --- who uses xfs_io besides xfstests? Any chance we > could consider dropping in some version of xfs_io into xfstests, or > actually moving it into xfstests from xfsprogs if xfstests is the > exclusive user of that program? I've been trying to get more people > to use xfstests, since it would be good if more companies and more > projects were using it --- and one of the things that makes it hard is > all of the dependencies that it has. If there was some way we could > gradually make xfstests more self-contained, it would certainly be > nice. > > - Ted These are the deps that I know xfstests has, to build and to run: BuildRequires: autoconf, libtool, xfsprogs-devel, e2fsprogs-devel BuildRequires: libacl-devel, libattr-devel, libaio-devel Requires: bash, xfsprogs, xfsdump, perl, acl, attr, bind-utils Requires: bc, indent, quota which isn't so bad... (and tests are just skipped if xfsdump isn't there) I'm not sure an xfsprogs dependency is so onerous; plenty has depended on e2fsprogs through the years and we've lived with that ;) But the lag time for xfsprogs to use released xfs_io functionality is a bit of a bummer. But I guess I don't have a great answer for who else uses xfs_io: xfs_io(8) xfs_io(8) NAME xfs_io - debug the I/O path of an XFS filesystem SYNOPSIS xfs_io [ -adFfmrRstx ] [ -c cmd ] ... [ -p prog ] file DESCRIPTION xfs_io is a debugging tool like xfs_db(8), but is aimed at examining the regular file I/O paths rather than the raw XFS volume itself. I guess it's not really advertised as a generic tool, and it's in the sbin path... I guess I could live with it either way - I suppose my main concern is that xfstests is a mess to package for a distro, and I really like easy access to xfs_io for my own use outside of xfstests. -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Jan 30, 2010 at 12:31:26PM -0600, Eric Sandeen wrote: > These are the deps that I know xfstests has, to build and to run: > > BuildRequires: autoconf, libtool, xfsprogs-devel, e2fsprogs-devel > BuildRequires: libacl-devel, libattr-devel, libaio-devel > > Requires: bash, xfsprogs, xfsdump, perl, acl, attr, bind-utils > Requires: bc, indent, quota > > which isn't so bad... Doesn't seem to bad. Indent is afaik only needed for the weird 122 test which doesn't apply to non-xfs filesystems. > I'm not sure an xfsprogs dependency is so onerous; plenty has depended > on e2fsprogs through the years and we've lived with that ;) But > the lag time for xfsprogs to use released xfs_io functionality is a > bit of a bummer. > > But I guess I don't have a great answer for who else uses xfs_io: I use xfs_io in lots various local scripts. It's a really handly tool for exercising some of the more weird I/O related syscalls. -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Christoph Hellwig wrote: > On Sat, Jan 30, 2010 at 12:31:26PM -0600, Eric Sandeen wrote: >> These are the deps that I know xfstests has, to build and to run: >> >> BuildRequires: autoconf, libtool, xfsprogs-devel, e2fsprogs-devel >> BuildRequires: libacl-devel, libattr-devel, libaio-devel >> >> Requires: bash, xfsprogs, xfsdump, perl, acl, attr, bind-utils >> Requires: bc, indent, quota >> >> which isn't so bad... > > Doesn't seem to bad. Indent is afaik only needed for the weird 122 test > which doesn't apply to non-xfs filesystems. and FWIW, we do: _require_command /usr/bin/indent so it'll just not run if it's not there (the above was for an rpm attempt I made, wishing to automatically pull in everything that might possibly be needed.) -Eric >> I'm not sure an xfsprogs dependency is so onerous; plenty has depended >> on e2fsprogs through the years and we've lived with that ;) But >> the lag time for xfsprogs to use released xfs_io functionality is a >> bit of a bummer. >> >> But I guess I don't have a great answer for who else uses xfs_io: > > I use xfs_io in lots various local scripts. It's a really handly > tool for exercising some of the more weird I/O related syscalls. > -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Index: xfstests-dev/src/aio-write.c =================================================================== --- xfstests-dev.orig/src/aio-write.c 2010-01-30 10:42:24.000000000 +0000 +++ xfstests-dev/src/aio-write.c 2010-01-30 10:45:30.000000000 +0000 @@ -42,7 +42,7 @@ void usage(void) /* * Scale value by kilo, mega, or giga. */ -loff_t scale_by_kmg(long long value, char scale) +long long scale_by_kmg(long long value, char scale) { switch (scale) { case 'g': @@ -69,7 +69,7 @@ switch (scale) { int main(int argc, char ** argv) { char filename[PATH_MAX]; - loff_t offset = 0; + long long offset = 0; size_t length = 0; int seed = 0xFF; int queue_depth = 8; @@ -95,12 +95,12 @@ int main(int argc, char ** argv) seed = (int)strtol(optarg, &endp, 0); break; case 'o': - offset = strtol(optarg, &endp, 0); - offset = scale_by_kmg((long long)offset, *endp); + offset = strtoll(optarg, &endp, 0); + offset = scale_by_kmg(offset, *endp); break; case 'l': length = strtol(optarg, &endp, 0); - length = scale_by_kmg((long long)length, *endp); + length = scale_by_kmg(length, *endp); break; case 'v': verbose++; @@ -141,7 +141,8 @@ int main(int argc, char ** argv) io_prep_pwrite(&iocb, fd, buf, length, offset); iocblist[0] = &iocb; if (verbose) - printf("submitting write of %zd bytes at offset %zd\n", length, offset); + printf("submitting write of %zd bytes at offset %lld\n", + length, offset); err = io_submit(io_ctx, 1, iocblist); if (err < 0) { printf("error submitting I/O requests: %s\n", strerror(-err));