[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 1/6] xen: add reference counter support


  • To: Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Thu, 16 Mar 2023 18:01:41 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; 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=K0zqmNGtyNBxwZJ7G0RJaTA+YW8M6jhF3wozVWt5Xvs=; b=jATczK8jYga5FPRmHY5vN3B2um3taOFKMKgDRus6CyPX3/LSxhXhKGd9OCmuuUEy7/v22X9cjnGCpPLPxhvKRrmBlqiQdMtM0TYi/RZcZ0L8KAUXDqtjVzPCqahYLVIEAgK+749+eW37+ms4J1AgG9Y/MJOP8CEMhAOBVtfv//ovQN0uZufDnzBlZAf9BGoTxK/djNj0MLE6SePF5mYgFsXlJAt/7KyB8o4SNgUYGayTCin3M/Xewn0fORgIgJSk2iCWxWwCkG26uXbxaD+GF5s/uTc/aULYaN3aDHg5K0sGFiof6tEh9qyPmAzbkdAQrW+riiriXV0qbVmHXwwwZw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=OSlPtAKY+/Z9Otkqw3uFhXazDznlebQD0WONTtHXusGsijD73GVNx0VnPSsWEFS5AI4oTMdQS6rC7fxAkpZlvwkgNMrEvAum0aKGKb8ew1wmwF1r7O4eZz8nmAdwMmG62FU8TypkHwff4l0Y9Aaa3npsDv1vkXn3DkMwRl/M6SSE0jK7awGCt5m2Axh3vVD4YHQ0N+MrcgrizLusXr822Ga3fP/Xgjvk5EVcOoBvs89JXxQyrOauUFKRMGiV/Ms7WpBCOL17jfkF2JD8xBYOeez72bNHDDaNGjHikWCMqO3d0GPiysM6Jrwy51NqMv4qXiTnFfrL8MSUSV6nDzaZJw==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Thu, 16 Mar 2023 17:01:58 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 14.03.2023 21:56, Volodymyr Babchuk wrote:
> +static inline void refcnt_put(refcnt_t *refcnt, void (*destructor)(refcnt_t 
> *refcnt))

Hmm, this means all callers need to pass (and agree on) the supposedly
single destructor function that needs calling. Wouldn't the destructor
function better be stored elsewhere (and supplied to e.g. refcnt_init())?

> +{
> +    int ret = atomic_dec_return(&refcnt->refcnt);
> +
> +    if ( ret == 0 )
> +        destructor(refcnt);
> +
> +    if ( unlikely(ret < 0))
> +    {
> +        atomic_set(&refcnt->refcnt, REFCNT_SATURATED);

It's undefined whether *refcnt still exists once the destructor was
called (which would have happened before we can make it here). While
even the atomic_dec_return() above would already have acted in an
unknown way in this case I don't think it's a good idea to access the
object yet another time. (Same for the "negative" case in
refcnt_get() then.)

Jan



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.