Mostly a Resend of ALL Bug fixes and SQUASHMEs - pnfs-submit 2.6.40 V7

PATCH In review Boaz Harrosh 2 participants 22 messages May 24, 2011

Subsystems (2)

NFS client NFS server

lore

Share & tools

lore

Boaz Harrosh

author

2011-05-24 14:52 UTC #0 lore raw

cover Mostly a Resend of ALL Bug fixes and SQUASHMEs - pnfs-submit 2.6.40 V7

As promised I'm resending all accumulated fixes from Benny's last release. They have been produced in the below order, each fix right after the patch that it needs to squash into.

* I have split those SQUASHMEs that belong to different patches.
* I have reordered some of benny's patchset and also split one patch into two. A patch that had both generic and objects code in it. I will resend also these patches the one that got split and a one that was reordered. Please see below.

Benny, Trond If you need to I can send the all ready-made patchset on call. It is right here waiting. If any body needs it. (Just didn't want to spam the list.

Please don't forget to throw away all the previous, fixes I sent this includes everything cleaned up.

It is all based on Benny's: [PATCHSET v6 0/26] pnfs for 2.6.40

Here below is the new patchset order interleaved with the patches I'm resending. Please apply them in that order and SQUASH

These patches that are indented in are left the same as before:

#pick 87482a8 NFSv4.1: use struct nfs_client to qualify deviceid #pick 21a93b1 pnfs: resolve header dependency in pnfs.h #pick d322d23 NFSv4.1: make deviceid cache global #pick e96d556 NFSv4.1: purge deviceid cache on nfs_free_client #pick 839fb76 pnfs: CB_NOTIFY_DEVICEID

pick b97ee18 NFSv4.1: use layout driver in global device cache - Resend
I'm resending this because I split out the objects code to the relevant patch below (I've put it here so to apply the fixes)

pick 898c6d6 SQUASHME: Bug in new global-device-cache code pick c04e8c7 SQUSHME: pnfs: BUG in _deviceid_purge_client
These two are a resend and a cleanup of the bug fixes I sent

pick c92b213 pnfs: layout_driver MUST set free_deviceid_node if using dev-cache
This one I also sent. It is not a SQUASHME it needs to be added here

#pick d1dc744 SUNRPC: introduce xdr_init_decode_pages #pick 020333f pnfs: Use byte-range for layoutget #pick 50a623c pnfs: align layoutget requests on page boundaries #pick 899e1e1 pnfs: Use byte-range for cb_layoutrecall #pick 24430f8 pnfs: client stats
#pick e005007 pnfs-obj: objlayoutdriver module skeleton #pick e679f1b pnfs-obj: pnfs_osd XDR definitions
pick d1fb331 SQUASHME: pnfs-obj: pnfs_osd_xdr.h Remove server definitions
Resend + more removed

#pick 98f3079 pnfs-obj: pnfs_osd XDR client implementation
pick a6d6418 SQUASHME: pnf-obj xdr_cli: Wrong type in comments
Resend

#pick c7b5197 pnfs-obj: decode layout, alloc/free lseg
pick 619d431 SQUASHME: objio alloc/free lseg Bugs fixes
Resend

#pick 756866c pnfs-obj: objio_osd device information retrieval and caching
pick 45f1e63 SQUASHME: pnfs-obj: use layout driver in global device cache pick b8b9d0e SQUASHME: pnfs-obj: Bugs in new global-device-cache code pick d2bff4d SQUASHME: pnfs-obj: objlayout wants to cache devices until unmount
All these where sent in different patches, here divided properly to apply cleanly to the original patch.

#pick 2f7beef pnfs: alloc and free layout_hdr layoutdriver methods #pick 603ceb3 pnfs-obj: define per-inode private structure #pick 608f381 pnfs: support for non-rpc layout drivers
pick 8be3eb4 SQUASHME: pnfs: Fall out from: non-rpc layout drivers
Resend

#pick 558f335 pnfs-obj: osd raid engine read/write implementation
pick c437fbb SQUASHME: objio read/write patch: Bugs fixes
Resend properly rebased this time

#pick db53862 pnfs: layoutreturn
#pick 6e323de pnfs: layoutret_on_setattr
#pick 385cc07 pnfs: encode_layoutreturn
#pick 8eb75c9 pnfs-obj: report errors and .encode_layoutreturn Implementation. #pick e5e510b pnfs: encode_layoutcommit
#pick bf2158f pnfs-obj: objlayout_encode_layoutcommit implementation

Boaz Harrosh

author

2011-05-24 15:04 UTC #1 lore raw

PATCH 1/12 NFSv4.1: use layout driver in global device cache

pnfs deviceids are unique per server, per layout type. struct nfs_client is currently used to distinguish deviceids from different nfs servers, yet these may clash between different layout types on the same server. Therefore, use the layout driver associated with each deviceid at insertion time to look it up, unhash, or delete it.

Signed-off-by Benny Halevy <[email protected]>

---
 fs/nfs/callback_proc.c  |    2 +-
 fs/nfs/nfs4filelayout.c |    3 ++-
 fs/nfs/pnfs.h           |    6 +++---
 fs/nfs/pnfs_dev.c       |   28 +++++++++++++++++-----------
 4 files changed, 23 insertions(+), 16 deletions(-)

fs/nfs/callback_proc.c +1 -1

index fb5e5b9..c73e7b2 100644
@@ -278,7 +278,7 @@ __be32 nfs4_callback_devicenotify(struct cb_devicenotifyargs *args,
 		if (dev->cbd_notify_type == NOTIFY_DEVICEID4_CHANGE)
 			dprintk("%s: NOTIFY_DEVICEID4_CHANGE not supported, "
 				"deleting instead\n", __func__);
-		nfs4_delete_deviceid(clp, &dev->cbd_dev_id);
+		nfs4_delete_deviceid(server->pnfs_curr_ld, clp, &dev->cbd_dev_id);
 	}
 
 out:

fs/nfs/nfs4filelayout.c +2 -1

index c181a8b..45866d0 100644
@@ -441,7 +441,8 @@ filelayout_check_layout(struct pnfs_layout_hdr *lo,
 	}
 
 	/* find and reference the deviceid */
