[PATCH 0/6] dcache/namei fixes for lustre

[PATCH 0/6] dcache/namei fixes for lustre

[NeilBrown]

This series is a revised version of two patches I sent
previously (one of which was sadly broken).
That patch has been broken into multiple parts for easy
review.  The other is included unchanged as the last of
this series.

I was drawn to look at this code due to the tests on
DCACHE_DISCONNECTED which are often wrong, and it turns out
they are used wrongly in lustre too.  Fixing one led to some
clean-up.  Fixing the other is straight forward.

A particular change here from the previous posting is
the first patch which tests for DCACHE_PAR_LOOKUP in ll_dcompare().
Without this patch, two threads can be looking up the same
name in a given directory in parallel.  This parallelism lead
to my concerns about needing improved locking in ll_splice_alias().
Instead of improving the locking, I now avoid the need for it
by fixing ll_dcompare.

This code passes basic "smoke tests".

Note that the cast to "struct dentry *" in the first patch is because
we have a "const struct dentry *" but d_in_lookup() requires a
pointer to a non-const structure.  I'll send a separate patch to
change d_in_lookup().

---

NeilBrown (6):
      staging: lustre: llite: handle DCACHE_PAR_LOOKUP in ll_dcompare
      staging: lustre: llite: use d_splice_alias for directories.
      staging: lustre: llite: remove directory-specific code from ll_find_alias()
      staging: lluste: llite: simplify ll_find_alias()
      staging: lustre: llite: refine ll_find_alias based on d_exact_alias
      staging: lustre: llite: fix incorrect DCACHE_DISCONNECTED test


 drivers/staging/lustre/lustre/llite/dcache.c       |   10 +++
 .../staging/lustre/lustre/llite/llite_internal.h   |    2 -
 drivers/staging/lustre/lustre/llite/namei.c        |   60 ++++++++++----------
 3 files changed, 40 insertions(+), 32 deletions(-)

--
Signature

[PATCH 1/6] staging: lustre: llite: handle DCACHE_PAR_LOOKUP in ll_dcompare

[NeilBrown]

ll_dcompare is used in two slightly different contexts.
It is called (from __d_lookup, __d_lookup_rcu, and d_exact_alias)
to compare a name against a dentry that is already in the dcache.
It is also called (from d_alloc_parallel) to compare a name against
a dentry that is not in the dcache yet, but is part of an active
"lookup" or "atomic_open" call.

In the first case we need to avoid matching against "invalid" dentries
as a match implies something about ldlm locks which is not accurate.
In the second case we need to allow matching against "invalid" dentries
as the dentry will always be invalid (set by ll_d_init()) but we still
want to guard against multiple concurrent lookups of the same name.
d_alloc_parallel() will repeat the call to ll_dcompare() after
the lookup has finished, and if the dentry is still invalid, the whole
d_alloc_parallel() process is repeated.  This assures us that it is safe
to report success whenever d_in_lookup().

With this patch, there will never be two threads concurrently in
ll_lookup_nd(), looking up the same name in the same directory.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lustre/llite/dcache.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/staging/lustre/lustre/llite/dcache.c b/drivers/staging/lustre/lustre/llite/dcache.c
index d20425fb8cbe..7cfe0babf99e 100644
--- a/drivers/staging/lustre/lustre/llite/dcache.c
+++ b/drivers/staging/lustre/lustre/llite/dcache.c
@@ -73,6 +73,12 @@ static void ll_release(struct dentry *de)
  * an AST before calling d_revalidate_it().  The dentry still exists (marked
  * INVALID) so d_lookup() matches it, but we have no lock on it (so
  * lock_match() fails) and we spin around real_lookup().
