On Wed, Feb 15, 2012 at 10:29:17PM -0500, Martin K. Petersen wrote:
> >>>>> "Vivek" == Vivek Goyal <vgoyal@xxxxxxxxxx> writes:
>
> Vivek,
>
> Vivek> Mike Snitzer mentioned that we do allow merging of one discard
> Vivek> request with another except following two cases.
>
> It's allowed by virtue of being marked mergeable. This goes back to the
> very first attempt we had a discard support. But we don't actually
> support merging of discard requests. The merge checks were just never
> updated to reflect this.
>
> I'm also trying to get rid of all the REQ_DISCARD special cases. Soon
> we'll have REQ_WRITE_SAME and REQ_COPY to worry about as well and the
> same rules apply to them. So I'm trying to leverage Tejun's recent
> cleanup to consolidate the merge checks.
Ok.
> >> - if (rq->cmd_type == REQ_TYPE_FS ||
> >> - (rq->cmd_flags & REQ_DISCARD)) {
> >> + if (rq->cmd_type == REQ_TYPE_FS) {
>
> Vivek> This is orthogonal to merging?
>
> REQ_DISCARD is REQ_TYPE_FS so the check is redundant.
Ok. So init_request_from_bio() will mark discard request also as
REQ_TYPE_FS. So this change is fine.
>
>
> >> break; case ELEVATOR_INSERT_SORT:
> >> - BUG_ON(rq->cmd_type != REQ_TYPE_FS &&
> >> - !(rq->cmd_flags & REQ_DISCARD));
> >> + BUG_ON(rq->cmd_type != REQ_TYPE_FS);
>
> Vivek> This change also looks orthogonal to merging. Will it not trigger
> Vivek> BUG_ON() when DISCARD request is being inserted into the
> Vivek> elevator?
>
> I thought the same thing. But I was unable to trigger it.
Given the fact that REQ_DISCARD will have REQ_TYPE_FS set, then second
condition is anyway redundant and that explains why bug is not triggered.
Thanks for the explanation.
Thanks
Vivek
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
