| Message ID | 20170619072628.12894-2-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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?
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
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.
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
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;
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(-)