Currently names of REQ_COPY_USER and __REQ_COPY_USER constants are confusing,
because they actually mean that the buffer for the corresponding requests
has space in the tail for padding in case of DMA padding restrictions.
This patch renames REQ_COPY_USER and __REQ_COPY_USER constants to more
descriptive names REQ_HAS_TAIL_SPACE_FOR_PADDING and __REQ_HAS_TAIL_SPACE_FOR_PADDING
correspondingly.
It's against 2.6.30.1, but if necessary, I can update it to any necessary
kernel version.
Signed-off-by: Vladislav Bolkhovitin <[email protected]>
block/blk-map.c | 6 +++---
block/blk-merge.c | 2 +-
include/linux/blkdev.h | 4 ++--
3 files changed, 6 insertions(+), 6 deletions(-)
diff -upkr linux-2.6.30.1/block/blk-map.c linux-2.6.30.1/block/blk-map.c
--- linux-2.6.30.1/block/blk-map.c 2009-06-10 07:05:27.000000000 +0400
+++ linux-2.6.30.1/block/blk-map.c 2009-07-08 21:18:53.000000000 +0400
@@ -154,7 +155,7 @@ int blk_rq_map_user(struct request_queue
}
if (!bio_flagged(bio, BIO_USER_MAPPED))
- rq->cmd_flags |= REQ_COPY_USER;
+ rq->cmd_flags |= REQ_HAS_TAIL_SPACE_FOR_PADDING;
rq->buffer = rq->data = NULL;
return 0;
@@ -230,7 +231,7 @@ int blk_rq_map_user_iov(struct request_q
}
if (!bio_flagged(bio, BIO_USER_MAPPED))
- rq->cmd_flags |= REQ_COPY_USER;
+ rq->cmd_flags |= REQ_HAS_TAIL_SPACE_FOR_PADDING;
blk_queue_bounce(q, &bio);
bio_get(bio);
@@ -309,7 +836,7 @@ int blk_rq_map_kern(struct request_queue
bio->bi_rw |= (1 << BIO_RW);
if (do_copy)
- rq->cmd_flags |= REQ_COPY_USER;
+ rq->cmd_flags |= REQ_HAS_TAIL_SPACE_FOR_PADDING;
blk_rq_bio_prep(q, rq, bio);
blk_queue_bounce(q, &rq->bio);
diff -upkr linux-2.6.30.1/block/blk-merge.c linux-2.6.30.1/block/blk-merge.c
--- linux-2.6.30.1/block/blk-merge.c 2009-06-10 07:05:27.000000000 +0400
+++ linux-2.6.30.1/block/blk-merge.c 2009-07-08 21:18:53.000000000 +0400
@@ -198,7 +198,7 @@ new_segment:
} /* segments in rq */
- if (unlikely(rq->cmd_flags & REQ_COPY_USER) &&
+ if ((rq->cmd_flags & REQ_HAS_TAIL_SPACE_FOR_PADDING) &&
(rq->data_len & q->dma_pad_mask)) {
unsigned int pad_len = (q->dma_pad_mask & ~rq->data_len) + 1;
diff -upkr linux-2.6.30.1/include/linux/blkdev.h linux-2.6.30.1/include/linux/blkdev.h
--- linux-2.6.30.1/include/linux/blkdev.h 2009-06-10 07:05:27.000000000 +0400
+++ linux-2.6.30.1/include/linux/blkdev.h 2009-07-08 21:18:53.000000000 +0400
@@ -115,7 +115,7 @@ enum rq_flag_bits {
__REQ_RW_SYNC, /* request is sync (sync write or read) */
__REQ_ALLOCED, /* request came from our alloc pool */
__REQ_RW_META, /* metadata io request */
- __REQ_COPY_USER, /* contains copies of user pages */
+ __REQ_HAS_TAIL_SPACE_FOR_PADDING, /* has space for padding in the tail */
__REQ_INTEGRITY, /* integrity metadata has been remapped */
__REQ_NOIDLE, /* Don't anticipate more IO after this one */
__REQ_IO_STAT, /* account I/O stat */
@@ -143,7 +143,7 @@ enum rq_flag_bits {
#define REQ_RW_SYNC (1 << __REQ_RW_SYNC)
#define REQ_ALLOCED (1 << __REQ_ALLOCED)
#define REQ_RW_META (1 << __REQ_RW_META)
-#define REQ_COPY_USER (1 << __REQ_COPY_USER)
+#define REQ_HAS_TAIL_SPACE_FOR_PADDING (1 << __REQ_HAS_TAIL_SPACE_FOR_PADDING)
#define REQ_INTEGRITY (1 << __REQ_INTEGRITY)
#define REQ_NOIDLE (1 << __REQ_NOIDLE)
#define REQ_IO_STAT (1 << __REQ_IO_STAT)
On Thu, Jul 09 2009, Vladislav Bolkhovitin wrote:
> Currently names of REQ_COPY_USER and __REQ_COPY_USER constants are confusing,
> because they actually mean that the buffer for the corresponding requests
> has space in the tail for padding in case of DMA padding restrictions.
No, that's not what it means, the fact that there's padding room is a
side effect of the map type. So I'd suggest adding a comment above that
if {} in blk_rq_map_sg(), something that should have been there from the
beginning.
--
Jens Axboe
Jens Axboe, on 07/09/2009 10:17 PM wrote:
> On Thu, Jul 09 2009, Vladislav Bolkhovitin wrote:
>> Currently names of REQ_COPY_USER and __REQ_COPY_USER constants are confusing,
>> because they actually mean that the buffer for the corresponding requests
>> has space in the tail for padding in case of DMA padding restrictions.
>
> No, that's not what it means, the fact that there's padding room is a
> side effect of the map type. So I'd suggest adding a comment above that
> if {} in blk_rq_map_sg(), something that should have been there from the
> beginning.
Can you elaborate a bit more about what REQ_COPY_USER should mean, please?
As far as I can see from the sources, currently it's used to only to
determine if there is the padding space. Maybe, the original meaning
doesn't make sense anymore?
Thanks,
Vlad
Vladislav Bolkhovitin, on 07/09/2009 10:30 PM wrote:
> Jens Axboe, on 07/09/2009 10:17 PM wrote:
>> On Thu, Jul 09 2009, Vladislav Bolkhovitin wrote:
>>> Currently names of REQ_COPY_USER and __REQ_COPY_USER constants are confusing,
>>> because they actually mean that the buffer for the corresponding requests
>>> has space in the tail for padding in case of DMA padding restrictions.
>> No, that's not what it means, the fact that there's padding room is a
>> side effect of the map type. So I'd suggest adding a comment above that
>> if {} in blk_rq_map_sg(), something that should have been there from the
>> beginning.
>
> Can you elaborate a bit more about what REQ_COPY_USER should mean, please?
>
> As far as I can see from the sources, currently it's used to only to
> determine if there is the padding space. Maybe, the original meaning
> doesn't make sense anymore?
Jens,
Sorry for bothering you, but with use of the kernel-generated buffers,
like in the patch I sent in
http://groups.google.com/group/linux.kernel/browse_thread/thread/b5127eeadbef934e/5da7ec7b424d2f36?lnk=raot
name REQ_COPY_USER becomes quite misleading.
For instance, for blk_rq_map_kern_sg() usage of REQ_COPY_USER the
buffers neither copied, nor from user. Plus, as I wrote above, in 2.6.30
REQ_COPY_USER used only to determine if there is the padding space in
the end. Hence, I suggested the rename.
Thanks,
Vlad