Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   "Wang, Wei W" <wei.w.wang@intel.com>
To:     David Matlack <dmatlack@google.com>
CC:     "Christopherson,, Sean" <seanjc@google.com>,
        "pbonzini@redhat.com" <pbonzini@redhat.com>,
        "vipinsh@google.com" <vipinsh@google.com>,
        "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH v1] KVM: selftests: name the threads
Thread-Topic: [PATCH v1] KVM: selftests: name the threads
Thread-Index: AQHY4kUEXwKyHG3+pEa5Gek2S9rL/a4V/BSAgACZBjA=
Date:   Thu, 20 Oct 2022 11:37:19 +0000
Message-ID: <DS0PR11MB6373943463AEA7C3617CAE30DC2A9@DS0PR11MB6373.namprd11.prod.outlook.com>
References: <20221017162448.257173-1-wei.w.wang@intel.com>
 <Y1Az6y1X4reOp2n6@google.com>
In-Reply-To: <Y1Az6y1X4reOp2n6@google.com>
Accept-Language: en-US
Content-Language: en-US
dlp-product: dlpe-windows
dlp-reaction: no-action
dlp-version: 11.6.500.17
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?O0Ny3LpRRGsEAlkbBr+BnmhGRjCJqef4LkguDdB2Q3o1utWjX0zHnEFFEBro?=
 =?us-ascii?Q?+XfxltseW3uTC8qhW5bljnQIKie69nt8VBWaNaAZNh7npOGvXvHtS8WfJuRg?=
 =?us-ascii?Q?eTC3WOlAuH6S1+5H3iFbhwCv5GE9qVF2va+rLpZJt0lRcJswwvDjMd1acASk?=
 =?us-ascii?Q?nn0GUAjIRBh1ZLD7Qj5LmBmkH5SNWvTQ2/h1ruOT4YuNZ3fKT6DqofKQqIE8?=
 =?us-ascii?Q?D10puUqV9pIvmy2Ey1GPcFMCMJOuaxXmB8F18f1/y8hZMKhJi44VzdQYkcI8?=
 =?us-ascii?Q?vUQxeMw3gykdDIW9uZeQKStPIJtYx1q7OD2HW/7VkTLJj0R+VNwVx0A5Iv3Q?=
 =?us-ascii?Q?Aak9uJP3WnesiZwJY3f6FV4LYEqOdQmpd2fwX4sQco2B4U7G/g0VxsnkFxdC?=
 =?us-ascii?Q?AFC5B9fLerNNZwJb98r5dGR7gTm1YBqo5GvEBcZ88SMfa2L2Dk92tkGehgnn?=
 =?us-ascii?Q?RS08sdrV4AkFG8RyVbnFaleD56ysoebsvJcpXt9o/kPuZRfmnIwYHBId22hY?=
 =?us-ascii?Q?2wuZzJLbw1IlB+Eh7lUlG4YqKOxwc+rdpfu36eUQMbuSXrQeBuDloA28Ch/W?=
 =?us-ascii?Q?aynVXjStXYSOrh7cphjvQEnbapMFO8AC75UECoM3Ad6nerMSSUZ8tUClDnZ7?=
 =?us-ascii?Q?G0ZbzlVXEFru9PkLcIdnavjAx5ncpMNF7k76OB6DeHsRZLoXk/YCAW5ANMpz?=
 =?us-ascii?Q?RfhUt1mvPUlCKOfa2owj6inVqkOfw9k+5lHye68oOCNer+kLfRfgkZFA5Nxw?=
 =?us-ascii?Q?5NX7sMPKGDoZPnHS3dYsmKs6pfTqY0359SHmzgye2anC3KkPKsd0GDioNIaL?=
 =?us-ascii?Q?+JkaE5szeYOoCe/JsIHQZ2WO5O/gWY0WAYWtgSUJDhXedZk+GFf7ttPPPwT6?=
 =?us-ascii?Q?r/ozX958uM+VwWN4CAk3HQF0MYpUhqvKVTkAH2CgKLpGq1txg+zAjaniogrE?=
 =?us-ascii?Q?QP0h8ZqvFzzrOd4750EmurmbfT5ridNIQduCv2o0XtroyCsOtR6GBmHjOHQw?=
 =?us-ascii?Q?vLwU3Jq/rC674uszwQedHOn0emxuZvHO+oqtgjQdbe+JO65cZwiDJY6/qvvP?=
 =?us-ascii?Q?2doJGcMgBkwTdivXA9Ln0WYyDG7uvva0AEWeFWKghEOyiIw9uwex3QGJSGRa?=
 =?us-ascii?Q?eBPcQ1tCIi7f0iV0OnNDzZTZLNW9p4sWMWQf8Qj5uCTS7iswe2AfcJUbKfsC?=
 =?us-ascii?Q?YnpTz8EKph4n9YnSHQnPFYf8z5vtQx3P3mXCeWKgOh4iWvkvZ+Js0BOuenIc?=
 =?us-ascii?Q?V2TF8R+XBV9pLLEnm4DDu2Jv7HQIXwN+G+B1K/3H1QArAnNnNGNflG2c6/nI?=
 =?us-ascii?Q?pUjICPzL3A5x4by5djISvSk+6OOh0JA7/kxwVMYCFEuVbZ3QOU2UH6hpK3CF?=
 =?us-ascii?Q?butRwXjTczXlu2kkaguuKB1YDgAgdrXLmpDk2//ee9GVn220ibdmD3yKbXgJ?=
 =?us-ascii?Q?F+//s1HksopqLzzjbD6kznP4xi9s89JHQjzo0Whwvqx7LzLcLVGxNf3lJevc?=
 =?us-ascii?Q?T7oCXKahxypTCzeKQo6ojkF0SvqpBaaxgnnWqLCDExZXcFk3Ld0NZORuwfFm?=
 =?us-ascii?Q?7qBHxMsX887DPVz1QcMv/SNXqrxm3YIdwZUQ2aNg?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: DS0PR11MB6373.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: f040ecb7-c4bc-4cb2-defa-08dab28f7281
