All of lore.kernel.org
 help / color / mirror / Atom feed
From: Luis Chamberlain <mcgrof@kernel.org>
To: Alexander Duyck <alexander.h.duyck@linux.intel.com>
Cc: len.brown@intel.com, bvanassche@acm.org,
	linux-pm@vger.kernel.org, gregkh@linuxfoundation.org,
	linux-nvdimm@lists.01.org, jiangshanlai@gmail.com,
	linux-kernel@vger.kernel.org, pavel@ucw.cz, zwisler@kernel.org,
	tj@kernel.org, akpm@linux-foundation.org, rafael@kernel.org
Subject: Re: [driver-core PATCH v8 0/9] Add NUMA aware async_schedule calls
Date: Mon, 10 Dec 2018 15:35:07 -0800	[thread overview]
Message-ID: <20181210233507.GK28501@garbanzo.do-not-panic.com> (raw)
In-Reply-To: <dd0b44d5dd9d19ac6735f80ae66bb437ec55b2cf.camel@linux.intel.com>

On Mon, Dec 10, 2018 at 03:25:04PM -0800, Alexander Duyck wrote:
> On Mon, 2018-12-10 at 11:22 -0800, Luis Chamberlain wrote:
> > On Wed, Dec 05, 2018 at 09:25:13AM -0800, Alexander Duyck wrote:
> > > This patch set provides functionality that will help to improve the
> > > locality of the async_schedule calls used to provide deferred
> > > initialization.
> > > 
> > > This patch set originally started out focused on just the one call to
> > > async_schedule_domain in the nvdimm tree that was being used to defer the
> > > device_add call however after doing some digging I realized the scope of
> > > this was much broader than I had originally planned. As such I went
> > > through and reworked the underlying infrastructure down to replacing the
> > > queue_work call itself with a function of my own and opted to try and
> > > provide a NUMA aware solution that would work for a broader audience.
> > > 
> > > In addition I have added several tweaks and/or clean-ups to the front of the
> > > patch set. Patches 1 through 4 address a number of issues that actually were
> > > causing the existing async_schedule calls to not show the performance that
> > > they could due to either not scaling on a per device basis, or due to issues
> > > that could result in a potential deadlock. For example, patch 4 addresses the
> > > fact that we were calling async_schedule once per driver instead of once
> > > per device, and as a result we would have still ended up with devices
> > > being probed on a non-local node without addressing this first.
> > 
> > No tests were added. Again, I think it would be good to add test
> > cases to showcase the old mechanisms, illustrate the new, and ensure
> > we don't regress both now and also help us ensure we don't regress
> > moving forward.
> > 
> > This is all too critical of a path for the kernel, and these changes
> > are rather instrusive. I'd readlly like to see test code for it now
> > rather than later.
> > 
> >   Luis
> 
> Sorry about that. I was more focused on the rewrite of patch 2 and
> overlooked the comment about lib/test_kmod.c.
> 
> I'll look into it and see if I can squeeze it in for v9.

Superb!

  Luis
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

WARNING: multiple messages have this Message-ID (diff)
From: Luis Chamberlain <mcgrof@kernel.org>
To: Alexander Duyck <alexander.h.duyck@linux.intel.com>
Cc: linux-kernel@vger.kernel.org, gregkh@linuxfoundation.org,
	linux-nvdimm@lists.01.org, tj@kernel.org,
	akpm@linux-foundation.org, linux-pm@vger.kernel.org,
	jiangshanlai@gmail.com, rafael@kernel.org, len.brown@intel.com,
	pavel@ucw.cz, zwisler@kernel.org, dan.j.williams@intel.com,
	dave.jiang@intel.com, bvanassche@acm.org
Subject: Re: [driver-core PATCH v8 0/9] Add NUMA aware async_schedule calls
Date: Mon, 10 Dec 2018 15:35:07 -0800	[thread overview]
Message-ID: <20181210233507.GK28501@garbanzo.do-not-panic.com> (raw)
In-Reply-To: <dd0b44d5dd9d19ac6735f80ae66bb437ec55b2cf.camel@linux.intel.com>

On Mon, Dec 10, 2018 at 03:25:04PM -0800, Alexander Duyck wrote:
> On Mon, 2018-12-10 at 11:22 -0800, Luis Chamberlain wrote:
> > On Wed, Dec 05, 2018 at 09:25:13AM -0800, Alexander Duyck wrote:
> > > This patch set provides functionality that will help to improve the
> > > locality of the async_schedule calls used to provide deferred
> > > initialization.
> > > 
> > > This patch set originally started out focused on just the one call to
> > > async_schedule_domain in the nvdimm tree that was being used to defer the
> > > device_add call however after doing some digging I realized the scope of
> > > this was much broader than I had originally planned. As such I went
> > > through and reworked the underlying infrastructure down to replacing the
> > > queue_work call itself with a function of my own and opted to try and
> > > provide a NUMA aware solution that would work for a broader audience.
> > > 
> > > In addition I have added several tweaks and/or clean-ups to the front of the
> > > patch set. Patches 1 through 4 address a number of issues that actually were
> > > causing the existing async_schedule calls to not show the performance that
> > > they could due to either not scaling on a per device basis, or due to issues
> > > that could result in a potential deadlock. For example, patch 4 addresses the
> > > fact that we were calling async_schedule once per driver instead of once
> > > per device, and as a result we would have still ended up with devices
> > > being probed on a non-local node without addressing this first.
> > 
> > No tests were added. Again, I think it would be good to add test
> > cases to showcase the old mechanisms, illustrate the new, and ensure
> > we don't regress both now and also help us ensure we don't regress
> > moving forward.
> > 
> > This is all too critical of a path for the kernel, and these changes
> > are rather instrusive. I'd readlly like to see test code for it now
> > rather than later.
> > 
> >   Luis
> 
> Sorry about that. I was more focused on the rewrite of patch 2 and
> overlooked the comment about lib/test_kmod.c.
> 
> I'll look into it and see if I can squeeze it in for v9.

Superb!

  Luis

WARNING: multiple messages have this Message-ID (diff)
From: Luis Chamberlain <mcgrof-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
To: Alexander Duyck
	<alexander.h.duyck-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Cc: len.brown-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org,
	bvanassche-HInyCGIudOg@public.gmane.org,
	linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org,
	linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw@public.gmane.org,
	jiangshanlai-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	pavel-+ZI9xUNit7I@public.gmane.org,
	zwisler-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org,
	rafael-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
Subject: Re: [driver-core PATCH v8 0/9] Add NUMA aware async_schedule calls
Date: Mon, 10 Dec 2018 15:35:07 -0800	[thread overview]
Message-ID: <20181210233507.GK28501@garbanzo.do-not-panic.com> (raw)
In-Reply-To: <dd0b44d5dd9d19ac6735f80ae66bb437ec55b2cf.camel-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

On Mon, Dec 10, 2018 at 03:25:04PM -0800, Alexander Duyck wrote:
> On Mon, 2018-12-10 at 11:22 -0800, Luis Chamberlain wrote:
> > On Wed, Dec 05, 2018 at 09:25:13AM -0800, Alexander Duyck wrote:
> > > This patch set provides functionality that will help to improve the
> > > locality of the async_schedule calls used to provide deferred
> > > initialization.
> > > 
> > > This patch set originally started out focused on just the one call to
> > > async_schedule_domain in the nvdimm tree that was being used to defer the
> > > device_add call however after doing some digging I realized the scope of
> > > this was much broader than I had originally planned. As such I went
> > > through and reworked the underlying infrastructure down to replacing the
> > > queue_work call itself with a function of my own and opted to try and
> > > provide a NUMA aware solution that would work for a broader audience.
> > > 
> > > In addition I have added several tweaks and/or clean-ups to the front of the
> > > patch set. Patches 1 through 4 address a number of issues that actually were
> > > causing the existing async_schedule calls to not show the performance that
> > > they could due to either not scaling on a per device basis, or due to issues
> > > that could result in a potential deadlock. For example, patch 4 addresses the
> > > fact that we were calling async_schedule once per driver instead of once
> > > per device, and as a result we would have still ended up with devices
> > > being probed on a non-local node without addressing this first.
> > 
> > No tests were added. Again, I think it would be good to add test
> > cases to showcase the old mechanisms, illustrate the new, and ensure
> > we don't regress both now and also help us ensure we don't regress
> > moving forward.
> > 
> > This is all too critical of a path for the kernel, and these changes
> > are rather instrusive. I'd readlly like to see test code for it now
> > rather than later.
> > 
> >   Luis
> 
> Sorry about that. I was more focused on the rewrite of patch 2 and
> overlooked the comment about lib/test_kmod.c.
> 
> I'll look into it and see if I can squeeze it in for v9.

Superb!

  Luis

  reply	other threads:[~2018-12-10 23:35 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-05 17:25 [driver-core PATCH v8 0/9] Add NUMA aware async_schedule calls Alexander Duyck
2018-12-05 17:25 ` [driver-core PATCH v8 1/9] driver core: Move async_synchronize_full call Alexander Duyck
2018-12-05 17:25   ` Alexander Duyck
2018-12-05 17:25 ` [driver-core PATCH v8 2/9] driver core: Establish order of operations for device_add and device_del via bitflag Alexander Duyck
2018-12-10 18:58   ` Dan Williams
2018-12-10 18:58     ` Dan Williams
2018-12-10 19:35     ` Alexander Duyck
2018-12-10 19:35       ` Alexander Duyck
2018-12-10 19:35       ` Alexander Duyck
2018-12-10 19:43       ` Dan Williams
2018-12-10 19:43         ` Dan Williams
2018-12-10 20:57         ` Alexander Duyck
2018-12-10 20:57           ` Alexander Duyck
2018-12-10 20:57           ` Alexander Duyck
2018-12-10 21:15           ` Dan Williams
2018-12-10 21:15             ` Dan Williams
2018-12-10 21:15             ` Dan Williams
2018-12-10 21:23             ` Dan Williams
2018-12-10 21:23               ` Dan Williams
2018-12-10 22:24               ` Alexander Duyck
2018-12-10 22:24                 ` Alexander Duyck
2018-12-10 22:24                 ` Alexander Duyck
2018-12-10 22:41                 ` Dan Williams
2018-12-10 22:41                   ` Dan Williams
2018-12-10 22:41                   ` Dan Williams
2018-12-05 17:25 ` [driver-core PATCH v8 3/9] device core: Consolidate locking and unlocking of parent and device Alexander Duyck
2018-12-05 17:25 ` [driver-core PATCH v8 4/9] driver core: Probe devices asynchronously instead of the driver Alexander Duyck
2018-12-05 17:25 ` [driver-core PATCH v8 5/9] workqueue: Provide queue_work_node to queue work near a given NUMA node Alexander Duyck
2018-12-05 17:25 ` [driver-core PATCH v8 6/9] async: Add support for queueing on specific " Alexander Duyck
2018-12-05 17:25 ` [driver-core PATCH v8 7/9] driver core: Attach devices on CPU local to device node Alexander Duyck
2018-12-05 17:25 ` [driver-core PATCH v8 8/9] PM core: Use new async_schedule_dev command Alexander Duyck
2018-12-05 17:26 ` [driver-core PATCH v8 9/9] libnvdimm: Schedule device registration on node local to the device Alexander Duyck
2018-12-10 19:22 ` [driver-core PATCH v8 0/9] Add NUMA aware async_schedule calls Luis Chamberlain
2018-12-10 19:22   ` Luis Chamberlain
2018-12-10 23:25   ` Alexander Duyck
2018-12-10 23:25     ` Alexander Duyck
2018-12-10 23:35     ` Luis Chamberlain [this message]
2018-12-10 23:35       ` Luis Chamberlain
2018-12-10 23:35       ` Luis Chamberlain

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=20181210233507.GK28501@garbanzo.do-not-panic.com \
    --to=mcgrof@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=alexander.h.duyck@linux.intel.com \
    --cc=bvanassche@acm.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=jiangshanlai@gmail.com \
    --cc=len.brown@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nvdimm@lists.01.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=pavel@ucw.cz \
    --cc=rafael@kernel.org \
    --cc=tj@kernel.org \
    --cc=zwisler@kernel.org \
    /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.