+ *
+ * This race doesn't apply to lookups in d_alloc_parallel(), and for
+ * those we want to ensure that only one dentry with a given name is
+ * in ll_lookup_nd() at a time.  So allow invalid dentries to match
+ * while d_in_lookup().  We will be called again when the lookup
+ * completes, and can give a different answer then.
  */
 static int ll_dcompare(const struct dentry *dentry,
 		       unsigned int len, const char *str,
@@ -92,6 +98,10 @@ static int ll_dcompare(const struct dentry *dentry,
 	if (d_mountpoint((struct dentry *)dentry))
 		return 0;
 
+	/* ensure exclusion against parallel lookup of the same name */
+	if (d_in_lookup((struct dentry*)dentry))
+		return 0;
+
 	if (d_lustre_invalid(dentry))
 		return 1;
 

[PATCH 2/6] staging: lustre: llite: use d_splice_alias for directories.

[NeilBrown]

In the Linux dcache a directory only ever has one dentry,
so d_splice_alias() can be used by ll_splice_alias() for directories.
It will find the one dentry whether it is DCACHE_DISCONNECTED or
IS_ROOT() or d_lustre_invalid().
Separating out the directories from non-directories will allow us
to simplify the non-directory code.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lustre/llite/namei.c |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/lustre/lustre/llite/namei.c b/drivers/staging/lustre/lustre/llite/namei.c
index a208a8b02c2c..5ac340a8ad6f 100644
--- a/drivers/staging/lustre/lustre/llite/namei.c
+++ b/drivers/staging/lustre/lustre/llite/namei.c
@@ -432,7 +432,7 @@ static struct dentry *ll_find_alias(struct inode *inode, struct dentry *dentry)
  */
 struct dentry *ll_splice_alias(struct inode *inode, struct dentry *de)
 {
-	if (inode) {
+	if (inode && !S_ISDIR(inode->i_mode)) {
 		struct dentry *new = ll_find_alias(inode, de);
 
 		if (new) {
@@ -443,8 +443,13 @@ struct dentry *ll_splice_alias(struct inode *inode, struct dentry *de)
 			      new, d_inode(new), d_count(new), new->d_flags);
 			return new;
 		}
+		d_add(de, inode);
+	} else {
+		struct dentry *new = d_splice_alias(inode, de);
+
+		if (new)
+			de = new;
 	}
-	d_add(de, inode);
 	CDEBUG(D_DENTRY, "Add dentry %p inode %p refc %d flags %#x\n",
 	       de, d_inode(de), d_count(de), de->d_flags);
 	return de;

[PATCH 3/6] staging: lustre: llite: remove directory-specific code from ll_find_alias()

[NeilBrown]

Now that ll_find_alias() is never called for directories,
we can remove code that only applies to directories.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lustre/llite/namei.c |   26 ++++++++------------------
 1 file changed, 8 insertions(+), 18 deletions(-)

diff --git a/drivers/staging/lustre/lustre/llite/namei.c b/drivers/staging/lustre/lustre/llite/namei.c
index 5ac340a8ad6f..329038a10a07 100644
--- a/drivers/staging/lustre/lustre/llite/namei.c
+++ b/drivers/staging/lustre/lustre/llite/namei.c
@@ -378,21 +378,15 @@ void ll_i2gids(__u32 *suppgids, struct inode *i1, struct inode *i2)
 }
 
 /*
- * try to reuse three types of dentry:
- * 1. unhashed alias, this one is unhashed by d_invalidate (but it may be valid
- *    by concurrent .revalidate).
- * 2. INVALID alias (common case for no valid ldlm lock held, but this flag may
- *    be cleared by others calling d_lustre_revalidate).
- * 3. DISCONNECTED alias.
+ * Try to reuse unhashed or invalidated dentries.
  */
 static struct dentry *ll_find_alias(struct inode *inode, struct dentry *dentry)
 {
-	struct dentry *alias, *discon_alias, *invalid_alias;
+	struct dentry *alias, *invalid_alias;
 
 	if (hlist_empty(&inode->i_dentry))
 		return NULL;
 
-	discon_alias = NULL;
 	invalid_alias = NULL;
 
 	spin_lock(&inode->i_lock);
@@ -400,22 +394,18 @@ static struct dentry *ll_find_alias(struct inode *inode, struct dentry *dentry)
 		LASSERT(alias != dentry);
 
 		spin_lock(&alias->d_lock);
-		if ((alias->d_flags & DCACHE_DISCONNECTED) &&
-		    S_ISDIR(inode->i_mode))
-			/* LASSERT(last_discon == NULL); LU-405, bz 20055 */
-			discon_alias = alias;
-		else if (alias->d_parent == dentry->d_parent	     &&
-			 alias->d_name.hash == dentry->d_name.hash       &&
-			 alias->d_name.len == dentry->d_name.len	 &&
-			 memcmp(alias->d_name.name, dentry->d_name.name,
-				dentry->d_name.len) == 0)
+		if (alias->d_parent == dentry->d_parent	     &&
+		    alias->d_name.hash == dentry->d_name.hash       &&
+		    alias->d_name.len == dentry->d_name.len	 &&
+		    memcmp(alias->d_name.name, dentry->d_name.name,
+			   dentry->d_name.len) == 0)
 			invalid_alias = alias;
 		spin_unlock(&alias->d_lock);
 
 		if (invalid_alias)
 			break;
 	}
-	alias = invalid_alias ?: discon_alias ?: NULL;
+	alias = invalid_alias ?: NULL;
 	if (alias) {
 		spin_lock(&alias->d_lock);
 		dget_dlock(alias);

[PATCH 4/6] staging: lluste: llite: simplify ll_find_alias()

[NeilBrown]

Now that ll_find_alias is only searching for one type
of dentry, we can return as soon as we find it.
This allows substantial simplification, and brings the
bonus that we don't need to take the d_lock again just
to increment the ref-count.  We can increment it immediately
that the dentry is found.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lustre/llite/namei.c |   23 ++++++++---------------
 1 file changed, 8 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/lustre/lustre/llite/namei.c b/drivers/staging/lustre/lustre/llite/namei.c
index 329038a10a07..51838eed501a 100644
--- a/drivers/staging/lustre/lustre/llite/namei.c
+++ b/drivers/staging/lustre/lustre/llite/namei.c
@@ -382,13 +382,11 @@ void ll_i2gids(__u32 *suppgids, struct inode *i1, struct inode *i2)
  */
 static struct dentry *ll_find_alias(struct inode *inode, struct dentry *dentry)
 {
-	struct dentry *alias, *invalid_alias;
+	struct dentry *alias;
 
 	if (hlist_empty(&inode->i_dentry))
 		return NULL;
 
-	invalid_alias = NULL;
-
 	spin_lock(&inode->i_lock);
 	hlist_for_each_entry(alias, &inode->i_dentry, d_u.d_alias) {
 		LASSERT(alias != dentry);
@@ -398,22 +396,17 @@ static struct dentry *ll_find_alias(struct inode *inode, struct dentry *dentry)
 		    alias->d_name.hash == dentry->d_name.hash       &&
 		    alias->d_name.len == dentry->d_name.len	 &&
 		    memcmp(alias->d_name.name, dentry->d_name.name,
-			   dentry->d_name.len) == 0)
-			invalid_alias = alias;
-		spin_unlock(&alias->d_lock);
-
-		if (invalid_alias)
-			break;
-	}
-	alias = invalid_alias ?: NULL;
-	if (alias) {
-		spin_lock(&alias->d_lock);
-		dget_dlock(alias);
+			   dentry->d_name.len) == 0) {
+			dget_dlock(alias);
+			spin_unlock(&alias->d_lock);
+			spin_unlock(&inode->i_lock);
+			return alias;
+		}
 		spin_unlock(&alias->d_lock);
 	}
 	spin_unlock(&inode->i_lock);
 
-	return alias;
+	return NULL;
 }
 
 /*

[PATCH 5/6] staging: lustre: llite: refine ll_find_alias based on d_exact_alias

[NeilBrown]

The task of ll_find_alias() is now very similar to d_exact_alias().
We cannot use that function directory, but we can copy much of
the structure so that the similarities and differences are more
obvious.
Examining d_exact_alias() shows that the d_lock spinlock does not
need to be held as much as it currently is.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lustre/llite/namei.c |   30 ++++++++++++++++++---------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/lustre/lustre/llite/namei.c b/drivers/staging/lustre/lustre/llite/namei.c
index 51838eed501a..8ed24ec1255d 100644
--- a/drivers/staging/lustre/lustre/llite/namei.c
+++ b/drivers/staging/lustre/lustre/llite/namei.c
@@ -379,6 +379,10 @@ void ll_i2gids(__u32 *suppgids, struct inode *i1, struct inode *i2)
 
 /*
  * Try to reuse unhashed or invalidated dentries.
+ * This is very similar to d_exact_alias(), and any changes in one should be
+ * considered for inclusion in the other.  The differences are that we don't
+ * need an unhashed alias, and we don't want d_compare to be used for
+ * comparison.
  */
 static struct dentry *ll_find_alias(struct inode *inode, struct dentry *dentry)
 {
@@ -390,19 +394,25 @@ static struct dentry *ll_find_alias(struct inode *inode, struct dentry *dentry)
 	spin_lock(&inode->i_lock);
 	hlist_for_each_entry(alias, &inode->i_dentry, d_u.d_alias) {
 		LASSERT(alias != dentry);
+		/*
+		 * Don't need alias->d_lock here, because aliases with
+		 * d_parent == entry->d_parent are not subject to name or
+		 * parent changes, because the parent inode i_mutex is held.
+		 */
 
-		spin_lock(&alias->d_lock);
-		if (alias->d_parent == dentry->d_parent	     &&
-		    alias->d_name.hash == dentry->d_name.hash       &&
-		    alias->d_name.len == dentry->d_name.len	 &&
+		if (alias->d_parent != dentry->d_parent)
+			continue;
+		if (alias->d_name.hash != dentry->d_name.hash)
+			continue;
+		if (alias->d_name.len != dentry->d_name.len ||
 		    memcmp(alias->d_name.name, dentry->d_name.name,
-			   dentry->d_name.len) == 0) {
-			dget_dlock(alias);
-			spin_unlock(&alias->d_lock);
-			spin_unlock(&inode->i_lock);
-			return alias;
-		}
+			   dentry->d_name.len) != 0)
+			continue;
+		spin_lock(&alias->d_lock);
+		dget_dlock(alias);
 		spin_unlock(&alias->d_lock);
+		spin_unlock(&inode->i_lock);
+		return alias;
 	}
 	spin_unlock(&inode->i_lock);
 

[PATCH 6/6] staging: lustre: llite: fix incorrect DCACHE_DISCONNECTED test

[NeilBrown]

It is almost always wrong to test DCACHE_DISCONNECTED, except in
"exportfs" code.
The flag tells us that this dentry *might* not be connected to the
root through a chain of d_parent links.  Following the d_parent
to an IS_ROOT() dentry *might* find one that is on the s_anon list
rather than s_root.

The code here needs to know if it is safe to call __d_drop(), and
the correct test is "!IS_ROOT(dentry)".
If an dentry IS_ROOT(), then it might be the filesystem root, or it
might be the root of a DCACHE_DISCONNECTED tree, and so be on
the s_anon list.  In these two cases it should not be __d_drop()ed.
If !IS_ROOT(), then the dentry is attached to its parent through
d_subdir, and can safely be unhashed.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 .../staging/lustre/lustre/llite/llite_internal.h   |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/lustre/lustre/llite/llite_internal.h b/drivers/staging/lustre/lustre/llite/llite_internal.h
index cd3311abf999..4854985bf4d3 100644
--- a/drivers/staging/lustre/lustre/llite/llite_internal.h
+++ b/drivers/staging/lustre/lustre/llite/llite_internal.h
@@ -1299,7 +1299,7 @@ static inline void d_lustre_invalidate(struct dentry *dentry, int nested)
 	 * If we unhashed such a dentry, unmount would not be able to find
 	 * it and busy inodes would be reported.
 	 */
