[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20090817121137.GW12579@kernel.dk>
Date: Mon, 17 Aug 2009 14:11:38 +0200
From: Jens Axboe <jens.axboe@...cle.com>
To: linux-kernel@...r.kernel.org
Cc: zach.brown@...cle.com, Trond.Myklebust@...app.com
Subject: Re: [PATCH 1/3] direct-io: make O_DIRECT IO path be page based
On Mon, Aug 17 2009, Jens Axboe wrote:
> Switch the aops->direct_IO() and below code to use page arrays
> instead, so that it doesn't make any assumptions about who the pages
> belong to. This works directly for all users but NFS, which just
> uses the same helper that the generic mapping read/write functions
> also call.
A quick NFS test reveals a simple problem - it wants to free the
pagevec on IO completion, but for O_DIRECT it should not do that anymore
since it'll simply inherit the given page map.
There's a flag structure in the nfs_{read,write}_data structure, but I
was not able to find out which flags (if any?!) it actually uses. So I
just stuck an int in there. Trond?
diff --git a/fs/nfs/direct.c b/fs/nfs/direct.c
index aff6169..2913caf 100644
--- a/fs/nfs/direct.c
+++ b/fs/nfs/direct.c
@@ -302,7 +302,7 @@ static ssize_t nfs_direct_read_schedule_segment(struct nfs_direct_req *dreq,
bytes = min(rsize,count);
result = -ENOMEM;
- data = nfs_readdata_alloc(nfs_page_array_len(pgbase, bytes));
+ data = nfs_readdata_alloc(nfs_page_array_len(pgbase, bytes), 0);
if (unlikely(!data))
break;
@@ -699,7 +699,7 @@ static ssize_t nfs_direct_write_schedule_segment(struct nfs_direct_req *dreq,
bytes = min(wsize,count);
result = -ENOMEM;
- data = nfs_writedata_alloc(nfs_page_array_len(pgbase, bytes));
+ data = nfs_writedata_alloc(nfs_page_array_len(pgbase, bytes),0);
if (unlikely(!data))
break;
diff --git a/fs/nfs/read.c b/fs/nfs/read.c
index 12c9e66..86f96a4 100644
--- a/fs/nfs/read.c
+++ b/fs/nfs/read.c
@@ -38,7 +38,8 @@ static mempool_t *nfs_rdata_mempool;
#define MIN_POOL_READ (32)
-struct nfs_read_data *nfs_readdata_alloc(unsigned int pagecount)
+struct nfs_read_data *nfs_readdata_alloc(unsigned int pagecount,
+ int need_pagevec)
{
struct nfs_read_data *p = mempool_alloc(nfs_rdata_mempool, GFP_NOFS);
@@ -47,9 +48,11 @@ struct nfs_read_data *nfs_readdata_alloc(unsigned int pagecount)
INIT_LIST_HEAD(&p->pages);
p->npages = pagecount;
p->res.seq_res.sr_slotid = NFS4_MAX_SLOT_TABLE;
- if (pagecount <= ARRAY_SIZE(p->page_array))
+ if (!need_pagevec || pagecount <= ARRAY_SIZE(p->page_array)) {
p->pagevec = p->page_array;
- else {
+ p->free_pagevec = 0;
+ } else {
+ p->free_pagevec = 1;
p->pagevec = kcalloc(pagecount, sizeof(struct page *), GFP_NOFS);
if (!p->pagevec) {
mempool_free(p, nfs_rdata_mempool);
@@ -62,7 +65,7 @@ struct nfs_read_data *nfs_readdata_alloc(unsigned int pagecount)
void nfs_readdata_free(struct nfs_read_data *p)
{
- if (p && (p->pagevec != &p->page_array[0]))
+ if (p && p->free_pagevec)
kfree(p->pagevec);
mempool_free(p, nfs_rdata_mempool);
}
@@ -256,7 +259,7 @@ static int nfs_pagein_multi(struct inode *inode, struct list_head *head, unsigne
do {
size_t len = min(nbytes,rsize);
- data = nfs_readdata_alloc(1);
+ data = nfs_readdata_alloc(1, 1);
if (!data)
goto out_bad;
list_add(&data->pages, &list);
@@ -306,7 +309,7 @@ static int nfs_pagein_one(struct inode *inode, struct list_head *head, unsigned
struct nfs_read_data *data;
int ret = -ENOMEM;
- data = nfs_readdata_alloc(npages);
+ data = nfs_readdata_alloc(npages, 1);
if (!data)
goto out_bad;
diff --git a/fs/nfs/write.c b/fs/nfs/write.c
index a34fae2..defb266 100644
--- a/fs/nfs/write.c
+++ b/fs/nfs/write.c
@@ -60,12 +60,13 @@ struct nfs_write_data *nfs_commitdata_alloc(void)
void nfs_commit_free(struct nfs_write_data *p)
{
- if (p && (p->pagevec != &p->page_array[0]))
+ if (p && p->free_pagevec)
kfree(p->pagevec);
mempool_free(p, nfs_commit_mempool);
}
-struct nfs_write_data *nfs_writedata_alloc(unsigned int pagecount)
+struct nfs_write_data *nfs_writedata_alloc(unsigned int pagecount,
+ int need_pagevec)
{
struct nfs_write_data *p = mempool_alloc(nfs_wdata_mempool, GFP_NOFS);
@@ -74,9 +75,11 @@ struct nfs_write_data *nfs_writedata_alloc(unsigned int pagecount)
INIT_LIST_HEAD(&p->pages);
p->npages = pagecount;
p->res.seq_res.sr_slotid = NFS4_MAX_SLOT_TABLE;
- if (pagecount <= ARRAY_SIZE(p->page_array))
+ if (!need_pagevec || pagecount <= ARRAY_SIZE(p->page_array)) {
p->pagevec = p->page_array;
- else {
+ p->free_pagevec = 0;
+ } else {
+ p->free_pagevec = 1;
p->pagevec = kcalloc(pagecount, sizeof(struct page *), GFP_NOFS);
if (!p->pagevec) {
mempool_free(p, nfs_wdata_mempool);
@@ -89,7 +92,7 @@ struct nfs_write_data *nfs_writedata_alloc(unsigned int pagecount)
void nfs_writedata_free(struct nfs_write_data *p)
{
- if (p && (p->pagevec != &p->page_array[0]))
+ if (p && p->free_pagevec)
kfree(p->pagevec);
mempool_free(p, nfs_wdata_mempool);
}
@@ -904,7 +907,7 @@ static int nfs_flush_multi(struct inode *inode, struct list_head *head, unsigned
do {
size_t len = min(nbytes, wsize);
- data = nfs_writedata_alloc(1);
+ data = nfs_writedata_alloc(1, 1);
if (!data)
goto out_bad;
list_add(&data->pages, &list);
@@ -960,7 +963,7 @@ static int nfs_flush_one(struct inode *inode, struct list_head *head, unsigned i
struct page **pages;
struct nfs_write_data *data;
- data = nfs_writedata_alloc(npages);
+ data = nfs_writedata_alloc(npages, 1);
if (!data)
goto out_bad;
diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
index dab9bf9..946317c 100644
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -498,7 +498,7 @@ nfs_have_writebacks(struct inode *inode)
/*
* Allocate nfs_write_data structures
*/
-extern struct nfs_write_data *nfs_writedata_alloc(unsigned int npages);
+extern struct nfs_write_data *nfs_writedata_alloc(unsigned int npages, int);
extern void nfs_writedata_free(struct nfs_write_data *);
/*
@@ -514,7 +514,7 @@ extern int nfs_readpage_async(struct nfs_open_context *, struct inode *,
/*
* Allocate nfs_read_data structures
*/
-extern struct nfs_read_data *nfs_readdata_alloc(unsigned int npages);
+extern struct nfs_read_data *nfs_readdata_alloc(unsigned int npages, int);
extern void nfs_readdata_free(struct nfs_read_data *);
/*
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index 62f63fb..45dd955 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -954,6 +954,7 @@ struct nfs_read_data {
struct nfs_page *req; /* multi ops per nfs_page */
struct page **pagevec;
unsigned int npages; /* Max length of pagevec */
+ int free_pagevec; /* free ->pagevec */
struct nfs_readargs args;
struct nfs_readres res;
#ifdef CONFIG_NFS_V4
@@ -973,6 +974,7 @@ struct nfs_write_data {
struct nfs_page *req; /* multi ops per nfs_page */
struct page **pagevec;
unsigned int npages; /* Max length of pagevec */
+ int free_pagevec; /* free ->pagevec */
struct nfs_writeargs args; /* argument struct */
struct nfs_writeres res; /* result struct */
#ifdef CONFIG_NFS_V4
--
Jens Axboe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists