2010-07-20 05:55:30

by Benny Halevy

[permalink] [raw]
Subject: [PATCH 0/9] layout driver api cleanup and usage

The following patches do some further cleanup on top of Andy's
2010-07-15's patchset and use the new api in the obj and block LDs

[PATCH 1/3] SQUASHME: pnfs-submit: add nr_pages back to write_pagelist api
[PATCH 2/3] SQUASHME: pnfs-submit: filelayout: add nr_pages back to write_pagelist api
[PATCH 3/3] SQUASHME: pnfs-submit: use nfs_page_array_len in pnfs_try_to_read_data

[PATCH 1/3] SQUASHME: pnfs-block: use new commit api
[PATCH 2/3] SQUASHME: pnfs-block: use new read_pagelist api
[PATCH 3/3] SQUASHME: pnfs-block: use new write_pagelist api

[PATCH 1/3] SQUASHME: pnfs-obj: use new commit api
[PATCH 2/3] SQAUSHME: pnfs-obj: use new read_pagelist api
[PATCH 3/3] SQUASHME: pnfs-obj: use new write_pagelist api


2010-07-20 14:10:03

by Trond Myklebust

[permalink] [raw]
Subject: Re: [PATCH 2/3] SQUASHME: pnfs-submit: filelayout: add nr_pages back to write_pagelist api

On Tue, 2010-07-20 at 17:07 +0300, Benny Halevy wrote:
> On 2010-07-20 16:50, William A. (Andy) Adamson wrote:
> > The file layout does not use nr_pages. The calculation is easy. No
>
> True, but we should strive for a coherent interface IMO,
> even if it's not the absolute minimal one.
>
> > need to pass the extra parameter. Why not just call
> > nfs_page_array_len() in the layout drivers that need it?
>
> It's defined in fs/nfs/internal.h, so to use it officially
> it should be moved to a public header file or exported as
> a public function.

There is nothing NFS-specific about that particular calculation: it is
100% generic. Feel free to move it into a public header.

Trond

> Benny
>
> >
> > -->Andy
> >
> > On Tue, Jul 20, 2010 at 1:57 AM, Benny Halevy <[email protected]> wrote:
> >> Signed-off-by: Benny Halevy <[email protected]>
> >> ---
> >> fs/nfs/nfs4filelayout.c | 2 +-
> >> 1 files changed, 1 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/fs/nfs/nfs4filelayout.c b/fs/nfs/nfs4filelayout.c
> >> index 9b6ba8d..b2ce478 100644
> >> --- a/fs/nfs/nfs4filelayout.c
> >> +++ b/fs/nfs/nfs4filelayout.c
> >> @@ -244,7 +244,7 @@ filelayout_read_pagelist(struct nfs_read_data *data, unsigned nr_pages)
> >>
> >> /* Perform async writes. */
> >> static enum pnfs_try_status
> >> -filelayout_write_pagelist(struct nfs_write_data *data, int sync)
> >> +filelayout_write_pagelist(struct nfs_write_data *data, unsigned nr_pages, int sync)
> >> {
> >> struct pnfs_layout_segment *lseg = data->pdata.lseg;
> >> struct nfs4_pnfs_ds *ds;
> >> --
> >> 1.7.1.1
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> >> the body of a message to [email protected]
> >> More majordomo info at http://vger.kernel.org/majordomo-info.html
> >>
>
>




2010-07-20 13:56:50

by Andy Adamson

[permalink] [raw]
Subject: Re: [PATCH 2/3] SQUASHME: pnfs-submit: filelayout: add nr_pages back to write_pagelist api

The file layout does not use nr_pages. The calculation is easy. No
need to pass the extra parameter. Why not just call
nfs_page_array_len() in the layout drivers that need it?

-->Andy

On Tue, Jul 20, 2010 at 1:57 AM, Benny Halevy <[email protected]> wrote:
> Signed-off-by: Benny Halevy <[email protected]>
> ---
> ?fs/nfs/nfs4filelayout.c | ? ?2 +-
> ?1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/fs/nfs/nfs4filelayout.c b/fs/nfs/nfs4filelayout.c
> index 9b6ba8d..b2ce478 100644
> --- a/fs/nfs/nfs4filelayout.c
> +++ b/fs/nfs/nfs4filelayout.c
> @@ -244,7 +244,7 @@ filelayout_read_pagelist(struct nfs_read_data *data, unsigned nr_pages)
>
> ?/* Perform async writes. */
> ?static enum pnfs_try_status
> -filelayout_write_pagelist(struct nfs_write_data *data, int sync)
> +filelayout_write_pagelist(struct nfs_write_data *data, unsigned nr_pages, int sync)
> ?{
> ? ? ? ?struct pnfs_layout_segment *lseg = data->pdata.lseg;
> ? ? ? ?struct nfs4_pnfs_ds *ds;
> --
> 1.7.1.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>

2010-07-20 14:07:11

by Benny Halevy

[permalink] [raw]
Subject: Re: [PATCH 2/3] SQUASHME: pnfs-submit: filelayout: add nr_pages back to write_pagelist api

On 2010-07-20 16:50, William A. (Andy) Adamson wrote:
> The file layout does not use nr_pages. The calculation is easy. No

True, but we should strive for a coherent interface IMO,
even if it's not the absolute minimal one.

> need to pass the extra parameter. Why not just call
> nfs_page_array_len() in the layout drivers that need it?

It's defined in fs/nfs/internal.h, so to use it officially
it should be moved to a public header file or exported as
a public function.

Benny

>
> -->Andy
>
> On Tue, Jul 20, 2010 at 1:57 AM, Benny Halevy <[email protected]> wrote:
>> Signed-off-by: Benny Halevy <[email protected]>
>> ---
>> fs/nfs/nfs4filelayout.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/fs/nfs/nfs4filelayout.c b/fs/nfs/nfs4filelayout.c
>> index 9b6ba8d..b2ce478 100644
>> --- a/fs/nfs/nfs4filelayout.c
>> +++ b/fs/nfs/nfs4filelayout.c
>> @@ -244,7 +244,7 @@ filelayout_read_pagelist(struct nfs_read_data *data, unsigned nr_pages)
>>
>> /* Perform async writes. */
>> static enum pnfs_try_status
>> -filelayout_write_pagelist(struct nfs_write_data *data, int sync)
>> +filelayout_write_pagelist(struct nfs_write_data *data, unsigned nr_pages, int sync)
>> {
>> struct pnfs_layout_segment *lseg = data->pdata.lseg;
>> struct nfs4_pnfs_ds *ds;
>> --
>> 1.7.1.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>


--
Benny Halevy
Software Architect
Panasas, Inc.
[email protected]
Tel/Fax: +972-3-647-8340
Mobile: +972-54-802-8340

Panasas: The Leader in Parallel Storage
http://www.panasas.com

2010-07-20 05:57:31

by Benny Halevy

[permalink] [raw]
Subject: [PATCH 1/3] SQUASHME: pnfs-submit: add nr_pages back to write_pagelist api

Keep it symmetrical with read_pagelist, using nfs_page_array_len.

Signed-off-by: Benny Halevy <[email protected]>
---
fs/nfs/pnfs.c | 4 +++-
include/linux/nfs4_pnfs.h | 2 +-
2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index 4de0b73..6ab1938 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -1269,7 +1269,9 @@ pnfs_try_to_write_data(struct nfs_write_data *wdata,
get_lseg(lseg);

wdata->pdata.lseg = lseg;
- trypnfs = nfss->pnfs_curr_ld->ld_io_ops->write_pagelist(wdata, how);
+ trypnfs = nfss->pnfs_curr_ld->ld_io_ops->write_pagelist(wdata,
+ nfs_page_array_len(wdata->args.pgbase, wdata->args.count),
+ how);

if (trypnfs == PNFS_NOT_ATTEMPTED) {
wdata->pdata.lseg = NULL;
diff --git a/include/linux/nfs4_pnfs.h b/include/linux/nfs4_pnfs.h
index 1c3cd49..6236687 100644
--- a/include/linux/nfs4_pnfs.h
+++ b/include/linux/nfs4_pnfs.h
@@ -130,7 +130,7 @@ struct layoutdriver_io_operations {
enum pnfs_try_status
(*read_pagelist) (struct nfs_read_data *nfs_data, unsigned nr_pages);
enum pnfs_try_status
- (*write_pagelist) (struct nfs_write_data *nfs_data, int how);
+ (*write_pagelist) (struct nfs_write_data *nfs_data, unsigned nr_pages, int how);

/* Consistency ops */
/* 2 problems:
--
1.7.1.1


2010-07-20 05:57:39

by Benny Halevy

[permalink] [raw]
Subject: [PATCH 2/3] SQUASHME: pnfs-submit: filelayout: add nr_pages back to write_pagelist api

Signed-off-by: Benny Halevy <[email protected]>
---
fs/nfs/nfs4filelayout.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/nfs/nfs4filelayout.c b/fs/nfs/nfs4filelayout.c
index 9b6ba8d..b2ce478 100644
--- a/fs/nfs/nfs4filelayout.c
+++ b/fs/nfs/nfs4filelayout.c
@@ -244,7 +244,7 @@ filelayout_read_pagelist(struct nfs_read_data *data, unsigned nr_pages)

/* Perform async writes. */
static enum pnfs_try_status
-filelayout_write_pagelist(struct nfs_write_data *data, int sync)
+filelayout_write_pagelist(struct nfs_write_data *data, unsigned nr_pages, int sync)
{
struct pnfs_layout_segment *lseg = data->pdata.lseg;
struct nfs4_pnfs_ds *ds;
--
1.7.1.1


2010-07-20 05:57:46

by Benny Halevy

[permalink] [raw]
Subject: [PATCH 3/3] SQUASHME: pnfs-submit: use nfs_page_array_len in pnfs_try_to_read_data

No sense in open coding the logic here.

Signed-off-by: Benny Halevy <[email protected]>
---
fs/nfs/pnfs.c | 13 ++-----------
1 files changed, 2 insertions(+), 11 deletions(-)

diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index 6ab1938..0f255f5 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -1296,7 +1296,6 @@ pnfs_try_to_read_data(struct nfs_read_data *rdata,
struct inode *inode = rdata->inode;
struct nfs_server *nfss = NFS_SERVER(inode);
struct pnfs_layout_segment *lseg = rdata->req->wb_lseg;
- int numpages, pgcount, temp;
enum pnfs_try_status trypnfs;

rdata->pdata.call_ops = call_ops;
@@ -1306,17 +1305,9 @@ pnfs_try_to_read_data(struct nfs_read_data *rdata,

get_lseg(lseg);

- /* Determine number of pages. */
- pgcount = rdata->args.pgbase + rdata->args.count;
- temp = pgcount % PAGE_CACHE_SIZE;
- numpages = pgcount / PAGE_CACHE_SIZE;
- if (temp != 0)
- numpages++;
-
- dprintk("%s: Calling layout driver read with %d pages\n",
- __func__, numpages);
rdata->pdata.lseg = lseg;
- trypnfs = nfss->pnfs_curr_ld->ld_io_ops->read_pagelist(rdata, numpages);
+ trypnfs = nfss->pnfs_curr_ld->ld_io_ops->read_pagelist(rdata,
+ nfs_page_array_len(rdata->args.pgbase, rdata->args.count));
if (trypnfs == PNFS_NOT_ATTEMPTED) {
rdata->pdata.lseg = NULL;
put_lseg(lseg);
--
1.7.1.1


2010-07-20 05:58:06

by Benny Halevy

[permalink] [raw]
Subject: [PATCH 1/3] SQUASHME: pnfs-block: use new commit api

Signed-off-by: Benny Halevy <[email protected]>
---
fs/nfs/blocklayout/blocklayout.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/fs/nfs/blocklayout/blocklayout.c b/fs/nfs/blocklayout/blocklayout.c
index 63d3b5a..e2ee90a 100644
--- a/fs/nfs/blocklayout/blocklayout.c
+++ b/fs/nfs/blocklayout/blocklayout.c
@@ -99,9 +99,8 @@ dont_like_caller(struct nfs_page *req)
}

static enum pnfs_try_status
-bl_commit(struct pnfs_layout_type *lo,
- int sync,
- struct nfs_write_data *nfs_data)
+bl_commit(struct nfs_write_data *nfs_data,
+ int sync)
{
dprintk("%s enter\n", __func__);
return PNFS_NOT_ATTEMPTED;
--
1.7.1.1


2010-07-20 05:58:14

by Benny Halevy

[permalink] [raw]
Subject: [PATCH 2/3] SQUASHME: pnfs-block: use new read_pagelist api

Signed-off-by: Benny Halevy <[email protected]>
---
fs/nfs/blocklayout/blocklayout.c | 14 ++++++--------
1 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/fs/nfs/blocklayout/blocklayout.c b/fs/nfs/blocklayout/blocklayout.c
index e2ee90a..80d2576 100644
--- a/fs/nfs/blocklayout/blocklayout.c
+++ b/fs/nfs/blocklayout/blocklayout.c
@@ -220,20 +220,18 @@ static void bl_rpc_do_nothing(struct rpc_task *task, void *calldata)
}

static enum pnfs_try_status
-bl_read_pagelist(struct pnfs_layout_type *lo,
- struct page **pages,
- unsigned int pgbase,
- unsigned nr_pages,
- loff_t f_offset,
- size_t count,
- struct nfs_read_data *rdata)
+bl_read_pagelist(struct nfs_read_data *rdata,
+ unsigned nr_pages)
{
int i, hole;
struct bio *bio = NULL;
struct pnfs_block_extent *be = NULL, *cow_read = NULL;
sector_t isect, extent_length = 0;
struct parallel_io *par;
- int pg_index = pgbase >> PAGE_CACHE_SHIFT;
+ loff_t f_offset = rdata->args.offset;
+ size_t count = rdata->args.count;
+ struct page **pages = rdata->args.pages;
+ int pg_index = rdata->args.pgbase >> PAGE_CACHE_SHIFT;

dprintk("%s enter nr_pages %u offset %lld count %Zd\n", __func__,
nr_pages, f_offset, count);
--
1.7.1.1


2010-07-20 05:58:21

by Benny Halevy

[permalink] [raw]
Subject: [PATCH 3/3] SQUASHME: pnfs-block: use new write_pagelist api

Signed-off-by: Benny Halevy <[email protected]>
---
fs/nfs/blocklayout/blocklayout.c | 16 +++++++---------
1 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/fs/nfs/blocklayout/blocklayout.c b/fs/nfs/blocklayout/blocklayout.c
index 80d2576..bfcef54 100644
--- a/fs/nfs/blocklayout/blocklayout.c
+++ b/fs/nfs/blocklayout/blocklayout.c
@@ -428,21 +428,19 @@ bl_end_par_io_write(void *data)
}

static enum pnfs_try_status
-bl_write_pagelist(struct pnfs_layout_type *lo,
- struct page **pages,
- unsigned int pgbase,
- unsigned nr_pages,
- loff_t offset,
- size_t count,
- int sync,
- struct nfs_write_data *wdata)
+bl_write_pagelist(struct nfs_write_data *wdata,
+ unsigned nr_pages,
+ int sync)
{
int i;
struct bio *bio = NULL;
struct pnfs_block_extent *be = NULL;
sector_t isect, extent_length = 0;
struct parallel_io *par;
- int pg_index = pgbase >> PAGE_CACHE_SHIFT;
+ loff_t offset = wdata->args.offset;
+ size_t count = wdata->args.count;
+ struct page **pages = wdata->args.pages;
+ int pg_index = wdata->args.pgbase >> PAGE_CACHE_SHIFT;

dprintk("%s enter, %Zu@%lld\n", __func__, count, offset);
if (!wdata->req->wb_lseg) {
--
1.7.1.1


2010-07-20 05:58:45

by Benny Halevy

[permalink] [raw]
Subject: [PATCH 1/3] SQUASHME: pnfs-obj: use new commit api

Signed-off-by: Benny Halevy <[email protected]>
---
fs/nfs/objlayout/objlayout.c | 4 +---
1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/fs/nfs/objlayout/objlayout.c b/fs/nfs/objlayout/objlayout.c
index 1381c60..6595bfc 100644
--- a/fs/nfs/objlayout/objlayout.c
+++ b/fs/nfs/objlayout/objlayout.c
@@ -297,9 +297,7 @@ static void _rpc_commit_complete(struct work_struct *work)
* Commit data remotely on OSDs
*/
enum pnfs_try_status
-objlayout_commit(struct pnfs_layout_type *pnfslay,
- int sync,
- struct nfs_write_data *wdata)
+objlayout_commit(struct nfs_write_data *wdata, int how)
{
int status = PNFS_ATTEMPTED;

--
1.7.1.1


2010-07-20 05:58:53

by Benny Halevy

[permalink] [raw]
Subject: [PATCH 2/3] SQAUSHME: pnfs-obj: use new read_pagelist api

Signed-off-by: Benny Halevy <[email protected]>
---
fs/nfs/objlayout/objlayout.c | 18 +++++++-----------
1 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/fs/nfs/objlayout/objlayout.c b/fs/nfs/objlayout/objlayout.c
index 6595bfc..c7daa16 100644
--- a/fs/nfs/objlayout/objlayout.c
+++ b/fs/nfs/objlayout/objlayout.c
@@ -352,23 +352,18 @@ objlayout_read_done(struct objlayout_io_state *state, ssize_t status, bool sync)
* Perform sync or async reads.
*/
enum pnfs_try_status
-objlayout_read_pagelist(struct pnfs_layout_type *pnfs_layout_type,
- struct page **pages,
- unsigned pgbase,
- unsigned nr_pages,
- loff_t offset,
- size_t count,
- struct nfs_read_data *rdata)
+objlayout_read_pagelist(struct nfs_read_data *rdata, unsigned nr_pages)
{
- struct inode *inode = PNFS_INODE(pnfs_layout_type);
+ loff_t offset = rdata->args.offset;
+ size_t count = rdata->args.count;
struct objlayout_io_state *state;
ssize_t status = 0;
loff_t eof;

dprintk("%s: Begin inode %p offset %llu count %d\n",
- __func__, inode, offset, (int)count);
+ __func__, rdata->inode, offset, (int)count);

- eof = i_size_read(inode);
+ eof = i_size_read(rdata->inode);
if (unlikely(offset + count > eof)) {
if (offset >= eof) {
status = 0;
@@ -379,7 +374,8 @@ objlayout_read_pagelist(struct pnfs_layout_type *pnfs_layout_type,
count = eof - offset;
}

- state = objlayout_alloc_io_state(pnfs_layout_type, pages, pgbase,
+ state = objlayout_alloc_io_state(NFS_I(rdata->inode)->layout,
+ rdata->args.pages, rdata->args.pgbase,
nr_pages, offset, count,
rdata->pdata.lseg, rdata);
if (unlikely(!state)) {
--
1.7.1.1


2010-07-20 05:59:00

by Benny Halevy

[permalink] [raw]
Subject: [PATCH 3/3] SQUASHME: pnfs-obj: use new write_pagelist api

Signed-off-by: Benny Halevy <[email protected]>
---
fs/nfs/objlayout/objlayout.c | 21 ++++++++++-----------
1 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/fs/nfs/objlayout/objlayout.c b/fs/nfs/objlayout/objlayout.c
index c7daa16..e8ea910 100644
--- a/fs/nfs/objlayout/objlayout.c
+++ b/fs/nfs/objlayout/objlayout.c
@@ -442,23 +442,22 @@ objlayout_write_done(struct objlayout_io_state *state, ssize_t status,
* Perform sync or async writes.
*/
enum pnfs_try_status
-objlayout_write_pagelist(struct pnfs_layout_type *pnfs_layout_type,
- struct page **pages,
- unsigned pgbase,
+objlayout_write_pagelist(struct nfs_write_data *wdata,
unsigned nr_pages,
- loff_t offset,
- size_t count,
- int how,
- struct nfs_write_data *wdata)
+ int how)
{
struct objlayout_io_state *state;
ssize_t status;

- dprintk("%s: Begin inode %p offset %llu count %d\n",
- __func__, PNFS_INODE(pnfs_layout_type), offset, (int)count);
+ dprintk("%s: Begin inode %p offset %llu count %u\n",
+ __func__, wdata->inode, wdata->args.offset, wdata->args.count);

- state = objlayout_alloc_io_state(pnfs_layout_type, pages, pgbase,
- nr_pages, offset, count,
+ state = objlayout_alloc_io_state(NFS_I(wdata->inode)->layout,
+ wdata->args.pages,
+ wdata->args.pgbase,
+ nr_pages,
+ wdata->args.offset,
+ wdata->args.count,
wdata->pdata.lseg, wdata);
if (unlikely(!state)) {
status = -ENOMEM;
--
1.7.1.1