-	if (d_count(dentry) == 0 && !(dentry->d_flags & DCACHE_DISCONNECTED))
+	if (d_count(dentry) == 0 && !IS_ROOT(dentry))
 		__d_drop(dentry);
 	spin_unlock(&dentry->d_lock);
 }

Re: [lustre-devel] [PATCH 0/6] dcache/namei fixes for lustre

[James Simmons]

> This series is a revised version of two patches I sent
> previously (one of which was sadly broken).
> That patch has been broken into multiple parts for easy
> review.  The other is included unchanged as the last of
> this series.
> 
> I was drawn to look at this code due to the tests on
> DCACHE_DISCONNECTED which are often wrong, and it turns out
> they are used wrongly in lustre too.  Fixing one led to some
> clean-up.  Fixing the other is straight forward.
> 
> A particular change here from the previous posting is
> the first patch which tests for DCACHE_PAR_LOOKUP in ll_dcompare().
> Without this patch, two threads can be looking up the same
> name in a given directory in parallel.  This parallelism lead
> to my concerns about needing improved locking in ll_splice_alias().
> Instead of improving the locking, I now avoid the need for it
> by fixing ll_dcompare.
> 
> This code passes basic "smoke tests".
> 
> Note that the cast to "struct dentry *" in the first patch is because
> we have a "const struct dentry *" but d_in_lookup() requires a
> pointer to a non-const structure.  I'll send a separate patch to
> change d_in_lookup().

