All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
To: Narcisa Ana Maria Vasile <navasile@linux.microsoft.com>
Cc: "Richardson, Bruce" <bruce.richardson@intel.com>,
	"david.marchand@redhat.com" <david.marchand@redhat.com>,
	"dev@dpdk.org" <dev@dpdk.org>,
	"dmitry.kozliuk@gmail.com" <dmitry.kozliuk@gmail.com>,
	"dmitrym@microsoft.com" <dmitrym@microsoft.com>,
	"khot@microsoft.com" <khot@microsoft.com>,
	"navasile@microsoft.com" <navasile@microsoft.com>,
	"ocardona@microsoft.com" <ocardona@microsoft.com>,
	"Kadam, Pallavi" <pallavi.kadam@intel.com>,
	"roretzla@microsoft.com" <roretzla@microsoft.com>,
	 "talshn@nvidia.com" <talshn@nvidia.com>,
	"thomas@monjalon.net" <thomas@monjalon.net>
Subject: RE: [PATCH v18 8/8] eal: implement functions for mutex management
Date: Wed, 9 Feb 2022 13:57:45 +0000	[thread overview]
Message-ID: <DM6PR11MB4491B209D9337458DB82C1349A2E9@DM6PR11MB4491.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20220209024755.GA9377@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>


> > Actually, please scrap that comment.
> > Obviously it wouldn't work for static variables,
> > and doesn't make much sense.
> > Though few thoughts remain:
> > for posix we probably don't need an indirection and
> > rte_thread_mutex can be just typedef of pthread_mutex_t.
> > also for posix we don't need RTE_INIT constructor for each
> > static mutex initialization.
> > Something like:
> > #define RTE_STATIC_INITIALIZED_MUTEX(mx) \
> > 	rte_thread_mutex_t mx = PTHREAD_MUTEX_INITIALIZER
> > should work, I think.
> > Konstantin
> 
> Thank you for reviewing, Konstantin!
> Some context for the current representation of mutex
> can be found in v9, patch 7/10 of this patchset.
> 
> Originally we've typedef'ed the pthread_mutex_t on POSIX, just
> like you are suggesting here.
> However, on Windows there's no static initializer similar to the pthread
> one. Still, we want ABI compatibility and same thread behavior between
> platforms. The most elegant solution we found was the current representation,
> as suggested by Dmitry K.

Yes, I agree it is a problem with Windows for static initializer.
But why we can't have different structs typedef for mutex 
for posix and windows platforms?
On posix it would be:

typedef pthread_mutex_t rte_thread_mutex_t;
#define RTE_STATIC_INITIALIZED_MUTEX(mx)   rte_thread_mutex_t mx = PTHREAD_MUTEX_INITIALIZER 

On windows it could be what Dimitry suggested:
 
typedef struct rte_thread_mutex {
        void *mutex_id;  /**< mutex identifier */
} rte_thread_mutex_t;

#define RTE_STATIC_INITIALIZED_MUTEX(private_lock)   \
rte_thread_mutex_t private_lock; \
RTE_INIT(__rte_ ## private_lock ## _init)\
{\
        RTE_VERIFY(rte_thread_mutex_init(&private_lock) == 0);\
}

API would remain the same, though it would be different underneath.
Yes, on Windows rte_thread_mutex still wouldn't work for MP,
but that's the same as with current design.
 
> I will address your other comments on the other thread.
> 
> Link to v9: http://patchwork.dpdk.org/project/dpdk/patch/1622850274-6946-8-git-send-email-navasile@linux.microsoft.com/





  reply	other threads:[~2022-02-09 13:57 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-07 16:02 [PATCH v18 8/8] eal: implement functions for mutex management Ananyev, Konstantin
2022-02-08  2:21 ` Ananyev, Konstantin
2022-02-09  2:47   ` Narcisa Ana Maria Vasile
2022-02-09 13:57     ` Ananyev, Konstantin [this message]
2022-02-20 21:56       ` Dmitry Kozlyuk
2022-02-23 17:08         ` Dmitry Kozlyuk
2022-02-24 17:29           ` Ananyev, Konstantin
2022-02-24 17:44             ` Stephen Hemminger
2022-03-08 21:36               ` Dmitry Kozlyuk
2022-03-08 21:33             ` Dmitry Kozlyuk
2022-02-09  3:08 ` Narcisa Ana Maria Vasile
2022-02-09 12:12   ` Ananyev, Konstantin
  -- strict thread matches above, loose matches on Subject: below --
2021-11-10  3:01 [dpdk-dev] [PATCH v17 00/13] eal: Add EAL API for threading Narcisa Ana Maria Vasile
2021-11-11  1:33 ` [PATCH v18 0/8] " Narcisa Ana Maria Vasile
2021-11-11  1:33   ` [PATCH v18 8/8] eal: implement functions for mutex management Narcisa Ana Maria Vasile
2021-12-13 20:27     ` Narcisa Ana Maria Vasile

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DM6PR11MB4491B209D9337458DB82C1349A2E9@DM6PR11MB4491.namprd11.prod.outlook.com \
    --to=konstantin.ananyev@intel.com \
    --cc=bruce.richardson@intel.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=dmitry.kozliuk@gmail.com \
    --cc=dmitrym@microsoft.com \
    --cc=khot@microsoft.com \
    --cc=navasile@linux.microsoft.com \
    --cc=navasile@microsoft.com \
    --cc=ocardona@microsoft.com \
    --cc=pallavi.kadam@intel.com \
    --cc=roretzla@microsoft.com \
    --cc=talshn@nvidia.com \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.