[PATCH] disk: Prioritize OS disk devices over memdisk and procfs

[PATCH] disk: Prioritize OS disk devices over memdisk and procfs

[Michael Chang via Grub-devel]

Refine iteration to prioritize returning system disk devices over
memdisk and procfs. This adjustment brings about a modest improvement in
search efficiency, particularly during file or UUID-based
searches for a root file system. Additionally, it helps mitigate
potential collisions in file-based searches where memdisk may mistakenly
take precedence over system disks.

Signed-off-by: Michael Chang <mchang@suse.com>
---
 include/grub/disk.h | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/include/grub/disk.h b/include/grub/disk.h
index fbf23df7f..b3e4f1c8b 100644
--- a/include/grub/disk.h
+++ b/include/grub/disk.h
@@ -242,7 +242,12 @@ grub_disk_dev_iterate (grub_disk_dev_iterate_hook_t hook, void *hook_data)
 
   for (pull = 0; pull < GRUB_DISK_PULL_MAX; pull++)
     for (p = grub_disk_dev_list; p; p = p->next)
-      if (p->disk_iterate && (p->disk_iterate) (hook, hook_data, pull))
+      if ((p->id != GRUB_DISK_DEVICE_MEMDISK_ID && p->id != GRUB_DISK_DEVICE_PROCFS_ID) && p->disk_iterate && (p->disk_iterate) (hook, hook_data, pull))
+	return 1;
+
+  for (pull = 0; pull < GRUB_DISK_PULL_MAX; pull++)
+    for (p = grub_disk_dev_list; p; p = p->next)
+      if ((p->id == GRUB_DISK_DEVICE_MEMDISK_ID || p->id == GRUB_DISK_DEVICE_PROCFS_ID) && p->disk_iterate && (p->disk_iterate) (hook, hook_data, pull))
 	return 1;
 
   return 0;
-- 
2.44.0


_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] disk: Prioritize OS disk devices over memdisk and procfs

[Daniel Kiper]

On Fri, Mar 15, 2024 at 05:00:28PM +0800, Michael Chang via Grub-devel wrote:
> Refine iteration to prioritize returning system disk devices over
> memdisk and procfs. This adjustment brings about a modest improvement in
> search efficiency, particularly during file or UUID-based
> searches for a root file system. Additionally, it helps mitigate
> potential collisions in file-based searches where memdisk may mistakenly
> take precedence over system disks.

It seems to me you could do this without adding additional loops. If it
is not possible the change begs for comment.

Daniel

> Signed-off-by: Michael Chang <mchang@suse.com>
> ---
>  include/grub/disk.h | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/include/grub/disk.h b/include/grub/disk.h
> index fbf23df7f..b3e4f1c8b 100644
> --- a/include/grub/disk.h
> +++ b/include/grub/disk.h
> @@ -242,7 +242,12 @@ grub_disk_dev_iterate (grub_disk_dev_iterate_hook_t hook, void *hook_data)
>
>    for (pull = 0; pull < GRUB_DISK_PULL_MAX; pull++)
>      for (p = grub_disk_dev_list; p; p = p->next)
> -      if (p->disk_iterate && (p->disk_iterate) (hook, hook_data, pull))
> +      if ((p->id != GRUB_DISK_DEVICE_MEMDISK_ID && p->id != GRUB_DISK_DEVICE_PROCFS_ID) && p->disk_iterate && (p->disk_iterate) (hook, hook_data, pull))
> +	return 1;
> +
> +  for (pull = 0; pull < GRUB_DISK_PULL_MAX; pull++)
> +    for (p = grub_disk_dev_list; p; p = p->next)
> +      if ((p->id == GRUB_DISK_DEVICE_MEMDISK_ID || p->id == GRUB_DISK_DEVICE_PROCFS_ID) && p->disk_iterate && (p->disk_iterate) (hook, hook_data, pull))
>  	return 1;
>
>    return 0;

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: Re: [PATCH] disk: Prioritize OS disk devices over memdisk and procfs

[Michael Chang via Grub-devel]

On Fri, Mar 15, 2024 at 04:09:02PM +0100, Daniel Kiper wrote:
> On Fri, Mar 15, 2024 at 05:00:28PM +0800, Michael Chang via Grub-devel wrote:
> > Refine iteration to prioritize returning system disk devices over
> > memdisk and procfs. This adjustment brings about a modest improvement in
> > search efficiency, particularly during file or UUID-based
> > searches for a root file system. Additionally, it helps mitigate
> > potential collisions in file-based searches where memdisk may mistakenly
> > take precedence over system disks.
> 
> It seems to me you could do this without adding additional loops. If it
> is not possible the change begs for comment.