To let you know this patch has been under going testing and we have a
ticket open to track the progess:

https://jira.hpdd.intel.com/browse/LU-9868

Your patch did reveal that a piece of a fix landed earlier is missing :-(
So currently the client can oops. I will send the fix shortly but this
work will have to rebased after. As soon as we can get some cycles we will
figure out what is going on. Thanks for helping out.
 
> NeilBrown (6):
>       staging: lustre: llite: handle DCACHE_PAR_LOOKUP in ll_dcompare
>       staging: lustre: llite: use d_splice_alias for directories.
>       staging: lustre: llite: remove directory-specific code from ll_find_alias()
>       staging: lluste: llite: simplify ll_find_alias()
>       staging: lustre: llite: refine ll_find_alias based on d_exact_alias
>       staging: lustre: llite: fix incorrect DCACHE_DISCONNECTED test
> 
> 
>  drivers/staging/lustre/lustre/llite/dcache.c       |   10 +++
>  .../staging/lustre/lustre/llite/llite_internal.h   |    2 -
>  drivers/staging/lustre/lustre/llite/namei.c        |   60 ++++++++++----------
>  3 files changed, 40 insertions(+), 32 deletions(-)
> 
> --
> Signature
> 
> _______________________________________________
> lustre-devel mailing list
> lustre-devel@lists.lustre.org
> http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org
> 

Re: [lustre-devel] [PATCH 0/6] dcache/namei fixes for lustre

[NeilBrown]

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

On Sun, Aug 20 2017, James Simmons wrote:

>> This series is a revised version of two patches I sent
>> previously (one of which was sadly broken).
>> That patch has been broken into multiple parts for easy
>> review.  The other is included unchanged as the last of
>> this series.
>> 
>> I was drawn to look at this code due to the tests on
>> DCACHE_DISCONNECTED which are often wrong, and it turns out
>> they are used wrongly in lustre too.  Fixing one led to some
>> clean-up.  Fixing the other is straight forward.
>> 
>> A particular change here from the previous posting is
>> the first patch which tests for DCACHE_PAR_LOOKUP in ll_dcompare().
>> Without this patch, two threads can be looking up the same
>> name in a given directory in parallel.  This parallelism lead
>> to my concerns about needing improved locking in ll_splice_alias().
>> Instead of improving the locking, I now avoid the need for it
>> by fixing ll_dcompare.
>> 
>> This code passes basic "smoke tests".
>> 
>> Note that the cast to "struct dentry *" in the first patch is because
>> we have a "const struct dentry *" but d_in_lookup() requires a
>> pointer to a non-const structure.  I'll send a separate patch to
>> change d_in_lookup().
>
> To let you know this patch has been under going testing and we have a
> ticket open to track the progess:
>
> https://jira.hpdd.intel.com/browse/LU-9868
>
> Your patch did reveal that a piece of a fix landed earlier is missing :-(
> So currently the client can oops. I will send the fix shortly but this
> work will have to rebased after. As soon as we can get some cycles we will
> figure out what is going on. Thanks for helping out.

Hi,
 what happened about this?  I had a look around the ticket and couldn't
 find anything about an oops.  If there is still a problem I'd be very
 happy to help work out what it is - but I don't know where to look.

Thanks,
NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [lustre-devel] [PATCH 0/6] dcache/namei fixes for lustre

[James Simmons]

> >> This series is a revised version of two patches I sent
> >> previously (one of which was sadly broken).
> >> That patch has been broken into multiple parts for easy
> >> review.  The other is included unchanged as the last of
> >> this series.
> >> 
> >> I was drawn to look at this code due to the tests on
> >> DCACHE_DISCONNECTED which are often wrong, and it turns out
> >> they are used wrongly in lustre too.  Fixing one led to some
> >> clean-up.  Fixing the other is straight forward.
> >> 
> >> A particular change here from the previous posting is
> >> the first patch which tests for DCACHE_PAR_LOOKUP in ll_dcompare().
> >> Without this patch, two threads can be looking up the same
> >> name in a given directory in parallel.  This parallelism lead
> >> to my concerns about needing improved locking in ll_splice_alias().
> >> Instead of improving the locking, I now avoid the need for it
> >> by fixing ll_dcompare.
> >> 
> >> This code passes basic "smoke tests".
> >> 
> >> Note that the cast to "struct dentry *" in the first patch is because
> >> we have a "const struct dentry *" but d_in_lookup() requires a
> >> pointer to a non-const structure.  I'll send a separate patch to
> >> change d_in_lookup().
> >
> > To let you know this patch has been under going testing and we have a
> > ticket open to track the progess:
> >
> > https://jira.hpdd.intel.com/browse/LU-9868
> >
> > Your patch did reveal that a piece of a fix landed earlier is missing :-(
> > So currently the client can oops. I will send the fix shortly but this
> > work will have to rebased after. As soon as we can get some cycles we will
> > figure out what is going on. Thanks for helping out.
> 
> Hi,
>  what happened about this?  I had a look around the ticket and couldn't
>  find anything about an oops.  If there is still a problem I'd be very
>  happy to help work out what it is - but I don't know where to look.

The oops is specific to the in kernel client. Some where along the way the
calls to ll_d_init() were removed from ll_splice_alias(). It was unnoticed
until your patch came along. I do have a fix that I will be pushing to 
the next staging tree very shortly.

I have been testing the patch series and for me I don't see any issue. Our 
test suite is reporting failures with this patch which I'm attempting to 
figure out how to reproduce locally on my test system. Once I have a 
reproducer I can send it to you. 

Re: [lustre-devel] [PATCH 0/6] dcache/namei fixes for lustre

[NeilBrown]

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

On Tue, Oct 24 2017, James Simmons wrote:

>> >> This series is a revised version of two patches I sent
>> >> previously (one of which was sadly broken).
>> >> That patch has been broken into multiple parts for easy
>> >> review.  The other is included unchanged as the last of
>> >> this series.
>> >> 
>> >> I was drawn to look at this code due to the tests on
>> >> DCACHE_DISCONNECTED which are often wrong, and it turns out
>> >> they are used wrongly in lustre too.  Fixing one led to some
>> >> clean-up.  Fixing the other is straight forward.
>> >> 
>> >> A particular change here from the previous posting is
>> >> the first patch which tests for DCACHE_PAR_LOOKUP in ll_dcompare().
>> >> Without this patch, two threads can be looking up the same
>> >> name in a given directory in parallel.  This parallelism lead
>> >> to my concerns about needing improved locking in ll_splice_alias().
>> >> Instead of improving the locking, I now avoid the need for it
>> >> by fixing ll_dcompare.
>> >> 
>> >> This code passes basic "smoke tests".
>> >> 
>> >> Note that the cast to "struct dentry *" in the first patch is because
>> >> we have a "const struct dentry *" but d_in_lookup() requires a
>> >> pointer to a non-const structure.  I'll send a separate patch to
>> >> change d_in_lookup().
>> >
>> > To let you know this patch has been under going testing and we have a
>> > ticket open to track the progess:
>> >
>> > https://jira.hpdd.intel.com/browse/LU-9868
>> >
>> > Your patch did reveal that a piece of a fix landed earlier is missing :-(
>> > So currently the client can oops. I will send the fix shortly but this
>> > work will have to rebased after. As soon as we can get some cycles we will
>> > figure out what is going on. Thanks for helping out.
>> 
>> Hi,
>>  what happened about this?  I had a look around the ticket and couldn't
>>  find anything about an oops.  If there is still a problem I'd be very
>>  happy to help work out what it is - but I don't know where to look.
>
> The oops is specific to the in kernel client. Some where along the way the
> calls to ll_d_init() were removed from ll_splice_alias(). It was unnoticed
> until your patch came along. I do have a fix that I will be pushing to 
> the next staging tree very shortly.

ll_d_init() doesn't need to be called from anywhere.  It is called by
__d_alloc (dentry->d_op->d_init) whenever a dentry is allocated.  That
is all that is needed.

>
> I have been testing the patch series and for me I don't see any issue. Our 
> test suite is reporting failures with this patch which I'm attempting to 
> figure out how to reproduce locally on my test system. Once I have a 
> reproducer I can send it to you. 

Can I see the failure report?  Or the oops?

I cannot find anything at the jira.hpdd.intel.com link you gave, or the
review.whamcloud.com that is linked from there.
Maybe it is behind testing.hpdd.intel.com that I need a login for (I've
registered and am waiting) ....


Thanks,
NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]