[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] sparc64: boot performance improvements
From: |
Eric Snowberg |
Subject: |
Re: [PATCH] sparc64: boot performance improvements |
Date: |
Sun, 11 Oct 2015 10:49:26 -0600 |
> On Oct 10, 2015, at 1:35 AM, Vladimir 'phcoder' Serbinenko <address@hidden>
> wrote:
>
>
> Le 10 oct. 2015 3:31 AM, "Eric Snowberg" <address@hidden> a écrit :
> >
> >
> > > On Oct 9, 2015, at 12:50 AM, Andrei Borzenkov <address@hidden> wrote:
> > >
> > > On Thu, Oct 8, 2015 at 2:20 AM, Eric Snowberg <address@hidden> wrote:
> > >>
> > >>> On Oct 7, 2015, at 2:36 AM, Andrei Borzenkov <address@hidden> wrote:
> > >>>
> > >>> On Tue, Oct 6, 2015 at 8:39 PM, Eric Snowberg <address@hidden> wrote:
> > >>>> Keep of devices open. This can save 6 - 7 seconds per open call and
> > >>>> can decrease boot times from over 10 minutes to 29 seconds on
> > >>>> larger SPARC systems. The open/close calls with some vendors'
> > >>>> SAS controllers cause the entire card to be reinitialized after
> > >>>> each close.
> > >>>>
> > >>>
> > >>> Is it necessary to close these handles before launching kernel?
> > >>
> > >> On SPARC hardware it is not necessary. I would be interested to hear if
> > >> it is necessary on other IEEE1275 platforms.
> > >>
> > >>> It sounds like it can accumulate quite a lot of them - are there any
> > >>> memory limits/restrictions? Also your patch is rather generic and so
> > >>> applies to any IEEE1275 platform, I think some of them may have less
> > >>> resources. Just trying to understand what run-time cost is.
> > >>
> > >> The most I have seen are two entries in the linked list. So the
> > >> additional memory requirements are very small. I have tried this on a
> > >> T2000, T4, and T5.
> > >
> > > I thought you were speaking about *larger* SPARC servers :)
> > >
> > > Anyway I'd still prefer if this would be explicitly restricted to
> > > Oracle servers. Please look at
> > > grub-core/kern/ieee1275/cmain.c:grub_ieee1275_find_options() if new
> > > quirk can be added to detect your platform(s).
> > >
> >
> > That makes sense, I’ll restrict it to all sun4v equipment.
> >
> Not good enough. Some of sun4v probably have the bug I told about in the
> other e-mail
I can get access to every sun4v platform. Could you provide any additional
information on which sun4v systems had this failure. If so, I’ll try to submit
a fix for that problem as well. It seems like there could be a better way to
handle this than resetting the SAS or SATA HBA before each read operation.
There are many problems with the upstream grub2 implementation for sun4v. I
will be submitting additional follow on patches, since it currently will not
even boot to the grub menu. My intention is to get grub2 working on every
sun4v platform.
> > >
> > >>
> > >> The path name sent into the grub_ieee1275_open function is not
> > >> consistent throughout the code, even though it is referencing the same
> > >> device. Originally I tried having a global containing the path sent
> > >> into grub_ieee1275_open, but this failed due to the various ways of
> > >> referencing the same device name. That is why I added the linked list
> > >> just to be safe. Caching the grub_ieee1275_ihandle_t this way saves a
> > >> significant amount of time, since OF calls are expensive. We were
> > >> seeing the same device opened 50+ times in the process of displaying the
> > >> grub menu and then launching the kernel.
> > >>
> > >>>
> > >>>> Signed-off-by: Eric Snowberg <address@hidden>
> > >>>> ---
> > >>>> grub-core/kern/ieee1275/ieee1275.c | 39
> > >>>> ++++++++++++++++++++++++++++++++++++
> > >>>> 1 files changed, 39 insertions(+), 0 deletions(-)
> > >>>>
> > >>>> diff --git a/grub-core/kern/ieee1275/ieee1275.c
> > >>>> b/grub-core/kern/ieee1275/ieee1275.c
> > >>>> index 9821702..30f973b 100644
> > >>>> --- a/grub-core/kern/ieee1275/ieee1275.c
> > >>>> +++ b/grub-core/kern/ieee1275/ieee1275.c
> > >>>> @@ -19,11 +19,24 @@
> > >>>>
> > >>>> #include <grub/ieee1275/ieee1275.h>
> > >>>> #include <grub/types.h>
> > >>>> +#include <grub/misc.h>
> > >>>> +#include <grub/list.h>
> > >>>> +#include <grub/mm.h>
> > >>>>
> > >>>> #define IEEE1275_PHANDLE_INVALID ((grub_ieee1275_cell_t) -1)
> > >>>> #define IEEE1275_IHANDLE_INVALID ((grub_ieee1275_cell_t) 0)
> > >>>> #define IEEE1275_CELL_INVALID ((grub_ieee1275_cell_t) -1)
> > >>>>
> > >>>> +struct grub_of_opened_device
> > >>>> +{
> > >>>> + struct grub_of_opened_device *next;
> > >>>> + struct grub_of_opened_device **prev;
> > >>>> + grub_ieee1275_ihandle_t ihandle;
> > >>>> + char *path;
> > >>>> +};
> > >>>> +
> > >>>> +static struct grub_of_opened_device *grub_of_opened_devices;
> > >>>> +
> > >>>>
> > >>>>
> > >>>> int
> > >>>> @@ -452,6 +465,18 @@ grub_ieee1275_open (const char *path,
> > >>>> grub_ieee1275_ihandle_t *result)
> > >>>> }
> > >>>> args;
> > >>>>
> > >>>> + struct grub_of_opened_device *dev;
> > >>>> +
> > >>>> + FOR_LIST_ELEMENTS(dev, grub_of_opened_devices)
> > >>>> + if (grub_strcmp(dev->path, path) == 0)
> > >>>> + break;
> > >>>> +
> > >>>> + if (dev)
> > >>>> + {
> > >>>> + *result = dev->ihandle;
> > >>>> + return 0;
> > >>>> + }
> > >>>> +
> > >>>> INIT_IEEE1275_COMMON (&args.common, "open", 1, 1);
> > >>>> args.path = (grub_ieee1275_cell_t) path;
> > >>>>
> > >>>> @@ -460,6 +485,11 @@ grub_ieee1275_open (const char *path,
> > >>>> grub_ieee1275_ihandle_t *result)
> > >>>> *result = args.result;
> > >>>> if (args.result == IEEE1275_IHANDLE_INVALID)
> > >>>> return -1;
> > >>>> +
> > >>>> + dev = grub_zalloc(sizeof(struct grub_of_opened_device));
> > >>>
> > >>> Error check
> > >>
> > >> I’ll resubmit V2 with this change
> > >>
> > >>
> > >>
> > >>>> + dev->path = grub_strdup(path);
> > >>>
> > >>> Ditto
> > >>>
> > >>
> > >> I’ll resubmit V2 with this change
> > >>
> > >>
> > >>>> + dev->ihandle = args.result;
> > >>>> + grub_list_push(GRUB_AS_LIST_P (&grub_of_opened_devices),
> > >>>> GRUB_AS_LIST (dev));
> > >>>> return 0;
> > >>>> }
> > >>>>
> > >>>> @@ -473,6 +503,15 @@ grub_ieee1275_close (grub_ieee1275_ihandle_t
> > >>>> ihandle)
> > >>>> }
> > >>>> args;
> > >>>>
> > >>>> + struct grub_of_opened_device *dev;
> > >>>> +
> > >>>> + FOR_LIST_ELEMENTS(dev, grub_of_opened_devices)
> > >>>> + if (dev->ihandle == ihandle)
> > >>>> + break;
> > >>>> +
> > >>>> + if (dev)
> > >>>> + return 0;
> > >>>> +
> > >>>
> > >>> How can we come here (i.e. can we get open handle without
> > >>> grub_ieee1275_open)?
> > >>>
> > >>
> > >> I don’t see it being possible with SPARC and would assume other
> > >> platforms could not get there either. I’m not familiar with the other
> > >> IEEE1275 platforms to know if this would be appropriate, but If there
> > >> isn’t a platform that needs this close function, could the function be
> > >> changed to do nothing but return 0?
> > >>
> > >>
> > >>
> > >>>> INIT_IEEE1275_COMMON (&args.common, "close", 1, 0);
> > >>>> args.ihandle = ihandle;
> > >>>>
> > >>>> --
> > >>>> 1.7.1
> > >>>>
> > >>>>
> > >>>> _______________________________________________
> > >>>> Grub-devel mailing list
> > >>>> address@hidden
> > >>>> https://lists.gnu.org/mailman/listinfo/grub-devel
> > >>>
> > >>> _______________________________________________
> > >>> Grub-devel mailing list
> > >>> address@hidden
> > >>> https://lists.gnu.org/mailman/listinfo/grub-devel
> > >>
> > >>
> > >> _______________________________________________
> > >> Grub-devel mailing list
> > >> address@hidden
> > >> https://lists.gnu.org/mailman/listinfo/grub-devel
> > >
> > > _______________________________________________
> > > Grub-devel mailing list
> > > address@hidden
> > > https://lists.gnu.org/mailman/listinfo/grub-devel
> >
> >
> > _______________________________________________
> > Grub-devel mailing list
> > address@hidden
> > https://lists.gnu.org/mailman/listinfo/grub-devel
> _______________________________________________
> Grub-devel mailing list
> address@hidden
> https://lists.gnu.org/mailman/listinfo/grub-devel
- [PATCH] sparc64: boot performance improvements, Eric Snowberg, 2015/10/08
- Re: [PATCH] sparc64: boot performance improvements, Andrei Borzenkov, 2015/10/08
- Re: [PATCH] sparc64: boot performance improvements, Eric Snowberg, 2015/10/08
- Re: [PATCH] sparc64: boot performance improvements, Andrei Borzenkov, 2015/10/09
- Re: [PATCH] sparc64: boot performance improvements, Eric Snowberg, 2015/10/09
- Re: [PATCH] sparc64: boot performance improvements, Vladimir 'phcoder' Serbinenko, 2015/10/10
- Re: [PATCH] sparc64: boot performance improvements,
Eric Snowberg <=
- Re: [PATCH] sparc64: boot performance improvements, Vladimir 'φ-coder/phcoder' Serbinenko, 2015/10/25
- Re: [PATCH] sparc64: boot performance improvements, Eric Snowberg, 2015/10/27
- Re: [PATCH] sparc64: boot performance improvements, Vladimir 'phcoder' Serbinenko, 2015/10/28
Re: [PATCH] sparc64: boot performance improvements, Vladimir 'φ-coder/phcoder' Serbinenko, 2015/10/10