On Sat, Aug 03, 2024 at 08:54:45PM +0300, Max Gurtovoy wrote: > > On 03/08/2024 15:39, Michael S. Tsirkin wrote: > > On Sat, Aug 03, 2024 at 01:07:27AM +0300, Max Gurtovoy wrote: > > > On 01/08/2024 20:56, Stefan Hajnoczi wrote: > > > > On Thu, Aug 01, 2024 at 06:56:44PM +0300, Max Gurtovoy wrote: > > > > > On 01/08/2024 18:43, Michael S. Tsirkin wrote: > > > > > > On Thu, Aug 01, 2024 at 06:39:16PM +0300, Max Gurtovoy wrote: > > > > > > > On 01/08/2024 18:29, Michael S. Tsirkin wrote: > > > > > > > > On Thu, Aug 01, 2024 at 06:17:21PM +0300, Max Gurtovoy wrote: > > > > > > > > > On 01/08/2024 18:13, Michael S. Tsirkin wrote: > > > > > > > > > > On Thu, Aug 01, 2024 at 06:11:37PM +0300, Max Gurtovoy wrote: > > > > > > > > > > > In this operation set the driver data of the hctx to point to the virtio > > > > > > > > > > > block queue. By doing so, we can use this reference in the and reduce > > > > > > > > > > in the .... ? > > > > > > > > > sorry for the type. > > > > > > > > > > > > > > > > > > should be : > > > > > > > > > > > > > > > > > > "By doing so, we can use this reference and reduce the number of operations in the fast path." > > > > > > > > ok. what kind of benefit do you see with this patch? > > > > > > > As mentioned. This is a micro optimization that reduce the number of > > > > > > > instructions/dereferences in the fast path. > > > > > > By how much? How random code tweaks affect object code is unpredictable. > > > > > > Pls show results of objdump to prove it does anything > > > > > > useful. > > > > > This is the way all modern block drivers such as NVMe PCI/RDMA/TCP use the > > > > > driver_data. > > > > > > > > > > These drivers don't have driver specific mechanisms to find the queue from > > > > > the hctx->queue->queuedata like vblk driver has for some unknown reason. > > > > > > > > > > It is pretty easy to review this patch and see its benefits, isn't it ? > > > > > > > > > > It is not expected to provide extreme perf improvement. > > > > > > > > > > It is introduced for aligning the driver to use common MQ mechanisms and > > > > > reduce dereferences. > > > > > > > > > > This is not "random code tweaks". > > > > If you cannot observe a performance change, then adjusting the commit > > > > description to explain this as a code cleanup to reduce dereferences and > > > > local variables, improving code readability seems fine to me. I think > > > > it's a nice cleanup when presented as such rather than a performance > > > > optimization. > > > > > > > > Stefan > > > Sure. Please check the bellow adjustment: > > > > > > virtio_blk: implement init_hctx MQ operation > > > > > > Set the driver data of the hardware context (hctx) to point directly to > > > the virtio block queue. This cleanup improves code readability, reduces > > > the number of dereferences, and minimizes local variables in the fast > > > path. > > I'd drop the local variables part, it is not at all clear why is that > > a win. > > We can drop it: > > virtio_blk: implement init_hctx MQ operation > > Set the driver data of the hardware context (hctx) to point directly to > the virtio block queue. This cleanup improves code readability and reduces > the number of dereferences in the fast path. > yep. also pls drop 1/1 from subject. Just [PATCH vX] pls repost with these commit log tweaks, I will queue. -- MST
