Hi Christoph, On Thu, 28 Oct 2010, Christoph Hellwig wrote: > On Thu, Oct 28, 2010 at 12:30:45PM +0200, Boaz Harrosh wrote: > > I've just noticed this patch. Was it posted to linux-fsdevel? Sorry > > for missing it. > > It has been, but only very recently. > > > I can't say I overly like it. There's no real point in dispatching this > out to a separate vector instead of just through ->ioctl. It's got > rather useless special case for ommiting the argument, and the whole > thing just doesn't seem generic enough to do it in the VFS. Well, learning from examples, that's what I am doing and ioctl_fiemap() is doing the same thing so I was using that as an example. And, of course, I am returning the amount of really discarded Bytes to the user and I just can not simply use user-space pointer, or maybe I do not understand you correctly. About the special case I am not entirely sure that it is useless, since there is no point in allocating the structure and setting range when you just want to discard the whole thing (in user-space of course). But the reason why to do it in ioctl_fstrim() is that otherwise some filesystems might decide to return EINVAL when no argument has been passed and some may handle it differently, hence it would not be consistent. > > Why did this common code go in through the ext4 tree as a start, and > without hitting linux-next before the merge window? I have posted it on both linux-fsdevel and linux-ext4 and we needed it for batched discard implementation in ext4 so that is probably why it went through ext4 tree. Unfortunately, I can not answer the second question... > > > What is the point of that? sb->s_bdev is not used anywhere in this code. > > If an FS published an sb->s_op->trim_fs, it should know what it wants > > No? Note that the FS in question does not even need to check. It already > > knows if it's block based or not. > > Indeed. Looks like copied from freeze/thaw which used to be block > device based, but don't really need this anymore either. > You're right. Thanks! -Lukas -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
