[01/10] pktcdvd: remove the call to blk_queue_bounce
diff mbox

Message ID 20170619072628.12894-2-hch@lst.de
State New
Headers show

Commit Message

Christoph Hellwig June 19, 2017, 7:26 a.m. UTC
pktcdvd is a make_request based stacking driver and thus doesn't have any
addressing limits on it's own.  It also doesn't use bio_data() or
page_address(), so it doesn't need a lowmem bounce either.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/block/pktcdvd.c | 2 --
 1 file changed, 2 deletions(-)

Comments

Ming Lei June 19, 2017, 2:34 p.m. UTC | #1
On Mon, Jun 19, 2017 at 3:26 PM, Christoph Hellwig <hch@lst.de> wrote:
> pktcdvd is a make_request based stacking driver and thus doesn't have any
> addressing limits on it's own.  It also doesn't use bio_data() or
> page_address(), so it doesn't need a lowmem bounce either.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/block/pktcdvd.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
> index 26c04baae967..7f2eca31eb6a 100644
> --- a/drivers/block/pktcdvd.c
> +++ b/drivers/block/pktcdvd.c
> @@ -2413,8 +2413,6 @@ static blk_qc_t pkt_make_request(struct request_queue *q, struct bio *bio)
>         char b[BDEVNAME_SIZE];
>         struct bio *split;
>
> -       blk_queue_bounce(q, &bio);
> -
>         blk_queue_split(q, &bio);
>
>         pd = q->queuedata;
> --
> 2.11.0
>

blk_queue_make_request() sets bounce for any highmem page for long time,
and in theory this patch might cause regression on 32bit arch, when
the controller can't handle higmem page really(especially in case of PAE), so
we may need to be careful about this change, especially on some old hardware.

thanks,
Ming Lei
Christoph Hellwig June 19, 2017, 3 p.m. UTC | #2
On Mon, Jun 19, 2017 at 10:34:36PM +0800, Ming Lei wrote:
> blk_queue_make_request() sets bounce for any highmem page for long time,
> and in theory this patch might cause regression on 32bit arch, when
> the controller can't handle higmem page really(especially in case of PAE), so
> we may need to be careful about this change, especially on some old hardware.

Which controller?
Ming Lei June 19, 2017, 3:13 p.m. UTC | #3
On Mon, Jun 19, 2017 at 11:00 PM, Christoph Hellwig <hch@lst.de> wrote:
> On Mon, Jun 19, 2017 at 10:34:36PM +0800, Ming Lei wrote:
>> blk_queue_make_request() sets bounce for any highmem page for long time,
>> and in theory this patch might cause regression on 32bit arch, when
>> the controller can't handle higmem page really(especially in case of PAE), so
>> we may need to be careful about this change, especially on some old hardware.
>
> Which controller?

I don't know.

IMO, we can't just remove it because it is a bio based driver, the
bounce is not related with
a bio or req based driver, and it depends if the controller can handle
highmem page.

Actually the bounce for highmem is here from v2.6.12, and looks you
might need to
investigate why it is introduced from start before the removing, :-)



Thanks,
Ming Lei
Christoph Hellwig June 19, 2017, 3:18 p.m. UTC | #4
On Mon, Jun 19, 2017 at 11:13:46PM +0800, Ming Lei wrote:
> On Mon, Jun 19, 2017 at 11:00 PM, Christoph Hellwig <hch@lst.de> wrote:
> > On Mon, Jun 19, 2017 at 10:34:36PM +0800, Ming Lei wrote:
> >> blk_queue_make_request() sets bounce for any highmem page for long time,
> >> and in theory this patch might cause regression on 32bit arch, when
> >> the controller can't handle higmem page really(especially in case of PAE), so
> >> we may need to be careful about this change, especially on some old hardware.
> >
> > Which controller?
> 
> I don't know.
> 
> IMO, we can't just remove it because it is a bio based driver, the
> bounce is not related with
> a bio or req based driver, and it depends if the controller can handle
> highmem page.

pktcdvd is a stacking driver and forwards all data through either
generic_make_request or blk_rq_map_kern + blk_execute_rq,
both of which call blk_queue_bounce internally.
Ming Lei June 19, 2017, 3:29 p.m. UTC | #5
On Mon, Jun 19, 2017 at 11:18 PM, Christoph Hellwig <hch@lst.de> wrote:
> On Mon, Jun 19, 2017 at 11:13:46PM +0800, Ming Lei wrote:
>> On Mon, Jun 19, 2017 at 11:00 PM, Christoph Hellwig <hch@lst.de> wrote:
>> > On Mon, Jun 19, 2017 at 10:34:36PM +0800, Ming Lei wrote:
>> >> blk_queue_make_request() sets bounce for any highmem page for long time,
>> >> and in theory this patch might cause regression on 32bit arch, when
>> >> the controller can't handle higmem page really(especially in case of PAE), so
>> >> we may need to be careful about this change, especially on some old hardware.
>> >
>> > Which controller?
>>
>> I don't know.
>>
>> IMO, we can't just remove it because it is a bio based driver, the
>> bounce is not related with
>> a bio or req based driver, and it depends if the controller can handle
>> highmem page.
>
> pktcdvd is a stacking driver and forwards all data through either
> generic_make_request or blk_rq_map_kern + blk_execute_rq,
> both of which call blk_queue_bounce internally.

OK, thanks for explaining, then looks it isn't needed from the start.

Thanks,
Ming Lei

Patch
diff mbox

diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
index 26c04baae967..7f2eca31eb6a 100644
--- a/drivers/block/pktcdvd.c
+++ b/drivers/block/pktcdvd.c
@@ -2413,8 +2413,6 @@  static blk_qc_t pkt_make_request(struct request_queue *q, struct bio *bio)
 	char b[BDEVNAME_SIZE];
 	struct bio *split;
 
-	blk_queue_bounce(q, &bio);
-
 	blk_queue_split(q, &bio);
 
 	pd = q->queuedata;