linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] NFS: switch back to ->iterate()
@ 2016-12-09 13:41 Benjamin Coddington
  2016-12-15 22:40 ` Trond Myklebust
  0 siblings, 1 reply; 3+ messages in thread
From: Benjamin Coddington @ 2016-12-09 13:41 UTC (permalink / raw)
  To: Alexander Viro, Trond Myklebust, Anna Schumaker
  Cc: linux-nfs, linux-fsdevel, linux-kernel

NFS has some optimizations for readdir to choose between using READDIR or
READDIRPLUS based on workload, and which NFS operation to use is determined
by subsequent interactions with lookup, d_revalidate, and getattr.

Concurrent use of nfs_readdir() via ->iterate_shared() can cause those
optimizations to repeatedly invalidate the pagecache used to store
directory entries during readdir(), which causes some very bad performance
for directories with many entries (more than about 10000).

There's a couple ways to fix this in NFS, but no fix would be as simple as
going back to ->iterate() to serialize nfs_readdir(), and neither fix I
tested performed as well as going back to ->iterate().

The first required taking the directory's i_lock for each entry, with the
result of terrible contention.

The second way adds another flag to the nfs_inode, and so keeps the
optimizations working for large directories.  The difference from using
->iterate() here is that much more memory is consumed for a given workload
without any performance gain.

The workings of nfs_readdir() are such that concurrent users are serialized
within read_cache_page() waiting to retrieve pages of entries from the
server.  By serializing this work in iterate_dir() instead, contention for
cache pages is reduced.  Waiting processes can have an uncontended pass at
the entirety of the directory's pagecache once previous processes have
completed filling it.

This reverts commit 9ac3d3e8460e ("nfs: switch to ->iterate_shared()")

Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
---
 fs/nfs/dir.c | 71 ++++++++++++++++++++++++------------------------------------
 1 file changed, 28 insertions(+), 43 deletions(-)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 7483722162fa..1905f3af5449 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -57,7 +57,7 @@ static void nfs_readdir_clear_array(struct page*);
 const struct file_operations nfs_dir_operations = {
 	.llseek		= nfs_llseek_dir,
 	.read		= generic_read_dir,
-	.iterate_shared	= nfs_readdir,
+	.iterate	= nfs_readdir,
 	.open		= nfs_opendir,
 	.release	= nfs_closedir,
 	.fsync		= nfs_fsync_dir,
@@ -145,7 +145,6 @@ struct nfs_cache_array_entry {
 };
 
 struct nfs_cache_array {
-	atomic_t refcount;
 	int size;
 	int eof_index;
 	u64 last_cookie;
@@ -201,20 +200,11 @@ void nfs_readdir_clear_array(struct page *page)
 	int i;
 
 	array = kmap_atomic(page);
-	if (atomic_dec_and_test(&array->refcount))
-		for (i = 0; i < array->size; i++)
-			kfree(array->array[i].string.name);
+	for (i = 0; i < array->size; i++)
+		kfree(array->array[i].string.name);
 	kunmap_atomic(array);
 }
 
-static bool grab_page(struct page *page)
-{
-	struct nfs_cache_array *array = kmap_atomic(page);
-	bool res = atomic_inc_not_zero(&array->refcount);
-	kunmap_atomic(array);
-	return res;
-}
-
 /*
  * the caller is responsible for freeing qstr.name
  * when called by nfs_readdir_add_to_array, the strings will be freed in
@@ -491,7 +481,6 @@ static
 void nfs_prime_dcache(struct dentry *parent, struct nfs_entry *entry)
 {
 	struct qstr filename = QSTR_INIT(entry->name, entry->len);
-	DECLARE_WAIT_QUEUE_HEAD_ONSTACK(wq);
 	struct dentry *dentry;
 	struct dentry *alias;
 	struct inode *dir = d_inode(parent);
@@ -519,13 +508,7 @@ void nfs_prime_dcache(struct dentry *parent, struct nfs_entry *entry)
 	filename.hash = full_name_hash(parent, filename.name, filename.len);
 
 	dentry = d_lookup(parent, &filename);
-again:
-	if (!dentry) {
-		dentry = d_alloc_parallel(parent, &filename, &wq);
-		if (IS_ERR(dentry))
-			return;
-	}
-	if (!d_in_lookup(dentry)) {
+	if (dentry != NULL) {
 		/* Is there a mountpoint here? If so, just exit */
 		if (!nfs_fsid_equal(&NFS_SB(dentry->d_sb)->fsid,
 					&entry->fattr->fsid))
@@ -541,8 +524,6 @@ void nfs_prime_dcache(struct dentry *parent, struct nfs_entry *entry)
 		} else {
 			d_invalidate(dentry);
 			dput(dentry);
-			dentry = NULL;
-			goto again;
 		}
 	}
 	if (!entry->fh->size) {
@@ -550,16 +531,23 @@ void nfs_prime_dcache(struct dentry *parent, struct nfs_entry *entry)
 		goto out;
 	}
 
+	dentry = d_alloc(parent, &filename);
+	if (dentry == NULL)
+		return;
+
 	inode = nfs_fhget(dentry->d_sb, entry->fh, entry->fattr, entry->label);
+	if (IS_ERR(inode))
+		goto out;
+
 	alias = d_splice_alias(inode, dentry);
-	d_lookup_done(dentry);
-	if (alias) {
-		if (IS_ERR(alias))
-			goto out;
-		dput(dentry);
-		dentry = alias;
-	}
-	nfs_set_verifier(dentry, nfs_save_change_attribute(dir));
+	if (IS_ERR(alias))
+		goto out;
+	else if (alias) {
+		nfs_set_verifier(alias, nfs_save_change_attribute(dir));
+		dput(alias);
+	} else
+		nfs_set_verifier(dentry, nfs_save_change_attribute(dir));
+
 out:
 	dput(dentry);
 }
@@ -680,7 +668,6 @@ int nfs_readdir_xdr_to_array(nfs_readdir_descriptor_t *desc, struct page *page,
 		goto out_label_free;
 	}
 	memset(array, 0, sizeof(struct nfs_cache_array));
-	atomic_set(&array->refcount, 1);
 	array->eof_index = -1;
 
 	status = nfs_readdir_alloc_pages(pages, array_size);
@@ -743,7 +730,8 @@ int nfs_readdir_filler(nfs_readdir_descriptor_t *desc, struct page* page)
 static
 void cache_page_release(nfs_readdir_descriptor_t *desc)
 {
-	nfs_readdir_clear_array(desc->page);
+	if (!desc->page->mapping)
+		nfs_readdir_clear_array(desc->page);
 	put_page(desc->page);
 	desc->page = NULL;
 }
@@ -751,16 +739,8 @@ void cache_page_release(nfs_readdir_descriptor_t *desc)
 static
 struct page *get_cache_page(nfs_readdir_descriptor_t *desc)
 {
-	struct page *page;
-
-	for (;;) {
-		page = read_cache_page(desc->file->f_mapping,
+	return read_cache_page(desc->file->f_mapping,
 			desc->page_index, (filler_t *)nfs_readdir_filler, desc);
-		if (IS_ERR(page) || grab_page(page))
-			break;
-		put_page(page);
-	}
-	return page;
 }
 
 /*
@@ -966,11 +946,13 @@ static int nfs_readdir(struct file *file, struct dir_context *ctx)
 
 static loff_t nfs_llseek_dir(struct file *filp, loff_t offset, int whence)
 {
+	struct inode *inode = file_inode(filp);
 	struct nfs_open_dir_context *dir_ctx = filp->private_data;
 
 	dfprintk(FILE, "NFS: llseek dir(%pD2, %lld, %d)\n",
 			filp, offset, whence);
 
+	inode_lock(inode);
 	switch (whence) {
 		case 1:
 			offset += filp->f_pos;
@@ -978,13 +960,16 @@ static loff_t nfs_llseek_dir(struct file *filp, loff_t offset, int whence)
 			if (offset >= 0)
 				break;
 		default:
-			return -EINVAL;
+			offset = -EINVAL;
+			goto out;
 	}
 	if (offset != filp->f_pos) {
 		filp->f_pos = offset;
 		dir_ctx->dir_cookie = 0;
 		dir_ctx->duped = 0;
 	}
+out:
+	inode_unlock(inode);
 	return offset;
 }
 
-- 
2.9.3


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

* Re: [PATCH] NFS: switch back to ->iterate()
  2016-12-09 13:41 [PATCH] NFS: switch back to ->iterate() Benjamin Coddington
@ 2016-12-15 22:40 ` Trond Myklebust
  2017-01-05 16:21   ` Benjamin Coddington
  0 siblings, 1 reply; 3+ messages in thread
From: Trond Myklebust @ 2016-12-15 22:40 UTC (permalink / raw)
  To: Benjamin Coddington
  Cc: Alexander Viro, Anna Schumaker, Linux NFS Mailing List,
	Linux FS-devel Mailing List, Linux Kernel Mailing List

DQo+IE9uIERlYyA5LCAyMDE2LCBhdCAwODo0MSwgQmVuamFtaW4gQ29kZGluZ3RvbiA8YmNvZGRp
bmdAcmVkaGF0LmNvbT4gd3JvdGU6DQo+IA0KPiBORlMgaGFzIHNvbWUgb3B0aW1pemF0aW9ucyBm
b3IgcmVhZGRpciB0byBjaG9vc2UgYmV0d2VlbiB1c2luZyBSRUFERElSIG9yDQo+IFJFQURESVJQ
TFVTIGJhc2VkIG9uIHdvcmtsb2FkLCBhbmQgd2hpY2ggTkZTIG9wZXJhdGlvbiB0byB1c2UgaXMg
ZGV0ZXJtaW5lZA0KPiBieSBzdWJzZXF1ZW50IGludGVyYWN0aW9ucyB3aXRoIGxvb2t1cCwgZF9y
ZXZhbGlkYXRlLCBhbmQgZ2V0YXR0ci4NCj4gDQo+IENvbmN1cnJlbnQgdXNlIG9mIG5mc19yZWFk
ZGlyKCkgdmlhIC0+aXRlcmF0ZV9zaGFyZWQoKSBjYW4gY2F1c2UgdGhvc2UNCj4gb3B0aW1pemF0
aW9ucyB0byByZXBlYXRlZGx5IGludmFsaWRhdGUgdGhlIHBhZ2VjYWNoZSB1c2VkIHRvIHN0b3Jl
DQo+IGRpcmVjdG9yeSBlbnRyaWVzIGR1cmluZyByZWFkZGlyKCksIHdoaWNoIGNhdXNlcyBzb21l
IHZlcnkgYmFkIHBlcmZvcm1hbmNlDQo+IGZvciBkaXJlY3RvcmllcyB3aXRoIG1hbnkgZW50cmll
cyAobW9yZSB0aGFuIGFib3V0IDEwMDAwKS4NCj4gDQo+IFRoZXJlJ3MgYSBjb3VwbGUgd2F5cyB0
byBmaXggdGhpcyBpbiBORlMsIGJ1dCBubyBmaXggd291bGQgYmUgYXMgc2ltcGxlIGFzDQo+IGdv
aW5nIGJhY2sgdG8gLT5pdGVyYXRlKCkgdG8gc2VyaWFsaXplIG5mc19yZWFkZGlyKCksIGFuZCBu
ZWl0aGVyIGZpeCBJDQo+IHRlc3RlZCBwZXJmb3JtZWQgYXMgd2VsbCBhcyBnb2luZyBiYWNrIHRv
IC0+aXRlcmF0ZSgpLg0KPiANCj4gVGhlIGZpcnN0IHJlcXVpcmVkIHRha2luZyB0aGUgZGlyZWN0
b3J5J3MgaV9sb2NrIGZvciBlYWNoIGVudHJ5LCB3aXRoIHRoZQ0KPiByZXN1bHQgb2YgdGVycmli
bGUgY29udGVudGlvbi4NCj4gDQo+IFRoZSBzZWNvbmQgd2F5IGFkZHMgYW5vdGhlciBmbGFnIHRv
IHRoZSBuZnNfaW5vZGUsIGFuZCBzbyBrZWVwcyB0aGUNCj4gb3B0aW1pemF0aW9ucyB3b3JraW5n
IGZvciBsYXJnZSBkaXJlY3Rvcmllcy4gIFRoZSBkaWZmZXJlbmNlIGZyb20gdXNpbmcNCj4gLT5p
dGVyYXRlKCkgaGVyZSBpcyB0aGF0IG11Y2ggbW9yZSBtZW1vcnkgaXMgY29uc3VtZWQgZm9yIGEg
Z2l2ZW4gd29ya2xvYWQNCj4gd2l0aG91dCBhbnkgcGVyZm9ybWFuY2UgZ2Fpbi4NCj4gDQo+IFRo
ZSB3b3JraW5ncyBvZiBuZnNfcmVhZGRpcigpIGFyZSBzdWNoIHRoYXQgY29uY3VycmVudCB1c2Vy
cyBhcmUgc2VyaWFsaXplZA0KPiB3aXRoaW4gcmVhZF9jYWNoZV9wYWdlKCkgd2FpdGluZyB0byBy
ZXRyaWV2ZSBwYWdlcyBvZiBlbnRyaWVzIGZyb20gdGhlDQo+IHNlcnZlci4gIEJ5IHNlcmlhbGl6
aW5nIHRoaXMgd29yayBpbiBpdGVyYXRlX2RpcigpIGluc3RlYWQsIGNvbnRlbnRpb24gZm9yDQo+
IGNhY2hlIHBhZ2VzIGlzIHJlZHVjZWQuICBXYWl0aW5nIHByb2Nlc3NlcyBjYW4gaGF2ZSBhbiB1
bmNvbnRlbmRlZCBwYXNzIGF0DQo+IHRoZSBlbnRpcmV0eSBvZiB0aGUgZGlyZWN0b3J5J3MgcGFn
ZWNhY2hlIG9uY2UgcHJldmlvdXMgcHJvY2Vzc2VzIGhhdmUNCj4gY29tcGxldGVkIGZpbGxpbmcg
aXQuDQo+IA0KPiBUaGlzIHJldmVydHMgY29tbWl0IDlhYzNkM2U4NDYwZSAoIm5mczogc3dpdGNo
IHRvIC0+aXRlcmF0ZV9zaGFyZWQoKSIpDQo+IA0KPiBTaWduZWQtb2ZmLWJ5OiBCZW5qYW1pbiBD
b2RkaW5ndG9uIDxiY29kZGluZ0ByZWRoYXQuY29tPg0KPiAtLS0NCj4gZnMvbmZzL2Rpci5jIHwg
NzEgKysrKysrKysrKysrKysrKysrKysrKysrLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0tDQo+IDEgZmlsZSBjaGFuZ2VkLCAyOCBpbnNlcnRpb25zKCspLCA0MyBkZWxldGlvbnMo
LSkNCj4gDQo+IGRpZmYgLS1naXQgYS9mcy9uZnMvZGlyLmMgYi9mcy9uZnMvZGlyLmMNCj4gaW5k
ZXggNzQ4MzcyMjE2MmZhLi4xOTA1ZjNhZjU0NDkgMTAwNjQ0DQo+IC0tLSBhL2ZzL25mcy9kaXIu
Yw0KPiArKysgYi9mcy9uZnMvZGlyLmMNCj4gQEAgLTU3LDcgKzU3LDcgQEAgc3RhdGljIHZvaWQg
bmZzX3JlYWRkaXJfY2xlYXJfYXJyYXkoc3RydWN0IHBhZ2UqKTsNCj4gY29uc3Qgc3RydWN0IGZp
bGVfb3BlcmF0aW9ucyBuZnNfZGlyX29wZXJhdGlvbnMgPSB7DQo+IAkubGxzZWVrCQk9IG5mc19s
bHNlZWtfZGlyLA0KPiAJLnJlYWQJCT0gZ2VuZXJpY19yZWFkX2RpciwNCj4gLQkuaXRlcmF0ZV9z
aGFyZWQJPSBuZnNfcmVhZGRpciwNCj4gKwkuaXRlcmF0ZQk9IG5mc19yZWFkZGlyLA0KPiAJLm9w
ZW4JCT0gbmZzX29wZW5kaXIsDQo+IAkucmVsZWFzZQk9IG5mc19jbG9zZWRpciwNCj4gCS5mc3lu
YwkJPSBuZnNfZnN5bmNfZGlyLA0KPiBAQCAtMTQ1LDcgKzE0NSw2IEBAIHN0cnVjdCBuZnNfY2Fj
aGVfYXJyYXlfZW50cnkgew0KPiB9Ow0KPiANCj4gc3RydWN0IG5mc19jYWNoZV9hcnJheSB7DQo+
IC0JYXRvbWljX3QgcmVmY291bnQ7DQo+IAlpbnQgc2l6ZTsNCj4gCWludCBlb2ZfaW5kZXg7DQo+
IAl1NjQgbGFzdF9jb29raWU7DQo+IEBAIC0yMDEsMjAgKzIwMCwxMSBAQCB2b2lkIG5mc19yZWFk
ZGlyX2NsZWFyX2FycmF5KHN0cnVjdCBwYWdlICpwYWdlKQ0KPiAJaW50IGk7DQo+IA0KPiAJYXJy
YXkgPSBrbWFwX2F0b21pYyhwYWdlKTsNCj4gLQlpZiAoYXRvbWljX2RlY19hbmRfdGVzdCgmYXJy
YXktPnJlZmNvdW50KSkNCj4gLQkJZm9yIChpID0gMDsgaSA8IGFycmF5LT5zaXplOyBpKyspDQo+
IC0JCQlrZnJlZShhcnJheS0+YXJyYXlbaV0uc3RyaW5nLm5hbWUpOw0KPiArCWZvciAoaSA9IDA7
IGkgPCBhcnJheS0+c2l6ZTsgaSsrKQ0KPiArCQlrZnJlZShhcnJheS0+YXJyYXlbaV0uc3RyaW5n
Lm5hbWUpOw0KPiAJa3VubWFwX2F0b21pYyhhcnJheSk7DQo+IH0NCj4gDQo+IC1zdGF0aWMgYm9v
bCBncmFiX3BhZ2Uoc3RydWN0IHBhZ2UgKnBhZ2UpDQo+IC17DQo+IC0Jc3RydWN0IG5mc19jYWNo
ZV9hcnJheSAqYXJyYXkgPSBrbWFwX2F0b21pYyhwYWdlKTsNCj4gLQlib29sIHJlcyA9IGF0b21p
Y19pbmNfbm90X3plcm8oJmFycmF5LT5yZWZjb3VudCk7DQo+IC0Ja3VubWFwX2F0b21pYyhhcnJh
eSk7DQo+IC0JcmV0dXJuIHJlczsNCj4gLX0NCj4gLQ0KPiAvKg0KPiAgKiB0aGUgY2FsbGVyIGlz
IHJlc3BvbnNpYmxlIGZvciBmcmVlaW5nIHFzdHIubmFtZQ0KPiAgKiB3aGVuIGNhbGxlZCBieSBu
ZnNfcmVhZGRpcl9hZGRfdG9fYXJyYXksIHRoZSBzdHJpbmdzIHdpbGwgYmUgZnJlZWQgaW4NCj4g
QEAgLTQ5MSw3ICs0ODEsNiBAQCBzdGF0aWMNCj4gdm9pZCBuZnNfcHJpbWVfZGNhY2hlKHN0cnVj
dCBkZW50cnkgKnBhcmVudCwgc3RydWN0IG5mc19lbnRyeSAqZW50cnkpDQo+IHsNCj4gCXN0cnVj
dCBxc3RyIGZpbGVuYW1lID0gUVNUUl9JTklUKGVudHJ5LT5uYW1lLCBlbnRyeS0+bGVuKTsNCj4g
LQlERUNMQVJFX1dBSVRfUVVFVUVfSEVBRF9PTlNUQUNLKHdxKTsNCj4gCXN0cnVjdCBkZW50cnkg
KmRlbnRyeTsNCj4gCXN0cnVjdCBkZW50cnkgKmFsaWFzOw0KPiAJc3RydWN0IGlub2RlICpkaXIg
PSBkX2lub2RlKHBhcmVudCk7DQo+IEBAIC01MTksMTMgKzUwOCw3IEBAIHZvaWQgbmZzX3ByaW1l
X2RjYWNoZShzdHJ1Y3QgZGVudHJ5ICpwYXJlbnQsIHN0cnVjdCBuZnNfZW50cnkgKmVudHJ5KQ0K
PiAJZmlsZW5hbWUuaGFzaCA9IGZ1bGxfbmFtZV9oYXNoKHBhcmVudCwgZmlsZW5hbWUubmFtZSwg
ZmlsZW5hbWUubGVuKTsNCj4gDQo+IAlkZW50cnkgPSBkX2xvb2t1cChwYXJlbnQsICZmaWxlbmFt
ZSk7DQo+IC1hZ2FpbjoNCj4gLQlpZiAoIWRlbnRyeSkgew0KPiAtCQlkZW50cnkgPSBkX2FsbG9j
X3BhcmFsbGVsKHBhcmVudCwgJmZpbGVuYW1lLCAmd3EpOw0KPiAtCQlpZiAoSVNfRVJSKGRlbnRy
eSkpDQo+IC0JCQlyZXR1cm47DQo+IC0JfQ0KPiAtCWlmICghZF9pbl9sb29rdXAoZGVudHJ5KSkg
ew0KPiArCWlmIChkZW50cnkgIT0gTlVMTCkgew0KDQpUaGlzIGFsbCBsb29rcyBsaWtlIGl0IGlz
IHJldmVydGluZyB0byB1c2luZyBhbiBvYnNvbGV0ZSBWRlMgQVBJLiBJ4oCZZCBwcmVmZXIgYW4g
QUNLIGZyb20gQWwgYXMgdG8gd2hldGhlciBvciBub3QgdGhpcyBpcyBhbGxvd2VkLiBQbGVhc2Ug
bm90ZSB0aGF0IHRoZSByZXN0IG9mIHRoZSBsb29rdXAgY29kZSBpcyBzdGlsbCBwYXJhbGxlbGlz
ZWQuDQoNCj4gCQkvKiBJcyB0aGVyZSBhIG1vdW50cG9pbnQgaGVyZT8gSWYgc28sIGp1c3QgZXhp
dCAqLw0KPiAJCWlmICghbmZzX2ZzaWRfZXF1YWwoJk5GU19TQihkZW50cnktPmRfc2IpLT5mc2lk
LA0KPiAJCQkJCSZlbnRyeS0+ZmF0dHItPmZzaWQpKQ0KPiBAQCAtNTQxLDggKzUyNCw2IEBAIHZv
aWQgbmZzX3ByaW1lX2RjYWNoZShzdHJ1Y3QgZGVudHJ5ICpwYXJlbnQsIHN0cnVjdCBuZnNfZW50
cnkgKmVudHJ5KQ0KPiAJCX0gZWxzZSB7DQo+IAkJCWRfaW52YWxpZGF0ZShkZW50cnkpOw0KPiAJ
CQlkcHV0KGRlbnRyeSk7DQo+IC0JCQlkZW50cnkgPSBOVUxMOw0KPiAtCQkJZ290byBhZ2FpbjsN
Cj4gCQl9DQo+IAl9DQo+IAlpZiAoIWVudHJ5LT5maC0+c2l6ZSkgew0KPiBAQCAtNTUwLDE2ICs1
MzEsMjMgQEAgdm9pZCBuZnNfcHJpbWVfZGNhY2hlKHN0cnVjdCBkZW50cnkgKnBhcmVudCwgc3Ry
dWN0IG5mc19lbnRyeSAqZW50cnkpDQo+IAkJZ290byBvdXQ7DQo+IAl9DQo+IA0KPiArCWRlbnRy
eSA9IGRfYWxsb2MocGFyZW50LCAmZmlsZW5hbWUpOw0KPiArCWlmIChkZW50cnkgPT0gTlVMTCkN
Cj4gKwkJcmV0dXJuOw0KPiArDQo+IAlpbm9kZSA9IG5mc19maGdldChkZW50cnktPmRfc2IsIGVu
dHJ5LT5maCwgZW50cnktPmZhdHRyLCBlbnRyeS0+bGFiZWwpOw0KPiArCWlmIChJU19FUlIoaW5v
ZGUpKQ0KPiArCQlnb3RvIG91dDsNCj4gKw0KPiAJYWxpYXMgPSBkX3NwbGljZV9hbGlhcyhpbm9k
ZSwgZGVudHJ5KTsNCj4gLQlkX2xvb2t1cF9kb25lKGRlbnRyeSk7DQo+IC0JaWYgKGFsaWFzKSB7
DQo+IC0JCWlmIChJU19FUlIoYWxpYXMpKQ0KPiAtCQkJZ290byBvdXQ7DQo+IC0JCWRwdXQoZGVu
dHJ5KTsNCj4gLQkJZGVudHJ5ID0gYWxpYXM7DQo+IC0JfQ0KPiAtCW5mc19zZXRfdmVyaWZpZXIo
ZGVudHJ5LCBuZnNfc2F2ZV9jaGFuZ2VfYXR0cmlidXRlKGRpcikpOw0KPiArCWlmIChJU19FUlIo
YWxpYXMpKQ0KPiArCQlnb3RvIG91dDsNCj4gKwllbHNlIGlmIChhbGlhcykgew0KPiArCQluZnNf
c2V0X3ZlcmlmaWVyKGFsaWFzLCBuZnNfc2F2ZV9jaGFuZ2VfYXR0cmlidXRlKGRpcikpOw0KPiAr
CQlkcHV0KGFsaWFzKTsNCj4gKwl9IGVsc2UNCj4gKwkJbmZzX3NldF92ZXJpZmllcihkZW50cnks
IG5mc19zYXZlX2NoYW5nZV9hdHRyaWJ1dGUoZGlyKSk7DQo+ICsNCj4gb3V0Og0KPiAJZHB1dChk
ZW50cnkpOw0KPiB9DQo+IEBAIC02ODAsNyArNjY4LDYgQEAgaW50IG5mc19yZWFkZGlyX3hkcl90
b19hcnJheShuZnNfcmVhZGRpcl9kZXNjcmlwdG9yX3QgKmRlc2MsIHN0cnVjdCBwYWdlICpwYWdl
LA0KPiAJCWdvdG8gb3V0X2xhYmVsX2ZyZWU7DQo+IAl9DQo+IAltZW1zZXQoYXJyYXksIDAsIHNp
emVvZihzdHJ1Y3QgbmZzX2NhY2hlX2FycmF5KSk7DQo+IC0JYXRvbWljX3NldCgmYXJyYXktPnJl
ZmNvdW50LCAxKTsNCj4gCWFycmF5LT5lb2ZfaW5kZXggPSAtMTsNCj4gDQo+IAlzdGF0dXMgPSBu
ZnNfcmVhZGRpcl9hbGxvY19wYWdlcyhwYWdlcywgYXJyYXlfc2l6ZSk7DQo+IEBAIC03NDMsNyAr
NzMwLDggQEAgaW50IG5mc19yZWFkZGlyX2ZpbGxlcihuZnNfcmVhZGRpcl9kZXNjcmlwdG9yX3Qg
KmRlc2MsIHN0cnVjdCBwYWdlKiBwYWdlKQ0KPiBzdGF0aWMNCj4gdm9pZCBjYWNoZV9wYWdlX3Jl
bGVhc2UobmZzX3JlYWRkaXJfZGVzY3JpcHRvcl90ICpkZXNjKQ0KPiB7DQo+IC0JbmZzX3JlYWRk
aXJfY2xlYXJfYXJyYXkoZGVzYy0+cGFnZSk7DQo+ICsJaWYgKCFkZXNjLT5wYWdlLT5tYXBwaW5n
KQ0KPiArCQluZnNfcmVhZGRpcl9jbGVhcl9hcnJheShkZXNjLT5wYWdlKTsNCj4gCXB1dF9wYWdl
KGRlc2MtPnBhZ2UpOw0KPiAJZGVzYy0+cGFnZSA9IE5VTEw7DQo+IH0NCj4gQEAgLTc1MSwxNiAr
NzM5LDggQEAgdm9pZCBjYWNoZV9wYWdlX3JlbGVhc2UobmZzX3JlYWRkaXJfZGVzY3JpcHRvcl90
ICpkZXNjKQ0KPiBzdGF0aWMNCj4gc3RydWN0IHBhZ2UgKmdldF9jYWNoZV9wYWdlKG5mc19yZWFk
ZGlyX2Rlc2NyaXB0b3JfdCAqZGVzYykNCj4gew0KPiAtCXN0cnVjdCBwYWdlICpwYWdlOw0KPiAt
DQo+IC0JZm9yICg7Oykgew0KPiAtCQlwYWdlID0gcmVhZF9jYWNoZV9wYWdlKGRlc2MtPmZpbGUt
PmZfbWFwcGluZywNCj4gKwlyZXR1cm4gcmVhZF9jYWNoZV9wYWdlKGRlc2MtPmZpbGUtPmZfbWFw
cGluZywNCj4gCQkJZGVzYy0+cGFnZV9pbmRleCwgKGZpbGxlcl90ICopbmZzX3JlYWRkaXJfZmls
bGVyLCBkZXNjKTsNCj4gLQkJaWYgKElTX0VSUihwYWdlKSB8fCBncmFiX3BhZ2UocGFnZSkpDQo+
IC0JCQlicmVhazsNCj4gLQkJcHV0X3BhZ2UocGFnZSk7DQo+IC0JfQ0KPiAtCXJldHVybiBwYWdl
Ow0KPiB9DQo+IA0KPiAvKg0KPiBAQCAtOTY2LDExICs5NDYsMTMgQEAgc3RhdGljIGludCBuZnNf
cmVhZGRpcihzdHJ1Y3QgZmlsZSAqZmlsZSwgc3RydWN0IGRpcl9jb250ZXh0ICpjdHgpDQo+IA0K
PiBzdGF0aWMgbG9mZl90IG5mc19sbHNlZWtfZGlyKHN0cnVjdCBmaWxlICpmaWxwLCBsb2ZmX3Qg
b2Zmc2V0LCBpbnQgd2hlbmNlKQ0KPiB7DQo+ICsJc3RydWN0IGlub2RlICppbm9kZSA9IGZpbGVf
aW5vZGUoZmlscCk7DQo+IAlzdHJ1Y3QgbmZzX29wZW5fZGlyX2NvbnRleHQgKmRpcl9jdHggPSBm
aWxwLT5wcml2YXRlX2RhdGE7DQo+IA0KPiAJZGZwcmludGsoRklMRSwgIk5GUzogbGxzZWVrIGRp
ciglcEQyLCAlbGxkLCAlZClcbiIsDQo+IAkJCWZpbHAsIG9mZnNldCwgd2hlbmNlKTsNCj4gDQo+
ICsJaW5vZGVfbG9jayhpbm9kZSk7DQo+IAlzd2l0Y2ggKHdoZW5jZSkgew0KPiAJCWNhc2UgMToN
Cj4gCQkJb2Zmc2V0ICs9IGZpbHAtPmZfcG9zOw0KPiBAQCAtOTc4LDEzICs5NjAsMTYgQEAgc3Rh
dGljIGxvZmZfdCBuZnNfbGxzZWVrX2RpcihzdHJ1Y3QgZmlsZSAqZmlscCwgbG9mZl90IG9mZnNl
dCwgaW50IHdoZW5jZSkNCj4gCQkJaWYgKG9mZnNldCA+PSAwKQ0KPiAJCQkJYnJlYWs7DQo+IAkJ
ZGVmYXVsdDoNCj4gLQkJCXJldHVybiAtRUlOVkFMOw0KPiArCQkJb2Zmc2V0ID0gLUVJTlZBTDsN
Cj4gKwkJCWdvdG8gb3V0Ow0KPiAJfQ0KPiAJaWYgKG9mZnNldCAhPSBmaWxwLT5mX3Bvcykgew0K
PiAJCWZpbHAtPmZfcG9zID0gb2Zmc2V0Ow0KPiAJCWRpcl9jdHgtPmRpcl9jb29raWUgPSAwOw0K
PiAJCWRpcl9jdHgtPmR1cGVkID0gMDsNCj4gCX0NCj4gK291dDoNCj4gKwlpbm9kZV91bmxvY2so
aW5vZGUpOw0KPiAJcmV0dXJuIG9mZnNldDsNCj4gfQ0KPiANCj4gLS0gDQo+IDIuOS4zDQo+IA0K
DQo=

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

* Re: [PATCH] NFS: switch back to ->iterate()
  2016-12-15 22:40 ` Trond Myklebust