X-MS-Exchange-CrossTenant-originalarrivaltime: 20 Oct 2022 11:37:19.7058
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: XI50yDgZK73Rb+fATaTU/93UIAvC7NBw6yYyEbUtXmDOr1f8yuyu/Oz+16aiYdwdbEtcQvFACjuekVnA6Q81Dw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL1PR11MB5431
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.6 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        SPF_HELO_NONE,SPF_NONE,URIBL_BLOCKED autolearn=ham autolearn_force=no
        version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Thursday, October 20, 2022 1:29 AM, David Matlack wrote:
> > +/*
> > + * Create a named thread
> > + *
> > + * Input Args:
>=20
> thread is missing here

It's an output, I think.

>=20
> > + *   attr - the attributes for the new thread
> > + *   start_routine - the routine to run in the thread context
> > + *   arg - the argument passed to start_routine
> > + *   vcpu_id - the id of the vcpu
> > + *
> > + * Output Args:
> > + *   thread - the thread to be created
> > + *
> > + * Create a vcpu thread with the name in "vcpu##id" format.
> > + */
> > +void kvm_create_vcpu_thread(pthread_t *thread, const pthread_attr_t
> > +*attr,
>=20
> If I'm reading the patch correctly, attr is always NULL for vCPU threads,=
 so just
> drop it until we need it?

Yeah, sounds good.

>=20
> > +			void *(*start_routine)(void *), void *arg, int vcpu_id)
>=20
> I think it would be helpful to tie the vcpu_id to something real, rather =
than
> leaving it up to the caller. How about passing in the struct kvm_vcpu her=
e and
> using vcpu->id in the thread name?

Yes, with the cleanup you mentioned below, it would be better.

>=20
> Another cleanup we could do on top of this series would be to stash the v=
CPU
> pthread_t into struct kvm_vcpu,=20

Yes, I also thought about this when I made this patch. I have other optimiz=
ations along
with this change, and this will have a lot more changes, but we will get mu=
ch better code
consolidation. Let me post them together in v2 to discuss.

> which would eliminate another parameter here
> and make this API super clean:
>=20
> void vcpu_create_thread(struct kvm_vcpu *vcpu, void *(*fn)(void *), void =
*arg) {
> 	...
> }
> > +{
> > +	char vcpu_name[6];
> > +
> > +	sprintf(vcpu_name, "%s%d", "vcpu", vcpu_id);
>=20
> There's no need to dynamically insert "vcpu". Also, could we include a da=
sh to
> make the name slightly easier to parse? Putting it together...

Right ;)
Thanks!