Re: remove a few uses of ->queuedata
On Tue, May 12, 2020 at 1:08 AM Christoph Hellwig wrote: > > On Sat, May 09, 2020 at 08:07:14AM -0700, Dan Williams wrote: > > > which are all used in the I/O submission path (generic_make_request / > > > generic_make_request_checks). This is mostly a prep cleanup patch to > > > also remove the pointless queue argument from ->make_request - then > > > ->queue is an extra dereference and extra churn. > > > > Ah ok. If the changelogs had been filled in with something like "In > > preparation for removing @q from make_request_fn, stop using > > ->queuedata", I probably wouldn't have looked twice. > > > > For the nvdimm/ driver updates you can add: > > > > Reviewed-by: Dan Williams > > > > ...or just let me know if you want me to pick those up through the nvdimm > > tree. > > I'd love you to pick them up through the nvdimm tree. Do you want > to fix up the commit message yourself? Will do, thanks.
Re: remove a few uses of ->queuedata
On Sat, May 09, 2020 at 08:07:14AM -0700, Dan Williams wrote: > > which are all used in the I/O submission path (generic_make_request / > > generic_make_request_checks). This is mostly a prep cleanup patch to > > also remove the pointless queue argument from ->make_request - then > > ->queue is an extra dereference and extra churn. > > Ah ok. If the changelogs had been filled in with something like "In > preparation for removing @q from make_request_fn, stop using > ->queuedata", I probably wouldn't have looked twice. > > For the nvdimm/ driver updates you can add: > > Reviewed-by: Dan Williams > > ...or just let me know if you want me to pick those up through the nvdimm > tree. I'd love you to pick them up through the nvdimm tree. Do you want to fix up the commit message yourself?
Re: remove a few uses of ->queuedata
On Sat, May 9, 2020 at 1:24 AM Christoph Hellwig wrote: > > On Fri, May 08, 2020 at 11:04:45AM -0700, Dan Williams wrote: > > On Fri, May 8, 2020 at 9:16 AM Christoph Hellwig wrote: > > > > > > Hi all, > > > > > > various bio based drivers use queue->queuedata despite already having > > > set up disk->private_data, which can be used just as easily. This > > > series cleans them up to only use a single private data pointer. > > > > ...but isn't the queue pretty much guaranteed to be cache hot and the > > gendisk cache cold? I'm not immediately seeing what else needs the > > gendisk in the I/O path. Is there another motivation I'm missing? > > ->private_data is right next to the ->queue pointer, pat0 and part_tbl > which are all used in the I/O submission path (generic_make_request / > generic_make_request_checks). This is mostly a prep cleanup patch to > also remove the pointless queue argument from ->make_request - then > ->queue is an extra dereference and extra churn. Ah ok. If the changelogs had been filled in with something like "In preparation for removing @q from make_request_fn, stop using ->queuedata", I probably wouldn't have looked twice. For the nvdimm/ driver updates you can add: Reviewed-by: Dan Williams ...or just let me know if you want me to pick those up through the nvdimm tree.
Re: remove a few uses of ->queuedata
On Sat, May 09, 2020 at 06:13:21AM +0800, Ming Lei wrote: > On Fri, May 08, 2020 at 06:15:02PM +0200, Christoph Hellwig wrote: > > Hi all, > > > > various bio based drivers use queue->queuedata despite already having > > set up disk->private_data, which can be used just as easily. This > > series cleans them up to only use a single private data pointer. > > > > blk-mq based drivers that have code pathes that can't easily get at > > the gendisk are unaffected by this series. > > Yeah, before adding disk, there still may be requests queued to LLD > for blk-mq based drivers. > > So are there this similar situation for these bio based drivers? bio submittsion is based on the gendisk, so we can't submit before it is added. The passthrough request based path obviously doesn't apply here.
Re: remove a few uses of ->queuedata
On Fri, May 08, 2020 at 11:04:45AM -0700, Dan Williams wrote: > On Fri, May 8, 2020 at 9:16 AM Christoph Hellwig wrote: > > > > Hi all, > > > > various bio based drivers use queue->queuedata despite already having > > set up disk->private_data, which can be used just as easily. This > > series cleans them up to only use a single private data pointer. > > ...but isn't the queue pretty much guaranteed to be cache hot and the > gendisk cache cold? I'm not immediately seeing what else needs the > gendisk in the I/O path. Is there another motivation I'm missing? ->private_data is right next to the ->queue pointer, pat0 and part_tbl which are all used in the I/O submission path (generic_make_request / generic_make_request_checks). This is mostly a prep cleanup patch to also remove the pointless queue argument from ->make_request - then ->queue is an extra dereference and extra churn.
Re: remove a few uses of ->queuedata
On Fri, May 08, 2020 at 06:15:02PM +0200, Christoph Hellwig wrote: > Hi all, > > various bio based drivers use queue->queuedata despite already having > set up disk->private_data, which can be used just as easily. This > series cleans them up to only use a single private data pointer. > > blk-mq based drivers that have code pathes that can't easily get at > the gendisk are unaffected by this series. Yeah, before adding disk, there still may be requests queued to LLD for blk-mq based drivers. So are there this similar situation for these bio based drivers? Thanks, Ming
Re: remove a few uses of ->queuedata
On Fri, May 8, 2020 at 9:16 AM Christoph Hellwig wrote: > > Hi all, > > various bio based drivers use queue->queuedata despite already having > set up disk->private_data, which can be used just as easily. This > series cleans them up to only use a single private data pointer. ...but isn't the queue pretty much guaranteed to be cache hot and the gendisk cache cold? I'm not immediately seeing what else needs the gendisk in the I/O path. Is there another motivation I'm missing?
remove a few uses of ->queuedata
Hi all, various bio based drivers use queue->queuedata despite already having set up disk->private_data, which can be used just as easily. This series cleans them up to only use a single private data pointer. blk-mq based drivers that have code pathes that can't easily get at the gendisk are unaffected by this series.