-	d = nfs4_find_get_deviceid(NFS_SERVER(lo->plh_inode)->nfs_client, id);
+	d = nfs4_find_get_deviceid(NFS_SERVER(lo->plh_inode)->pnfs_curr_ld,
+				   NFS_SERVER(lo->plh_inode)->nfs_client, id);
 	if (d == NULL) {
 		dsaddr = get_device_info(lo->plh_inode, id, gfp_flags);
 		if (dsaddr == NULL)

fs/nfs/pnfs.h +3 -3

index 7f29e3a..b7f88d4 100644
@@ -171,9 +171,9 @@ struct nfs4_deviceid_node {
 };
 
 void nfs4_print_deviceid(const struct nfs4_deviceid *dev_id);
-struct nfs4_deviceid_node *nfs4_find_get_deviceid(const struct nfs_client *, const struct nfs4_deviceid *);
-struct nfs4_deviceid_node *nfs4_unhash_put_deviceid(const struct nfs_client *, const struct nfs4_deviceid *);
-void nfs4_delete_deviceid(const struct nfs_client *, const struct nfs4_deviceid *);
+struct nfs4_deviceid_node *nfs4_find_get_deviceid(const struct pnfs_layoutdriver_type *, const struct nfs_client *, const struct nfs4_deviceid *);
+struct nfs4_deviceid_node *nfs4_unhash_put_deviceid(const struct pnfs_layoutdriver_type *, const struct nfs_client *, const struct nfs4_deviceid *);
+void nfs4_delete_deviceid(const struct pnfs_layoutdriver_type *, const struct nfs_client *, const struct nfs4_deviceid *);
 void nfs4_init_deviceid_node(struct nfs4_deviceid_node *,
 			     const struct pnfs_layoutdriver_type *,
 			     const struct nfs_client *,

fs/nfs/pnfs_dev.c +17 ~~-11~~

index f830616..7997899 100644
@@ -67,14 +67,16 @@ nfs4_deviceid_hash(const struct nfs4_deviceid *id)
 }
 
 static struct nfs4_deviceid_node *
-_lookup_deviceid(const struct nfs_client *clp, const struct nfs4_deviceid *id,
+_lookup_deviceid(const struct pnfs_layoutdriver_type *ld,
+		 const struct nfs_client *clp, const struct nfs4_deviceid *id,
 		 long hash)
 {
 	struct nfs4_deviceid_node *d;
 	struct hlist_node *n;
 
 	hlist_for_each_entry_rcu(d, n, &nfs4_deviceid_cache[hash], node)
-		if (d->nfs_client == clp && !memcmp(&d->deviceid, id, sizeof(*id))) {
+		if (d->ld == ld && d->nfs_client == clp &&
+		    !memcmp(&d->deviceid, id, sizeof(*id))) {
 			if (atomic_read(&d->ref))
 				return d;
 			else

@@ -90,13 +92,14 @@ _lookup_deviceid(const struct nfs_client *clp, const struct nfs4_deviceid *id,
  * @id deviceid to look up
  */
 struct nfs4_deviceid_node *
-_find_get_deviceid(const struct nfs_client *clp, const struct nfs4_deviceid *id,
+_find_get_deviceid(const struct pnfs_layoutdriver_type *ld,
+		   const struct nfs_client *clp, const struct nfs4_deviceid *id,
 		   long hash)
 {
 	struct nfs4_deviceid_node *d;
 
 	rcu_read_lock();
-	d = _lookup_deviceid(clp, id, hash);
+	d = _lookup_deviceid(ld, clp, id, hash);
 	if (!atomic_inc_not_zero(&d->ref))
 		d = NULL;
 	rcu_read_unlock();

@@ -104,9 +107,10 @@ _find_get_deviceid(const struct nfs_client *clp, const struct nfs4_deviceid *id,
 }
 
 struct nfs4_deviceid_node *
-nfs4_find_get_deviceid(const struct nfs_client *clp, const struct nfs4_deviceid *id)
+nfs4_find_get_deviceid(const struct pnfs_layoutdriver_type *ld,
+		       const struct nfs_client *clp, const struct nfs4_deviceid *id)
 {
-	return _find_get_deviceid(clp, id, nfs4_deviceid_hash(id));
+	return _find_get_deviceid(ld, clp, id, nfs4_deviceid_hash(id));
 }
 EXPORT_SYMBOL_GPL(nfs4_find_get_deviceid);

@@ -119,13 +123,14 @@ EXPORT_SYMBOL_GPL(nfs4_find_get_deviceid);
  * @ret the unhashed node, if found and dereferenced to zero, NULL otherwise.
  */
 struct nfs4_deviceid_node *
-nfs4_unhash_put_deviceid(const struct nfs_client *clp, const struct nfs4_deviceid *id)
+nfs4_unhash_put_deviceid(const struct pnfs_layoutdriver_type *ld,
+			 const struct nfs_client *clp, const struct nfs4_deviceid *id)
 {
 	struct nfs4_deviceid_node *d;
 
 	spin_lock(&nfs4_deviceid_lock);
 	rcu_read_lock();
-	d = _lookup_deviceid(clp, id, nfs4_deviceid_hash(id));
+	d = _lookup_deviceid(ld, clp, id, nfs4_deviceid_hash(id));
 	rcu_read_unlock();
 	if (!d) {
 		spin_unlock(&nfs4_deviceid_lock);

@@ -150,11 +155,12 @@ EXPORT_SYMBOL_GPL(nfs4_unhash_put_deviceid);
  * @id deviceid to delete
  */
 void
-nfs4_delete_deviceid(const struct nfs_client *clp, const struct nfs4_deviceid *id)
+nfs4_delete_deviceid(const struct pnfs_layoutdriver_type *ld,
+		     const struct nfs_client *clp, const struct nfs4_deviceid *id)
 {
 	struct nfs4_deviceid_node *d;
 
-	d = nfs4_unhash_put_deviceid(clp, id);
+	d = nfs4_unhash_put_deviceid(ld, clp, id);
 	if (!d)
 		return;
 	if (d->ld->free_deviceid_node)

@@ -197,7 +203,7 @@ nfs4_insert_deviceid_node(struct nfs4_deviceid_node *new)
 
 	spin_lock(&nfs4_deviceid_lock);
 	hash = nfs4_deviceid_hash(&new->deviceid);
-	d = _find_get_deviceid(new->nfs_client, &new->deviceid, hash);
+	d = _find_get_deviceid(new->ld, new->nfs_client, &new->deviceid, hash);
 	if (d) {
 		spin_unlock(&nfs4_deviceid_lock);
 		return d;

Signature · 1 line

1.7.2.3

Boaz Harrosh

author

2011-05-24 15:05 UTC #2 lore raw

PATCH 2/12 SQUASHME: Bug in new global-device-cache code

In reply to Boaz Harrosh

NULL deref on first ever call. (When device is not found)

Signed-off-by Boaz Harrosh <[email protected]>

---
 fs/nfs/pnfs_dev.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

fs/nfs/pnfs_dev.c +1 -1

index 7997899..7e5542c 100644
@@ -100,7 +100,7 @@ _find_get_deviceid(const struct pnfs_layoutdriver_type *ld,
 
 	rcu_read_lock();
 	d = _lookup_deviceid(ld, clp, id, hash);
-	if (!atomic_inc_not_zero(&d->ref))
+	if (!d || !atomic_inc_not_zero(&d->ref))
 		d = NULL;
 	rcu_read_unlock();
 	return d;

Signature · 1 line

1.7.2.3

Benny Halevy

2011-05-24 16:52 UTC #3 lore raw

Re: [PATCH 02/12] SQUASHME: Bug in new global-device-cache code

Quoted 12 lines · Boaz Harrosh

NULL deref on first ever call. (When device is not found)
Signed-off-by Boaz Harrosh <[email protected]>
---
 fs/nfs/pnfs_dev.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
fs/nfs/pnfs_dev.c +0 -0
index 7997899..7e5542c 100644

fs/nfs/pnfs_dev.c _find_get_deviceid(const struct pnfs_layoutdriver_type *ld, · line 103

Quoted 5 lines

fs/nfs/pnfs_dev.c +1 -1

@@ -100,7 +100,7 @@ _find_get_deviceid(const struct pnfs_layoutdriver_type *ld,
 
 	rcu_read_lock();
 	d = _lookup_deviceid(ld, clp, id, hash);
-	if (!atomic_inc_not_zero(&d->ref))
+	if (!d || !atomic_inc_not_zero(&d->ref))

This makes more sense, no?
+ if (d && !atomic_inc_not_zero(&d->ref))

Benny

Quoted 3 lines

fs/nfs/pnfs_dev.c +0 -0

 		d = NULL;
 	rcu_read_unlock();
 	return d;

Boaz Harrosh

author

2011-05-24 17:01 UTC #4 lore raw

Re: [PATCH 02/12] SQUASHME: Bug in new global-device-cache code

Quoted 22 lines · Benny Halevy

Quoted 12 lines · Boaz Harrosh
NULL deref on first ever call. (When device is not found)
Signed-off-by Boaz Harrosh <[email protected]>
---
 fs/nfs/pnfs_dev.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
fs/nfs/pnfs_dev.c +0 -0
index 7997899..7e5542c 100644
fs/nfs/pnfs_dev.c _find_get_deviceid(const struct pnfs_layoutdriver_type *ld, · line 103
Quoted 5 lines
fs/nfs/pnfs_dev.c +1 -1
@@ -100,7 +100,7 @@ _find_get_deviceid(const struct pnfs_layoutdriver_type *ld,
 
 	rcu_read_lock();
 	d = _lookup_deviceid(ld, clp, id, hash);
-	if (!atomic_inc_not_zero(&d->ref))
+	if (!d || !atomic_inc_not_zero(&d->ref))
This makes more sense, no?
(quoted diff) +1 -0
+	if (d && !atomic_inc_not_zero(&d->ref))

Quoted 3 lines

Benny
Quoted 1 line
fs/nfs/pnfs_dev.c +0 -0
 		d = NULL;

Sure, since then d is already set to NULL, I guess

Quoted 3 lines

fs/nfs/pnfs_dev.c _find_get_deviceid(const struct pnfs_layoutdriver_type *ld, · line 106
Quoted 2 lines
fs/nfs/pnfs_dev.c +0 -0
 	rcu_read_unlock();
 	return d;

Boaz

Benny Halevy

2011-05-24 17:02 UTC #5 lore raw

Re: [PATCH 02/12] SQUASHME: Bug in new global-device-cache code

Quoted 23 lines · Boaz Harrosh

Quoted 22 lines · Benny Halevy
Quoted 12 lines · Boaz Harrosh
NULL deref on first ever call. (When device is not found)
Signed-off-by Boaz Harrosh <[email protected]>
---
 fs/nfs/pnfs_dev.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
fs/nfs/pnfs_dev.c +0 -0
index 7997899..7e5542c 100644
fs/nfs/pnfs_dev.c _find_get_deviceid(const struct pnfs_layoutdriver_type *ld, · line 103
Quoted 5 lines
fs/nfs/pnfs_dev.c +1 -1
@@ -100,7 +100,7 @@ _find_get_deviceid(const struct pnfs_layoutdriver_type *ld,
 
 	rcu_read_lock();
 	d = _lookup_deviceid(ld, clp, id, hash);
-	if (!atomic_inc_not_zero(&d->ref))
+	if (!d || !atomic_inc_not_zero(&d->ref))
This makes more sense, no?
(quoted diff) +1 -0
+	if (d && !atomic_inc_not_zero(&d->ref))

Quoted 6 lines

Quoted 3 lines
Benny
Quoted 1 line
fs/nfs/pnfs_dev.c +0 -0
 		d = NULL;
Sure, since then d is already set to NULL, I guess

Right.

Quoted 5 lines

Quoted 3 lines
fs/nfs/pnfs_dev.c _find_get_deviceid(const struct pnfs_layoutdriver_type *ld, · line 106
Quoted 2 lines
fs/nfs/pnfs_dev.c +0 -0
 	rcu_read_unlock();
 	return d;
Boaz

Boaz Harrosh

author

2011-05-24 15:05 UTC #6 lore raw

PATCH 3/12 SQUSHME: pnfs: BUG in _deviceid_purge_client

In reply to Boaz Harrosh

The atomic_dec_and_test() returns *true* when zero. The ! belongs to atomic_dec(). Fixit

Signed-off-by Boaz Harrosh <[email protected]>

---
 fs/nfs/pnfs_dev.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

fs/nfs/pnfs_dev.c +1 -1

index 7e5542c..37ca215 100644
@@ -258,7 +258,7 @@ _deviceid_purge_client(const struct nfs_client *clp, long hash)
 
 	synchronize_rcu();
 	hlist_for_each_entry_safe(d, n, next, &tmp, node)
-		if (!atomic_dec_and_test(&d->ref)) {
+		if (atomic_dec_and_test(&d->ref)) {
 			if (d->ld->free_deviceid_node)
 				d->ld->free_deviceid_node(d);
 			else

Signature · 1 line

1.7.2.3

Benny Halevy

2011-05-24 16:57 UTC #7 lore raw

Re: [PATCH 03/12] SQUSHME: pnfs: BUG in _deviceid_purge_client

Quoted 2 lines · Boaz Harrosh

The atomic_dec_and_test() returns *true* when zero. The ! belongs to atomic_dec(). Fixit

Thanks!

Benny

Quoted 19 lines

Signed-off-by Boaz Harrosh <[email protected]>

---
 fs/nfs/pnfs_dev.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

fs/nfs/pnfs_dev.c +1 -1

index 7e5542c..37ca215 100644
@@ -258,7 +258,7 @@ _deviceid_purge_client(const struct nfs_client *clp, long hash)
 
 	synchronize_rcu();
 	hlist_for_each_entry_safe(d, n, next, &tmp, node)
-		if (!atomic_dec_and_test(&d->ref)) {
+		if (atomic_dec_and_test(&d->ref)) {
 			if (d->ld->free_deviceid_node)
 				d->ld->free_deviceid_node(d);
 			else

Boaz Harrosh

author

2011-05-24 15:06 UTC #8 lore raw

PATCH 4/12 pnfs: layout_driver MUST set free_deviceid_node if using dev-cache

In reply to Boaz Harrosh

A device cache is not a matter of memory store. It is a matter of mounting/login and unmounting/logout. So it is not logical to not set free_deviceid_node. Who will do the unmount?

It is better to crash the developer then let him leak mounts.

Signed-off-by Boaz Harrosh <[email protected]>

---
 fs/nfs/pnfs_dev.c |   16 ++++------------
 1 files changed, 4 insertions(+), 12 deletions(-)

fs/nfs/pnfs_dev.c +4 ~~-12~~

index 37ca215..1b592d9 100644
@@ -163,10 +163,7 @@ nfs4_delete_deviceid(const struct pnfs_layoutdriver_type *ld,
 	d = nfs4_unhash_put_deviceid(ld, clp, id);
 	if (!d)
 		return;
-	if (d->ld->free_deviceid_node)
-		d->ld->free_deviceid_node(d);
-	else
-		kfree(d);
+	d->ld->free_deviceid_node(d);
 }
 EXPORT_SYMBOL_GPL(nfs4_delete_deviceid);

@@ -232,8 +229,7 @@ nfs4_put_deviceid_node(struct nfs4_deviceid_node *d)
 	hlist_del_init_rcu(&d->node);
 	spin_unlock(&nfs4_deviceid_lock);
 	synchronize_rcu();
-	if (d->ld->free_deviceid_node)
-		d->ld->free_deviceid_node(d);
+	d->ld->free_deviceid_node(d);
 	return true;
 }
 EXPORT_SYMBOL_GPL(nfs4_put_deviceid_node);

@@ -258,12 +254,8 @@ _deviceid_purge_client(const struct nfs_client *clp, long hash)
 
 	synchronize_rcu();
 	hlist_for_each_entry_safe(d, n, next, &tmp, node)
-		if (atomic_dec_and_test(&d->ref)) {
-			if (d->ld->free_deviceid_node)
-				d->ld->free_deviceid_node(d);
-			else
-				kfree(d);
-		}
+		if (atomic_dec_and_test(&d->ref))
+			d->ld->free_deviceid_node(d);
 }
 
 void

Signature · 1 line

1.7.2.3

Benny Halevy

2011-05-24 17:04 UTC #9 lore raw

Re: [PATCH 04/12] pnfs: layout_driver MUST set free_deviceid_node if using dev-cache

Quoted 7 lines · Boaz Harrosh

A device cache is not a matter of memory store. It is a matter of mounting/login and unmounting/logout. So it is not logical to not set free_deviceid_node. Who will do the unmount?
It is better to crash the developer then let him leak mounts.
Signed-off-by Boaz Harrosh <[email protected]>

OK, we can do that.

Benny

Quoted 45 lines

---
 fs/nfs/pnfs_dev.c |   16 ++++------------
 1 files changed, 4 insertions(+), 12 deletions(-)

fs/nfs/pnfs_dev.c +4 ~~-12~~

index 37ca215..1b592d9 100644
@@ -163,10 +163,7 @@ nfs4_delete_deviceid(const struct pnfs_layoutdriver_type *ld,
 	d = nfs4_unhash_put_deviceid(ld, clp, id);
 	if (!d)
 		return;
-	if (d->ld->free_deviceid_node)
-		d->ld->free_deviceid_node(d);
-	else
-		kfree(d);
+	d->ld->free_deviceid_node(d);
 }
 EXPORT_SYMBOL_GPL(nfs4_delete_deviceid);

@@ -232,8 +229,7 @@ nfs4_put_deviceid_node(struct nfs4_deviceid_node *d)
 	hlist_del_init_rcu(&d->node);
 	spin_unlock(&nfs4_deviceid_lock);
 	synchronize_rcu();
-	if (d->ld->free_deviceid_node)
-		d->ld->free_deviceid_node(d);
+	d->ld->free_deviceid_node(d);
 	return true;
 }
 EXPORT_SYMBOL_GPL(nfs4_put_deviceid_node);

@@ -258,12 +254,8 @@ _deviceid_purge_client(const struct nfs_client *clp, long hash)
 
 	synchronize_rcu();
 	hlist_for_each_entry_safe(d, n, next, &tmp, node)
-		if (atomic_dec_and_test(&d->ref)) {
-			if (d->ld->free_deviceid_node)
-				d->ld->free_deviceid_node(d);
-			else
-				kfree(d);
-		}
+		if (atomic_dec_and_test(&d->ref))
+			d->ld->free_deviceid_node(d);
 }
 
 void

Boaz Harrosh

author

2011-05-24 15:06 UTC #10 lore raw

PATCH 5/12 SQUASHME: pnfs-obj: pnfs_osd_xdr.h Remove server definitions

In reply to Boaz Harrosh

These are later added by the Server patchset

Signed-off-by Boaz Harrosh <[email protected]>

---
 include/linux/pnfs_osd_xdr.h |    5 -----
 1 files changed, 0 insertions(+), 5 deletions(-)

include/linux/pnfs_osd_xdr.h +0 -5

index 747d06d..76efbdd 100644
@@ -336,15 +336,10 @@ extern void pnfs_osd_xdr_decode_deviceaddr(
 extern int
 pnfs_osd_xdr_encode_layoutupdate(struct xdr_stream *xdr,
 				 struct pnfs_osd_layoutupdate *lou);
-extern __be32 *
-pnfs_osd_xdr_decode_layoutupdate(struct pnfs_osd_layoutupdate *lou, __be32 *p);
 
 /* osd_ioerror encoding/decoding (layout_return) */
 /* Client */
 extern __be32 *pnfs_osd_xdr_ioerr_reserve_space(struct xdr_stream *xdr);
 extern void pnfs_osd_xdr_encode_ioerr(__be32 *p, struct pnfs_osd_ioerr *ioerr);
-/* Server*/
-extern __be32 *
-pnfs_osd_xdr_decode_ioerr(struct pnfs_osd_ioerr *ioerr, __be32 *p);
 
 #endif /* __PNFS_OSD_XDR_H__ */

Signature · 1 line

1.7.2.3

Boaz Harrosh

author

2011-05-24 15:06 UTC #11 lore raw

PATCH 6/12 SQUASHME: pnf-obj xdr_cli: Wrong type in comments

In reply to Boaz Harrosh

Signed-off-by Boaz Harrosh <[email protected]>

---
 fs/nfs/objlayout/pnfs_osd_xdr_cli.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

fs/nfs/objlayout/pnfs_osd_xdr_cli.c +2 -2

index dd3052c..16fc758 100644
@@ -47,7 +47,7 @@
 
 /*
  * struct pnfs_osd_objid {
- *	struct pnfs_deviceid	oid_device_id;
+ *	struct nfs4_deviceid	oid_device_id;
  *	u64			oid_partition_id;
  *	u64			oid_object_id;
  * }; // xdr size 32 bytes

@@ -366,7 +366,7 @@ pnfs_osd_xdr_encode_layoutupdate(struct xdr_stream *xdr,
 
 /*
  * struct pnfs_osd_objid {
- *	struct pnfs_deviceid	oid_device_id;
+ *	struct nfs4_deviceid	oid_device_id;
  *	u64			oid_partition_id;
  *	u64			oid_object_id;
  * }; // xdr size 32 bytes

Signature · 1 line

1.7.2.3

Boaz Harrosh

author

2011-05-24 15:07 UTC #12 lore raw

PATCH 7/12 SQUASHME: pnfs-obj: use layout driver in global device cache

In reply to Boaz Harrosh

From: Benny Halevy <[email protected]>

This is the objio_osd part of the patch:
NFSv4.1: use layout driver in global device cache

I have move that patch to the begining of the submit toghther with the bug fixing of the new code

This here should just be squashed into the single objlayout part of the get_devic_info handling

Signed-off-by Benny Halevy <[email protected]>

---
 fs/nfs/objlayout/objio_osd.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

fs/nfs/objlayout/objio_osd.c +3 -3

index bcc8468..a4201d8 100644
@@ -60,12 +60,12 @@ objio_free_deviceid_node(struct nfs4_deviceid_node *d)
 	kfree(de);
 }
 
-static struct objio_dev_ent *_dev_list_find(const struct nfs_client *clp,
+static struct objio_dev_ent *_dev_list_find(const struct nfs_server *nfss,
 	const struct nfs4_deviceid *d_id)
 {
 	struct nfs4_deviceid_node *d;
 
-	d = nfs4_find_get_deviceid(clp, d_id);
+	d = nfs4_find_get_deviceid(nfss->pnfs_curr_ld, nfss->nfs_client, d_id);
 	if (!d)
 		return NULL;
 	return container_of(d, struct objio_dev_ent, id_node);

@@ -141,7 +141,7 @@ static struct objio_dev_ent *_device_lookup(struct pnfs_layout_hdr *pnfslay,
 
 	d_id = &objio_seg->comps[comp].oc_object_id.oid_device_id;
 
-	ode = _dev_list_find(NFS_SERVER(pnfslay->plh_inode)->nfs_client, d_id);
+	ode = _dev_list_find(NFS_SERVER(pnfslay->plh_inode), d_id);
 	if (ode)
 		return ode;

Signature · 1 line

1.7.2.3

Boaz Harrosh

author

2011-05-24 15:07 UTC #13 lore raw

PATCH 8/12 SQUASHME: objio alloc/free lseg Bugs fixes

In reply to Boaz Harrosh

Wrong allocation and pointering in lseg_alloc.

Signed-off-by Boaz Harrosh <[email protected]>

---
 fs/nfs/objlayout/objio_osd.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

fs/nfs/objlayout/objio_osd.c +5 -4

index a4201d8..167cd1e 100644
@@ -117,7 +117,7 @@ struct objio_segment {
 	unsigned comps_index;
 	unsigned num_comps;
 	/* variable length */
-	struct objio_dev_ent *ods[1];
+	struct objio_dev_ent *ods[0];
 };
 
 static inline struct objio_segment *

@@ -278,7 +278,6 @@ extern int objio_alloc_lseg(struct pnfs_layout_segment **outp,
 	struct pnfs_osd_layout layout;
 	struct pnfs_osd_object_cred *cur_comp, src_comp;
 	struct caps_buffers *caps_p;
-
 	int err;
 
 	err = pnfs_osd_xdr_decode_layout_map(&layout, &iter, xdr);

@@ -289,14 +288,16 @@ extern int objio_alloc_lseg(struct pnfs_layout_segment **outp,
 	if (unlikely(err))
 		return err;
 
-	objio_seg = kzalloc(sizeof(*objio_seg) +
+	objio_seg = kzalloc(sizeof(*objio_seg) + 
+			    sizeof(objio_seg->ods[0]) * layout.olo_num_comps +
 			    sizeof(*objio_seg->comps) * layout.olo_num_comps +
 			    sizeof(struct caps_buffers) * layout.olo_num_comps,
 			    gfp_flags);
 	if (!objio_seg)
 		return -ENOMEM;
 
-	cur_comp = objio_seg->comps = (void *)(objio_seg + 1);
+	objio_seg->comps = (void *)(objio_seg->ods + layout.olo_num_comps);
+	cur_comp = objio_seg->comps;
 	caps_p = (void *)(cur_comp + layout.olo_num_comps);
 	while (pnfs_osd_xdr_decode_layout_comp(&src_comp, &iter, xdr, &err))
 		copy_single_comp(cur_comp++, &src_comp, caps_p++);

Signature · 1 line

1.7.2.3

Benny Halevy

2011-05-24 17:06 UTC #14 lore raw

Re: [PATCH 08/12] SQUASHME: objio alloc/free lseg Bugs fixes

Quoted 29 lines · Boaz Harrosh

Wrong allocation and pointering in lseg_alloc.

Signed-off-by Boaz Harrosh <[email protected]>

---
 fs/nfs/objlayout/objio_osd.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

fs/nfs/objlayout/objio_osd.c +1 -2

index a4201d8..167cd1e 100644
@@ -117,7 +117,7 @@ struct objio_segment {
 	unsigned comps_index;
 	unsigned num_comps;
 	/* variable length */
-	struct objio_dev_ent *ods[1];
+	struct objio_dev_ent *ods[0];
 };
 
 static inline struct objio_segment *

@@ -278,7 +278,6 @@ extern int objio_alloc_lseg(struct pnfs_layout_segment **outp,
 	struct pnfs_osd_layout layout;
 	struct pnfs_osd_object_cred *cur_comp, src_comp;
 	struct caps_buffers *caps_p;
-
 	int err;
 
 	err = pnfs_osd_xdr_decode_layout_map(&layout, &iter, xdr);

fs/nfs/objlayout/objio_osd.c extern int objio_alloc_lseg(struct pnfs_layout_segment **outp, · line 291

Quoted 5 lines

fs/nfs/objlayout/objio_osd.c +1 -1

@@ -289,14 +288,16 @@ extern int objio_alloc_lseg(struct pnfs_layout_segment **outp,
 	if (unlikely(err))
 		return err;
 
-	objio_seg = kzalloc(sizeof(*objio_seg) +
+	objio_seg = kzalloc(sizeof(*objio_seg) +

nit: While at it, the trailing space is extraneous...

Benny

Quoted 13 lines

fs/nfs/objlayout/objio_osd.c +3 -1

+			    sizeof(objio_seg->ods[0]) * layout.olo_num_comps +
 			    sizeof(*objio_seg->comps) * layout.olo_num_comps +
 			    sizeof(struct caps_buffers) * layout.olo_num_comps,
 			    gfp_flags);
 	if (!objio_seg)
 		return -ENOMEM;
 
-	cur_comp = objio_seg->comps = (void *)(objio_seg + 1);
+	objio_seg->comps = (void *)(objio_seg->ods + layout.olo_num_comps);
+	cur_comp = objio_seg->comps;
 	caps_p = (void *)(cur_comp + layout.olo_num_comps);
 	while (pnfs_osd_xdr_decode_layout_comp(&src_comp, &iter, xdr, &err))
 		copy_single_comp(cur_comp++, &src_comp, caps_p++);

Boaz Harrosh

author

2011-05-24 15:07 UTC #15 lore raw

PATCH 9/12 SQUASHME: pnfs-obj: Bugs in new global-device-cache code

In reply to Boaz Harrosh

Fix BUGs in the new "Use global-device-cache".

One thing I don't understand is why the compiler did not complain when the code was returning the wrong type of structure

Signed-off-by Boaz Harrosh <[email protected]>

---
 fs/nfs/objlayout/objio_osd.c |   28 +++++++++++++++++++---------
 1 files changed, 19 insertions(+), 9 deletions(-)

fs/nfs/objlayout/objio_osd.c +19 -9

index 167cd1e..faacde2 100644
@@ -56,6 +56,7 @@ objio_free_deviceid_node(struct nfs4_deviceid_node *d)
 {
 	struct objio_dev_ent *de = container_of(d, struct objio_dev_ent, id_node);
 
+	dprintk("%s: free od=%p\n", __func__, de->od);
 	osduld_put_device(de->od);
 	kfree(de);
 }

@@ -64,14 +65,18 @@ static struct objio_dev_ent *_dev_list_find(const struct nfs_server *nfss,
 	const struct nfs4_deviceid *d_id)
 {
 	struct nfs4_deviceid_node *d;
+	struct objio_dev_ent *de;
 
 	d = nfs4_find_get_deviceid(nfss->pnfs_curr_ld, nfss->nfs_client, d_id);
 	if (!d)
 		return NULL;
-	return container_of(d, struct objio_dev_ent, id_node);
+
+	de = container_of(d, struct objio_dev_ent, id_node);
+	return de;
 }
 
-static int _dev_list_add(const struct nfs_server *nfss,
+static struct objio_dev_ent *
+_dev_list_add(const struct nfs_server *nfss,
 	const struct nfs4_deviceid *d_id, struct osd_dev *od,
 	gfp_t gfp_flags)
 {

@@ -79,9 +84,12 @@ static int _dev_list_add(const struct nfs_server *nfss,
 	struct objio_dev_ent *de = kzalloc(sizeof(*de), gfp_flags);
 	struct objio_dev_ent *n;
 
-	if (!de)
-		return -ENOMEM;
+	if (!de) {
+		dprintk("%s: -ENOMEM od=%p\n", __func__, od);
+		return NULL;
+	}
 
+	dprintk("%s: Adding od=%p\n", __func__, od);
 	nfs4_init_deviceid_node(&de->id_node,
 				nfss->pnfs_curr_ld,
 				nfss->nfs_client,

@@ -91,11 +99,12 @@ static int _dev_list_add(const struct nfs_server *nfss,
 	d = nfs4_insert_deviceid_node(&de->id_node);
 	n = container_of(d, struct objio_dev_ent, id_node);
 	if (n != de) {
-		BUG_ON(n->od != od);
+		dprintk("%s: Race with other n->od=%p\n", __func__, n->od);
 		objio_free_deviceid_node(&de->id_node);
+		de = n;
 	}
 
-	return 0;
+	return de;
 }
 
 struct caps_buffers {

@@ -117,7 +126,7 @@ struct objio_segment {
 	unsigned comps_index;
 	unsigned num_comps;
 	/* variable length */
-	struct objio_dev_ent *ods[0];
+	struct objio_dev_ent *ods[];
 };
 
 static inline struct objio_segment *

@@ -176,12 +185,13 @@ static struct objio_dev_ent *_device_lookup(struct pnfs_layout_hdr *pnfslay,
 		goto out;
 	}
 
-	_dev_list_add(NFS_SERVER(pnfslay->plh_inode), d_id, od, gfp_flags);
+	ode = _dev_list_add(NFS_SERVER(pnfslay->plh_inode), d_id, od,
+			    gfp_flags);
 
 out:
 	dprintk("%s: return=%d\n", __func__, err);
 	objlayout_put_deviceinfo(deviceaddr);
-	return err ? ERR_PTR(err) : od;
+	return err ? ERR_PTR(err) : ode;
 }
 
 static int objio_devices_lookup(struct pnfs_layout_hdr *pnfslay,

Signature · 1 line

1.7.2.3

Benny Halevy

2011-05-24 17:14 UTC #16 lore raw

Re: [PATCH 09/12] SQUASHME: pnfs-obj: Bugs in new global-device-cache code

Quoted 26 lines · Boaz Harrosh

Fix BUGs in the new "Use global-device-cache".

One thing I don't understand is why the compiler did not complain when the code was returning the wrong type of structure

Signed-off-by Boaz Harrosh <[email protected]>

---
 fs/nfs/objlayout/objio_osd.c |   28 +++++++++++++++++++---------
 1 files changed, 19 insertions(+), 9 deletions(-)

fs/nfs/objlayout/objio_osd.c +1 -0

index 167cd1e..faacde2 100644
@@ -56,6 +56,7 @@ objio_free_deviceid_node(struct nfs4_deviceid_node *d)
 {
 	struct objio_dev_ent *de = container_of(d, struct objio_dev_ent, id_node);
 
+	dprintk("%s: free od=%p\n", __func__, de->od);
 	osduld_put_device(de->od);
 	kfree(de);
 }

@@ -64,14 +65,18 @@ static struct objio_dev_ent *_dev_list_find(const struct nfs_server *nfss,
 	const struct nfs4_deviceid *d_id)
 {

fs/nfs/objlayout/objio_osd.c static struct objio_dev_ent *_dev_list_find(const struct nfs_server *nfss, · line 75

Quoted 10 lines

fs/nfs/objlayout/objio_osd.c +4 -1

 	struct nfs4_deviceid_node *d;
+	struct objio_dev_ent *de;
 
 	d = nfs4_find_get_deviceid(nfss->pnfs_curr_ld, nfss->nfs_client, d_id);
 	if (!d)
 		return NULL;
-	return container_of(d, struct objio_dev_ent, id_node);
+
+	de = container_of(d, struct objio_dev_ent, id_node);
+	return de;

That's not really required as container_of() does the type casting for you.

Quoted 9 lines

fs/nfs/objlayout/objio_osd.c +2 -1

 }
 
-static int _dev_list_add(const struct nfs_server *nfss,
+static struct objio_dev_ent *
+_dev_list_add(const struct nfs_server *nfss,
 	const struct nfs4_deviceid *d_id, struct osd_dev *od,
 	gfp_t gfp_flags)
 {

fs/nfs/objlayout/objio_osd.c static int _dev_list_add(const struct nfs_server *nfss, · line 89

Quoted 8 lines

fs/nfs/objlayout/objio_osd.c +3 -2

@@ -79,9 +84,12 @@ static int _dev_list_add(const struct nfs_server *nfss,
 	struct objio_dev_ent *de = kzalloc(sizeof(*de), gfp_flags);
 	struct objio_dev_ent *n;
 
-	if (!de)
-		return -ENOMEM;
+	if (!de) {
+		dprintk("%s: -ENOMEM od=%p\n", __func__, od);
+		return NULL;

better return ERR_PTR(-ENOMEM)
that will percolate up the stack via _device_lookup.

Thanks!

Benny

Quoted 46 lines

fs/nfs/objlayout/objio_osd.c +9 -5

+	}
 
+	dprintk("%s: Adding od=%p\n", __func__, od);
 	nfs4_init_deviceid_node(&de->id_node,
 				nfss->pnfs_curr_ld,
 				nfss->nfs_client,

@@ -91,11 +99,12 @@ static int _dev_list_add(const struct nfs_server *nfss,
 	d = nfs4_insert_deviceid_node(&de->id_node);
 	n = container_of(d, struct objio_dev_ent, id_node);
 	if (n != de) {
-		BUG_ON(n->od != od);
+		dprintk("%s: Race with other n->od=%p\n", __func__, n->od);
 		objio_free_deviceid_node(&de->id_node);
+		de = n;
 	}
 
-	return 0;
+	return de;
 }
 
 struct caps_buffers {

@@ -117,7 +126,7 @@ struct objio_segment {
 	unsigned comps_index;
 	unsigned num_comps;
 	/* variable length */
-	struct objio_dev_ent *ods[0];
+	struct objio_dev_ent *ods[];
 };
 
 static inline struct objio_segment *

@@ -176,12 +185,13 @@ static struct objio_dev_ent *_device_lookup(struct pnfs_layout_hdr *pnfslay,
 		goto out;
 	}
 
-	_dev_list_add(NFS_SERVER(pnfslay->plh_inode), d_id, od, gfp_flags);
+	ode = _dev_list_add(NFS_SERVER(pnfslay->plh_inode), d_id, od,
+			    gfp_flags);
 
 out:
 	dprintk("%s: return=%d\n", __func__, err);
 	objlayout_put_deviceinfo(deviceaddr);
-	return err ? ERR_PTR(err) : od;
+	return err ? ERR_PTR(err) : ode;
 }
 
 static int objio_devices_lookup(struct pnfs_layout_hdr *pnfslay,

Boaz Harrosh

author

2011-05-24 17:18 UTC #17 lore raw

Re: [PATCH 09/12] SQUASHME: pnfs-obj: Bugs in new global-device-cache code

Quoted 30 lines · Benny Halevy

Quoted 29 lines · Boaz Harrosh

Fix BUGs in the new "Use global-device-cache".

One thing I don't understand is why the compiler did not complain when the code was returning the wrong type of structure

Signed-off-by Boaz Harrosh <[email protected]>

---
 fs/nfs/objlayout/objio_osd.c |   28 +++++++++++++++++++---------
 1 files changed, 19 insertions(+), 9 deletions(-)

fs/nfs/objlayout/objio_osd.c +2 -0

index 167cd1e..faacde2 100644
@@ -56,6 +56,7 @@ objio_free_deviceid_node(struct nfs4_deviceid_node *d)
 {
 	struct objio_dev_ent *de = container_of(d, struct objio_dev_ent, id_node);
 
+	dprintk("%s: free od=%p\n", __func__, de->od);
 	osduld_put_device(de->od);
 	kfree(de);
 }

@@ -64,14 +65,18 @@ static struct objio_dev_ent *_dev_list_find(const struct nfs_server *nfss,
 	const struct nfs4_deviceid *d_id)
 {
 	struct nfs4_deviceid_node *d;
+	struct objio_dev_ent *de;

Quoted 11 lines

fs/nfs/objlayout/objio_osd.c static struct objio_dev_ent *_dev_list_find(const struct nfs_server *nfss, · line 75
Quoted 7 lines
fs/nfs/objlayout/objio_osd.c +3 -1
 	d = nfs4_find_get_deviceid(nfss->pnfs_curr_ld, nfss->nfs_client, d_id);
 	if (!d)
 		return NULL;
-	return container_of(d, struct objio_dev_ent, id_node);
+
+	de = container_of(d, struct objio_dev_ent, id_node);
+	return de;
That's not really required as container_of() does the type casting for you.

Ye, I know that change is just a left over from a print between the set and the return. (Else how could I debug this)
I than removed the print because these come a lot in a git clone for example.

Quoted 12 lines

fs/nfs/objlayout/objio_osd.c +2 -1

 }
 
-static int _dev_list_add(const struct nfs_server *nfss,
+static struct objio_dev_ent *
+_dev_list_add(const struct nfs_server *nfss,
 	const struct nfs4_deviceid *d_id, struct osd_dev *od,
 	gfp_t gfp_flags)
 {

@@ -79,9 +84,12 @@ static int _dev_list_add(const struct nfs_server *nfss,
 	struct objio_dev_ent *de = kzalloc(sizeof(*de), gfp_flags);
 	struct objio_dev_ent *n;

Quoted 9 lines

fs/nfs/objlayout/objio_osd.c static int _dev_list_add(const struct nfs_server *nfss, · line 89
Quoted 5 lines
fs/nfs/objlayout/objio_osd.c +3 -2
-	if (!de)
-		return -ENOMEM;
+	if (!de) {
+		dprintk("%s: -ENOMEM od=%p\n", __func__, od);
+		return NULL;
better return ERR_PTR(-ENOMEM)
that will percolate up the stack via _device_lookup.

Right missed that one

Quoted 2 lines

Thanks!

What thanks? beers on you next time! ;-)

Quoted 2 lines

Benny

Boaz

Quoted 47 lines

Quoted 46 lines

fs/nfs/objlayout/objio_osd.c +9 -5

+	}
 
+	dprintk("%s: Adding od=%p\n", __func__, od);
 	nfs4_init_deviceid_node(&de->id_node,
 				nfss->pnfs_curr_ld,
 				nfss->nfs_client,

@@ -91,11 +99,12 @@ static int _dev_list_add(const struct nfs_server *nfss,
 	d = nfs4_insert_deviceid_node(&de->id_node);
 	n = container_of(d, struct objio_dev_ent, id_node);
 	if (n != de) {
-		BUG_ON(n->od != od);
+		dprintk("%s: Race with other n->od=%p\n", __func__, n->od);
 		objio_free_deviceid_node(&de->id_node);
+		de = n;
 	}
 
-	return 0;
+	return de;
 }
 
 struct caps_buffers {

@@ -117,7 +126,7 @@ struct objio_segment {
 	unsigned comps_index;
 	unsigned num_comps;
 	/* variable length */
-	struct objio_dev_ent *ods[0];
+	struct objio_dev_ent *ods[];
 };
 
 static inline struct objio_segment *

@@ -176,12 +185,13 @@ static struct objio_dev_ent *_device_lookup(struct pnfs_layout_hdr *pnfslay,
 		goto out;
 	}
 
-	_dev_list_add(NFS_SERVER(pnfslay->plh_inode), d_id, od, gfp_flags);
+	ode = _dev_list_add(NFS_SERVER(pnfslay->plh_inode), d_id, od,
+			    gfp_flags);
 
 out:
 	dprintk("%s: return=%d\n", __func__, err);
 	objlayout_put_deviceinfo(deviceaddr);
-	return err ? ERR_PTR(err) : od;
+	return err ? ERR_PTR(err) : ode;
 }
 
 static int objio_devices_lookup(struct pnfs_layout_hdr *pnfslay,

Boaz Harrosh

author

2011-05-24 15:08 UTC #18 lore raw

PATCH 10/12 SQUASHME: pnfs-obj: objlayout wants to cache devices until unmount

In reply to Boaz Harrosh

Take an extra reference on a device insert. So devices keep around in the cache until nfs_client release. (This was the behavior of the old cache)

The extra reference will be removed in nfs4_deviceid_purge_client(). I tested this and it works perfectly.

TODO: Define an nfs4_get_deviceid()
Currently accesing did->ref directly

TODO: nfs4_insert_deviceid_node should check if there are too many
devices and start purging them. Say by time from last use.

Signed-off-by Boaz Harrosh <[email protected]>

---
 fs/nfs/objlayout/objio_osd.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

fs/nfs/objlayout/objio_osd.c +1 -0

index faacde2..8b05b16 100644
@@ -104,6 +104,7 @@ _dev_list_add(const struct nfs_server *nfss,
 		de = n;
 	}
 
+	atomic_inc(&de->id_node.ref);
 	return de;
 }

Signature · 1 line

1.7.2.3

Benny Halevy

2011-05-24 17:17 UTC #19 lore raw

Re: [PATCH 10/12] SQUASHME: pnfs-obj: objlayout wants to cache devices until unmount

Quoted 9 lines · Boaz Harrosh

Take an extra reference on a device insert. So devices keep around in the cache until nfs_client release. (This was the behavior of the old cache)
The extra reference will be removed in nfs4_deviceid_purge_client(). I tested this and it works perfectly.
TODO: Define an nfs4_get_deviceid()
Currently accesing did->ref directly

This is the right time to do it, why wait?

Quoted 3 lines

TODO: nfs4_insert_deviceid_node should check if there are too many
devices and start purging them. Say by time from last use.

yeah, an lru list would be helpful to prune the lists and get rid of unused devices. As we discussed before, this can be done periodically, and all devices not used for a long enough interval can be deleted.

Benny

Quoted 18 lines

Signed-off-by Boaz Harrosh <[email protected]>

---
 fs/nfs/objlayout/objio_osd.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

fs/nfs/objlayout/objio_osd.c +1 -0

index faacde2..8b05b16 100644
@@ -104,6 +104,7 @@ _dev_list_add(const struct nfs_server *nfss,
 		de = n;
 	}
 
+	atomic_inc(&de->id_node.ref);
 	return de;
 }

Boaz Harrosh

author

2011-05-24 15:09 UTC #20 lore raw

PATCH 11/12 SQUASHME: pnfs: Fall out from: non-rpc layout drivers

In reply to Boaz Harrosh

the de-ref in pnfs_ld_read/write_done in the error case is not needed. I only tested the write path but I suspect it is all symetric

Signed-off-by Boaz Harrosh <[email protected]>

---
 fs/nfs/pnfs.c |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)

fs/nfs/pnfs.c +0 -4

index 0f59802..1716621 100644
@@ -1069,8 +1069,6 @@ pnfs_ld_write_done(struct nfs_write_data *data)
 		return 0;
 	}
 
-	put_lseg(data->lseg);
-	data->lseg = NULL;
 	dprintk("%s: pnfs_error=%d, retry via MDS\n", __func__,
 		data->pnfs_error);
 	status = nfs_initiate_write(data, NFS_CLIENT(data->inode),

@@ -1118,8 +1116,6 @@ pnfs_ld_read_done(struct nfs_read_data *data)
 		return 0;
 	}
 
-	put_lseg(data->lseg);
-	data->lseg = NULL;
 	dprintk("%s: pnfs_error=%d, retry via MDS\n", __func__,
 		data->pnfs_error);
 	status = nfs_initiate_read(data, NFS_CLIENT(data->inode),

Signature · 1 line

1.7.2.3

Boaz Harrosh

author

2011-05-24 15:11 UTC #21 lore raw

PATCH 12/12 SQUASHME: objio read/write patch: Bugs fixes

In reply to Boaz Harrosh

Cap BIO size it one page

Signed-off-by Boaz Harrosh <[email protected]>

---
 fs/nfs/objlayout/objio_osd.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

fs/nfs/objlayout/objio_osd.c +3 -0

index bc5aa46..06bc032 100644
@@ -561,6 +561,9 @@ static int _add_stripe_unit(struct objio_state *ios,  unsigned *cur_pg,
 		unsigned bio_size = (ios->ol_state.nr_pages + pages_in_stripe) /
 				    stripes;
 
+		if (BIO_MAX_PAGES_KMALLOC < bio_size)
+			bio_size = BIO_MAX_PAGES_KMALLOC;
+
 		per_dev->bio = bio_kmalloc(gfp_flags, bio_size);
 		if (unlikely(!per_dev->bio)) {
 			dprintk("Faild to allocate BIO size=%u\n", bio_size);

Signature · 1 line

1.7.2.3