Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1628368 > unrolled thread

[PATCH v1 1/8] lib/uuid: Introduce uuid_{be|le}_cmp_p{p}() helpers

Back to article view | Back to linux.kernel

Contents

  [PATCH v1 1/8] lib/uuid: Introduce uuid_{be|le}_cmp_p{p}() helpers Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2017-04-21 19:10 +0200
    [PATCH v1 5/8] vmbus: Use recently introduced uuid_le_cmp_p{p}() helpers Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2017-04-21 19:10 +0200
    [PATCH v1 6/8] mei: Use recently introduced uuid_le_cmp_p{p}() helpers Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2017-04-21 20:50 +0200
    [PATCH v1 3/8] HID: intel_ish-hid: Use recently introduced uuid_le_cmp_p{p}() Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2017-04-21 21:20 +0200
    RE: [PATCH v1 1/8] lib/uuid: Introduce uuid_{be|le}_cmp_p{p}()  helpers "Winkler, Tomas" <tomas.winkler@intel.com> - 2017-04-23 12:40 +0200
      Re: [PATCH v1 1/8] lib/uuid: Introduce uuid_{be|le}_cmp_p{p}() helpers Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-04-23 14:50 +0200
        Re: [PATCH v1 1/8] lib/uuid: Introduce uuid_{be|le}_cmp_p{p}()  helpers "Winkler, Tomas" <tomas.winkler@intel.com> - 2017-04-23 22:30 +0200
          Re: [PATCH v1 1/8] lib/uuid: Introduce uuid_{be|le}_cmp_p{p}()  helpers Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2017-04-24 11:00 +0200
      Re: [PATCH v1 1/8] lib/uuid: Introduce uuid_{be|le}_cmp_p{p}()  helpers Lukas Wunner <lukas@wunner.de> - 2017-04-24 12:50 +0200

#1628368 — [PATCH v1 1/8] lib/uuid: Introduce uuid_{be|le}_cmp_p{p}() helpers

New helpers take pointers to uuid_{be|le} as parameters.

When using them on a raw data we don't need to do an ugly dereference
and, in some cases, a type casting.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Vinod Koul <vinod.koul@intel.com>
Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cc: Kirti Wankhede <kwankhede@nvidia.com>
Cc: Alex Williamson <alex.williamson@redhat.com>
Cc: "K. Y. Srinivasan" <kys@microsoft.com>
Cc: Haiyang Zhang <haiyangz@microsoft.com>
Cc: Stephen Hemminger <sthemmin@microsoft.com>
Cc: Tomas Winkler <tomas.winkler@intel.com>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 include/linux/uuid.h | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/include/linux/uuid.h b/include/linux/uuid.h
index 4dff73a89758..45312cb5ac65 100644
--- a/include/linux/uuid.h
+++ b/include/linux/uuid.h
@@ -58,6 +58,26 @@ static inline int uuid_be_cmp(const uuid_be u1, const uuid_be u2)
 	return memcmp(&u1, &u2, sizeof(uuid_be));
 }
 
+static inline int uuid_le_cmp_p(const uuid_le *pu1, const uuid_le u2)
+{
+	return memcmp(pu1, &u2, sizeof(uuid_le));
+}
+
+static inline int uuid_be_cmp_p(const uuid_be *pu1, const uuid_be u2)
+{
+	return memcmp(pu1, &u2, sizeof(uuid_be));
+}
+
+static inline int uuid_le_cmp_pp(const uuid_le *pu1, const uuid_le *pu2)
+{
+	return memcmp(pu1, pu2, sizeof(uuid_le));
+}
+
+static inline int uuid_be_cmp_pp(const uuid_be *pu1, const uuid_be *pu2)
+{
+	return memcmp(pu1, pu2, sizeof(uuid_be));
+}
+
 void generate_random_uuid(unsigned char uuid[16]);
 
 extern void uuid_le_gen(uuid_le *u);
-- 
2.11.0

[toc] | [next] | [standalone]

#1628369 — [PATCH v1 5/8] vmbus: Use recently introduced uuid_le_cmp_p{p}() helpers

Recently introduced helpers take pointers to uuid_{be|le} instead of
reference.

Using them makes code less ugly.

Cc: "K. Y. Srinivasan" <kys@microsoft.com>
Cc: Haiyang Zhang <haiyangz@microsoft.com>
Cc: Stephen Hemminger <sthemmin@microsoft.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/hv/channel_mgmt.c | 10 +++++-----
 drivers/hv/vmbus_drv.c    |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 735f9363f2e4..01570c50ed04 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -176,7 +176,7 @@ static bool is_unsupported_vmbus_devs(const uuid_le *guid)
 	int i;
 
 	for (i = 0; i < ARRAY_SIZE(vmbus_unsupported_devs); i++)
-		if (!uuid_le_cmp(*guid, vmbus_unsupported_devs[i].guid))
+		if (!uuid_le_cmp_p(guid, vmbus_unsupported_devs[i].guid))
 			return true;
 	return false;
 }
@@ -190,7 +190,7 @@ static u16 hv_get_dev_type(const struct vmbus_channel *channel)
 		return HV_UNKNOWN;
 
 	for (i = HV_IDE; i < HV_UNKNOWN; i++) {
-		if (!uuid_le_cmp(*guid, vmbus_devs[i].guid))
+		if (!uuid_le_cmp_p(guid, vmbus_devs[i].guid))
 			return i;
 	}
 	pr_info("Unknown GUID: %pUl\n", guid);
@@ -456,9 +456,9 @@ static void vmbus_process_offer(struct vmbus_channel *newchannel)
 
 	list_for_each_entry(channel, &vmbus_connection.chn_list, listentry) {
 		if (!uuid_le_cmp(channel->offermsg.offer.if_type,
-			newchannel->offermsg.offer.if_type) &&
-			!uuid_le_cmp(channel->offermsg.offer.if_instance,
-				newchannel->offermsg.offer.if_instance)) {
+				 newchannel->offermsg.offer.if_type) &&
+		    !uuid_le_cmp(channel->offermsg.offer.if_instance,
+				 newchannel->offermsg.offer.if_instance)) {
 			fnew = false;
 			break;
 		}
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 0087b49095eb..b41a2be778f6 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -557,7 +557,7 @@ static const struct hv_vmbus_device_id *hv_vmbus_get_id(struct hv_driver *drv,
 	/* Look at the dynamic ids first, before the static ones */
 	spin_lock(&drv->dynids.lock);
 	list_for_each_entry(dynid, &drv->dynids.list, node) {
-		if (!uuid_le_cmp(dynid->id.guid, *guid)) {
+		if (!uuid_le_cmp_p(guid, dynid->id.guid)) {
 			id = &dynid->id;
 			break;
 		}
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1628483 — [PATCH v1 6/8] mei: Use recently introduced uuid_le_cmp_p{p}() helpers

Recently introduced helpers take pointers to uuid_{be|le} instead of
reference.

Using them makes code less ugly.

Cc: Tomas Winkler <tomas.winkler@intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/misc/mei/bus-fixup.c | 2 +-
 drivers/misc/mei/bus.c       | 2 +-
 drivers/misc/mei/client.c    | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/misc/mei/bus-fixup.c b/drivers/misc/mei/bus-fixup.c
index 0208c4b027c5..de0fcd98d162 100644
--- a/drivers/misc/mei/bus-fixup.c
+++ b/drivers/misc/mei/bus-fixup.c
@@ -416,7 +416,7 @@ void mei_cl_bus_dev_fixup(struct mei_cl_device *cldev)
 
 		f = &mei_fixups[i];
 		if (uuid_le_cmp(f->uuid, MEI_UUID_ANY) == 0 ||
-		    uuid_le_cmp(f->uuid, *uuid) == 0)
+		    uuid_le_cmp_p(uuid, f->uuid) == 0)
 			f->hook(cldev);
 	}
 }
diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
index d1928fdd0f43..e15549f71891 100644
--- a/drivers/misc/mei/bus.c
+++ b/drivers/misc/mei/bus.c
@@ -615,7 +615,7 @@ struct mei_cl_device_id *mei_cl_device_find(struct mei_cl_device *cldev,
 
 	id = cldrv->id_table;
 	while (uuid_le_cmp(NULL_UUID_LE, id->uuid)) {
-		if (!uuid_le_cmp(*uuid, id->uuid)) {
+		if (!uuid_le_cmp_p(uuid, id->uuid)) {
 			match = true;
 
 			if (cldev->name[0])
diff --git a/drivers/misc/mei/client.c b/drivers/misc/mei/client.c
index be64969d986a..590accb369bd 100644
--- a/drivers/misc/mei/client.c
+++ b/drivers/misc/mei/client.c
@@ -148,7 +148,7 @@ static struct mei_me_client *__mei_me_cl_by_uuid(struct mei_device *dev,
 
 	list_for_each_entry(me_cl, &dev->me_clients, list) {
 		pn = &me_cl->props.protocol_name;
-		if (uuid_le_cmp(*uuid, *pn) == 0)
+		if (uuid_le_cmp_pp(uuid, pn) == 0)
 			return mei_me_cl_get(me_cl);
 	}
 
@@ -228,7 +228,7 @@ static struct mei_me_client *__mei_me_cl_by_uuid_id(struct mei_device *dev,
 
 	list_for_each_entry(me_cl, &dev->me_clients, list) {
 		pn = &me_cl->props.protocol_name;
-		if (uuid_le_cmp(*uuid, *pn) == 0 &&
+		if (uuid_le_cmp_pp(uuid, pn) == 0 &&
 		    me_cl->client_id == client_id)
 			return mei_me_cl_get(me_cl);
 	}
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1628523 — [PATCH v1 3/8] HID: intel_ish-hid: Use recently introduced uuid_le_cmp_p{p}()

Recently introduced helpers take pointers to uuid_{be|le} instead of
reference.

Using them makes code less ugly.

Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/hid/intel-ish-hid/ishtp/bus.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.c b/drivers/hid/intel-ish-hid/ishtp/bus.c
index 5f382fedc2ab..165e21b7ee9f 100644
--- a/drivers/hid/intel-ish-hid/ishtp/bus.c
+++ b/drivers/hid/intel-ish-hid/ishtp/bus.c
@@ -138,8 +138,7 @@ int ishtp_fw_cl_by_uuid(struct ishtp_device *dev, const uuid_le *uuid)
 	int i, res = -ENOENT;
 
 	for (i = 0; i < dev->fw_clients_num; ++i) {
-		if (uuid_le_cmp(*uuid, dev->fw_clients[i].props.protocol_name)
-				== 0) {
+		if (!uuid_le_cmp_p(uuid, dev->fw_clients[i].props.protocol_name)) {
 			res = i;
 			break;
 		}
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1628988 — RE: [PATCH v1 1/8] lib/uuid: Introduce uuid_{be|le}_cmp_p{p}() helpers

> New helpers take pointers to uuid_{be|le} as parameters.
> 
> When using them on a raw data we don't need to do an ugly dereference and,
> in some cases, a type casting.
> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Liam Girdwood <lgirdwood@gmail.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Vinod Koul <vinod.koul@intel.com>
> Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Cc: Kirti Wankhede <kwankhede@nvidia.com>
> Cc: Alex Williamson <alex.williamson@redhat.com>
> Cc: "K. Y. Srinivasan" <kys@microsoft.com>
> Cc: Haiyang Zhang <haiyangz@microsoft.com>
> Cc: Stephen Hemminger <sthemmin@microsoft.com>
> Cc: Tomas Winkler <tomas.winkler@intel.com>
> Cc: Matt Fleming <matt@codeblueprint.co.uk>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  include/linux/uuid.h | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/include/linux/uuid.h b/include/linux/uuid.h index
> 4dff73a89758..45312cb5ac65 100644
> --- a/include/linux/uuid.h
> +++ b/include/linux/uuid.h
> @@ -58,6 +58,26 @@ static inline int uuid_be_cmp(const uuid_be u1, const
> uuid_be u2)
>  	return memcmp(&u1, &u2, sizeof(uuid_be));  }
> 
> +static inline int uuid_le_cmp_p(const uuid_le *pu1, const uuid_le u2) {
> +	return memcmp(pu1, &u2, sizeof(uuid_le)); }
> +
> +static inline int uuid_be_cmp_p(const uuid_be *pu1, const uuid_be u2) {
> +	return memcmp(pu1, &u2, sizeof(uuid_be)); }
> +
> +static inline int uuid_le_cmp_pp(const uuid_le *pu1, const uuid_le
> +*pu2) {
> +	return memcmp(pu1, pu2, sizeof(uuid_le)); }
> +
> +static inline int uuid_be_cmp_pp(const uuid_be *pu1, const uuid_be
> +*pu2) {
> +	return memcmp(pu1, pu2, sizeof(uuid_be)); }
> +
>  void generate_random_uuid(unsigned char uuid[16]);
> 
>  extern void uuid_le_gen(uuid_le *u);

I think this going overboard, the _pp types  are just enough. 
Tomas

[toc] | [prev] | [next] | [standalone]

#1629019

On Sun, Apr 23, 2017 at 1:29 PM, Winkler, Tomas <tomas.winkler@intel.com> wrote:
>> New helpers take pointers to uuid_{be|le} as parameters.
>>
>> When using them on a raw data we don't need to do an ugly dereference and,
>> in some cases, a type casting.

> I think this going overboard, the _pp types  are just enough.
I looked at existing users and there are cases like
#define XXX_UUID UUID_...(a, b, c, ...)

uuid_.*_cmp(value, XXX_UUID)

For _pp variant if would be _cmp_pp(value, &XXX_UUID) which is
slightly worse than for _p variant.

-- 
With Best Regards,
Andy Shevchenko

[toc] | [prev] | [next] | [standalone]

#1629077 — Re: [PATCH v1 1/8] lib/uuid: Introduce uuid_{be|le}_cmp_p{p}() helpers

On Sun, 2017-04-23 at 15:42 +0300, Andy Shevchenko wrote:
> On Sun, Apr 23, 2017 at 1:29 PM, Winkler, Tomas <tomas.winkler@intel.
> com> wrote:
> > > New helpers take pointers to uuid_{be|le} as parameters.
> > > 
> > > When using them on a raw data we don't need to do an ugly
> > > dereference and,
> > > in some cases, a type casting.
> > I think this going overboard, the _pp types  are just enough.
> 
> I looked at existing users and there are cases like
> #define XXX_UUID UUID_...(a, b, c, ...)
> 
> uuid_.*_cmp(value, XXX_UUID)
> 
> For _pp variant if would be _cmp_pp(value, &XXX_UUID) which is
> slightly worse than for _p variant.


Maybe it's worth to actually replace the defines with variables than to
create an interface with all the permutations.

Tomas

[toc] | [prev] | [next] | [standalone]

#1629320 — Re: [PATCH v1 1/8] lib/uuid: Introduce uuid_{be|le}_cmp_p{p}() helpers

On Sun, 2017-04-23 at 20:20 +0000, Winkler, Tomas wrote:
> On Sun, 2017-04-23 at 15:42 +0300, Andy Shevchenko wrote:
> > On Sun, Apr 23, 2017 at 1:29 PM, Winkler, Tomas <tomas.winkler@intel
> > .
> > com> wrote:
> > > > New helpers take pointers to uuid_{be|le} as parameters.
> > > > 
> > > > When using them on a raw data we don't need to do an ugly
> > > > dereference and,
> > > > in some cases, a type casting.
> > > 
> > > I think this going overboard, the _pp types  are just enough.
> > 
> > I looked at existing users and there are cases like
> > #define XXX_UUID UUID_...(a, b, c, ...)
> > 
> > uuid_.*_cmp(value, XXX_UUID)
> > 
> > For _pp variant if would be _cmp_pp(value, &XXX_UUID) which is
> > slightly worse than for _p variant.
> 
> 
> Maybe it's worth to actually replace the defines with variables than
> to
> create an interface with all the permutations.

Maybe. I didn't estimate the number of users that would be in a scope.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

[toc] | [prev] | [next] | [standalone]

#1629414 — Re: [PATCH v1 1/8] lib/uuid: Introduce uuid_{be|le}_cmp_p{p}() helpers

> New helpers take pointers to uuid_{be|le} as parameters.
> 
> When using them on a raw data we don't need to do an ugly dereference and,
> in some cases, a type casting.
> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Liam Girdwood <lgirdwood@gmail.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Vinod Koul <vinod.koul@intel.com>
> Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Cc: Kirti Wankhede <kwankhede@nvidia.com>
> Cc: Alex Williamson <alex.williamson@redhat.com>
> Cc: "K. Y. Srinivasan" <kys@microsoft.com>
> Cc: Haiyang Zhang <haiyangz@microsoft.com>
> Cc: Stephen Hemminger <sthemmin@microsoft.com>
> Cc: Tomas Winkler <tomas.winkler@intel.com>
> Cc: Matt Fleming <matt@codeblueprint.co.uk>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  include/linux/uuid.h | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/include/linux/uuid.h b/include/linux/uuid.h index
> 4dff73a89758..45312cb5ac65 100644
> --- a/include/linux/uuid.h
> +++ b/include/linux/uuid.h
> @@ -58,6 +58,26 @@ static inline int uuid_be_cmp(const uuid_be u1, const
> uuid_be u2)
>  	return memcmp(&u1, &u2, sizeof(uuid_be));  }
> 
> +static inline int uuid_le_cmp_p(const uuid_le *pu1, const uuid_le u2) {
> +	return memcmp(pu1, &u2, sizeof(uuid_le)); }
> +
> +static inline int uuid_be_cmp_p(const uuid_be *pu1, const uuid_be u2) {
> +	return memcmp(pu1, &u2, sizeof(uuid_be)); }
> +
> +static inline int uuid_le_cmp_pp(const uuid_le *pu1, const uuid_le
> +*pu2) {
> +	return memcmp(pu1, pu2, sizeof(uuid_le)); }
> +
> +static inline int uuid_be_cmp_pp(const uuid_be *pu1, const uuid_be
> +*pu2) {
> +	return memcmp(pu1, pu2, sizeof(uuid_be)); }
> +
>  void generate_random_uuid(unsigned char uuid[16]);
> 
>  extern void uuid_le_gen(uuid_le *u);

There's a bug in gcc wherein constant compound literals are generated
on the stack at runtime, rather than stored in rodata.  The bug occurs
if the compound literal is passed by reference.  It does not manifest
itself when passing by value:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68725

Hence this patch is unfortunately counterproductive if the UUIDs are
declared const.

FWIW I've posted a series back in January to constify UUIDs as much as
possible, but I got some objections and lacked the time so far to
address them.  In fact I'm thinking that gcc needs to be fixed first,
then we can focus on improving the kernel side of things:

https://www.spinics.net/lists/linux-efi/msg10093.html

Best regards,

Lukas

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web