2017-04-18 15:40:12

by Benjamin Coddington

[permalink] [raw]
Subject: [PATCH 1/2] NFS: move nfs_pgarray_set() to open code

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 <[email protected]>
---
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



2017-04-18 15:40:12

by Benjamin Coddington

[permalink] [raw]
Subject: [PATCH 2/2] NFS: Use GFP_NOIO for two allocations in writeback

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

Signed-off-by: Benjamin Coddington <[email protected]>
---
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


2017-04-18 16:42:54

by Trond Myklebust

[permalink] [raw]
Subject: Re: [PATCH 2/2] NFS: Use GFP_NOIO for two allocations in writeback

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=

2017-04-18 20:32:06

by Benjamin Coddington

[permalink] [raw]
Subject: Re: [PATCH 2/2] NFS: Use GFP_NOIO for two allocations in writeback

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 <[email protected]>
>> ---
>>  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

2017-04-19 10:08:04

by Benjamin Coddington

[permalink] [raw]
Subject: Re: [PATCH 2/2] NFS: Use GFP_NOIO for two allocations in writeback

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 <[email protected]>
>>> ---
>>>  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

2017-04-19 12:26:19

by Benjamin Coddington

[permalink] [raw]
Subject: Re: [PATCH 2/2] NFS: Use GFP_NOIO for two allocations in writeback

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 <[email protected]>
>>>> ---
>>>>  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

2017-04-19 14:11:37

by Benjamin Coddington

[permalink] [raw]
Subject: [PATCH v2 3/3] NFS: move rw_mode to nfs_pageio_header

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

Signed-off-by: Benjamin Coddington <[email protected]>
---
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


2017-04-19 14:11:42

by Benjamin Coddington

[permalink] [raw]
Subject: [PATCH v2 2/3] NFS: move nfs_pgarray_set() to open code

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 <[email protected]>
---
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


2017-04-19 14:11:42

by Benjamin Coddington

[permalink] [raw]
Subject: [PATCH v2 1/3] NFS: Use GFP_NOIO for two allocations in writeback

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

Signed-off-by: Benjamin Coddington <[email protected]>
---
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