Re: [PATCH 1/5] block: Implement support for WRITE SAME

["Martin K. Petersen" <martin.petersen@oracle.com>]

>>>>> "Vivek" == Vivek Goyal <vgoyal@redhat.com> 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.


>> - 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.


>> 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.

-- 
Martin K. Petersen	Oracle Linux Engineering