Ah - I see now. OK - numpages in write_pagelist.
-->Andy
On Tue, Jul 20, 2010 at 12:33 AM, Benny Halevy <bhalevy@xxxxxxxxxxx> wrote:
> On Jul. 20, 2010, 7:26 +0300, Benny Halevy <bhalevy@xxxxxxxxxxx> wrote:
>> On Jul. 15, 2010, 19:13 +0300, andros@xxxxxxxxxx wrote:
>>> From: Andy Adamson <andros@xxxxxxxxxx>
>>>
>>> Remove the numpages calculation which is not used by the file layout driver.
>>> Layout drivers that need the number of pages can call nfs_page_array_len in
>>> their write_pagelist operation.
>>>
>>> NOTE: API change.
>>>
>>> Signed-off-by: Andy Adamson <andros@xxxxxxxxxx>
>>> ---
>>> fs/nfs/pnfs.c | 24 +++---------------------
>>> include/linux/nfs4_pnfs.h | 5 +----
>>> 2 files changed, 4 insertions(+), 25 deletions(-)
>>>
>>> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
>>> index 576946b..51bd66f 100644
>>> --- a/fs/nfs/pnfs.c
>>> +++ b/fs/nfs/pnfs.c
>>> @@ -1271,39 +1271,21 @@ enum pnfs_try_status
>>> pnfs_try_to_write_data(struct nfs_write_data *wdata,
>>> const struct rpc_call_ops *call_ops, int how)
>>> {
>>> - struct nfs_writeargs *args = &wdata->args;
>>> struct inode *inode = wdata->inode;
>>> enum pnfs_try_status trypnfs;
>>> struct nfs_server *nfss = NFS_SERVER(inode);
>>> - struct nfs_inode *nfsi = NFS_I(inode);
>>> struct pnfs_layout_segment *lseg = wdata->req->wb_lseg;
>>> - int numpages;
>>>
>>> wdata->pdata.call_ops = call_ops;
>>> wdata->pdata.how = how;
>>>
>>> - dprintk("%s: Writing ino:%lu %u@%llu\n", __func__,
>>> - inode->i_ino, args->count, args->offset);
>>> + dprintk("%s: Writing ino:%lu %u@%llu (how %d)\n", __func__,
>>> + inode->i_ino, wdata->args.count, wdata->args.offset, how);
>>>
>>> get_lseg(lseg);
>>>
>>> - /* Determine number of pages
>>> - */
>>> - numpages = nfs_page_array_len(args->pgbase, args->count);
>>
>> Andy, removing this from the interface is a bit problematic
>> since nfs_page_array_len is private to the nfs module and I want to leave
>> the logic there rather than copying it to the layout driver.
>> Also, we still pass in its equivalent to read_pagelist so for symmetry
>> reasons it would be cleaner to pass to to the write path as well.
>
> and we should use nfs_page_array_len for the read path as well.
> In any case the calculation is straight forward so we should either do
> it in the generic layer in both cases or neither.
>
> Benny
>
>>
>> Benny
>>
>>> -
>>> - dprintk("%s: Calling layout driver (how %d) write with %d pages\n",
>>> - __func__, how, numpages);
>>> -
>>> wdata->pdata.lseg = lseg;
>>> - trypnfs = nfss->pnfs_curr_ld->ld_io_ops->write_pagelist(
>>> - nfsi->layout,
>>> - args->pages,
>>> - args->pgbase,
>>> - numpages,
>>> - (loff_t)args->offset,
>>> - args->count,
>>> - how,
>>> - wdata);
>>> + trypnfs = nfss->pnfs_curr_ld->ld_io_ops->write_pagelist(wdata, 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 b379225..13f71ad 100644
>>> --- a/include/linux/nfs4_pnfs.h
>>> +++ b/include/linux/nfs4_pnfs.h
>>> @@ -130,10 +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 pnfs_layout_type *layoutid,
>>> - struct page **pages, unsigned int pgbase,
>>> - unsigned nr_pages, loff_t offset, size_t count,
>>> - int sync, struct nfs_write_data *nfs_data);
>>> + (*write_pagelist) (struct nfs_write_data *nfs_data, int how);
>>>
>>> /* Consistency ops */
>>> /* 2 problems:
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
