On Mon, 15 Jul 2013 13:55:51 -0700, Zach Brown wrote: > I'd get rid of all this code by only copying each input argument on to > the stack as it's needed and by getting rid of the writable output > struct fields. (more on this later)
> As I said, I'd get rid of the output fields. Like the other vectored io > syscalls, the return value can indicate the number of initial > consecutive bytes that worked. When no progess was made then it can > return errors. Userspace is left to sort out the resulting state and > figure out the extents to retry in exactly the same way that it found > the initial extents to attempt to dedupe in the first place. > > (And imagine strace trying to print the inputs and outputs. Poor, poor, > strace! :)) The dedup branch that uses this syscall[1] doesn't compare files before submitting them anymore (the kernel will do it, and ranges may not fit in cache once I get rid of an unnecessary loop). I don't have strong opinions on the return style, but it would be good to have the syscall always make progress by finding at least one good range before bailing out, and signaling which files were involved. With those constraints, the current struct seems like the cleanest way to pass the data. The early return you suggest is a good idea if Mark agrees, but the return condition should be something like: if one range with bytes_deduped != 0 doesn't get bytes_deduped incremented by iteration_len in this iteration, bail out. That's sufficient to guarantee progress and to know which ranges were involved. > I hope this helps! > > - z Thank you and everyone involved for the progress on this. [1] https://github.com/g2p/bedup/tree/wip/dedup-syscall -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