Thanks for review. The added loop for proc and memdisk in the final
output is required for the `ls' to output list of all devices. I'll add
this to the comment and send v2 patch.

Thanks,
Michael

> 
> Daniel
> 
> > Signed-off-by: Michael Chang <mchang@suse.com>
> > ---
> >  include/grub/disk.h | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/grub/disk.h b/include/grub/disk.h
> > index fbf23df7f..b3e4f1c8b 100644
> > --- a/include/grub/disk.h
> > +++ b/include/grub/disk.h
> > @@ -242,7 +242,12 @@ grub_disk_dev_iterate (grub_disk_dev_iterate_hook_t hook, void *hook_data)
> >
> >    for (pull = 0; pull < GRUB_DISK_PULL_MAX; pull++)
> >      for (p = grub_disk_dev_list; p; p = p->next)
> > -      if (p->disk_iterate && (p->disk_iterate) (hook, hook_data, pull))
> > +      if ((p->id != GRUB_DISK_DEVICE_MEMDISK_ID && p->id != GRUB_DISK_DEVICE_PROCFS_ID) && p->disk_iterate && (p->disk_iterate) (hook, hook_data, pull))
> > +	return 1;
> > +
> > +  for (pull = 0; pull < GRUB_DISK_PULL_MAX; pull++)
> > +    for (p = grub_disk_dev_list; p; p = p->next)
> > +      if ((p->id == GRUB_DISK_DEVICE_MEMDISK_ID || p->id == GRUB_DISK_DEVICE_PROCFS_ID) && p->disk_iterate && (p->disk_iterate) (hook, hook_data, pull))
> >  	return 1;
> >
> >    return 0;

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: Re: [PATCH] disk: Prioritize OS disk devices over memdisk and procfs

[Daniel Kiper]

On Mon, Mar 18, 2024 at 02:21:36PM +0800, Michael Chang wrote:
> On Fri, Mar 15, 2024 at 04:09:02PM +0100, Daniel Kiper wrote:
> > On Fri, Mar 15, 2024 at 05:00:28PM +0800, Michael Chang via Grub-devel wrote:
> > > Refine iteration to prioritize returning system disk devices over
> > > memdisk and procfs. This adjustment brings about a modest improvement in
> > > search efficiency, particularly during file or UUID-based
> > > searches for a root file system. Additionally, it helps mitigate
> > > potential collisions in file-based searches where memdisk may mistakenly
> > > take precedence over system disks.
> >
> > It seems to me you could do this without adding additional loops. If it
> > is not possible the change begs for comment.
>
> Thanks for review. The added loop for proc and memdisk in the final
> output is required for the `ls' to output list of all devices. I'll add
> this to the comment and send v2 patch.

I understand that. However, by adding two ifs and two variables or so
you would be able to avoid two additional loops. And I would really
prefer that...

Daniel

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: Re: Re: [PATCH] disk: Prioritize OS disk devices over memdisk and procfs

[Michael Chang via Grub-devel]

On Mon, Mar 18, 2024 at 12:34:37PM +0100, Daniel Kiper wrote:
> On Mon, Mar 18, 2024 at 02:21:36PM +0800, Michael Chang wrote:
> > On Fri, Mar 15, 2024 at 04:09:02PM +0100, Daniel Kiper wrote:
> > > On Fri, Mar 15, 2024 at 05:00:28PM +0800, Michael Chang via Grub-devel wrote:
> > > > Refine iteration to prioritize returning system disk devices over
> > > > memdisk and procfs. This adjustment brings about a modest improvement in
> > > > search efficiency, particularly during file or UUID-based
> > > > searches for a root file system. Additionally, it helps mitigate
> > > > potential collisions in file-based searches where memdisk may mistakenly
> > > > take precedence over system disks.
> > >
> > > It seems to me you could do this without adding additional loops. If it
> > > is not possible the change begs for comment.
> >
> > Thanks for review. The added loop for proc and memdisk in the final
> > output is required for the `ls' to output list of all devices. I'll add
> > this to the comment and send v2 patch.
> 
> I understand that. However, by adding two ifs and two variables or so
> you would be able to avoid two additional loops. And I would really
> prefer that...

The intention behind using two loops is to facilitate two separate
rounds of disk pull. The first round specifically targets system disks,
helping to exclude undesired factors such as memdisk or procfs in
between totally. In my opinion, this sequential approach appears to
offer advantages over a single loop where all disk types are mixed
together in one type of pull.

Thanks,
Michael

> 
> Daniel

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel