All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] NFS: move nfs_pgarray_set() to open code
@ 2017-04-18 15:40 Benjamin Coddington
  2017-04-18 15:40 ` [PATCH 2/2] NFS: Use GFP_NOIO for two allocations in writeback Benjamin Coddington
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Benjamin Coddington @ 2017-04-18 15:40 UTC (permalink / raw)
  To: Trond Myklebust, Anna Schumaker; +Cc: linux-nfs

Since commit 00bfa30abe86 ("NFS: Create a common pgio_alloc and
pgio_release function"), nfs_pgarray_set() has only a single caller.  Let's
open code it to make it easier to use the correct GFP_ flags in a following
patch.

Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
---
 fs/nfs/pagelist.c | 29 ++++++++++++-----------------
 1 file changed, 12 insertions(+), 17 deletions(-)

diff --git a/fs/nfs/pagelist.c b/fs/nfs/pagelist.c
index 6e629b856a00..19eaae0dee51 100644
--- a/fs/nfs/pagelist.c
+++ b/fs/nfs/pagelist.c
@@ -29,19 +29,6 @@
 static struct kmem_cache *nfs_page_cachep;
 static const struct rpc_call_ops nfs_pgio_common_ops;
 
-static bool nfs_pgarray_set(struct nfs_page_array *p, unsigned int pagecount)
-{
-	p->npages = pagecount;
-	if (pagecount <= ARRAY_SIZE(p->page_array))
-		p->pagevec = p->page_array;
-	else {
-		p->pagevec = kcalloc(pagecount, sizeof(struct page *), GFP_KERNEL);
-		if (!p->pagevec)
-			p->npages = 0;
-	}
-	return p->pagevec != NULL;
-}
-
 struct nfs_pgio_mirror *
 nfs_pgio_current_mirror(struct nfs_pageio_descriptor *desc)
 {
@@ -754,13 +741,21 @@ int nfs_generic_pgio(struct nfs_pageio_descriptor *desc,
 				*last_page;
 	struct list_head *head = &mirror->pg_list;
 	struct nfs_commit_info cinfo;
+	struct nfs_page_array *pg_array = &hdr->page_array;
 	unsigned int pagecount, pageused;
 
 	pagecount = nfs_page_array_len(mirror->pg_base, mirror->pg_count);
-	if (!nfs_pgarray_set(&hdr->page_array, pagecount)) {
-		nfs_pgio_error(hdr);
-		desc->pg_error = -ENOMEM;
-		return desc->pg_error;
+
+	if (pagecount <= ARRAY_SIZE(pg_array->page_array))
+		pg_array->pagevec = pg_array->page_array;
+	else {
+		pg_array->pagevec = kcalloc(pagecount, sizeof(struct page *), GFP_KERNEL);
+		if (!pg_array->pagevec) {
+			pg_array->npages = 0;
+			nfs_pgio_error(hdr);
+			desc->pg_error = -ENOMEM;
+			return desc->pg_error;
+		}
 	}
 
 	nfs_init_cinfo(&cinfo, desc->pg_inode, desc->pg_dreq);
-- 
2.9.3


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

* [PATCH 2/2] NFS: Use GFP_NOIO for two allocations in writeback
  2017-04-18 15:40 [PATCH 1/2] NFS: move nfs_pgarray_set() to open code Benjamin Coddington
@ 2017-04-18 15:40 ` Benjamin Coddington
  2017-04-18 16:42   ` Trond Myklebust
  2017-04-19 14:11 ` [PATCH v2 1/3] " Benjamin Coddington
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Benjamin Coddington @ 2017-04-18 15:40 UTC (permalink / raw)
  To: Trond Myklebust, Anna Schumaker; +Cc: linux-nfs

Prevent a deadlock that can occur if we wait on allocations
that try to write back our pages.

Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
---
 fs/nfs/pagelist.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/pagelist.c b/fs/nfs/pagelist.c
index 19eaae0dee51..fa2924ce5a78 100644
--- a/fs/nfs/pagelist.c
+++ b/fs/nfs/pagelist.c
@@ -668,6 +668,7 @@ void nfs_pageio_init(struct nfs_pageio_descriptor *desc,
 {
 	struct nfs_pgio_mirror *new;
 	int i;
+	gfp_t gfp_flags = GFP_KERNEL;
 
 	desc->pg_moreio = 0;
 	desc->pg_inode = inode;
@@ -687,8 +688,10 @@ void nfs_pageio_init(struct nfs_pageio_descriptor *desc,
 	if (pg_ops->pg_get_mirror_count) {
 		/* until we have a request, we don't have an lseg and no
 		 * idea how many mirrors there will be */
+		if (desc->pg_rw_ops->rw_mode == FMODE_WRITE)
+			gfp_flags = GFP_NOIO;
 		new = kcalloc(NFS_PAGEIO_DESCRIPTOR_MIRROR_MAX,
-			      sizeof(struct nfs_pgio_mirror), GFP_KERNEL);
+			      sizeof(struct nfs_pgio_mirror), gfp_flags);
 		desc->pg_mirrors_dynamic = new;
 		desc->pg_mirrors = new;
 
@@ -743,13 +746,16 @@ int nfs_generic_pgio(struct nfs_pageio_descriptor *desc,
 	struct nfs_commit_info cinfo;
 	struct nfs_page_array *pg_array = &hdr->page_array;
 	unsigned int pagecount, pageused;
+	gfp_t gfp_flags = GFP_KERNEL;
 
 	pagecount = nfs_page_array_len(mirror->pg_base, mirror->pg_count);
 
 	if (pagecount <= ARRAY_SIZE(pg_array->page_array))
 		pg_array->pagevec = pg_array->page_array;
 	else {
-		pg_array->pagevec = kcalloc(pagecount, sizeof(struct page *), GFP_KERNEL);
+		if (desc->pg_rw_ops->rw_mode == FMODE_WRITE)
+			gfp_flags = GFP_NOIO;
+		pg_array->pagevec = kcalloc(pagecount, sizeof(struct page *), gfp_flags);
 		if (!pg_array->pagevec) {
 			pg_array->npages = 0;
 			nfs_pgio_error(hdr);
-- 
2.9.3


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

* Re: [PATCH 2/2] NFS: Use GFP_NOIO for two allocations in writeback
  2017-04-18 15:40 ` [PATCH 2/2] NFS: Use GFP_NOIO for two allocations in writeback Benjamin Coddington
@ 2017-04-18 16:42   ` Trond Myklebust
  2017-04-18 20:32     ` Benjamin Coddington
  0 siblings, 1 reply; 9+ messages in thread
From: Trond Myklebust @ 2017-04-18 16:42 UTC (permalink / raw)
  To: bcodding, anna.schumaker; +Cc: linux-nfs

SGkgQmVuLA0KDQpPbiBUdWUsIDIwMTctMDQtMTggYXQgMTE6NDAgLTA0MDAsIEJlbmphbWluIENv
ZGRpbmd0b24gd3JvdGU6DQo+IFByZXZlbnQgYSBkZWFkbG9jayB0aGF0IGNhbiBvY2N1ciBpZiB3
ZSB3YWl0IG9uIGFsbG9jYXRpb25zDQo+IHRoYXQgdHJ5IHRvIHdyaXRlIGJhY2sgb3VyIHBhZ2Vz
Lg0KPiANCj4gU2lnbmVkLW9mZi1ieTogQmVuamFtaW4gQ29kZGluZ3RvbiA8YmNvZGRpbmdAcmVk
aGF0LmNvbT4NCj4gLS0tDQo+IMKgZnMvbmZzL3BhZ2VsaXN0LmMgfCAxMCArKysrKysrKy0tDQo+
IMKgMSBmaWxlIGNoYW5nZWQsIDggaW5zZXJ0aW9ucygrKSwgMiBkZWxldGlvbnMoLSkNCj4gDQo+
IGRpZmYgLS1naXQgYS9mcy9uZnMvcGFnZWxpc3QuYyBiL2ZzL25mcy9wYWdlbGlzdC5jDQo+IGlu
ZGV4IDE5ZWFhZTBkZWU1MS4uZmEyOTI0Y2U1YTc4IDEwMDY0NA0KPiAtLS0gYS9mcy9uZnMvcGFn
ZWxpc3QuYw0KPiArKysgYi9mcy9uZnMvcGFnZWxpc3QuYw0KPiBAQCAtNjY4LDYgKzY2OCw3IEBA
IHZvaWQgbmZzX3BhZ2Vpb19pbml0KHN0cnVjdCBuZnNfcGFnZWlvX2Rlc2NyaXB0b3INCj4gKmRl
c2MsDQo+IMKgew0KPiDCoAlzdHJ1Y3QgbmZzX3BnaW9fbWlycm9yICpuZXc7DQo+IMKgCWludCBp
Ow0KPiArCWdmcF90IGdmcF9mbGFncyA9IEdGUF9LRVJORUw7DQo+IMKgDQo+IMKgCWRlc2MtPnBn
X21vcmVpbyA9IDA7DQo+IMKgCWRlc2MtPnBnX2lub2RlID0gaW5vZGU7DQo+IEBAIC02ODcsOCAr
Njg4LDEwIEBAIHZvaWQgbmZzX3BhZ2Vpb19pbml0KHN0cnVjdA0KPiBuZnNfcGFnZWlvX2Rlc2Ny
aXB0b3IgKmRlc2MsDQo+IMKgCWlmIChwZ19vcHMtPnBnX2dldF9taXJyb3JfY291bnQpIHsNCj4g
wqAJCS8qIHVudGlsIHdlIGhhdmUgYSByZXF1ZXN0LCB3ZSBkb24ndCBoYXZlIGFuIGxzZWcNCj4g
YW5kIG5vDQo+IMKgCQnCoCogaWRlYSBob3cgbWFueSBtaXJyb3JzIHRoZXJlIHdpbGwgYmUgKi8N
Cj4gKwkJaWYgKGRlc2MtPnBnX3J3X29wcy0+cndfbW9kZSA9PSBGTU9ERV9XUklURSkNCg0KQ2Fu
IHdlIHJhdGhlciByZXBsYWNlIHRoaXMgd2l0aCBhIG5ldyBmaWVsZCBpbiBzdHJ1Y3QNCm5mc19w
YWdlaW9fZGVzY3JpcHRvcj8gSSB3YW50IHRvIGdldCByaWQgb2YgcGdfcndfb3BzLT5yd19tb2Rl
OyBpdCdzDQpzb21ldGhpbmcgdGhhdCBzbGlwcGVkIHRocm91Z2ggdGhlIGNyYWNrcy4NCg0KQWxz
bywgcGxlYXNlIHB1dCB0aGlzIHBhdGNoIGJlZm9yZSB0aGUgb3RoZXIgc28gdGhhdCBpdCBjYW4g
YmUgZWFzaWx5DQpwcm9wYWdhdGVkIGFzIGEgc3RhYmxlIHBhdGNoLg0KDQpUaGFua3MhDQogIFRy
b25kDQoNCi0tIA0KVHJvbmQgTXlrbGVidXN0DQpMaW51eCBORlMgY2xpZW50IG1haW50YWluZXIs
IFByaW1hcnlEYXRhDQp0cm9uZC5teWtsZWJ1c3RAcHJpbWFyeWRhdGEuY29tDQo=

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

* Re: [PATCH 2/2] NFS: Use GFP_NOIO for two allocations in writeback
  2017-04-18 16:42   ` Trond Myklebust
@ 2017-04-18 20:32     ` Benjamin Coddington
  2017-04-19 10:08       ` Benjamin Coddington
  0 siblings, 1 reply; 9+ messages in thread
From: Benjamin Coddington @ 2017-04-18 20:32 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: anna.schumaker, linux-nfs

On 18 Apr 2017, at 12:42, Trond Myklebust wrote:

> Hi Ben,
>
> On Tue, 2017-04-18 at 11:40 -0400, Benjamin Coddington wrote:
>> Prevent a deadlock that can occur if we wait on allocations
>> that try to write back our pages.
>>
>> Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
>> ---
>>  fs/nfs/pagelist.c | 10 ++++++++--
>>  1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/nfs/pagelist.c b/fs/nfs/pagelist.c
>> index 19eaae0dee51..fa2924ce5a78 100644
>> --- a/fs/nfs/pagelist.c
>> +++ b/fs/nfs/pagelist.c
>> @@ -668,6 +668,7 @@ void nfs_pageio_init(struct nfs_pageio_descriptor
>> *desc,
>>  {
>>  	struct nfs_pgio_mirror *new;
>>  	int i;
>> +	gfp_t gfp_flags = GFP_KERNEL;
>>  
>>  	desc->pg_moreio = 0;
>>  	desc->pg_inode = inode;
>> @@ -687,8 +688,10 @@ void nfs_pageio_init(struct
>> nfs_pageio_descriptor *desc,
>>  	if (pg_ops->pg_get_mirror_count) {
>>  		/* until we have a request, we don't have an lseg
>> and no
>>  		 * idea how many mirrors there will be */
>> +		if (desc->pg_rw_ops->rw_mode == FMODE_WRITE)
>
> Can we rather replace this with a new field in struct
> nfs_pageio_descriptor? I want to get rid of pg_rw_ops->rw_mode; it's
> something that slipped through the cracks.

I can do this.  It might make sense to just replace rw_mode with something
like rw_gfp_flags..  Otherwise, this looks like another argument to
nfs_pageio_init(), unless we can piggy-back on pg_ioflags.

> Also, please put this patch before the other so that it can be easily
> propagated as a stable patch.

Of course, that makes more sense too.

Ben

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

* Re: [PATCH 2/2] NFS: Use GFP_NOIO for two allocations in writeback
  2017-04-18 20:32     ` Benjamin Coddington
@ 2017-04-19 10:08       ` Benjamin Coddington
  2017-04-19 12:26         ` Benjamin Coddington
  0 siblings, 1 reply; 9+ messages in thread
From: Benjamin Coddington @ 2017-04-19 10:08 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: anna.schumaker, linux-nfs

On 18 Apr 2017, at 16:32, Benjamin Coddington wrote:

> On 18 Apr 2017, at 12:42, Trond Myklebust wrote:
>
>> Hi Ben,
>>
>> On Tue, 2017-04-18 at 11:40 -0400, Benjamin Coddington wrote:
>>> Prevent a deadlock that can occur if we wait on allocations
>>> that try to write back our pages.
>>>
>>> Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
>>> ---
>>>  fs/nfs/pagelist.c | 10 ++++++++--
>>>  1 file changed, 8 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/fs/nfs/pagelist.c b/fs/nfs/pagelist.c
>>> index 19eaae0dee51..fa2924ce5a78 100644
>>> --- a/fs/nfs/pagelist.c
>>> +++ b/fs/nfs/pagelist.c
>>> @@ -668,6 +668,7 @@ void nfs_pageio_init(struct nfs_pageio_descriptor
>>> *desc,
>>>  {
>>>  	struct nfs_pgio_mirror *new;
>>>  	int i;
>>> +	gfp_t gfp_flags = GFP_KERNEL;
>>>  
>>>  	desc->pg_moreio = 0;
>>>  	desc->pg_inode = inode;
>>> @@ -687,8 +688,10 @@ void nfs_pageio_init(struct
>>> nfs_pageio_descriptor *desc,
>>>  	if (pg_ops->pg_get_mirror_count) {
>>>  		/* until we have a request, we don't have an lseg
>>> and no
>>>  		 * idea how many mirrors there will be */
>>> +		if (desc->pg_rw_ops->rw_mode == FMODE_WRITE)
>>
>> Can we rather replace this with a new field in struct
>> nfs_pageio_descriptor? I want to get rid of pg_rw_ops->rw_mode; it's
>> something that slipped through the cracks.
>
> I can do this.  It might make sense to just replace rw_mode with something
> like rw_gfp_flags..  Otherwise, this looks like another argument to
> nfs_pageio_init(), unless we can piggy-back on pg_ioflags.

But that won't work, since we'll need something to laster figure out whether
a read or write delegation stateid can be used.  Maybe you were instead
suggesting to move the rw_mode to the pageio_descriptor or header..

Ben

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

* Re: [PATCH 2/2] NFS: Use GFP_NOIO for two allocations in writeback
  2017-04-19 10:08       ` Benjamin Coddington
@ 2017-04-19 12:26         ` Benjamin Coddington
  0 siblings, 0 replies; 9+ messages in thread
From: Benjamin Coddington @ 2017-04-19 12:26 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: anna.schumaker, linux-nfs

On 19 Apr 2017, at 6:08, Benjamin Coddington wrote:

> On 18 Apr 2017, at 16:32, Benjamin Coddington wrote:
>
>> On 18 Apr 2017, at 12:42, Trond Myklebust wrote:
>>
>>> Hi Ben,
>>>
>>> On Tue, 2017-04-18 at 11:40 -0400, Benjamin Coddington wrote:
>>>> Prevent a deadlock that can occur if we wait on allocations
>>>> that try to write back our pages.
>>>>
>>>> Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
>>>> ---
>>>>  fs/nfs/pagelist.c | 10 ++++++++--
>>>>  1 file changed, 8 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/fs/nfs/pagelist.c b/fs/nfs/pagelist.c
>>>> index 19eaae0dee51..fa2924ce5a78 100644
>>>> --- a/fs/nfs/pagelist.c
>>>> +++ b/fs/nfs/pagelist.c
>>>> @@ -668,6 +668,7 @@ void nfs_pageio_init(struct 
>>>> nfs_pageio_descriptor
>>>> *desc,
>>>>  {
>>>>  	struct nfs_pgio_mirror *new;
>>>>  	int i;
>>>> +	gfp_t gfp_flags = GFP_KERNEL;
>>>>  
>>>>  	desc->pg_moreio = 0;
>>>>  	desc->pg_inode = inode;
>>>> @@ -687,8 +688,10 @@ void nfs_pageio_init(struct
>>>> nfs_pageio_descriptor *desc,
>>>>  	if (pg_ops->pg_get_mirror_count) {
>>>>  		/* until we have a request, we don't have an lseg
>>>> and no
>>>>  		 * idea how many mirrors there will be */
>>>> +		if (desc->pg_rw_ops->rw_mode == FMODE_WRITE)
>>>
>>> Can we rather replace this with a new field in struct
>>> nfs_pageio_descriptor? I want to get rid of pg_rw_ops->rw_mode; it's
>>> something that slipped through the cracks.
>>
>> I can do this.  It might make sense to just replace rw_mode with 
>> something
>> like rw_gfp_flags..  Otherwise, this looks like another argument to
>> nfs_pageio_init(), unless we can piggy-back on pg_ioflags.
>
> But that won't work, since we'll need something to laster figure out 
> whether
> a read or write delegation stateid can be used.  Maybe you were 
> instead
> suggesting to move the rw_mode to the pageio_descriptor or header..

I've been trying to figure out why you want to get rid of
pg_rw_ops->rw_mode, and think that it's because it would be better to 
have
it in a cacheline in nfs4_proc_pgio_rpc_prepare(), so I'm going to move 
it
to the nfs_pgio_header.  There's a 4-byte hole after nfs_writeverf on
x86_64.

The updates will continue until a request for decreased verbosity.  :)

Ben

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

* [PATCH v2 1/3] NFS: Use GFP_NOIO for two allocations in writeback
  2017-04-18 15:40 [PATCH 1/2] NFS: move nfs_pgarray_set() to open code Benjamin Coddington
  2017-04-18 15:40 ` [PATCH 2/2] NFS: Use GFP_NOIO for two allocations in writeback Benjamin Coddington
@ 2017-04-19 14:11 ` Benjamin Coddington
  2017-04-19 14:11 ` [PATCH v2 2/3] NFS: move nfs_pgarray_set() to open code Benjamin Coddington
  2017-04-19 14:11 ` [PATCH v2 3/3] NFS: move rw_mode to nfs_pageio_header Benjamin Coddington
  3 siblings, 0 replies; 9+ messages in thread
From: Benjamin Coddington @ 2017-04-19 14:11 UTC (permalink / raw)
  To: Trond Myklebust, Anna Schumaker; +Cc: linux-nfs

Prevent a deadlock that can occur if we wait on allocations
that try to write back our pages.

Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
---
 fs/nfs/pagelist.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/fs/nfs/pagelist.c b/fs/nfs/pagelist.c
index 6e629b856a00..cd6ec9bdd1c7 100644
--- a/fs/nfs/pagelist.c
+++ b/fs/nfs/pagelist.c
@@ -29,13 +29,14 @@
 static struct kmem_cache *nfs_page_cachep;
 static const struct rpc_call_ops nfs_pgio_common_ops;
 
-static bool nfs_pgarray_set(struct nfs_page_array *p, unsigned int pagecount)
+static bool nfs_pgarray_set(struct nfs_page_array *p, unsigned int pagecount,
+					gfp_t gfp_flags)
 {
 	p->npages = pagecount;
 	if (pagecount <= ARRAY_SIZE(p->page_array))
 		p->pagevec = p->page_array;
 	else {
-		p->pagevec = kcalloc(pagecount, sizeof(struct page *), GFP_KERNEL);
+		p->pagevec = kcalloc(pagecount, sizeof(struct page *), gfp_flags);
 		if (!p->pagevec)
 			p->npages = 0;
 	}
@@ -681,6 +682,7 @@ void nfs_pageio_init(struct nfs_pageio_descriptor *desc,
 {
 	struct nfs_pgio_mirror *new;
 	int i;
+	gfp_t gfp_flags = GFP_KERNEL;
 
 	desc->pg_moreio = 0;
 	desc->pg_inode = inode;
@@ -700,8 +702,10 @@ void nfs_pageio_init(struct nfs_pageio_descriptor *desc,
 	if (pg_ops->pg_get_mirror_count) {
 		/* until we have a request, we don't have an lseg and no
 		 * idea how many mirrors there will be */
+		if (desc->pg_rw_ops->rw_mode == FMODE_WRITE)
+			gfp_flags = GFP_NOIO;
 		new = kcalloc(NFS_PAGEIO_DESCRIPTOR_MIRROR_MAX,
-			      sizeof(struct nfs_pgio_mirror), GFP_KERNEL);
+			      sizeof(struct nfs_pgio_mirror), gfp_flags);
 		desc->pg_mirrors_dynamic = new;
 		desc->pg_mirrors = new;
 
@@ -755,9 +759,12 @@ int nfs_generic_pgio(struct nfs_pageio_descriptor *desc,
 	struct list_head *head = &mirror->pg_list;
 	struct nfs_commit_info cinfo;
 	unsigned int pagecount, pageused;
+	gfp_t gfp_flags = GFP_KERNEL;
 
 	pagecount = nfs_page_array_len(mirror->pg_base, mirror->pg_count);
-	if (!nfs_pgarray_set(&hdr->page_array, pagecount)) {
+	if (desc->pg_rw_ops->rw_mode == FMODE_WRITE)
+		gfp_flags = GFP_NOIO;
+	if (!nfs_pgarray_set(&hdr->page_array, pagecount, gfp_flags)) {
 		nfs_pgio_error(hdr);
 		desc->pg_error = -ENOMEM;
 		return desc->pg_error;
-- 
2.9.3


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

* [PATCH v2 2/3] NFS: move nfs_pgarray_set() to open code
  2017-04-18 15:40 [PATCH 1/2] NFS: move nfs_pgarray_set() to open code Benjamin Coddington
  2017-04-18 15:40 ` [PATCH 2/2] NFS: Use GFP_NOIO for two allocations in writeback Benjamin Coddington
  2017-04-19 14:11 ` [PATCH v2 1/3] " Benjamin Coddington
@ 2017-04-19 14:11 ` Benjamin Coddington
  2017-04-19 14:11 ` [PATCH v2 3/3] NFS: move rw_mode to nfs_pageio_header Benjamin Coddington
  3 siblings, 0 replies; 9+ messages in thread
From: Benjamin Coddington @ 2017-04-19 14:11 UTC (permalink / raw)
  To: Trond Myklebust, Anna Schumaker; +Cc: linux-nfs

Since commit 00bfa30abe86 ("NFS: Create a common pgio_alloc and
pgio_release function"), nfs_pgarray_set() has only a single caller.  Let's
open code it.

Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
---
 fs/nfs/pagelist.c | 34 ++++++++++++++--------------------
 1 file changed, 14 insertions(+), 20 deletions(-)

diff --git a/fs/nfs/pagelist.c b/fs/nfs/pagelist.c
index cd6ec9bdd1c7..fa2924ce5a78 100644
--- a/fs/nfs/pagelist.c
+++ b/fs/nfs/pagelist.c
@@ -29,20 +29,6 @@
 static struct kmem_cache *nfs_page_cachep;
 static const struct rpc_call_ops nfs_pgio_common_ops;
 
-static bool nfs_pgarray_set(struct nfs_page_array *p, unsigned int pagecount,
-					gfp_t gfp_flags)
-{
-	p->npages = pagecount;
-	if (pagecount <= ARRAY_SIZE(p->page_array))
-		p->pagevec = p->page_array;
-	else {
-		p->pagevec = kcalloc(pagecount, sizeof(struct page *), gfp_flags);
-		if (!p->pagevec)
-			p->npages = 0;
-	}
-	return p->pagevec != NULL;
-}
-
 struct nfs_pgio_mirror *
 nfs_pgio_current_mirror(struct nfs_pageio_descriptor *desc)
 {
@@ -758,16 +744,24 @@ int nfs_generic_pgio(struct nfs_pageio_descriptor *desc,
 				*last_page;
 	struct list_head *head = &mirror->pg_list;
 	struct nfs_commit_info cinfo;
+	struct nfs_page_array *pg_array = &hdr->page_array;
 	unsigned int pagecount, pageused;
 	gfp_t gfp_flags = GFP_KERNEL;
 
 	pagecount = nfs_page_array_len(mirror->pg_base, mirror->pg_count);
-	if (desc->pg_rw_ops->rw_mode == FMODE_WRITE)
-		gfp_flags = GFP_NOIO;
-	if (!nfs_pgarray_set(&hdr->page_array, pagecount, gfp_flags)) {
-		nfs_pgio_error(hdr);
-		desc->pg_error = -ENOMEM;
-		return desc->pg_error;
+
+	if (pagecount <= ARRAY_SIZE(pg_array->page_array))
+		pg_array->pagevec = pg_array->page_array;
+	else {
+		if (desc->pg_rw_ops->rw_mode == FMODE_WRITE)
+			gfp_flags = GFP_NOIO;
+		pg_array->pagevec = kcalloc(pagecount, sizeof(struct page *), gfp_flags);
+		if (!pg_array->pagevec) {
+			pg_array->npages = 0;
+			nfs_pgio_error(hdr);
+			desc->pg_error = -ENOMEM;
+			return desc->pg_error;
+		}
 	}
 
 	nfs_init_cinfo(&cinfo, desc->pg_inode, desc->pg_dreq);
-- 
2.9.3


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

* [PATCH v2 3/3] NFS: move rw_mode to nfs_pageio_header
  2017-04-18 15:40 [PATCH 1/2] NFS: move nfs_pgarray_set() to open code Benjamin Coddington
                   ` (2 preceding siblings ...)
  2017-04-19 14:11 ` [PATCH v2 2/3] NFS: move nfs_pgarray_set() to open code Benjamin Coddington
@ 2017-04-19 14:11 ` Benjamin Coddington
  3 siblings, 0 replies; 9+ messages in thread
From: Benjamin Coddington @ 2017-04-19 14:11 UTC (permalink / raw)
  To: Trond Myklebust, Anna Schumaker; +Cc: linux-nfs

Let's try to have it in a cacheline in nfs4_proc_pgio_rpc_prepare().

Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
---
 fs/nfs/nfs4proc.c        | 2 +-
 fs/nfs/pagelist.c        | 8 +++-----
 fs/nfs/read.c            | 9 ++++++---
 fs/nfs/write.c           | 7 ++++---
 include/linux/nfs_page.h | 4 ++--
 include/linux/nfs_xdr.h  | 1 +
 6 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 201ca3f2c4ba..a725de5b7a08 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -4610,7 +4610,7 @@ static int nfs4_proc_pgio_rpc_prepare(struct rpc_task *task,
 		return 0;
 	if (nfs4_set_rw_stateid(&hdr->args.stateid, hdr->args.context,
 				hdr->args.lock_context,
-				hdr->rw_ops->rw_mode) == -EIO)
+				hdr->rw_mode) == -EIO)
 		return -EIO;
 	if (unlikely(test_bit(NFS_CONTEXT_BAD, &hdr->args.context->flags)))
 		return -EIO;
diff --git a/fs/nfs/pagelist.c b/fs/nfs/pagelist.c
index fa2924ce5a78..fac370bc0d6e 100644
--- a/fs/nfs/pagelist.c
+++ b/fs/nfs/pagelist.c
@@ -664,11 +664,11 @@ void nfs_pageio_init(struct nfs_pageio_descriptor *desc,
 		     const struct nfs_pgio_completion_ops *compl_ops,
 		     const struct nfs_rw_ops *rw_ops,
 		     size_t bsize,
-		     int io_flags)
+		     int io_flags,
+		     gfp_t gfp_flags)
 {
 	struct nfs_pgio_mirror *new;
 	int i;
-	gfp_t gfp_flags = GFP_KERNEL;
 
 	desc->pg_moreio = 0;
 	desc->pg_inode = inode;
@@ -688,8 +688,6 @@ void nfs_pageio_init(struct nfs_pageio_descriptor *desc,
 	if (pg_ops->pg_get_mirror_count) {
 		/* until we have a request, we don't have an lseg and no
 		 * idea how many mirrors there will be */
-		if (desc->pg_rw_ops->rw_mode == FMODE_WRITE)
-			gfp_flags = GFP_NOIO;
 		new = kcalloc(NFS_PAGEIO_DESCRIPTOR_MIRROR_MAX,
 			      sizeof(struct nfs_pgio_mirror), gfp_flags);
 		desc->pg_mirrors_dynamic = new;
@@ -753,7 +751,7 @@ int nfs_generic_pgio(struct nfs_pageio_descriptor *desc,
 	if (pagecount <= ARRAY_SIZE(pg_array->page_array))
 		pg_array->pagevec = pg_array->page_array;
 	else {
-		if (desc->pg_rw_ops->rw_mode == FMODE_WRITE)
+		if (hdr->rw_mode == FMODE_WRITE)
 			gfp_flags = GFP_NOIO;
 		pg_array->pagevec = kcalloc(pagecount, sizeof(struct page *), gfp_flags);
 		if (!pg_array->pagevec) {
diff --git a/fs/nfs/read.c b/fs/nfs/read.c
index defc9233e985..a8421d9dab6a 100644
--- a/fs/nfs/read.c
+++ b/fs/nfs/read.c
@@ -35,7 +35,11 @@ static struct kmem_cache *nfs_rdata_cachep;
 
 static struct nfs_pgio_header *nfs_readhdr_alloc(void)
 {
-	return kmem_cache_zalloc(nfs_rdata_cachep, GFP_KERNEL);
+	struct nfs_pgio_header *p = kmem_cache_zalloc(nfs_rdata_cachep, GFP_KERNEL);
+
+	if (p)
+		p->rw_mode = FMODE_READ;
+	return p;
 }
 
 static void nfs_readhdr_free(struct nfs_pgio_header *rhdr)
@@ -64,7 +68,7 @@ void nfs_pageio_init_read(struct nfs_pageio_descriptor *pgio,
 		pg_ops = server->pnfs_curr_ld->pg_read_ops;
 #endif
 	nfs_pageio_init(pgio, inode, pg_ops, compl_ops, &nfs_rw_read_ops,
-			server->rsize, 0);
+			server->rsize, 0, GFP_KERNEL);
 }
 EXPORT_SYMBOL_GPL(nfs_pageio_init_read);
 
@@ -451,7 +455,6 @@ void nfs_destroy_readpagecache(void)
 }
 
 static const struct nfs_rw_ops nfs_rw_read_ops = {
-	.rw_mode		= FMODE_READ,
 	.rw_alloc_header	= nfs_readhdr_alloc,
 	.rw_free_header		= nfs_readhdr_free,
 	.rw_done		= nfs_readpage_done,
diff --git a/fs/nfs/write.c b/fs/nfs/write.c
index abb2c8a3be42..859a95b67abe 100644
--- a/fs/nfs/write.c
+++ b/fs/nfs/write.c
@@ -82,8 +82,10 @@ static struct nfs_pgio_header *nfs_writehdr_alloc(void)
 {
 	struct nfs_pgio_header *p = mempool_alloc(nfs_wdata_mempool, GFP_NOIO);
 
-	if (p)
+	if (p) {
 		memset(p, 0, sizeof(*p));
+		p->rw_mode = FMODE_WRITE;
+	}
 	return p;
 }
 
@@ -1368,7 +1370,7 @@ void nfs_pageio_init_write(struct nfs_pageio_descriptor *pgio,
 		pg_ops = server->pnfs_curr_ld->pg_write_ops;
 #endif
 	nfs_pageio_init(pgio, inode, pg_ops, compl_ops, &nfs_rw_write_ops,
-			server->wsize, ioflags);
+			server->wsize, ioflags, GFP_NOIO);
 }
 EXPORT_SYMBOL_GPL(nfs_pageio_init_write);
 
@@ -2108,7 +2110,6 @@ void nfs_destroy_writepagecache(void)
 }
 
 static const struct nfs_rw_ops nfs_rw_write_ops = {
-	.rw_mode		= FMODE_WRITE,
 	.rw_alloc_header	= nfs_writehdr_alloc,
 	.rw_free_header		= nfs_writehdr_free,
 	.rw_done		= nfs_writeback_done,
diff --git a/include/linux/nfs_page.h b/include/linux/nfs_page.h
index 957049f72290..6f01e28bba27 100644
--- a/include/linux/nfs_page.h
+++ b/include/linux/nfs_page.h
@@ -64,7 +64,6 @@ struct nfs_pageio_ops {
 };
 
 struct nfs_rw_ops {
-	const fmode_t rw_mode;
 	struct nfs_pgio_header *(*rw_alloc_header)(void);
 	void (*rw_free_header)(struct nfs_pgio_header *);
 	int  (*rw_done)(struct rpc_task *, struct nfs_pgio_header *,
@@ -124,7 +123,8 @@ extern	void nfs_pageio_init(struct nfs_pageio_descriptor *desc,
 			     const struct nfs_pgio_completion_ops *compl_ops,
 			     const struct nfs_rw_ops *rw_ops,
 			     size_t bsize,
-			     int how);
+			     int how,
+			     gfp_t gfp_flags);
 extern	int nfs_pageio_add_request(struct nfs_pageio_descriptor *,
 				   struct nfs_page *);
 extern  int nfs_pageio_resend(struct nfs_pageio_descriptor *,
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index 348f7c158084..51e27f9746ee 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -1427,6 +1427,7 @@ struct nfs_pgio_header {
 	struct list_head	pages;
 	struct nfs_page		*req;
 	struct nfs_writeverf	verf;		/* Used for writes */
+	fmode_t			rw_mode;
 	struct pnfs_layout_segment *lseg;
 	loff_t			io_start;
 	const struct rpc_call_ops *mds_ops;
-- 
2.9.3


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

end of thread, other threads:[~2017-04-19 14:11 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-18 15:40 [PATCH 1/2] NFS: move nfs_pgarray_set() to open code Benjamin Coddington
2017-04-18 15:40 ` [PATCH 2/2] NFS: Use GFP_NOIO for two allocations in writeback Benjamin Coddington
2017-04-18 16:42   ` Trond Myklebust
2017-04-18 20:32     ` Benjamin Coddington
2017-04-19 10:08       ` Benjamin Coddington
2017-04-19 12:26         ` Benjamin Coddington
2017-04-19 14:11 ` [PATCH v2 1/3] " Benjamin Coddington
2017-04-19 14:11 ` [PATCH v2 2/3] NFS: move nfs_pgarray_set() to open code Benjamin Coddington
2017-04-19 14:11 ` [PATCH v2 3/3] NFS: move rw_mode to nfs_pageio_header Benjamin Coddington

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.