From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from AM0PR83CU005.outbound.protection.outlook.com (mail-westeuropeazon11020125.outbound.protection.outlook.com [52.101.69.125]) by lore.virtuozzo.com (Postfix) with ESMTPS id 9E1D380474 for ; Wed, 28 Oct 2026 11:48:55 +0000 (UTC) ARC-Seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=CaYac/opKOQ+BVrGUQaBfGyPq9SAFaMH2bNajM39okyPAzm0JvKszsGYZSKp/DpnX+X+22FJ3xh5AWChRAe/2SCruGhjWRKSC2H8vgy59YkIThNC9rvZ0QXMJxYWgF0+NR+zuzDVStGbajtMsEDofg7M5LhUod4Ioy3NlysBdkOw8A38GEaQuX2VKJHvIGzFug6dU9pu3w+kGGc1YwswfsA+7W7D/rjb0VaEzr97rFbud0hYINmplsazbb3r4U3Y6g8kKZfJTw504blfUZcFaoIw+bQAvIWEc0MxzXjwkCvJyLU5DLuJPueKvq8Ah2/F6DAO28GJM3i9PN4N1aTWLw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector10001; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=jZYMkCUKpmRn8lpcGNBLyYjyIKiIZ05E9K8aUZzw+fM=; b=WcpZsGlzvmJaSoMmE/YKt9i8hbtBJyhbdqRVO0YO6PEr1fEduAYPqRxvdV/lX1J+hawjttq+oftIFYfe6hbNV/YvMjdYMK0pQPOJauwGhgvcP2I67RlkyGUh8BRI7/hLdmBNP7s45WNwp6bThwmNdhJGVEQRAUfw3gRqOqwLepDa14HouggvC3C6ebr395GsGSlk/5u0Zg9xYFLzLM6duPRMx93T5hMITKDzIuh23kIHV642tfMr0EJ8AL+OGVytyhaFnFz3TT+NSi/hYAt4l75Muio0HQy3TExyy/KRmLKjI5oFTCX8BZQL/YNShrevdmvT/NFV4r9kS08fWV2RoA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=virtuozzo.com; dmarc=pass action=none header.from=virtuozzo.com; dkim=pass header.d=virtuozzo.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=virtuozzo.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=jZYMkCUKpmRn8lpcGNBLyYjyIKiIZ05E9K8aUZzw+fM=; b=j+ieFQ9wfZ2lTkl6Mx4mX5c3aKRct4pQ5Bhi246QrC5l1wsmGTfd+oTKJoDbBIrOUPFDi9gCUytyswGKQiEvIsRMOftuhPkZYzF5/b9FIPyW2nUOUkARwhEr/hGfbLwd7RcglH7+isp/u+QjvWTz/e9HjCtq9fwVBLRpu5zVZ0dMfq7z55/erYMy23THv4R1QRc5bDDWpGgb6ggQWjMt+w3tz8BEkr+wYj+vP0Ij8KRZtjWWoVIZbRL1TcSe0ytPtnh0SAsxCpRvz83x5DqeH1qdZB4l2+P+IMBj/k3X/IrhDRMTNdZL5IPBwg+ltmbdLDqfkgPOv/xHSx3Y+e3DZA== Authentication-Results: mx.microsoft.com 1; dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=virtuozzo.com; Received: from VI0PR08MB10743.eurprd08.prod.outlook.com (2603:10a6:800:205::19) by AS2PR08MB9223.eurprd08.prod.outlook.com (2603:10a6:20b:59d::7) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.21.546.22; Wed, 28 Oct 2026 11:48:53 +0000 Received: from VI0PR08MB10743.eurprd08.prod.outlook.com ([fe80::7a7c:d078:f125:f16a]) by VI0PR08MB10743.eurprd08.prod.outlook.com ([fe80::7a7c:d078:f125:f16a%6]) with mapi id 15.21.0546.017; Wed, 28 Oct 2026 11:48:53 +0000 Message-ID: <6e8b3fa4-b100-4044-ba32-67087f9d970d@virtuozzo.com> Date: Wed, 28 Oct 2026 12:48:50 +0100 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH hci-8.1 v2 1/2] vhost-blk: create kernel threads with the mainstream worker ioctls To: Andrey Drobyshev , andrey.zhadchenko@virtuozzo.com Cc: svt-core@virtuozzo.com References: <20261027124759.2979043-1-alexander.ivanov@virtuozzo.com> <20261027124759.2979043-2-alexander.ivanov@virtuozzo.com> <1b58bb6c-b384-4925-bdfa-a2021c59a2bd@virtuozzo.com> Content-Language: en-US From: Alexander Ivanov In-Reply-To: <1b58bb6c-b384-4925-bdfa-a2021c59a2bd@virtuozzo.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-ClientProxiedBy: ZR1PEPF000077B0.CHEP278.PROD.OUTLOOK.COM (2603:10a6:918::425) To VI0PR08MB10743.eurprd08.prod.outlook.com (2603:10a6:800:205::19) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: VI0PR08MB10743:EE_|AS2PR08MB9223:EE_ X-MS-Office365-Filtering-Correlation-Id: 958ad068-1071-4771-5292-08df34e97030 X-LD-Processed: 0bc7f26d-0264-416e-a6fc-8352af79c58f,ExtAddr List-Id: svt-core@virtuozzo.com X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0;ARA:13230040|366016|1800799024|376014|23010399003|56012099006|4143699003|261026140227099003|18092099006|18002099003|22082099003|5113699003|10067099003|261021121427099003|261009223027099003|261022075727099003|261009221227099003|261022105027099003|261026121527099003; X-Microsoft-Antispam-Message-Info: 6PJhEqxrNwhd2Xcx6G3F+/lixZ/xEKy6qxqU3wY4J+Pw5SpQibfN5aCNPdcPLoVYDE5VN7Z9hEnbIIy7sx3bfZlpwMjJjvhxMfZ8u++8u6y9zjewekYTVDQfjumcroa9XWs4PRxDHSzAMvxYlvlNpJLyPFU+BUTrB70ER51o8XYkd08qaX9o8YGDnBRt2qVwy6XaU9xjUB/G6ubjjk9mmoTAiNb81eI9vrEEUYKFAZOaxE6/V618Rug7/dhIHLt7c/HUZ0+QE5PvwIT7M2T6kxgvQ0Hyjyw6vc93eMXeSikboO3v/Afzo0RXJ8waCe68ncp1RP+lky5Dv4b9FaWAmJJ9OiZpNWHMYMPfBlplPPKrz2rOJWxORskq5y948bFJhef2RTPCjuDYJj4IJ9IhkXM9GI8fgPeQaoZ/ItzOPT2lM7ZL2N0UcJfyk67mfbFQL1xum9PgnFUrdgSl/EtJDIjhvyW+q0VbNbMh92hk1M709eqZWBETAzhBw3YFeGPFXnoG26OMHCXf8dUdEVTh9rFtHFDW643mkucXVbarEAdsomekm9HK1ds1CEw8iz+o2jVoYqQgCc3ICXslZxidVCzVjoNpvU23P2CQ4Kj3RDL115PhSVwutDA+tZylCPLiyN78RBpqcoz+7n2qxHsIA8RXR7uGGwkr+Ey9cDQeRl8= X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:VI0PR08MB10743.eurprd08.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230040)(366016)(1800799024)(376014)(23010399003)(56012099006)(4143699003)(261026140227099003)(18092099006)(18002099003)(22082099003)(5113699003)(10067099003)(261021121427099003)(261009223027099003)(261022075727099003)(261009221227099003)(261022105027099003)(261026121527099003);DIR:OUT;SFP:1102; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?RmE1UVJBZ0VwK0pWRU4xUWZ5UlVDMXh6WklxaUhMS2J6ZlgvSDNYT1ZNWXlx?= =?utf-8?B?THN2aFJhU29SMGpScFRrWC9TNVNPWFdOZnFLbnRUUkNDaUYra01qRHowTHg0?= =?utf-8?B?M0JDYzI5anU4YnJGSTdGRWJ6WVNHV2M0RlBQcStvc2t6STRBNHJPSXJKcnMw?= =?utf-8?B?ZzNnTHowbGttTWRPMHRnNXdQbW9EbU5hbG9ESkhvbmNkb2VyYmtzUmtZNHdL?= =?utf-8?B?TGxxd3IrenhrempzTFhQam5IY1k0QmJ0T1pHZzN5alo4SDZPUkRLaVMwcXBQ?= =?utf-8?B?MzZhYjB5UVU2T0Vzbk0xOTNGSkJ1emVPS2NRbHJXSFArSjArdTE3M0VWT2p2?= =?utf-8?B?SGY4N0h3SWlsZXJkQ0ptMjBxWm5NYmhtMXgrTmFZa3hPeE5OMUYrUUlvd3Jh?= =?utf-8?B?RnpNVTVaOEFicVk0QktMQVl0eHl1VDMwWFEyMDlYWVJSYURDc1VwUEU0MDBR?= =?utf-8?B?RVNMb3p0T25LTWRKcW9MZmNGQTQ5ZC9sdjNmRDd0NzFSTFMrM3A0TlcycXZ0?= =?utf-8?B?SnNTaElJb0tsT2ZSRzE5cjJ2TVdqUUllWkRCZlB0UnVYYks5M2orRXQ3OFd1?= =?utf-8?B?Nmoza0F3Tzl1TFZ5Y3A1T0kxNnVMVCtpRVVBUVl4UmZBTnlKZTlPRjNjZS9F?= =?utf-8?B?SXFRRVFHTkdFNHgxZDFkZXRNdDdzeEh3Tk1PaGtiZ3VpSUsrZlF0enhOREU2?= =?utf-8?B?dlJ1ODJhQjBjRjI0aGVFcEdHY0swMkpHeDNXd2FvSUJjZVlhbEZYclk3dWo3?= =?utf-8?B?V1RoRkVIQzVLK3J3T283M2ZrWTRkT3BreENLTTFob1ZkZnBWYkFWNXYwdFVn?= =?utf-8?B?VlFTTHZvRnFHUW5FdVFwUTlUVitHcW9LUlZ5YnZ2VTArZC83bSs0aDB5MU5J?= =?utf-8?B?NkY0bDdWWEQxY1o5cm9oYlcreUtHeHY2K3R5VmN2aUo2dHVhaXlXbldhMUVT?= =?utf-8?B?Z3lVTjR3UmpJb0FtRFA4K0ZxSHFRWDZjYy9aSVc2YXhWazY2NWMrQW9vN2tM?= =?utf-8?B?cjUwdGtiajQ3TmJOMDNUN3dWcHd1cjNwbU5UTGhtVDdsSmtCbFpSTjRlYk5V?= =?utf-8?B?ZUxiOVU1UWVCMEdraUNjWGZMOENvbTMwL2crVjZrWnRTMVhLUVdIdXRYcTZH?= =?utf-8?B?MThLdjBrNFhuVWxpem1nUTJJZnUyZ2NuTFk4eHJ3eGxIUnArQWUyZVQ5SmFL?= =?utf-8?B?U0NuVUlXTnY5VnR3Vk5aU1RSZk9sY0U4bXB4N0VxbnlhY2RkT0RCV3JXQ2Jk?= =?utf-8?B?cytHbEJsVHJxc2ZUU0ZSTWE3UlBDVnFuOVFRYkl3RGpKY0FqZmJFdUFKRXc1?= =?utf-8?B?cXYzeWwzdnBsMFFQcmU4T1pRVExtRk8vb2YyTmhTNmVrTE9NM3pkRyticjdQ?= =?utf-8?B?NE40eWhnMjRmZS9mRCtaa0h5Z1lPQ3F3cUVQaEtsdTZqWC9OYnJMSmRoOHJ3?= =?utf-8?B?dmFIbWNlN3RQMFplMTNLN0FZYmd1OHY3bUpIZktqQkQxRWpYYVlGczhaL1lK?= =?utf-8?B?aVBRYk5sdlVjWkIxVElTb0lhL3c1b2RQUW1mSkc4NVgyRDNuZjBoaEFtaC9U?= =?utf-8?B?RmorYlNqbVRUTDdrY0QyUXFnSDR3NFBvL2pVUnBQcDNEUzByWTlsNmMvdGJ1?= =?utf-8?B?NVRRbGFMQVRBeEpybmZRRzdtcm9tT2NWK2JuVnFzYStDbEVBKzZNQWxDMWdk?= =?utf-8?B?RHpkaVN0TjlUL2JYU1Q2RjM2K29JRUt6bVlNQkEzTlF2Q2hzRTg3eXZYMVZ2?= =?utf-8?B?aGxCTUd0R2wwenlnaDdJRmxJMmNDUk1ZOGlmbG85YXFyelU2OVliWXNKVnFW?= =?utf-8?B?VEF1UzVLTDNLTnJMeEFibmNac3J3VUswbHpXWVU5eWRHNlhGNGZmN2kyWllI?= =?utf-8?B?UWFwQ0hvam8xK0IrL3YwdE9FWEtJdVRMRzJUTVRPdG52ZTdTYnNWUHVmUE82?= =?utf-8?B?UnJLajlFNzhxeS9SSWY0Y3U4WlMwcHNmaHpVSU5laUxzQ2syM1pVb0lhT3Ri?= =?utf-8?B?LzQ1RFZPWXRWOEs1SkR5UjZ1aXhvcGd0RGJzZktoYUhKdzJUY0NQNkxvZ0F2?= =?utf-8?B?MzJuWG41OFkyakxXYzF5b0dVREEyVElJMk9jVktySEU2RFFMb1RSMWE2emxG?= =?utf-8?B?YmtON3FwZ0hlTlhuZmFZN0pwNlZaYjNPVW1Db3k0aCtpcldPSURMWFpiV1Jt?= =?utf-8?B?RFhhMTVZRC9tbDBFVnNvNEh4R2h0cUlTSVovaU1qTmVBU0ZWRVl1ZE1SbTNF?= =?utf-8?B?bXg5U3liRmxuY3FiV0RVVzdwU1VHQmVURW1sVitLWTV4YjAwUFlOd3IwZkZl?= =?utf-8?B?WWpLalY5QXZlNXZZaFltYmNvMGxsWTlXbVZNZERJTmg4WDRlYU9OdHFwdzRI?= =?utf-8?Q?mCQQNwAwpVchicfs=3D?= X-Auto-Response-Suppress: DR, OOF, AutoReply X-OriginatorOrg: virtuozzo.com X-MS-Exchange-CrossTenant-Network-Message-Id: 958ad068-1071-4771-5292-08df34e97030 X-MS-Exchange-CrossTenant-AuthSource: VI0PR08MB10743.eurprd08.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Oct 2026 11:48:52.8679 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 0bc7f26d-0264-416e-a6fc-8352af79c58f X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: hgcdyXNDNXCqsjKMVOV1vgMommm6btfNLEuYqabROpVEpyz0ySDcEdT4yN7p7XHqetiTHFF9MtfNuVm5v+9lOwDOAsMinaKN1e9a77NIpRQ= X-MS-Exchange-Transport-CrossTenantHeadersStamped: AS2PR08MB9223 > The overall logic is fine, but I'd like patch to be simplified. > > First, there're dedicated vhost_ops->vhost_new_worker() and > vhost_ops->vhost_attach_vring_worker() calls. We should reuse them. > For consistency, let's add another patch "vhost-backend: add > vhost_set_fork_from_owner op". Smth like: > >> /* hw/virtio/vhost-backend.c */ >> static int vhost_kernel_set_fork_from_owner(struct vhost_dev *dev, >> uint8_t fork_owner) >> { >> return vhost_kernel_call(dev, VHOST_SET_FORK_FROM_OWNER, &fork_owner); >> } Done. > Other issues (by Claude): > > 1. Dead fallback in vhost_blk_set_workers(). The ENOTTY && i == 1 branch > handles a kernel that has VHOST_SET_FORK_FROM_OWNER (6.17 / vz10) but > not VHOST_NEW_WORKER (6.4) — no such kernel exists. Once the fork-mode > ioctl succeeded, a worker-ioctl failure is a real error. Drop, do a > single fallback point in start(). The fallback is not dead. VHOST_SET_FORK_FROM_OWNER is in our 6.12 kernel as a RHEL 10 backport. The worker ioctls are generic, but each vhost driver has to route them to vhost_worker_ioctl(), and vhost-blk only does that with "drivers/vhost/blk: add support for worker ioctls" (VSTOR-107537), which is not merged yet. > 2. Fork-mode block in vhost_blk_start() is hard to read. A declaration > inside the if, the ENOTTY test nested under 'ret != -ENOTTY', the > 'nworkers = 1' fall-through at the bottom. Switch to flat three-way if > (ret == -ENOTTY) … else if (ret < 0) … at the call site, ioctl plumbing > in the backend. Done. > 3. Redundant clamp. MIN(nworkers, VHOST_BLK_MAX_QUEUES) is already > guaranteed by nworkers ≤ nvqs ≤ VHOST_BLK_MAX_QUEUES (realize, :666); a > clamp hides an invariant. Switch to assert(nworkers <= ARRAY_SIZE(ids)). Done, switched to the assert. To make it hold, I added a patch that rejects num-queues above VHOST_BLK_MAX_QUEUES at realize: before, only the auto value was capped there. > 4. memset() + field stores for two structs per iteration. Switch to > designated initializers, and the two loops merged into one: thread w is > created by its first user vq (i < nworkers), then attached — same ioctl > count, one error path per ioctl, loop body reads as "which thread, > create if first, attach". Done.  The structs use designated initializers, and the two loops are merged into one: a thread is created by its first vq, then attached. > 5. s->dev. repeated throughout. Use struct vhost_dev *hdev = &s->dev; > once, as vhost-scsi does; vhost_blk_set_workers() takes hdev and no > longer needs VHostBlk. Done. vhost_blk_start() uses hdev throughout, and vhost_blk_set_workers() takes hdev instead of VHostBlk. > 6. Commit message nits (1/2). "The kernel is dropping it" needs the > kernel change/ticket; say that VHOST_NEW_WORKER returns -EFAULT (not > ENOTTY) in kthread mode, which is why the mode switch — not the worker > ioctl — is the capability probe; "default worker being the first one" > belongs in the code comment, where it already is. > > Here's what the changed version might look like: > >> /* >> * Spread the virtqueues round-robin over nworkers kernel threads. Thread 0 >> * is the default worker VHOST_SET_OWNER created; vq 0 and every nworkers-th >> * vq after it stay on it. >> */ >> static int vhost_blk_set_workers(struct vhost_dev *hdev, int nworkers) >> { >> uint32_t ids[VHOST_BLK_MAX_QUEUES]; /* ids[0]: default worker, unused */ >> int i, ret; >> >> assert(nworkers <= ARRAY_SIZE(ids)); >> >> for (i = 1; i < hdev->nvqs; i++) { >> int w = i % nworkers; >> struct vhost_vring_worker vq_worker = { .index = i }; >> >> if (w == 0) { >> continue; >> } >> >> /* vq i < nworkers is the first user of thread i: create it here */ >> if (i < nworkers) { >> struct vhost_worker_state worker = { }; >> >> ret = hdev->vhost_ops->vhost_new_worker(hdev, &worker); >> if (ret < 0) { >> error_report("vhost-blk: unable to create kernel thread: %s", >> strerror(-ret)); >> return ret; >> } >> ids[w] = worker.worker_id; >> } >> >> vq_worker.worker_id = ids[w]; >> ret = hdev->vhost_ops->vhost_attach_vring_worker(hdev, &vq_worker); >> if (ret < 0) { >> error_report("vhost-blk: unable to attach kernel thread to " >> "queue %d: %s", i, strerror(-ret)); >> return ret; >> } >> } >> >> return 0; >> } >> >> static int vhost_blk_start(VirtIODevice *vdev) >> { >> VHostBlk *s = VHOST_BLK(vdev); >> struct vhost_dev *hdev = &s->dev; >> ... >> int ret, i, nworkers; >> ... >> if (s->vhost_started) { >> return 0; >> } >> >> nworkers = MIN(s->conf.num_threads, hdev->nvqs); >> if (nworkers > 1) { >> /* >> * VHOST_NEW_WORKER needs vhost_task mode; our kernel defaults to >> * kthreads and VHOST_RESET_OWNER brings that default back, so ask >> * on every start. Must precede VHOST_SET_OWNER. >> */ >> ret = hdev->vhost_ops->vhost_set_fork_from_owner(hdev, >> VHOST_FORK_OWNER_TASK); >> if (ret == -ENOTTY) { >> warn_report_once("vhost-blk: kernel does not support vhost tasks, " >> "using a single kernel thread"); >> nworkers = 1; >> } else if (ret < 0) { >> error_report("vhost-blk: unable to set vhost task mode: %s", >> strerror(-ret)); >> return ret; >> } >> } >> >> if (ioctl(s->vhostfd, VHOST_SET_OWNER, NULL)) { >> error_report("vhost-blk: unable to set owner"); >> return -ENOSYS; >> } >> >> if (nworkers > 1) { >> ret = vhost_blk_set_workers(hdev, nworkers); >> if (ret < 0) { >> return ret; >> } >> } >> >> if (s->conf.serial) { >> ... Done.