I'm sending some more fixes.
After launch I will re-send all the fixes needed ontop of Benny's
last tree as SQUASHMEs. One SQUASHME for each patch in the set that
needs one.
Last sent fixes will have conflicts with newest code. So these re-sent
will resolve that.
With this code, I'm able to use the global-device-cache the way It
was with the old one. Also including all the fixes I'm able to do
IO, and checkout a git tree!
*BUT*: Currently IO is very very slow, because wsize/rsize is set
to default 64k which is just one strip_unit for default
objects. I used to set server->w/rsize on .set_layoutdriver
but Benny removed that. What do you guys want to do with this?
I can see to options:
1. Return the .set_layoutdriver where ld can set defaults
the way it likes it.
2. If pnfs is used set w/rsize to MAX_UINT and let pg_test
cap the sizes as see fit.
Please advise?
For now these:
SQUSHME: pnfs: BUG in _deviceid_purge_client
pnfs: layout_driver MUST set free_deviceid_node if using dev-cache
SQUASHME: pnfs-obj: objlayout wants to cache devices until unmount
Thanks
Boaz
Take an extra reference on a device insert. So devices keep
around in the cache until nfs_client release.
(This was the behaviour 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 accessing did->ref directly
TODO: nfs4_insert_deviceid_node should check if there are too many
devices and start purging them. Say by longest 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(-)
diff --git a/fs/nfs/objlayout/objio_osd.c b/fs/nfs/objlayout/objio_osd.c
index c09066f..47e8cb5 100644
--- a/fs/nfs/objlayout/objio_osd.c
+++ b/fs/nfs/objlayout/objio_osd.c
@@ -110,6 +110,7 @@ _dev_list_add(const struct nfs_server *nfss,
de = n;
}
+ atomic_inc(&de->id_node.ref);
return de;
}
--
1.7.2.3
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(-)
diff --git a/fs/nfs/pnfs_dev.c b/fs/nfs/pnfs_dev.c
index 37ca215..1b592d9 100644
--- a/fs/nfs/pnfs_dev.c
+++ b/fs/nfs/pnfs_dev.c
@@ -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
--
1.7.2.3
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(-)
diff --git a/fs/nfs/pnfs_dev.c b/fs/nfs/pnfs_dev.c
index 7e5542c..37ca215 100644
--- a/fs/nfs/pnfs_dev.c
+++ b/fs/nfs/pnfs_dev.c
@@ -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
--
1.7.2.3