The NFS4CLNT_LAYOUTRECALL tests in pnfs_layout_process and
pnfs_update_layout are redundant.
In the case of a bulk layout recall, we're always testing for
the NFS_LAYOUT_BULK_RECALL flay anyway.
In the case of a file or segment recall, the call to
pnfs_set_layout_stateid() updates the layout_header 'barrier'
sequence id, which triggers the test in pnfs_layoutgets_blocked()
and is less race-prone than NFS4CLNT_LAYOUTRECALL anyway.
Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/pnfs.c | 6 ++----
1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index a534216..9c19f92 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -966,8 +966,7 @@ pnfs_update_layout(struct inode *ino,
}
/* Do we even need to bother with this? */
- if (test_bit(NFS4CLNT_LAYOUTRECALL, &clp->cl_state) ||
- test_bit(NFS_LAYOUT_BULK_RECALL, &lo->plh_flags)) {
+ if (test_bit(NFS_LAYOUT_BULK_RECALL, &lo->plh_flags)) {
dprintk("%s matches recall, use MDS\n", __func__);
goto out_unlock;
}
@@ -1048,8 +1047,7 @@ pnfs_layout_process(struct nfs4_layoutget *lgp)
}
spin_lock(&ino->i_lock);
- if (test_bit(NFS4CLNT_LAYOUTRECALL, &clp->cl_state) ||
- test_bit(NFS_LAYOUT_BULK_RECALL, &lo->plh_flags)) {
+ if (test_bit(NFS_LAYOUT_BULK_RECALL, &lo->plh_flags)) {
dprintk("%s forget reply due to recall\n", __func__);
goto out_forget_reply;
}
--
1.7.7.6
The NFS4CLNT_LAYOUTRECALL bit is a long-term impediment to scalability. It
basically stops all other recalls by a given server once any layout recall
is requested.
If the recall is for a different file, then we don't care.
If the recall applies to the same file, then we're in one of two situations:
Either we are in the case of a replay of an existing request, in which case
the session is supposed to deal with matters, or we are dealing with a
completely different request, in which case we should just try to process
it.
Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/callback_proc.c | 6 +-----
fs/nfs/nfs4_fs.h | 1 -
2 files changed, 1 insertions(+), 6 deletions(-)
diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
index f71978d..0e0865e 100644
--- a/fs/nfs/callback_proc.c
+++ b/fs/nfs/callback_proc.c
@@ -233,17 +233,13 @@ static u32 initiate_bulk_draining(struct nfs_client *clp,
static u32 do_callback_layoutrecall(struct nfs_client *clp,
struct cb_layoutrecallargs *args)
{
- u32 res = NFS4ERR_DELAY;
+ u32 res;
dprintk("%s enter, type=%i\n", __func__, args->cbl_recall_type);
- if (test_and_set_bit(NFS4CLNT_LAYOUTRECALL, &clp->cl_state))
- goto out;
if (args->cbl_recall_type == RETURN_FILE)
res = initiate_file_draining(clp, args);
else
res = initiate_bulk_draining(clp, args);
- clear_bit(NFS4CLNT_LAYOUTRECALL, &clp->cl_state);
-out:
dprintk("%s returning %i\n", __func__, res);
return res;
diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index b133b50..19079ec 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -20,7 +20,6 @@ enum nfs4_client_state {
NFS4CLNT_RECLAIM_REBOOT,
NFS4CLNT_RECLAIM_NOGRACE,
NFS4CLNT_DELEGRETURN,
- NFS4CLNT_LAYOUTRECALL,
NFS4CLNT_SESSION_RESET,
NFS4CLNT_RECALL_SLOT,
NFS4CLNT_LEASE_CONFIRM,
--
1.7.7.6
On 02/29/2012 06:34 PM, Trond Myklebust wrote:
> The NFS4CLNT_LAYOUTRECALL tests in pnfs_layout_process and
> pnfs_update_layout are redundant.
>
> In the case of a bulk layout recall, we're always testing for
> the NFS_LAYOUT_BULK_RECALL flay anyway.
> In the case of a file or segment recall, the call to
> pnfs_set_layout_stateid() updates the layout_header 'barrier'
> sequence id, which triggers the test in pnfs_layoutgets_blocked()
> and is less race-prone than NFS4CLNT_LAYOUTRECALL anyway.
>
> Signed-off-by: Trond Myklebust <[email protected]>
> ---
> fs/nfs/pnfs.c | 6 ++----
> 1 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
> index a534216..9c19f92 100644
> --- a/fs/nfs/pnfs.c
> +++ b/fs/nfs/pnfs.c
> @@ -966,8 +966,7 @@ pnfs_update_layout(struct inode *ino,
> }
>
> /* Do we even need to bother with this? */
> - if (test_bit(NFS4CLNT_LAYOUTRECALL, &clp->cl_state) ||
> - test_bit(NFS_LAYOUT_BULK_RECALL, &lo->plh_flags)) {
> + if (test_bit(NFS_LAYOUT_BULK_RECALL, &lo->plh_flags)) {
> dprintk("%s matches recall, use MDS\n", __func__);
> goto out_unlock;
> }
> @@ -1048,8 +1047,7 @@ pnfs_layout_process(struct nfs4_layoutget *lgp)
> }
>
> spin_lock(&ino->i_lock);
> - if (test_bit(NFS4CLNT_LAYOUTRECALL, &clp->cl_state) ||
> - test_bit(NFS_LAYOUT_BULK_RECALL, &lo->plh_flags)) {
This was the only place that the clp structure was used, so you could probably remove it to avoid this warning:
fs/nfs/pnfs.c: In function 'pnfs_layout_process':
fs/nfs/pnfs.c:1058:21: warning: unused variable 'clp' [-Wunused-variable]
- Bryan
> + if (test_bit(NFS_LAYOUT_BULK_RECALL, &lo->plh_flags)) {
> dprintk("%s forget reply due to recall\n", __func__);
> goto out_forget_reply;
> }
T24gVGh1LCAyMDEyLTAzLTAxIGF0IDEwOjM5IC0wNTAwLCBCcnlhbiBTY2h1bWFrZXIgd3JvdGU6
DQo+IE9uIDAyLzI5LzIwMTIgMDY6MzQgUE0sIFRyb25kIE15a2xlYnVzdCB3cm90ZToNCj4gDQo+
ID4gVGhlIE5GUzRDTE5UX0xBWU9VVFJFQ0FMTCB0ZXN0cyBpbiBwbmZzX2xheW91dF9wcm9jZXNz
IGFuZA0KPiA+IHBuZnNfdXBkYXRlX2xheW91dCBhcmUgcmVkdW5kYW50Lg0KPiA+IA0KPiA+IElu
IHRoZSBjYXNlIG9mIGEgYnVsayBsYXlvdXQgcmVjYWxsLCB3ZSdyZSBhbHdheXMgdGVzdGluZyBm
b3INCj4gPiB0aGUgTkZTX0xBWU9VVF9CVUxLX1JFQ0FMTCBmbGF5IGFueXdheS4NCj4gPiBJbiB0
aGUgY2FzZSBvZiBhIGZpbGUgb3Igc2VnbWVudCByZWNhbGwsIHRoZSBjYWxsIHRvDQo+ID4gcG5m
c19zZXRfbGF5b3V0X3N0YXRlaWQoKSB1cGRhdGVzIHRoZSBsYXlvdXRfaGVhZGVyICdiYXJyaWVy
Jw0KPiA+IHNlcXVlbmNlIGlkLCB3aGljaCB0cmlnZ2VycyB0aGUgdGVzdCBpbiBwbmZzX2xheW91
dGdldHNfYmxvY2tlZCgpDQo+ID4gYW5kIGlzIGxlc3MgcmFjZS1wcm9uZSB0aGFuIE5GUzRDTE5U
X0xBWU9VVFJFQ0FMTCBhbnl3YXkuDQo+ID4gDQo+ID4gU2lnbmVkLW9mZi1ieTogVHJvbmQgTXlr
bGVidXN0IDxUcm9uZC5NeWtsZWJ1c3RAbmV0YXBwLmNvbT4NCj4gPiAtLS0NCj4gPiAgZnMvbmZz
L3BuZnMuYyB8ICAgIDYgKystLS0tDQo+ID4gIDEgZmlsZXMgY2hhbmdlZCwgMiBpbnNlcnRpb25z
KCspLCA0IGRlbGV0aW9ucygtKQ0KPiA+IA0KPiA+IGRpZmYgLS1naXQgYS9mcy9uZnMvcG5mcy5j
IGIvZnMvbmZzL3BuZnMuYw0KPiA+IGluZGV4IGE1MzQyMTYuLjljMTlmOTIgMTAwNjQ0DQo+ID4g
LS0tIGEvZnMvbmZzL3BuZnMuYw0KPiA+ICsrKyBiL2ZzL25mcy9wbmZzLmMNCj4gPiBAQCAtOTY2
LDggKzk2Niw3IEBAIHBuZnNfdXBkYXRlX2xheW91dChzdHJ1Y3QgaW5vZGUgKmlubywNCj4gPiAg
CX0NCj4gPiAgDQo+ID4gIAkvKiBEbyB3ZSBldmVuIG5lZWQgdG8gYm90aGVyIHdpdGggdGhpcz8g
Ki8NCj4gPiAtCWlmICh0ZXN0X2JpdChORlM0Q0xOVF9MQVlPVVRSRUNBTEwsICZjbHAtPmNsX3N0
YXRlKSB8fA0KPiA+IC0JICAgIHRlc3RfYml0KE5GU19MQVlPVVRfQlVMS19SRUNBTEwsICZsby0+
cGxoX2ZsYWdzKSkgew0KPiA+ICsJaWYgKHRlc3RfYml0KE5GU19MQVlPVVRfQlVMS19SRUNBTEws
ICZsby0+cGxoX2ZsYWdzKSkgew0KPiA+ICAJCWRwcmludGsoIiVzIG1hdGNoZXMgcmVjYWxsLCB1
c2UgTURTXG4iLCBfX2Z1bmNfXyk7DQo+ID4gIAkJZ290byBvdXRfdW5sb2NrOw0KPiA+ICAJfQ0K
PiA+IEBAIC0xMDQ4LDggKzEwNDcsNyBAQCBwbmZzX2xheW91dF9wcm9jZXNzKHN0cnVjdCBuZnM0
X2xheW91dGdldCAqbGdwKQ0KPiA+ICAJfQ0KPiA+ICANCj4gPiAgCXNwaW5fbG9jaygmaW5vLT5p
X2xvY2spOw0KPiA+IC0JaWYgKHRlc3RfYml0KE5GUzRDTE5UX0xBWU9VVFJFQ0FMTCwgJmNscC0+
Y2xfc3RhdGUpIHx8DQo+ID4gLQkgICAgdGVzdF9iaXQoTkZTX0xBWU9VVF9CVUxLX1JFQ0FMTCwg
JmxvLT5wbGhfZmxhZ3MpKSB7DQo+IA0KPiANCj4gVGhpcyB3YXMgdGhlIG9ubHkgcGxhY2UgdGhh
dCB0aGUgY2xwIHN0cnVjdHVyZSB3YXMgdXNlZCwgc28geW91IGNvdWxkIHByb2JhYmx5IHJlbW92
ZSBpdCB0byBhdm9pZCB0aGlzIHdhcm5pbmc6DQo+IA0KPiBmcy9uZnMvcG5mcy5jOiBJbiBmdW5j
dGlvbiAncG5mc19sYXlvdXRfcHJvY2Vzcyc6DQo+IGZzL25mcy9wbmZzLmM6MTA1ODoyMTogd2Fy
bmluZzogdW51c2VkIHZhcmlhYmxlICdjbHAnIFstV3VudXNlZC12YXJpYWJsZV0NCg0KRG9uZS4g
VGhhbmtzIGZvciBzcG90dGluZyB0aGF0IQ0KDQotLSANClRyb25kIE15a2xlYnVzdA0KTGludXgg
TkZTIGNsaWVudCBtYWludGFpbmVyDQoNCk5ldEFwcA0KVHJvbmQuTXlrbGVidXN0QG5ldGFwcC5j
b20NCnd3dy5uZXRhcHAuY29tDQoNCg==