On Fri, Jul 02 2010 at 3:49pm -0400,
Mikulas Patocka <mpatocka@xxxxxxxxxx> wrote:
> > As we discussed, we have a challenge where we need DM to avoid issuing
> > a barrier before the discard IFF a target doesn't support the discard
> > (which the barrier is paired with).
> >
> > My understanding is that blkdev_issue_discard() only cares if the
> > discard was supported. Barrier is used just to decorate the discard
> > (for correctness). So by returning -EOPNOTSUPP we're saying the discard
> > isn't supported; we're not making any claims about the implict barrier,
> > so best to avoid the barrier entirely.
> >
> > Otherwise we'll be issuing unnecessary barriers (and associated
> > performance loss).
> >
> > So yet another TODO item... Anyway:
> >
> > Acked-by: Mike Snitzer <snitzer@xxxxxxxxxx>
>
> Unnecessary barriers are issued anyway. With each freed extent.
>
> The code must issue a "SYNCHRONIZE CACHE" to flush cache for previous
> writes, then "UNMAP" and then another "SYNCHRONIZE CACHE" to commit that
> unmap to disk. And this in loop for all extents in
> "release_blocks_on_commit".
You're delving into the mechanics of the discard when it is supported;
which is fine but tangential to my point above. My point was DM
shouldn't issue any barrier(s) at all if it the discard will not be sent
(because a device doesn't support discards).
> One idea behind "discard barriers" was to submit a discard request and not
> wait for it. Then the request would need a barrier so that it doesn't get
> reordered with further writes (that may potentially write to the same area
> as the discarded area). But discard isn't used this way anyway,
> sb_issue_discard waits for completion, so the barrier isn't needed.
>
> Even if ext4 developers wanted asynchronous discard requests, they should
> fire all the discards at once and then submit one zero-sized barrier. Not
> barrier with each discard request.
sb_issue_discard() is the block layer api that ext4 uses for discards.
Ext4, or any other filesystem that uses sb_issue_discard(), has no
control over the barriers that are issued.
> This is up to ext4 developers to optimize and remove the barriers and we
> can't do anything with it. Just send "SYNCHRONIZE
> CACHE"+"UNMAP"+"SYNCHRONIZE CACHE" like the barrier specification wants...
In practice that is what I see when I remove a file in ext4:
kdmflush-2537 [000] 911436.484481: scsi_dispatch_cmd_start: host_no=5 channel=0 id=0 lun=0 data_sgl=0 prot_sgl=0 cmnd=(SYNCHRONIZE_CACHE - raw=35 00 00 00 00 00 00 00 00 00)
kdmflush-2537 [000] 911436.484482: scsi_dispatch_cmd_done: host_no=5 channel=0 id=0 lun=0 data_sgl=0 prot_sgl=0 cmnd=(SYNCHRONIZE_CACHE - raw=35 00 00 00 00 00 00 00 00 00) result=(driver=DRIVER_OK host=DID_OK message=COMMAND_COMPLETE status=SAM_STAT_GOOD)
kdmflush-2537 [000] 911436.484500: scsi_dispatch_cmd_start: host_no=5 channel=0 id=0 lun=0 data_sgl=1 prot_sgl=0 cmnd=(UNMAP regions=1 raw=42 00 00 00 00 00 00 00 18 00)
<idle>-0 [000] 911436.485238: scsi_dispatch_cmd_done: host_no=5 channel=0 id=0 lun=0 data_sgl=1 prot_sgl=0 cmnd=(UNMAP regions=1 raw=42 00 00 00 00 00 00 00 18 00) result=(driver=DRIVER_OK host=DID_OK message=COMMAND_COMPLETE status=SAM_STAT_GOOD)
kdmflush-2537 [000] 911436.485283: scsi_dispatch_cmd_start: host_no=5 channel=0 id=0 lun=0 data_sgl=0 prot_sgl=0 cmnd=(SYNCHRONIZE_CACHE - raw=35 00 00 00 00 00 00 00 00 00)
kdmflush-2537 [000] 911436.485284: scsi_dispatch_cmd_done: host_no=5 channel=0 id=0 lun=0 data_sgl=0 prot_sgl=0 cmnd=(SYNCHRONIZE_CACHE - raw=35 00 00 00 00 00 00 00 00 00) result=(driver=DRIVER_OK host=DID_OK message=COMMAND_COMPLETE status=SAM_STAT_GOOD)
Mike
--
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
