Fred git blame points to you:
What was the meaning of the below code:
@@ -1014,18 +1015,22 @@ static int nfs_flush_one(struct nfs_pageio_descriptor *desc)
*pages++ = req->wb_page;
}
req = nfs_list_entry(data->pages.next);
if ((!lseg) && list_is_singular(&data->pages))
lseg = pnfs_update_layout(desc->pg_inode, req->wb_context,
req_offset(req), desc->pg_count,
IOMODE_RW);
I mean why only the list_is_singular() case gets a pnfs_update_layout?
Because I have a very funny BUG:
If I do:
dd if=/dev/zero of=/mnt/pnfs/dd1 bs=4k count=1
I get a nice pnfs write out.
But if I do:
dd if=/dev/zero of=/mnt/pnfs/dd1 bs=4k count=2 (or any > 1)
I see all IO going to MDS
Below patch fixes that:
@@ -1014,18 +1015,22 @@ static int nfs_flush_one(struct nfs_pageio_descriptor *desc)
*pages++ = req->wb_page;
}
req = nfs_list_entry(data->pages.next);
- if ((!lseg) && list_is_singular(&data->pages))
+ if ((!lseg) /*&& list_is_singular(&data->pages)*/)
lseg = pnfs_update_layout(desc->pg_inode, req->wb_context,
req_offset(req), desc->pg_count,
IOMODE_RW);
Which points to what I'm saying all along. pNFS Write path had no testing
but by me.
(BTW am still fighting the unbalanced lseg ref)
Thanks
Boaz
On Sat, May 21, 2011 at 8:16 PM, Boaz Harrosh <[email protected]> wrote:
> On 05/22/2011 02:45 AM, Boaz Harrosh wrote:
>>
>> Fred git blame points to you:
>>
>> What was the meaning of the below code:
>>
>
> OK Smack *ME* on the head
>
> A layout driver must have a .pg_test function else
> that's what happens. pNFS only for a single page.
> I'd say it should be opposite no?
>
Right now, a driver with no pg_test function erroneously misses the
pnfs_update_layout call in multi-page io. I'll send a patch to fix
that shortly.
Fred
On 2011-05-23 17:22, Fred Isaman wrote:
> On Sat, May 21, 2011 at 8:16 PM, Boaz Harrosh <[email protected]> wrote:
>> On 05/22/2011 02:45 AM, Boaz Harrosh wrote:
>>>
>>> Fred git blame points to you:
>>>
>>> What was the meaning of the below code:
>>>
>>
>> OK Smack *ME* on the head
>>
>> A layout driver must have a .pg_test function else
>> that's what happens. pNFS only for a single page.
>> I'd say it should be opposite no?
>>
>
> Right now, a driver with no pg_test function erroneously misses the
> pnfs_update_layout call in multi-page io. I'll send a patch to fix
> that shortly.
>
After discussing this with Trond how about having
pnfs_pageio_init_{read,write} always set
pgio->pg_test = ld->pg_test
and then let the layout driver initialize its vector to
the generic pnfs_{read,write}_pg_test functions, respectively
if it requires no layout-type specific code.
(and EXPORT_SYMBOL_GPL them for the layout drivers)
Benny
> Fred
On 2011-05-23 20:50, Benny Halevy wrote:
> On 2011-05-23 17:22, Fred Isaman wrote:
>> On Sat, May 21, 2011 at 8:16 PM, Boaz Harrosh <[email protected]> wrote:
>>> On 05/22/2011 02:45 AM, Boaz Harrosh wrote:
>>>>
>>>> Fred git blame points to you:
>>>>
>>>> What was the meaning of the below code:
>>>>
>>>
>>> OK Smack *ME* on the head
>>>
>>> A layout driver must have a .pg_test function else
>>> that's what happens. pNFS only for a single page.
>>> I'd say it should be opposite no?
>>>
>>
>> Right now, a driver with no pg_test function erroneously misses the
>> pnfs_update_layout call in multi-page io. I'll send a patch to fix
>> that shortly.
>>
>
> After discussing this with Trond how about having
> pnfs_pageio_init_{read,write} always set
> pgio->pg_test = ld->pg_test
>
> and then let the layout driver initialize its vector to
> the generic pnfs_{read,write}_pg_test functions, respectively
> if it requires no layout-type specific code.
> (and EXPORT_SYMBOL_GPL them for the layout drivers)
one more note: pnfs_{read,write}_pg_test should, in this model,
return 1 where they call
NFS_SERVER(pgio->pg_inode)->pnfs_curr_ld->pg_test today.
Benny
>
> Benny
>
>> Fred
> --
> 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
On 05/22/2011 02:45 AM, Boaz Harrosh wrote:
>
> Fred git blame points to you:
>
> What was the meaning of the below code:
>
OK Smack *ME* on the head
A layout driver must have a .pg_test function else
that's what happens. pNFS only for a single page.
I'd say it should be opposite no?
Boaz
> @@ -1014,18 +1015,22 @@ static int nfs_flush_one(struct nfs_pageio_descriptor *desc)
> *pages++ = req->wb_page;
> }
> req = nfs_list_entry(data->pages.next);
> if ((!lseg) && list_is_singular(&data->pages))
> lseg = pnfs_update_layout(desc->pg_inode, req->wb_context,
> req_offset(req), desc->pg_count,
> IOMODE_RW);
>
> I mean why only the list_is_singular() case gets a pnfs_update_layout?
>
> Because I have a very funny BUG:
> If I do:
> dd if=/dev/zero of=/mnt/pnfs/dd1 bs=4k count=1
> I get a nice pnfs write out.
> But if I do:
> dd if=/dev/zero of=/mnt/pnfs/dd1 bs=4k count=2 (or any > 1)
> I see all IO going to MDS
>
> Below patch fixes that:
> @@ -1014,18 +1015,22 @@ static int nfs_flush_one(struct nfs_pageio_descriptor *desc)
> *pages++ = req->wb_page;
> }
> req = nfs_list_entry(data->pages.next);
> - if ((!lseg) && list_is_singular(&data->pages))
> + if ((!lseg) /*&& list_is_singular(&data->pages)*/)
> lseg = pnfs_update_layout(desc->pg_inode, req->wb_context,
> req_offset(req), desc->pg_count,
> IOMODE_RW);
>
> Which points to what I'm saying all along. pNFS Write path had no testing
> but by me.
>
> (BTW am still fighting the unbalanced lseg ref)
>
> Thanks
> Boaz
>
> --
> 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