* root=PARTUUID for MBR/NT disk signatures? @ 2012-08-17 22:10 Stephen Warren 2012-08-20 18:22 ` Tejun Heo 0 siblings, 1 reply; 7+ messages in thread From: Stephen Warren @ 2012-08-17 22:10 UTC (permalink / raw) To: linux-kernel Cc: Will Drewry, cross-distro, Jens Axboe, Tejun Heo, Kay Sievers I was considering extending the kernel command-line option root=PARTUUID= to also support MBR (NT disk signatures). I was thinking of a syntax along the lines of: root=PARTUUID=UUUUUUUU-PP[/PARTNROFF=%d] ... where UUUUUUUU is the hex representation of the NT disk signature, and PP is the hex representation of the partition number. Like GPT, /PARTNROFF could be used too if desired. Related, I was thinking of changing struct partition_meta_info's uuid field to be a string, so that it could simply be strcmp'd against the UUID value on the kernel command-line. That way, the type of the UUID is irrelevant. Does anyone have any objection to that? The reason I aim for that syntax rather than say: root=MBRSIG=UUUUUUUU-PP[/PARTNROFF=%d] ... is to allow boot-loaders (e.g. U-Boot on ARM) to store just the partition ID in a variable, and prepend all the Linux-specific stuff on the front, e.g. # For GPT: setenv kernel_part_uuid b2f82cda-2535-4779-b467-094a210fbae7 # For MBR: setenv kernel_part_uuid UUUUUUUU-PP In fact, those hard-coded statements would probably be replaced with a run-time command: part uuid mmc 0:1 kernel_part_uuid # Then in a common script: setenv bootargs root=PARTUUID=${kernel_part_uuid} Otherwise, the value of the uuid variable (or result of the "part uuid" command) would need to prepend the PARTUUID= or MBRSIG= to the "uuid" variable's value, and that's probably Linux-specific rather than part of a generic UUID for the partition. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: root=PARTUUID for MBR/NT disk signatures? 2012-08-17 22:10 root=PARTUUID for MBR/NT disk signatures? Stephen Warren @ 2012-08-20 18:22 ` Tejun Heo 2012-08-20 18:30 ` Stephen Warren 0 siblings, 1 reply; 7+ messages in thread From: Tejun Heo @ 2012-08-20 18:22 UTC (permalink / raw) To: Stephen Warren Cc: linux-kernel, Will Drewry, cross-distro, Jens Axboe, Kay Sievers Hello, On Fri, Aug 17, 2012 at 04:10:52PM -0600, Stephen Warren wrote: > I was considering extending the kernel command-line option > root=PARTUUID= to also support MBR (NT disk signatures). I was thinking > of a syntax along the lines of: > > root=PARTUUID=UUUUUUUU-PP[/PARTNROFF=%d] > > ... where UUUUUUUU is the hex representation of the NT disk signature, > and PP is the hex representation of the partition number. Like GPT, > /PARTNROFF could be used too if desired. > > Related, I was thinking of changing struct partition_meta_info's uuid > field to be a string, so that it could simply be strcmp'd against the > UUID value on the kernel command-line. That way, the type of the UUID is > irrelevant. > > Does anyone have any objection to that? Wouldn't that be able to break setups which work currently? Thanks. -- tejun ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: root=PARTUUID for MBR/NT disk signatures? 2012-08-20 18:22 ` Tejun Heo @ 2012-08-20 18:30 ` Stephen Warren 2012-08-21 4:47 ` Will Drewry 0 siblings, 1 reply; 7+ messages in thread From: Stephen Warren @ 2012-08-20 18:30 UTC (permalink / raw) To: Tejun Heo Cc: linux-kernel, Will Drewry, cross-distro, Jens Axboe, Kay Sievers On 08/20/2012 12:22 PM, Tejun Heo wrote: > Hello, > > On Fri, Aug 17, 2012 at 04:10:52PM -0600, Stephen Warren wrote: >> I was considering extending the kernel command-line option >> root=PARTUUID= to also support MBR (NT disk signatures). I was thinking >> of a syntax along the lines of: >> >> root=PARTUUID=UUUUUUUU-PP[/PARTNROFF=%d] >> >> ... where UUUUUUUU is the hex representation of the NT disk signature, >> and PP is the hex representation of the partition number. Like GPT, >> /PARTNROFF could be used too if desired. >> >> Related, I was thinking of changing struct partition_meta_info's uuid >> field to be a string, so that it could simply be strcmp'd against the >> UUID value on the kernel command-line. That way, the type of the UUID is >> irrelevant. >> >> Does anyone have any objection to that? > > Wouldn't that be able to break setups which work currently? I don't believe so: Since the newly supported UUID syntax wouldn't ever match any EFI UUID (the lengths differ in all cases), I don't believe the new syntax would affect behavior for any existing usage. Obviously, part_efi.c would be modified to initialize struct partition_meta_info's uuid field to the appropriate string representation of the UUID so that the str(case)cmp would still succeed for existing command-lines. I ended up coding up that part of the change late Friday, and the feature was certainly still working OK. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: root=PARTUUID for MBR/NT disk signatures? 2012-08-20 18:30 ` Stephen Warren @ 2012-08-21 4:47 ` Will Drewry 2012-08-21 6:38 ` Michael Tokarev 2012-08-21 18:08 ` Stephen Warren 0 siblings, 2 replies; 7+ messages in thread From: Will Drewry @ 2012-08-21 4:47 UTC (permalink / raw) To: Stephen Warren Cc: Tejun Heo, linux-kernel, cross-distro, Jens Axboe, Kay Sievers On Mon, Aug 20, 2012 at 1:30 PM, Stephen Warren <swarren@wwwdotorg.org> wrote: > On 08/20/2012 12:22 PM, Tejun Heo wrote: >> Hello, >> >> On Fri, Aug 17, 2012 at 04:10:52PM -0600, Stephen Warren wrote: >>> I was considering extending the kernel command-line option >>> root=PARTUUID= to also support MBR (NT disk signatures). I was thinking >>> of a syntax along the lines of: >>> >>> root=PARTUUID=UUUUUUUU-PP[/PARTNROFF=%d] >>> >>> ... where UUUUUUUU is the hex representation of the NT disk signature, >>> and PP is the hex representation of the partition number. Like GPT, >>> /PARTNROFF could be used too if desired. >>> >>> Related, I was thinking of changing struct partition_meta_info's uuid >>> field to be a string, so that it could simply be strcmp'd against the >>> UUID value on the kernel command-line. That way, the type of the UUID is >>> irrelevant. >>> >>> Does anyone have any objection to that? >> >> Wouldn't that be able to break setups which work currently? > > I don't believe so: > > Since the newly supported UUID syntax wouldn't ever match any EFI UUID > (the lengths differ in all cases), I don't believe the new syntax would > affect behavior for any existing usage. > > Obviously, part_efi.c would be modified to initialize struct > partition_meta_info's uuid field to the appropriate string > representation of the UUID so that the str(case)cmp would still succeed > for existing command-lines. I ended up coding up that part of the change > late Friday, and the feature was certainly still working OK. Functionally, I suspect this will work fine, but I am concerned that it is a bad move from an efficiency perspective (not unfixable though). Right now, the user-supplied value is converted from string-uuid to packed-uuid. This is then memcmp'd across any and all partitions - be it 2 or 200 - across all attached storage. If we move to a pure string, then we end up needing to unpack every packed UUID at disk scan time (or search, depending on impl) rather than just the one user supplied value. Perhaps the cost is negligible on modern machines, but it seems like the wrong place to put the cost (per entry rather than per search value). I'd be happy to test out any proposed patch to see if I can measure any differences in my specific environments, but I don't know if it will slow down partition scanning for other EFI UUID users out there. Maybe the NT disk sigs could be massaged to be memcmp friendly instead of the opposite? thanks! will ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: root=PARTUUID for MBR/NT disk signatures? 2012-08-21 4:47 ` Will Drewry @ 2012-08-21 6:38 ` Michael Tokarev 2012-08-21 18:08 ` Stephen Warren 1 sibling, 0 replies; 7+ messages in thread From: Michael Tokarev @ 2012-08-21 6:38 UTC (permalink / raw) To: Will Drewry Cc: Stephen Warren, Tejun Heo, linux-kernel, cross-distro, Jens Axboe, Kay Sievers On 21.08.2012 08:47, Will Drewry wrote: [] > Functionally, I suspect this will work fine, but I am concerned that > it is a bad move from an efficiency perspective (not unfixable > though). Right now, the user-supplied value is converted from > string-uuid to packed-uuid. This is then memcmp'd across any and all > partitions - be it 2 or 200 - across all attached storage. If we move > to a pure string, then we end up needing to unpack every packed UUID > at disk scan time (or search, depending on impl) rather than just the > one user supplied value. > > Perhaps the cost is negligible on modern machines, but it seems like > the wrong place to put the cost (per entry rather than per search > value). Amount of work needed to READ all the partition tables might be quite a bit larger than strcmp'ing it all. I think. /mjt ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: root=PARTUUID for MBR/NT disk signatures? 2012-08-21 4:47 ` Will Drewry 2012-08-21 6:38 ` Michael Tokarev @ 2012-08-21 18:08 ` Stephen Warren 2012-08-21 18:47 ` Will Drewry 1 sibling, 1 reply; 7+ messages in thread From: Stephen Warren @ 2012-08-21 18:08 UTC (permalink / raw) To: Will Drewry Cc: Tejun Heo, linux-kernel, cross-distro, Jens Axboe, Kay Sievers On 08/20/2012 10:47 PM, Will Drewry wrote: > On Mon, Aug 20, 2012 at 1:30 PM, Stephen Warren <swarren@wwwdotorg.org> wrote: >> On 08/20/2012 12:22 PM, Tejun Heo wrote: >>> Hello, >>> >>> On Fri, Aug 17, 2012 at 04:10:52PM -0600, Stephen Warren wrote: >>>> I was considering extending the kernel command-line option >>>> root=PARTUUID= to also support MBR (NT disk signatures). I was thinking >>>> of a syntax along the lines of: >>>> >>>> root=PARTUUID=UUUUUUUU-PP[/PARTNROFF=%d] >>>> >>>> ... where UUUUUUUU is the hex representation of the NT disk signature, >>>> and PP is the hex representation of the partition number. Like GPT, >>>> /PARTNROFF could be used too if desired. >>>> >>>> Related, I was thinking of changing struct partition_meta_info's uuid >>>> field to be a string, so that it could simply be strcmp'd against the >>>> UUID value on the kernel command-line. That way, the type of the UUID is >>>> irrelevant. >>>> >>>> Does anyone have any objection to that? >>> >>> Wouldn't that be able to break setups which work currently? >> >> I don't believe so: >> >> Since the newly supported UUID syntax wouldn't ever match any EFI UUID >> (the lengths differ in all cases), I don't believe the new syntax would >> affect behavior for any existing usage. >> >> Obviously, part_efi.c would be modified to initialize struct >> partition_meta_info's uuid field to the appropriate string >> representation of the UUID so that the str(case)cmp would still succeed >> for existing command-lines. I ended up coding up that part of the change >> late Friday, and the feature was certainly still working OK. > > Functionally, I suspect this will work fine, but I am concerned that > it is a bad move from an efficiency perspective (not unfixable > though). Right now, the user-supplied value is converted from > string-uuid to packed-uuid. This is then memcmp'd across any and all > partitions - be it 2 or 200 - across all attached storage. If we move > to a pure string, then we end up needing to unpack every packed UUID > at disk scan time (or search, depending on impl) rather than just the > one user supplied value. The EFI partition code actually does the following already: 1) Unpack the UUID from the binary on-disk representation to a temporary string. 2) Repack the temporary string into the internal UUID buffer. The comments imply this is in order to do endian conversions. Switching the internal representation to a string avoids step (2) above, plus avoids having to pack the string on the kernel command-line into a binary UUID before the comparison. I doubt the difference between memcmp vs. strcasecmp is worth considering. So, I think it's overall a win. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: root=PARTUUID for MBR/NT disk signatures? 2012-08-21 18:08 ` Stephen Warren @ 2012-08-21 18:47 ` Will Drewry 0 siblings, 0 replies; 7+ messages in thread From: Will Drewry @ 2012-08-21 18:47 UTC (permalink / raw) To: Stephen Warren Cc: Tejun Heo, linux-kernel, cross-distro, Jens Axboe, Kay Sievers On Tue, Aug 21, 2012 at 1:08 PM, Stephen Warren <swarren@wwwdotorg.org> wrote: > On 08/20/2012 10:47 PM, Will Drewry wrote: >> On Mon, Aug 20, 2012 at 1:30 PM, Stephen Warren <swarren@wwwdotorg.org> wrote: >>> On 08/20/2012 12:22 PM, Tejun Heo wrote: >>>> Hello, >>>> >>>> On Fri, Aug 17, 2012 at 04:10:52PM -0600, Stephen Warren wrote: >>>>> I was considering extending the kernel command-line option >>>>> root=PARTUUID= to also support MBR (NT disk signatures). I was thinking >>>>> of a syntax along the lines of: >>>>> >>>>> root=PARTUUID=UUUUUUUU-PP[/PARTNROFF=%d] >>>>> >>>>> ... where UUUUUUUU is the hex representation of the NT disk signature, >>>>> and PP is the hex representation of the partition number. Like GPT, >>>>> /PARTNROFF could be used too if desired. >>>>> >>>>> Related, I was thinking of changing struct partition_meta_info's uuid >>>>> field to be a string, so that it could simply be strcmp'd against the >>>>> UUID value on the kernel command-line. That way, the type of the UUID is >>>>> irrelevant. >>>>> >>>>> Does anyone have any objection to that? >>>> >>>> Wouldn't that be able to break setups which work currently? >>> >>> I don't believe so: >>> >>> Since the newly supported UUID syntax wouldn't ever match any EFI UUID >>> (the lengths differ in all cases), I don't believe the new syntax would >>> affect behavior for any existing usage. >>> >>> Obviously, part_efi.c would be modified to initialize struct >>> partition_meta_info's uuid field to the appropriate string >>> representation of the UUID so that the str(case)cmp would still succeed >>> for existing command-lines. I ended up coding up that part of the change >>> late Friday, and the feature was certainly still working OK. >> >> Functionally, I suspect this will work fine, but I am concerned that >> it is a bad move from an efficiency perspective (not unfixable >> though). Right now, the user-supplied value is converted from >> string-uuid to packed-uuid. This is then memcmp'd across any and all >> partitions - be it 2 or 200 - across all attached storage. If we move >> to a pure string, then we end up needing to unpack every packed UUID >> at disk scan time (or search, depending on impl) rather than just the >> one user supplied value. > > The EFI partition code actually does the following already: > > 1) Unpack the UUID from the binary on-disk representation to a temporary > string. > 2) Repack the temporary string into the internal UUID buffer. > > The comments imply this is in order to do endian conversions. > > Switching the internal representation to a string avoids step (2) above, > plus avoids having to pack the string on the kernel command-line into a > binary UUID before the comparison. I doubt the difference between memcmp > vs. strcasecmp is worth considering. So, I think it's overall a win. Sounds reasonable to me then. Thanks! will ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2012-08-21 18:47 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-08-17 22:10 root=PARTUUID for MBR/NT disk signatures? Stephen Warren 2012-08-20 18:22 ` Tejun Heo 2012-08-20 18:30 ` Stephen Warren 2012-08-21 4:47 ` Will Drewry 2012-08-21 6:38 ` Michael Tokarev 2012-08-21 18:08 ` Stephen Warren 2012-08-21 18:47 ` Will Drewry
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).