Linux-NVDIMM Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] Consider namespace with size as active namespace
@ 2019-08-07  4:39 Aneesh Kumar K.V
  2019-10-16 22:31 ` Verma, Vishal L
  0 siblings, 1 reply; 5+ messages in thread
From: Aneesh Kumar K.V @ 2019-08-07  4:39 UTC (permalink / raw)
  To: dan.j.williams; +Cc: Aneesh Kumar K.V, linux-nvdimm

This enables us to mark a namespace as disabled due to pfn_sb
mismatch. We have pending kernel patches at that will mark the
namespace disabled when the PAGE_SIZE or struct page size didn't
match with the value stored in pfn_sb.

We need to make sure we don't use this disabled namespace as seed namespace
for new namespace creation.

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
 ndctl/namespace.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ndctl/namespace.c b/ndctl/namespace.c
index 58a9e3c53474..1f212a2b3a9b 100644
--- a/ndctl/namespace.c
+++ b/ndctl/namespace.c
@@ -455,7 +455,8 @@ static int is_namespace_active(struct ndctl_namespace *ndns)
 	return ndns && (ndctl_namespace_is_enabled(ndns)
 		|| ndctl_namespace_get_pfn(ndns)
 		|| ndctl_namespace_get_dax(ndns)
-		|| ndctl_namespace_get_btt(ndns));
+		|| ndctl_namespace_get_btt(ndns)
+		|| ndctl_namespace_get_size(ndns));
 }
 
 /*
-- 
2.21.0

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* Re: [PATCH] Consider namespace with size as active namespace
  2019-08-07  4:39 [PATCH] Consider namespace with size as active namespace Aneesh Kumar K.V
@ 2019-10-16 22:31 ` Verma, Vishal L
  2019-10-17  3:05   ` Aneesh Kumar K.V
  0 siblings, 1 reply; 5+ messages in thread
From: Verma, Vishal L @ 2019-10-16 22:31 UTC (permalink / raw)
  To: aneesh.kumar, Williams, Dan J; +Cc: linux-nvdimm

On Wed, 2019-08-07 at 10:09 +0530, Aneesh Kumar K.V wrote:
> This enables us to mark a namespace as disabled due to pfn_sb
> mismatch. We have pending kernel patches at that will mark the
> namespace disabled when the PAGE_SIZE or struct page size didn't
> match with the value stored in pfn_sb.
> 
> We need to make sure we don't use this disabled namespace as seed namespace
> for new namespace creation.
> 
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
>  ndctl/namespace.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/ndctl/namespace.c b/ndctl/namespace.c
> index 58a9e3c53474..1f212a2b3a9b 100644
> --- a/ndctl/namespace.c
> +++ b/ndctl/namespace.c
> @@ -455,7 +455,8 @@ static int is_namespace_active(struct ndctl_namespace *ndns)
>  	return ndns && (ndctl_namespace_is_enabled(ndns)
>  		|| ndctl_namespace_get_pfn(ndns)
>  		|| ndctl_namespace_get_dax(ndns)
> -		|| ndctl_namespace_get_btt(ndns));
> +		|| ndctl_namespace_get_btt(ndns)
> +		|| ndctl_namespace_get_size(ndns));
>  }
>  
>  /*

Hi Aneesh,

I was going through pending ndctl patches and found this - this seems to
break some of the unit tests. Also, have the relevant kernel patches
been posted?

The failing unit tests are sector-mode.sh and dax.sh

	-Vishal
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* Re: [PATCH] Consider namespace with size as active namespace
  2019-10-16 22:31 ` Verma, Vishal L
@ 2019-10-17  3:05   ` Aneesh Kumar K.V
  2019-10-30 23:39     ` Verma, Vishal L
  0 siblings, 1 reply; 5+ messages in thread
From: Aneesh Kumar K.V @ 2019-10-17  3:05 UTC (permalink / raw)
  To: Verma, Vishal L, Williams, Dan J; +Cc: linux-nvdimm

On 10/17/19 4:01 AM, Verma, Vishal L wrote:
> On Wed, 2019-08-07 at 10:09 +0530, Aneesh Kumar K.V wrote:
>> This enables us to mark a namespace as disabled due to pfn_sb
>> mismatch. We have pending kernel patches at that will mark the
>> namespace disabled when the PAGE_SIZE or struct page size didn't
>> match with the value stored in pfn_sb.
>>
>> We need to make sure we don't use this disabled namespace as seed namespace
>> for new namespace creation.
>>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>> ---
>>   ndctl/namespace.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/ndctl/namespace.c b/ndctl/namespace.c
>> index 58a9e3c53474..1f212a2b3a9b 100644
>> --- a/ndctl/namespace.c
>> +++ b/ndctl/namespace.c
>> @@ -455,7 +455,8 @@ static int is_namespace_active(struct ndctl_namespace *ndns)
>>   	return ndns && (ndctl_namespace_is_enabled(ndns)
>>   		|| ndctl_namespace_get_pfn(ndns)
>>   		|| ndctl_namespace_get_dax(ndns)
>> -		|| ndctl_namespace_get_btt(ndns));
>> +		|| ndctl_namespace_get_btt(ndns)
>> +		|| ndctl_namespace_get_size(ndns));
>>   }
>>   
>>   /*
> 
> Hi Aneesh,
> 
> I was going through pending ndctl patches and found this - this seems to
> break some of the unit tests. Also, have the relevant kernel patches
> been posted?

Yes. The required changes in kernel got merged as part of

commit 1c97afa714098aab2ca588cc654f8ff67dd46dcb
Author: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Date:   Thu Sep 5 21:15:58 2019 +0530

     libnvdimm/pmem: Advance namespace seed for specific probe errors


> 
> The failing unit tests are sector-mode.sh and dax.sh
> 

I will see if i can run them on ppc64. We still had issues in getting 
ndctl check to be running on ppc64.

-aneesh


_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* Re: [PATCH] Consider namespace with size as active namespace
  2019-10-17  3:05   ` Aneesh Kumar K.V
@ 2019-10-30 23:39     ` Verma, Vishal L
  2019-10-31 18:10       ` Dan Williams
  0 siblings, 1 reply; 5+ messages in thread
From: Verma, Vishal L @ 2019-10-30 23:39 UTC (permalink / raw)
  To: aneesh.kumar, Williams, Dan J; +Cc: linux-nvdimm

On Thu, 2019-10-17 at 08:35 +0530, Aneesh Kumar K.V wrote:
> 
> > > ---
> > >   ndctl/namespace.c | 3 ++-
> > >   1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/ndctl/namespace.c b/ndctl/namespace.c
> > > index 58a9e3c53474..1f212a2b3a9b 100644
> > > --- a/ndctl/namespace.c
> > > +++ b/ndctl/namespace.c
> > > @@ -455,7 +455,8 @@ static int is_namespace_active(struct ndctl_namespace *ndns)
> > >   	return ndns && (ndctl_namespace_is_enabled(ndns)
> > >   		|| ndctl_namespace_get_pfn(ndns)
> > >   		|| ndctl_namespace_get_dax(ndns)
> > > -		|| ndctl_namespace_get_btt(ndns));
> > > +		|| ndctl_namespace_get_btt(ndns)
> > > +		|| ndctl_namespace_get_size(ndns));
> > >   }
> > >   
> > >   /*
[..]
> 
> > The failing unit tests are sector-mode.sh and dax.sh
> > 
> 
> I will see if i can run them on ppc64. We still had issues in getting 
> ndctl check to be running on ppc64.
> 

I dug into this a bit more.

The failure happens on 'legacy' namespaces (ND_DEVICE_NAMESPACE_IO).

There is an assumption that legacy namespaces cannot be fully deleted,
so as part of a reconfigure, when it comes time to delete the namespace
(ndctl_namespace_delete()), we refuse to do that, and bail, before
setting the size to zero.

libndctl.c:4467

	case ND_DEVICE_NAMESPACE_BLK:
		break;
	default:
		dbg(ctx, "%s: nstype: %d not deletable\n",
				ndctl_namespace_get_devname(ndns),
				ndctl_namespace_get_type(ndns));
		return 0;
	}

	rc = namespace_set_size(ndns, 0);
...

Indeed, destroy namespace wouldn't even get to that point, because that
assumption is repeated in namespace_destroy(), where we switch on
namespace type, and potentially skip over the ndctl_namespace_destroy
call entirely.

If setting the size to zero is now significant we'd need to rework both
of these sites. In destroy_namespace(), delay the did_zero checking
until after ndctl_namespace_delete(), and in ndctl_namespace_delete(),
set the size to zero before the type check.

Dan, does the above make sense - was there reason to refrain from
touching the size on legacy namespaces?

	-Vishal
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* Re: [PATCH] Consider namespace with size as active namespace
  2019-10-30 23:39     ` Verma, Vishal L
@ 2019-10-31 18:10       ` Dan Williams
  0 siblings, 0 replies; 5+ messages in thread
From: Dan Williams @ 2019-10-31 18:10 UTC (permalink / raw)
  To: Verma, Vishal L; +Cc: aneesh.kumar, linux-nvdimm

On Wed, Oct 30, 2019 at 4:39 PM Verma, Vishal L
<vishal.l.verma@intel.com> wrote:
>
> On Thu, 2019-10-17 at 08:35 +0530, Aneesh Kumar K.V wrote:
> >
> > > > ---
> > > >   ndctl/namespace.c | 3 ++-
> > > >   1 file changed, 2 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/ndctl/namespace.c b/ndctl/namespace.c
> > > > index 58a9e3c53474..1f212a2b3a9b 100644
> > > > --- a/ndctl/namespace.c
> > > > +++ b/ndctl/namespace.c
> > > > @@ -455,7 +455,8 @@ static int is_namespace_active(struct ndctl_namespace *ndns)
> > > >           return ndns && (ndctl_namespace_is_enabled(ndns)
> > > >                   || ndctl_namespace_get_pfn(ndns)
> > > >                   || ndctl_namespace_get_dax(ndns)
> > > > -         || ndctl_namespace_get_btt(ndns));
> > > > +         || ndctl_namespace_get_btt(ndns)
> > > > +         || ndctl_namespace_get_size(ndns));
> > > >   }
> > > >
> > > >   /*
> [..]
> >
> > > The failing unit tests are sector-mode.sh and dax.sh
> > >
> >
> > I will see if i can run them on ppc64. We still had issues in getting
> > ndctl check to be running on ppc64.
> >
>
> I dug into this a bit more.
>
> The failure happens on 'legacy' namespaces (ND_DEVICE_NAMESPACE_IO).
>
> There is an assumption that legacy namespaces cannot be fully deleted,
> so as part of a reconfigure, when it comes time to delete the namespace
> (ndctl_namespace_delete()), we refuse to do that, and bail, before
> setting the size to zero.
>
> libndctl.c:4467
>
>         case ND_DEVICE_NAMESPACE_BLK:
>                 break;
>         default:
>                 dbg(ctx, "%s: nstype: %d not deletable\n",
>                                 ndctl_namespace_get_devname(ndns),
>                                 ndctl_namespace_get_type(ndns));
>                 return 0;
>         }
>
>         rc = namespace_set_size(ndns, 0);
> ...
>
> Indeed, destroy namespace wouldn't even get to that point, because that
> assumption is repeated in namespace_destroy(), where we switch on
> namespace type, and potentially skip over the ndctl_namespace_destroy
> call entirely.
>
> If setting the size to zero is now significant we'd need to rework both
> of these sites. In destroy_namespace(), delay the did_zero checking
> until after ndctl_namespace_delete(), and in ndctl_namespace_delete(),
> set the size to zero before the type check.
>
> Dan, does the above make sense - was there reason to refrain from
> touching the size on legacy namespaces?

It's because the size is read-only on legacy namespaces, so writes
will always fail so the assumption is that ndctl_namespace_delete() is
a nop. Hmm, but that makes me think that size == read-only might be a
good gate for this idle check, i.e.:

if (size_is_writable(ndns) && size(ndns) != 0)
    return not_idle;
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-07  4:39 [PATCH] Consider namespace with size as active namespace Aneesh Kumar K.V
2019-10-16 22:31 ` Verma, Vishal L
2019-10-17  3:05   ` Aneesh Kumar K.V
2019-10-30 23:39     ` Verma, Vishal L
2019-10-31 18:10       ` Dan Williams

Linux-NVDIMM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-nvdimm/0 linux-nvdimm/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-nvdimm linux-nvdimm/ https://lore.kernel.org/linux-nvdimm \
		linux-nvdimm@lists.01.org
	public-inbox-index linux-nvdimm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.01.lists.linux-nvdimm


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