lkml.org 
[lkml]   [2014]   [Dec]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH 2/2] Drivers: hv: balloon: Fix the deadlock issue in the memory hot-add code
Date


> -----Original Message-----
> From: Michal Hocko [mailto:mhocko@suse.cz]
> Sent: Tuesday, December 9, 2014 2:56 AM
> To: Yasuaki Ishimatsu
> Cc: KY Srinivasan; gregkh@linuxfoundation.org; linux-
> kernel@vger.kernel.org; devel@linuxdriverproject.org; olaf@aepfle.de;
> apw@canonical.com; linux-mm@kvack.org
> Subject: Re: [PATCH 2/2] Drivers: hv: balloon: Fix the deadlock issue in the
> memory hot-add code
>
> On Tue 09-12-14 19:25:50, Yasuaki Ishimatsu wrote:
> > (2014/12/09 18:08), Michal Hocko wrote:
> [...]
> > >Doesn't udev retry the operation if it gets EBUSY or EAGAIN?
> >
> > It depend on implementation of udev.rules. So we can retry
> > online/offline operation in udev.rules.
> [...]
>
> # Memory hotadd request
> SUBSYSTEM=="memory", ACTION=="add",
> DEVPATH=="/devices/system/memory/memory*[0-9]",
> TEST=="/sys$devpath/state", RUN+="/bin/sh -c 'echo online >
> /sys$devpath/state'"
>
> OK so this is not prepared for a temporary failures and retries.
>
> > >And again, why cannot we simply make the onlining fail or try_lock
> > >and retry internally if the event consumer cannot cope with errors?
> >
> > Did you mean the following Srinivasan's first patch looks good to you?
> > https://lkml.org/lkml/2014/12/2/662
>
> Heh, I was just about to post this. Because I haven't noticed the previous
> patch yet. Yeah, Something like that. Except that I would expect EAGAIN or
> EBUSY rather than ERESTARTSYS which should never leak into userspace. And
> that would happen here AFAICS because signal_pending will not be true
> usually.
Michal,

I agree that the fix to this problem must be outside the clients of add_memory() and that
is the reason I had sent that patch: https://lkml.org/lkml/2014/12/2/662. Let me know if
you want me to resend this patch with the correct return value.

Regards,

K. Y
>
> So there are two options. Either make the udev rule more robust and retry
> within RUN section or do the retry withing online_pages (try_lock and go into
> interruptible sleep which gets signaled by finished add_memory()). The later
> option is safer wrt. the userspace because the operation wouldn't fail
> unexpectedly.
> Another option would be generating the sysfs file after all the internal
> initialization is done and call it outside of the memory hotplug lock.
>
> --
> Michal Hocko
> SUSE Labs


\
 
 \ /
  Last update: 2014-12-11 02:01    [W:0.057 / U:0.164 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site