linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Vasilis Liaskovitis <vasilis.liaskovitis@profitbricks.com>
To: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Toshi Kani <toshi.kani@hp.com>,
	ACPI Devel Maling List <linux-acpi@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	isimatu.yasuaki@jp.fujitsu.com, Len Brown <lenb@kernel.org>,
	linux-mm@kvack.org
Subject: Re: [PATCH 2/2 v2, RFC] Driver core: Introduce offline/online callbacks for memory blocks
Date: Mon, 6 May 2013 18:28:12 +0200	[thread overview]
Message-ID: <20130506162812.GB4929@dhcp-192-168-178-175.profitbricks.localdomain> (raw)
In-Reply-To: <19540491.PRsM4lKIYM@vostro.rjw.lan>

Hi,

On Sat, May 04, 2013 at 01:21:16PM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Introduce .offline() and .online() callbacks for memory_subsys
> that will allow the generic device_offline() and device_online()
> to be used with device objects representing memory blocks.  That,
> in turn, allows the ACPI subsystem to use device_offline() to put
> removable memory blocks offline, if possible, before removing
> memory modules holding them.
> 
> The 'online' sysfs attribute of memory block devices will attempt to
> put them offline if 0 is written to it and will attempt to apply the
> previously used online type when onlining them (i.e. when 1 is
> written to it).
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/base/memory.c  |  105 +++++++++++++++++++++++++++++++++++++------------
>  include/linux/memory.h |    1 
>  2 files changed, 81 insertions(+), 25 deletions(-)
>
[...]

> @@ -686,10 +735,16 @@ int offline_memory_block(struct memory_b
>  {
>  	int ret = 0;
>  
> +	lock_device_hotplug();
>  	mutex_lock(&mem->state_mutex);
> -	if (mem->state != MEM_OFFLINE)
> -		ret = __memory_block_change_state(mem, MEM_OFFLINE, MEM_ONLINE, -1);
> +	if (mem->state != MEM_OFFLINE) {
> +		ret = __memory_block_change_state_uevent(mem, MEM_OFFLINE,
> +							 MEM_ONLINE, -1);
> +		if (!ret)
> +			mem->dev.offline = true;
> +	}
>  	mutex_unlock(&mem->state_mutex);
> +	unlock_device_hotplug();

(Testing with qemu...)
offline_memory_block is called from remove_memory, which in turn is called from
acpi_memory_device_remove (detach operation) during acpi_bus_trim. We already
hold the device_hotplug lock when we trim (acpi_scan_hot_remove), so we
don't need to lock/unlock_device_hotplug in offline_memory_block.

A more general issue is that there are now two memory offlining efforts:

1) from acpi_bus_offline_companions during device offline
2) from mm: remove_memory during device detach (offline_memory_block_cb)

The 2nd is only called if the device offline operation was already succesful, so
it seems ineffective or redundant now, at least for x86_64/acpi_memhotplug machine
(unless the blocks were re-onlined in between).
On the other hand, the 2nd effort has some more intelligence in offlining, as it
tries to offline twice in the precense of memcg, see commits df3e1b91 or
reworked 0baeab16. Maybe we need to consolidate the logic.

remove_memory is called from device_detach, during trim that can't fail, so it
should not fail. However this function can still fail in 2 cases:
- offline_memory_block_cb
- is_memblock_offlined_cb
in the case of re-onlined memblocks in between device-offline and device detach.
This seems possible I think, since we do not hold lock_memory_hotplug for the
duration of the hot-remove operation.

thanks,

- Vasilis

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2013-05-06 16:28 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1576321.HU0tZ4cGWk@vostro.rjw.lan>
     [not found] ` <3166726.elbgrUIZ0L@vostro.rjw.lan>
2013-05-04  1:01   ` [PATCH 0/3 RFC] Driver core: Add offline/online callbacks for memory_subsys Rafael J. Wysocki
2013-05-04  1:03     ` [PATCH 1/3 RFC] ACPI / memhotplug: Bind removable memory blocks to ACPI device nodes Rafael J. Wysocki
2013-05-04  1:04     ` [PATCH 2/3 RFC] Driver core: Introduce types of device "online" Rafael J. Wysocki
2013-05-04  1:06     ` [PATCH 3/3 RFC] Driver core: Introduce offline/online callbacks for memory blocks Rafael J. Wysocki
2013-05-04 11:11     ` [PATCH 0/2 v2, RFC] Driver core: Add offline/online callbacks for memory_subsys Rafael J. Wysocki
2013-05-04 11:12       ` [PATCH 1/2 v2, RFC] ACPI / memhotplug: Bind removable memory blocks to ACPI device nodes Rafael J. Wysocki
2013-05-21  6:50         ` Tang Chen
2013-05-04 11:21       ` [PATCH 2/2 v2, RFC] Driver core: Introduce offline/online callbacks for memory blocks Rafael J. Wysocki
2013-05-06 16:28         ` Vasilis Liaskovitis [this message]
2013-05-07  0:59           ` Rafael J. Wysocki
2013-05-07 10:59             ` Vasilis Liaskovitis
2013-05-07 12:11               ` Rafael J. Wysocki
2013-05-07 21:03                 ` Toshi Kani
2013-05-07 22:10                   ` Rafael J. Wysocki
2013-05-07 22:45                     ` Toshi Kani
2013-05-07 23:17                       ` Rafael J. Wysocki
2013-05-07 23:59                         ` Toshi Kani
2013-05-08  0:24                           ` Rafael J. Wysocki
2013-05-08  0:37                             ` Toshi Kani
2013-05-08 11:53                               ` Rafael J. Wysocki
2013-05-08 14:38                                 ` Toshi Kani
2013-05-06 17:20         ` Greg Kroah-Hartman
2013-05-06 19:46           ` Rafael J. Wysocki
2013-05-21  6:37         ` Tang Chen
2013-05-21 11:15           ` Rafael J. Wysocki
2013-05-22  4:45             ` Tang Chen
2013-05-22 10:42               ` Rafael J. Wysocki
2013-05-22 22:06               ` [PATCH] Driver core / memory: Simplify __memory_block_change_state() Rafael J. Wysocki
2013-05-22 22:14                 ` Greg Kroah-Hartman
2013-05-22 23:29                   ` Rafael J. Wysocki
2013-05-23  4:37                 ` Tang Chen
2013-05-06 10:48       ` [PATCH 0/2 v2, RFC] Driver core: Add offline/online callbacks for memory_subsys Rafael J. Wysocki

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=20130506162812.GB4929@dhcp-192-168-178-175.profitbricks.localdomain \
    --to=vasilis.liaskovitis@profitbricks.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=isimatu.yasuaki@jp.fujitsu.com \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=rjw@sisk.pl \
    --cc=toshi.kani@hp.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox