Linux-Block Archive on lore.kernel.org
 help / Atom feed
* [PATCH v1] : Switch to use new generic UUID API
@ 2019-01-10 14:30 Andy Shevchenko
  2019-01-21  8:47 ` Christoph Hellwig
  0 siblings, 1 reply; 10+ messages in thread
From: Andy Shevchenko @ 2019-01-10 14:30 UTC (permalink / raw)
  To: Matias Bjorling, linux-block, Christoph Hellwig, linux-kernel
  Cc: Andy Shevchenko

There are new types and helpers that are supposed to be used in new code.

As a preparation to get rid of legacy types and API functions do
the conversion here.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/lightnvm/pblk.h | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h
index 85e38ed62f85..0a0aeb6ef314 100644
--- a/drivers/lightnvm/pblk.h
+++ b/drivers/lightnvm/pblk.h
@@ -1362,10 +1362,7 @@ static inline unsigned int pblk_get_secs(struct bio *bio)
 
 static inline void pblk_setup_uuid(struct pblk *pblk)
 {
-	uuid_le uuid;
-
-	uuid_le_gen(&uuid);
-	memcpy(pblk->instance_uuid, uuid.b, 16);
+	guid_gen((guid_t *)&pblk->instance_uuid);
 }
 
 static inline char *pblk_disk_name(struct pblk *pblk)
-- 
2.20.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1] : Switch to use new generic UUID API
  2019-01-10 14:30 [PATCH v1] : Switch to use new generic UUID API Andy Shevchenko
@ 2019-01-21  8:47 ` Christoph Hellwig
  2019-01-24 12:16   ` Andy Shevchenko
  0 siblings, 1 reply; 10+ messages in thread
From: Christoph Hellwig @ 2019-01-21  8:47 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Matias Bjorling, linux-block, Christoph Hellwig, linux-kernel

On Thu, Jan 10, 2019 at 04:30:51PM +0200, Andy Shevchenko wrote:
> There are new types and helpers that are supposed to be used in new code.
> 
> As a preparation to get rid of legacy types and API functions do
> the conversion here.

This seems to miss a "lightnvm" in the subject line.

>  static inline void pblk_setup_uuid(struct pblk *pblk)
>  {
> +	guid_gen((guid_t *)&pblk->instance_uuid);
>  }

I think we can just kill this wrapper.

But more importantly the instance_uuid fied, and the header.uuid one
it is copied from should be turned into an actual guid_t, the memcpys
and memcmps should also be replaced with the proper UUID API.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1] : Switch to use new generic UUID API
  2019-01-21  8:47 ` Christoph Hellwig
@ 2019-01-24 12:16   ` Andy Shevchenko
  2019-01-24 13:17     ` Javier González
  0 siblings, 1 reply; 10+ messages in thread
From: Andy Shevchenko @ 2019-01-24 12:16 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Matias Bjorling, linux-block, linux-kernel

On Mon, Jan 21, 2019 at 09:47:32AM +0100, Christoph Hellwig wrote:
> On Thu, Jan 10, 2019 at 04:30:51PM +0200, Andy Shevchenko wrote:
> > There are new types and helpers that are supposed to be used in new code.
> > 
> > As a preparation to get rid of legacy types and API functions do
> > the conversion here.
> 
> This seems to miss a "lightnvm" in the subject line.
> 
> >  static inline void pblk_setup_uuid(struct pblk *pblk)
> >  {
> > +	guid_gen((guid_t *)&pblk->instance_uuid);
> >  }
> 
> I think we can just kill this wrapper.
> 
> But more importantly the instance_uuid fied, and the header.uuid one
> it is copied from should be turned into an actual guid_t, the memcpys
> and memcmps should also be replaced with the proper UUID API.

header.uuid is defined using __u8 type, I'm not sure we can use guid_t there.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1] : Switch to use new generic UUID API
  2019-01-24 12:16   ` Andy Shevchenko
@ 2019-01-24 13:17     ` Javier González
  2019-01-24 13:36       ` Andy Shevchenko
  0 siblings, 1 reply; 10+ messages in thread
From: Javier González @ 2019-01-24 13:17 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Christoph Hellwig, Matias Bjørling, linux-block, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1028 bytes --]


> On 24 Jan 2019, at 13.16, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> 
> On Mon, Jan 21, 2019 at 09:47:32AM +0100, Christoph Hellwig wrote:
>> On Thu, Jan 10, 2019 at 04:30:51PM +0200, Andy Shevchenko wrote:
>>> There are new types and helpers that are supposed to be used in new code.
>>> 
>>> As a preparation to get rid of legacy types and API functions do
>>> the conversion here.
>> 
>> This seems to miss a "lightnvm" in the subject line.
>> 
>>> static inline void pblk_setup_uuid(struct pblk *pblk)
>>> {
>>> +	guid_gen((guid_t *)&pblk->instance_uuid);
>>> }
>> 
>> I think we can just kill this wrapper.
>> 
>> But more importantly the instance_uuid fied, and the header.uuid one
>> it is copied from should be turned into an actual guid_t, the memcpys
>> and memcmps should also be replaced with the proper UUID API.
> 
> header.uuid is defined using __u8 type, I'm not sure we can use guid_t there.
> 

We can turn it into a guid_t and bump the minor version.

Javier

[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1] : Switch to use new generic UUID API
  2019-01-24 13:17     ` Javier González
@ 2019-01-24 13:36       ` Andy Shevchenko
  2019-01-24 13:45         ` Javier González
  0 siblings, 1 reply; 10+ messages in thread
From: Andy Shevchenko @ 2019-01-24 13:36 UTC (permalink / raw)
  To: Javier González
  Cc: Andy Shevchenko, Christoph Hellwig, Matias Bjørling,
	linux-block, Linux Kernel Mailing List

On Thu, Jan 24, 2019 at 3:19 PM Javier González <javier@javigon.com> wrote:
> > On 24 Jan 2019, at 13.16, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > On Mon, Jan 21, 2019 at 09:47:32AM +0100, Christoph Hellwig wrote:
> >> On Thu, Jan 10, 2019 at 04:30:51PM +0200, Andy Shevchenko wrote:
> >>> There are new types and helpers that are supposed to be used in new code.
> >>>
> >>> As a preparation to get rid of legacy types and API functions do
> >>> the conversion here.
> >>
> >> This seems to miss a "lightnvm" in the subject line.
> >>
> >>> static inline void pblk_setup_uuid(struct pblk *pblk)
> >>> {
> >>> +   guid_gen((guid_t *)&pblk->instance_uuid);
> >>> }
> >>
> >> I think we can just kill this wrapper.
> >>
> >> But more importantly the instance_uuid fied, and the header.uuid one
> >> it is copied from should be turned into an actual guid_t, the memcpys
> >> and memcmps should also be replaced with the proper UUID API.
> >
> > header.uuid is defined using __u8 type, I'm not sure we can use guid_t there.
> >
>
> We can turn it into a guid_t and bump the minor version.

It's not so easy. __uXX types are dedicated for external APIs. guid_t
is kernel internal type disregard of (still) presence some uapi bits.
So, the question is those __uXX types in the driver definition is a
simple mistake, (weird) style decision, or what?

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1] : Switch to use new generic UUID API
  2019-01-24 13:36       ` Andy Shevchenko
@ 2019-01-24 13:45         ` Javier González
  2019-01-24 14:13           ` Andy Shevchenko
  0 siblings, 1 reply; 10+ messages in thread
From: Javier González @ 2019-01-24 13:45 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Andy Shevchenko, Christoph Hellwig, Matias Bjørling,
	linux-block, Linux Kernel Mailing List

[-- Attachment #1: Type: text/plain, Size: 1782 bytes --]


> On 24 Jan 2019, at 14.36, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> 
> On Thu, Jan 24, 2019 at 3:19 PM Javier González <javier@javigon.com> wrote:
>>> On 24 Jan 2019, at 13.16, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
>>> On Mon, Jan 21, 2019 at 09:47:32AM +0100, Christoph Hellwig wrote:
>>>> On Thu, Jan 10, 2019 at 04:30:51PM +0200, Andy Shevchenko wrote:
>>>>> There are new types and helpers that are supposed to be used in new code.
>>>>> 
>>>>> As a preparation to get rid of legacy types and API functions do
>>>>> the conversion here.
>>>> 
>>>> This seems to miss a "lightnvm" in the subject line.
>>>> 
>>>>> static inline void pblk_setup_uuid(struct pblk *pblk)
>>>>> {
>>>>> +   guid_gen((guid_t *)&pblk->instance_uuid);
>>>>> }
>>>> 
>>>> I think we can just kill this wrapper.
>>>> 
>>>> But more importantly the instance_uuid fied, and the header.uuid one
>>>> it is copied from should be turned into an actual guid_t, the memcpys
>>>> and memcmps should also be replaced with the proper UUID API.
>>> 
>>> header.uuid is defined using __u8 type, I'm not sure we can use guid_t there.
>> 
>> We can turn it into a guid_t and bump the minor version.
> 
> It's not so easy. __uXX types are dedicated for external APIs. guid_t
> is kernel internal type disregard of (still) presence some uapi bits.
> So, the question is those __uXX types in the driver definition is a
> simple mistake, (weird) style decision, or what?
> 

I would define it as a mistake and I think it is worth fixing it. At the
moment we are only using this uuid for recovery purposes, to discard
data from a different pblk instance, so there should not be a big impact
outside of pblk itself. Am I missing something?

Javier

[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1] : Switch to use new generic UUID API
  2019-01-24 13:45         ` Javier González
@ 2019-01-24 14:13           ` Andy Shevchenko
  2019-01-24 14:36             ` Javier González
  0 siblings, 1 reply; 10+ messages in thread
From: Andy Shevchenko @ 2019-01-24 14:13 UTC (permalink / raw)
  To: Javier González
  Cc: Andy Shevchenko, Christoph Hellwig, Matias Bjørling,
	linux-block, Linux Kernel Mailing List

On Thu, Jan 24, 2019 at 3:45 PM Javier González <javier@javigon.com> wrote:
> > On 24 Jan 2019, at 14.36, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > On Thu, Jan 24, 2019 at 3:19 PM Javier González <javier@javigon.com> wrote:
> >>> On 24 Jan 2019, at 13.16, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> >>> header.uuid is defined using __u8 type, I'm not sure we can use guid_t there.
> >>
> >> We can turn it into a guid_t and bump the minor version.
> >
> > It's not so easy. __uXX types are dedicated for external APIs. guid_t
> > is kernel internal type disregard of (still) presence some uapi bits.
> > So, the question is those __uXX types in the driver definition is a
> > simple mistake, (weird) style decision, or what?
> >
>
> I would define it as a mistake and I think it is worth fixing it. At the
> moment we are only using this uuid for recovery purposes, to discard
> data from a different pblk instance,

Does this come from outside of the kernel in any mean (user space,
data from device, etc)?
It sounds to me like it does. In this case there is no mistake and we
may not use guid_t there.

> so there should not be a big impact
> outside of pblk itself. Am I missing something?

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1] : Switch to use new generic UUID API
  2019-01-24 14:13           ` Andy Shevchenko
@ 2019-01-24 14:36             ` Javier González
  2019-01-24 16:38               ` Andy Shevchenko
  0 siblings, 1 reply; 10+ messages in thread
From: Javier González @ 2019-01-24 14:36 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Andy Shevchenko, Christoph Hellwig, Matias Bjørling,
	linux-block, Linux Kernel Mailing List

[-- Attachment #1: Type: text/plain, Size: 1699 bytes --]


> On 24 Jan 2019, at 15.13, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> 
> On Thu, Jan 24, 2019 at 3:45 PM Javier González <javier@javigon.com> wrote:
>>> On 24 Jan 2019, at 14.36, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>>> On Thu, Jan 24, 2019 at 3:19 PM Javier González <javier@javigon.com> wrote:
>>>>> On 24 Jan 2019, at 13.16, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> 
>>>>> header.uuid is defined using __u8 type, I'm not sure we can use guid_t there.
>>>> 
>>>> We can turn it into a guid_t and bump the minor version.
>>> 
>>> It's not so easy. __uXX types are dedicated for external APIs. guid_t
>>> is kernel internal type disregard of (still) presence some uapi bits.
>>> So, the question is those __uXX types in the driver definition is a
>>> simple mistake, (weird) style decision, or what?
>> 
>> I would define it as a mistake and I think it is worth fixing it. At the
>> moment we are only using this uuid for recovery purposes, to discard
>> data from a different pblk instance,
> 
> Does this come from outside of the kernel in any mean (user space,
> data from device, etc)?
> It sounds to me like it does. In this case there is no mistake and we
> may not use guid_t there.

pblk manages the metadata layout without involvement of the device or
user space, so no, no dependency at this moment.

It is not pushed anywhere yet, but I have been working on a tool to make
a pblk recovery tool to enable FTL repairs if something fails in the
kernel recovery path. Here, I use this uuid to identify the
instance - is there a way to reconcile guid_t with user space, which
currently uses the __u8?

Javier

[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1] : Switch to use new generic UUID API
  2019-01-24 14:36             ` Javier González
@ 2019-01-24 16:38               ` Andy Shevchenko
  2019-01-24 16:41                 ` Javier González
  0 siblings, 1 reply; 10+ messages in thread
From: Andy Shevchenko @ 2019-01-24 16:38 UTC (permalink / raw)
  To: Javier González
  Cc: Andy Shevchenko, Christoph Hellwig, Matias Bjørling,
	linux-block, Linux Kernel Mailing List

On Thu, Jan 24, 2019 at 4:36 PM Javier González <javier@javigon.com> wrote:

> It is not pushed anywhere yet, but I have been working on a tool to make
> a pblk recovery tool to enable FTL repairs if something fails in the
> kernel recovery path. Here, I use this uuid to identify the
> instance - is there a way to reconcile guid_t with user space, which
> currently uses the __u8?

For Linux there is util-linux which contains libuuid. There is uuid_t type.
Unfortunately there is no so called LE (little endian) variant.
Perhaps someone would need to extend the support for that.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1] : Switch to use new generic UUID API
  2019-01-24 16:38               ` Andy Shevchenko
@ 2019-01-24 16:41                 ` Javier González
  0 siblings, 0 replies; 10+ messages in thread
From: Javier González @ 2019-01-24 16:41 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Andy Shevchenko, Christoph Hellwig, Matias Bjørling,
	linux-block, Linux Kernel Mailing List

[-- Attachment #1: Type: text/plain, Size: 914 bytes --]


> On 24 Jan 2019, at 17.38, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> 
> On Thu, Jan 24, 2019 at 4:36 PM Javier González <javier@javigon.com> wrote:
> 
>> It is not pushed anywhere yet, but I have been working on a tool to make
>> a pblk recovery tool to enable FTL repairs if something fails in the
>> kernel recovery path. Here, I use this uuid to identify the
>> instance - is there a way to reconcile guid_t with user space, which
>> currently uses the __u8?
> 
> For Linux there is util-linux which contains libuuid. There is uuid_t type.
> Unfortunately there is no so called LE (little endian) variant.
> Perhaps someone would need to extend the support for that.

Ok. I can look into that when releasing pblk-tools. But for now, I am OK
with applying with the changes Christoph suggested and aligning with
guid_t if you also think helps maintaining common helpers.

Javier

[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, back to index

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-10 14:30 [PATCH v1] : Switch to use new generic UUID API Andy Shevchenko
2019-01-21  8:47 ` Christoph Hellwig
2019-01-24 12:16   ` Andy Shevchenko
2019-01-24 13:17     ` Javier González
2019-01-24 13:36       ` Andy Shevchenko
2019-01-24 13:45         ` Javier González
2019-01-24 14:13           ` Andy Shevchenko
2019-01-24 14:36             ` Javier González
2019-01-24 16:38               ` Andy Shevchenko
2019-01-24 16:41                 ` Javier González

Linux-Block Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-block/0 linux-block/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-block linux-block/ https://lore.kernel.org/linux-block \
		linux-block@vger.kernel.org linux-block@archiver.kernel.org
	public-inbox-index linux-block


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-block


AGPL code for this site: git clone https://public-inbox.org/ public-inbox