On Mon, May 02 2011 at 6:24am -0400, Lukas Czerner <lczerner@xxxxxxxxxx> wrote: > On Mon, 2 May 2011, Christoph Hellwig wrote: > > > On Mon, May 02, 2011 at 09:13:08AM +0100, Alasdair G Kergon wrote: > > > On Mon, May 02, 2011 at 09:16:21AM +0200, Lukas Czerner wrote: > > > > However when we have dm device where part of the device supports > > > > discard due to underlying hardware capability we just can not return > > > > EOPNOTSUPP from blkdev_issue_discard, because it is just not true! > > > > > > EOPNOTSUPP from dm means the operation was not supported on that *one* bio. > > > It does *not* tell you anything in general about the device, or whether > > > you'd get the same error from different bios in future. > > > > Exactly. We already have the information in the queue limits to tell > > the filesystem if discard is supported at all or not. > > > > So I gave it a try. First of all the device composed of SSD and spinning > disk does export discard_support information properly, however it also > advertise discard_zeroes_data which is wrong and possibly dangerous and > should be fixed! You're effectively advocating that blk_stack_limits() needs to clear the topmost device's discard_zeroes_data flag if any one bottom device does not have discard_zeroes_data. > And second of all, strictly speaking if EOPNOTSUPP from dm means that > operation was not supported on that *one* bio, blkdev_issue_discard() > should handle that and do not return EOPNOTSUPP further if queue limits > tells that device has discard support. Is this acceptable solution for > you guys ? I can make that change since I am changing blkdev_issue_discard() > anyway. Or, we can make that change in filesystem where we disable > discard on mount time, when we notice that discard mount option was > specified, but the device does not support it (we should probably do > this regardless on blkdev_issue_discard() change). The blkdev_issue_discard() change you propose could be fine (mask EOPNOTSUPP return if device advertises support for discards) -- though Eric said we shouldn't ever say we did something when we didn't. But that blkdev_issue_discard() change is really only safe if the discard_zeroes_data flag is cleared by blk_stack_limits() if finds inconsistent discard_zeroes_data support. Mike -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel
