On Fri, 2011-09-23 at 19:17 +0200, Lukas Czerner wrote: > On Fri, 23 Sep 2011, Alex Elder wrote: > > > On Wed, 2011-09-21 at 11:42 +0200, Lukas Czerner wrote: > > > In xfs_ioc_trim it is possible that computing the last allocation group > > > to discard might overflow for big start & len values, because the result > > > might be bigger then xfs_agnumber_t which is 32 bit long. Fix this by not > > > allowing the start and end block of the range to be beyond the end of the > > > file system. > > > > > > Note that if the start is beyond the end of the file system we have to > > > return -EINVAL, but in the "end" case we have to truncate it to the fs > > > size. > > > > > > Also introduce "end" variable, rather than using start+len which which > > > might be more confusing to get right as this bug shows. > > > > > > Signed-off-by: Lukas Czerner <lczerner@xxxxxxxxxx> > > > > There are cases where we're (still) not trimming > > blocks within the range specified when we could. > > I have an idea about how to do that but I'll send > > it out later and will commit this as-is. > > What cases do you have in mind ? I have actually noticed that you are > discarding even more than user asked for in case that the free extent > and the range to trim overlaps, but that perfectly fine since the > duration of the discard command does not usually depends on the extent > size, so it is good thing to do. Suppose blocks from byte offset 4096 through 20480 are free, a FSB is 4096 bytes, and minlen computed in xfs_ioc_trim() is computed to be 4096. Now suppose a request comes in to trim range 7680 = (8KB - 512B) for 5KB = 5120 bytes (i.e., bytes 7680 through 12799). In xfs_ioc_trim(), "start" will be truncated to FSB 1, byte offset 4096. In computing "end", range.len will be truncated to 1 FSB or 4096 bytes. So the resulting "end" byte will be 4096 + 4096 - 1 or 8191, thereby *not* trimming FSB 2 at offset 8192B even though it is entirely within the requested range, and was free. As pointed out by Christoph this isn't a big deal because it's an advisory interface. I have a way to tighten this up but it'll be easier to see with code, which I'll try to post next week. -Alex _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
