| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1726699.Z30ifEcQDQ@vostro.rjw.lan>
Date: Mon, 20 May 2013 21:47:25 +0200
From: "Rafael J. Wysocki" <rjw@...k.pl>
To: Toshi Kani <toshi.kani@...com>
Cc: ACPI Devel Maling List <linux-acpi@...r.kernel.org>,
LKML <linux-kernel@...r.kernel.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Wen Congyang <wency@...fujitsu.com>,
Tang Chen <tangchen@...fujitsu.com>,
Yasuaki Ishimatsu <isimatu.yasuaki@...fujitsu.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Jiang Liu <liuj97@...il.com>,
Vasilis Liaskovitis <vasilis.liaskovitis@...fitbricks.com>,
linux-mm@...ck.org
Subject: Re: [PATCH 5/5] ACPI / memhotplug: Drop unnecessary code
On Monday, May 20, 2013 11:27:56 AM Toshi Kani wrote:
> On Sun, 2013-05-19 at 01:34 +0200, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> >
> > Now that the memory offlining should be taken care of by the
> > companion device offlining code in acpi_scan_hot_remove(), the
> > ACPI memory hotplug driver doesn't need to offline it in
> > acpi_memory_remove_memory() any more. Consequently, it doesn't
> > need to call remove_memory() any more, which means that that
> > funtion may be dropped entirely, because acpi_memory_remove_memory()
> > is the only user of it.
>
> The off-lining part of remove_memory() can be removed, but not the
> hot-delete part. Please see my comments below.
>
> > Make the changes described above to get rid of the dead code.
> >
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> > ---
> > drivers/acpi/acpi_memhotplug.c | 15 ------
> > include/linux/memory_hotplug.h | 1
> > mm/memory_hotplug.c | 102 -----------------------------------------
> > 3 files changed, 2 insertions(+), 116 deletions(-)
> >
> > Index: linux-pm/drivers/acpi/acpi_memhotplug.c
> > ===================================================================
> > --- linux-pm.orig/drivers/acpi/acpi_memhotplug.c
> > +++ linux-pm/drivers/acpi/acpi_memhotplug.c
> > @@ -271,31 +271,20 @@ static int acpi_memory_enable_device(str
> > return 0;
> > }
> >
> > -static int acpi_memory_remove_memory(struct acpi_memory_device *mem_device)
> > +static void acpi_memory_remove_memory(struct acpi_memory_device *mem_device)
> > {
> > acpi_handle handle = mem_device->device->handle;
> > - int result = 0, nid;
> > struct acpi_memory_info *info, *n;
> >
> > - nid = acpi_get_node(handle);
> > -
> > list_for_each_entry_safe(info, n, &mem_device->res_list, list) {
> > if (!info->enabled)
> > continue;
> >
> > - if (nid < 0)
> > - nid = memory_add_physaddr_to_nid(info->start_addr);
> > -
> > + /* All of the memory blocks are offline at this point. */
> > acpi_unbind_memory_blocks(info, handle);
> > - result = remove_memory(nid, info->start_addr, info->length);
>
> We still need to call remove_memory().
>
> > - if (result)
> > - return result;
> > -
> > list_del(&info->list);
> > kfree(info);
> > }
> > -
> > - return result;
> > }
> >
> > static void acpi_memory_device_free(struct acpi_memory_device *mem_device)
> > Index: linux-pm/include/linux/memory_hotplug.h
> > ===================================================================
> > --- linux-pm.orig/include/linux/memory_hotplug.h
> > +++ linux-pm/include/linux/memory_hotplug.h
> > @@ -252,7 +252,6 @@ extern int add_memory(int nid, u64 start
> > extern int arch_add_memory(int nid, u64 start, u64 size);
> > extern int offline_pages(unsigned long start_pfn, unsigned long nr_pages);
> > extern bool is_memblock_offlined(struct memory_block *mem);
> > -extern int remove_memory(int nid, u64 start, u64 size);
> > extern int sparse_add_one_section(struct zone *zone, unsigned long start_pfn,
> > int nr_pages);
> > extern void sparse_remove_one_section(struct zone *zone, struct mem_section *ms);
> > Index: linux-pm/mm/memory_hotplug.c
> > ===================================================================
> > --- linux-pm.orig/mm/memory_hotplug.c
> > +++ linux-pm/mm/memory_hotplug.c
> > @@ -1670,41 +1670,6 @@ int walk_memory_range(unsigned long star
> > }
>
> :
>
> > -
> > -int __ref remove_memory(int nid, u64 start, u64 size)
> > -{
> > - unsigned long start_pfn, end_pfn;
> > - int ret = 0;
> > - int retry = 1;
> > -
> > - start_pfn = PFN_DOWN(start);
> > - end_pfn = PFN_UP(start + size - 1);
> > -
> > - /*
> > - * When CONFIG_MEMCG is on, one memory block may be used by other
> > - * blocks to store page cgroup when onlining pages. But we don't know
> > - * in what order pages are onlined. So we iterate twice to offline
> > - * memory:
> > - * 1st iterate: offline every non primary memory block.
> > - * 2nd iterate: offline primary (i.e. first added) memory block.
> > - */
> > -repeat:
> > - walk_memory_range(start_pfn, end_pfn, &ret,
> > - offline_memory_block_cb);
> > - if (ret) {
> > - if (!retry)
> > - return ret;
> > -
> > - retry = 0;
> > - ret = 0;
> > - goto repeat;
> > - }
>
> The above procedure can be removed as it is for off-lining.
>
> > - lock_memory_hotplug();
> > -
> > - /*
> > - * we have offlined all memory blocks like this:
> > - * 1. lock memory hotplug
> > - * 2. offline a memory block
> > - * 3. unlock memory hotplug
> > - *
> > - * repeat step1-3 to offline the memory block. All memory blocks
> > - * must be offlined before removing memory. But we don't hold the
> > - * lock in the whole operation. So we should check whether all
> > - * memory blocks are offlined.
> > - */
> > -
> > - ret = walk_memory_range(start_pfn, end_pfn, NULL,
> > - is_memblock_offlined_cb);
> > - if (ret) {
> > - unlock_memory_hotplug();
> > - return ret;
> > - }
> > -
>
> I think the above procedure is still useful for safe guard.
But then it shoud to BUG_ON() instead of returning an error (which isn't very
useful for anything now).
> > - /* remove memmap entry */
> > - firmware_map_remove(start, start + size, "System RAM");
> > -
> > - arch_remove_memory(start, size);
> > -
> > - try_offline_node(nid);
>
> The above procedure performs memory hot-delete specific operations and
> is necessary.
OK, I see. I'll replace this patch with something simpler, then.
What about the other patches in the series?
Thanks,
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists