From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1kLVh3-0002Cu-Um for mharc-grub-devel@gnu.org; Thu, 24 Sep 2020 14:10:09 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:47094) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kLUmY-0001j9-AK for grub-devel@gnu.org; Thu, 24 Sep 2020 13:11:46 -0400 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:36294) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kLUmT-0005iK-Ud for grub-devel@gnu.org; Thu, 24 Sep 2020 13:11:46 -0400 Received: from pps.filterd (m0098393.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.42/8.16.0.42) with SMTP id 08OH6T8T053786 for ; Thu, 24 Sep 2020 13:11:38 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=mime-version : content-type : content-transfer-encoding : date : from : to : subject : message-id; s=pp1; bh=GeK3Ir+tPnIdey1OZ+6rveTMlW4RP2E30Oa7MZlsjMs=; b=MluXYAo0mjQDZKVC1KQy5oCdat/D/+zKxkIWClLP9C9sYZJpmzHBTqsaUpWyxiRQsHoC Yz570TMlasTERBspATpIvAiok0ZxBZgPFi0D+nVs9miqUrcwCnpvhJ412gmEfAfBvnaQ BcmhioVmywJb/xfJ6chwFMvtQ9OOeTpMYNPvPKpA0kzSSgJU8PnmtzkphA28qVpcEFRm fAnX/MvWcSDUMwC93H18G/hH1LVM5bEHLO1p8fdWyUdc1LjXSLT5xybCWhLi6iIks+xm etxOA2T/TV+LM7gD/wOp7twQ/jLDtwyuh3m4aRnWSk8XtCVk3qdEWB2Rp8/aoeWa0jIY hw== Received: from ppma02wdc.us.ibm.com (aa.5b.37a9.ip4.static.sl-reverse.com [169.55.91.170]) by mx0a-001b2d01.pphosted.com with ESMTP id 33rwyvu770-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Thu, 24 Sep 2020 13:11:38 -0400 Received: from pps.filterd (ppma02wdc.us.ibm.com [127.0.0.1]) by ppma02wdc.us.ibm.com (8.16.0.42/8.16.0.42) with SMTP id 08OH2sMU002872 for ; Thu, 24 Sep 2020 17:11:37 GMT Received: from b01cxnp23033.gho.pok.ibm.com (b01cxnp23033.gho.pok.ibm.com [9.57.198.28]) by ppma02wdc.us.ibm.com with ESMTP id 33n9m9j6a4-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Thu, 24 Sep 2020 17:11:37 +0000 Received: from b01ledav001.gho.pok.ibm.com (b01ledav001.gho.pok.ibm.com [9.57.199.106]) by b01cxnp23033.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 08OHBa2G25493888 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Thu, 24 Sep 2020 17:11:36 GMT Received: from b01ledav001.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id B618428059 for ; Thu, 24 Sep 2020 17:11:36 +0000 (GMT) Received: from b01ledav001.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 6735928058 for ; Thu, 24 Sep 2020 17:11:36 +0000 (GMT) Received: from ltc.linux.ibm.com (unknown [9.16.170.189]) by b01ledav001.gho.pok.ibm.com (Postfix) with ESMTP for ; Thu, 24 Sep 2020 17:11:36 +0000 (GMT) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Thu, 24 Sep 2020 14:11:35 -0300 From: diegodo To: grub-devel@gnu.org Subject: [RFC] ieee1275: Avoiding many unnecessary open/close Message-ID: X-Sender: diegodo@linux.vnet.ibm.com User-Agent: Roundcube Webmail/1.0.1 X-TM-AS-GCONF: 00 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.235, 18.0.687 definitions=2020-09-24_09:2020-09-24, 2020-09-24 signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 bulkscore=0 clxscore=1011 mlxlogscore=999 lowpriorityscore=0 suspectscore=3 spamscore=0 phishscore=0 malwarescore=0 impostorscore=0 adultscore=0 priorityscore=1501 mlxscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2006250000 definitions=main-2009240123 Received-SPF: none client-ip=148.163.156.1; envelope-from=diegodo@linux.vnet.ibm.com; helo=mx0a-001b2d01.pphosted.com X-detected-operating-system: by eggs.gnu.org: First seen = 2020/09/24 13:11:38 X-ACL-Warn: Detected OS = Linux 3.x [generic] [fuzzy] X-Spam_score_int: -26 X-Spam_score: -2.7 X-Spam_bar: -- X-Spam_report: (-2.7 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_NONE=0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-Mailman-Approved-At: Thu, 24 Sep 2020 14:10:08 -0400 X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 24 Sep 2020 17:11:46 -0000 Hi, we are facing some performance issues with ieee1275 platform (ppc64le architecture to be more specific) and I would like to suggest a couple of changes to try to avoid them. Sometimes the system can take more than 10 minutes to boot (in fact, it can takes more than 30 and 40 minutes in some cases). From my investigations, I noticied the following points: 1. Some types of storage devices (NVMe, Fibre Channel, ...) can have a long time for shutdown notification. Our investigation here showed us this shutdown increase with the size of the storage. For example, 6.4TB NVMe can takes something like 7 seconds just to issue a shutdown notification. 2. The grub calls a lot for OPEN/READ/CLOSE for each device. I ran some tests with just one disk (NVMe 6.4TB) and found that, even with one disk, grub call the OPEN/CLOSE functions more than 60 times. Considering the opening time of the device something like 1.5 seconds, we have almost 9 minutes just opening and closing the disk. I did some changes in the code and the time during the boot dropped drastically - from 492s to only 15s. The idea is to handle the _actual close_ while _opening_ the disk. So, during the _grub_ofdisk_open_ function, we check if the requested disk is the same as the already opened and close the previous one if we are dealing with a different disk. Yet, I removed the OPEN/CLOSE calls inside the _grub_ofdisk_get_block_size_ function and let it just checking for the block size. What are your suggestions? Is there a better way to handle this? I really appreciate your help here. Thanks --- grub-core/disk/ieee1275/ofdisk.c | 63 +++++++++++++++++--------------- 1 file changed, 34 insertions(+), 29 deletions(-) diff --git a/grub-core/disk/ieee1275/ofdisk.c b/grub-core/disk/ieee1275/ofdisk.c index d887d4b..d0ee4ae 100644 --- a/grub-core/disk/ieee1275/ofdisk.c +++ b/grub-core/disk/ieee1275/ofdisk.c @@ -44,7 +44,7 @@ struct ofdisk_hash_ent }; static grub_err_t -grub_ofdisk_get_block_size (const char *device, grub_uint32_t *block_size, +grub_ofdisk_get_block_size (grub_uint32_t *block_size, struct ofdisk_hash_ent *op); #define OFDISK_HASH_SZ 8 @@ -461,6 +461,7 @@ grub_ofdisk_open (const char *name, grub_disk_t disk) grub_ssize_t actual; grub_uint32_t block_size = 0; grub_err_t err; + struct ofdisk_hash_ent *op; if (grub_strncmp (name, "ieee1275/", sizeof ("ieee1275/") - 1) != 0) return grub_error (GRUB_ERR_UNKNOWN_DEVICE, @@ -471,6 +472,34 @@ grub_ofdisk_open (const char *name, grub_disk_t disk) grub_dprintf ("disk", "Opening `%s'.\n", devpath); + op = ofdisk_hash_find (devpath); + if (!op) + op = ofdisk_hash_add (devpath, NULL); + if (!op) + { + grub_free (devpath); + return grub_errno; + } + + /* Check if the call to open is the same to the last disk already opened */ + if (last_devpath && !grub_strcmp(op->open_path,last_devpath)) + { + goto finish; + } + + /* If not, we need to close the previous disk and open the new one */ + else { + if (last_ihandle){ + grub_ieee1275_close (last_ihandle); + } + last_ihandle = 0; + last_devpath = NULL; + + grub_ieee1275_open (op->open_path, &last_ihandle); + if (! last_ihandle) + return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "can't open device"); + } + if (grub_ieee1275_finddevice (devpath, &dev)) { grub_free (devpath); @@ -491,25 +520,18 @@ grub_ofdisk_open (const char *name, grub_disk_t disk) return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "not a block device"); } + + finish: /* XXX: There is no property to read the number of blocks. There should be a property `#blocks', but it is not there. Perhaps it is possible to use seek for this. */ disk->total_sectors = GRUB_DISK_SIZE_UNKNOWN; { - struct ofdisk_hash_ent *op; - op = ofdisk_hash_find (devpath); - if (!op) - op = ofdisk_hash_add (devpath, NULL); - if (!op) - { - grub_free (devpath); - return grub_errno; - } disk->id = (unsigned long) op; disk->data = op->open_path; - err = grub_ofdisk_get_block_size (devpath, &block_size, op); + err = grub_ofdisk_get_block_size (&block_size, op); if (err) { grub_free (devpath); @@ -532,13 +554,6 @@ grub_ofdisk_open (const char *name, grub_disk_t disk) static void grub_ofdisk_close (grub_disk_t disk) { - if (disk->data == last_devpath) - { - if (last_ihandle) - grub_ieee1275_close (last_ihandle); - last_ihandle = 0; - last_devpath = NULL; - } disk->data = 0; } @@ -685,7 +700,7 @@ grub_ofdisk_init (void) } static grub_err_t -grub_ofdisk_get_block_size (const char *device, grub_uint32_t *block_size, +grub_ofdisk_get_block_size (grub_uint32_t *block_size, struct ofdisk_hash_ent *op) { struct size_args_ieee1275 @@ -698,16 +713,6 @@ grub_ofdisk_get_block_size (const char *device, grub_uint32_t *block_size, grub_ieee1275_cell_t size2; } args_ieee1275; - if (last_ihandle) - grub_ieee1275_close (last_ihandle); - - last_ihandle = 0; - last_devpath = NULL; - - grub_ieee1275_open (device, &last_ihandle); - if (! last_ihandle) - return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "can't open device"); - *block_size = 0; if (op->block_size_fails >= 2) -- 2.21.3