@ 2017-01-05 16:21   ` Benjamin Coddington
  0 siblings, 0 replies; 3+ messages in thread
From: Benjamin Coddington @ 2017-01-05 16:21 UTC (permalink / raw)
  To: Alexander Viro
  Cc: Anna Schumaker, Trond Myklebust, Linux NFS Mailing List,
	Linux FS-devel Mailing List, Linux Kernel Mailing List


On 15 Dec 2016, at 17:40, Trond Myklebust wrote:

>> On Dec 9, 2016, at 08:41, Benjamin Coddington <bcodding@redhat.com> 
>> wrote:
>> @@ -519,13 +508,7 @@ void nfs_prime_dcache(struct dentry *parent, 
>> struct nfs_entry *entry)
>> 	filename.hash = full_name_hash(parent, filename.name, filename.len);
>>
>> 	dentry = d_lookup(parent, &filename);
>> -again:
>> -	if (!dentry) {
>> -		dentry = d_alloc_parallel(parent, &filename, &wq);
>> -		if (IS_ERR(dentry))
>> -			return;
>> -	}
>> -	if (!d_in_lookup(dentry)) {
>> +	if (dentry != NULL) {
>
> This all looks like it is reverting to using an obsolete VFS API. 
> I’d
> prefer an ACK from Al as to whether or not this is allowed. Please 
> note
> that the rest of the lookup code is still parallelised.

I should've made sure the revert wasn't going to jump back to older VFS
usage.  I'll go back over this to make sure that's not the case.

Al, are you hoping to get rid of ->iterate completely?  If so, better to
work on this another way.

Ben

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

end of thread, other threads:[~2017-01-05 16:21 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-09 13:41 [PATCH] NFS: switch back to ->iterate() Benjamin Coddington
2016-12-15 22:40 ` Trond Myklebust
2017-01-05 16:21   ` Benjamin Coddington

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).