On 06/09/2011 09:43 PM, Benny Halevy wrote:
> On 2011-06-10 00:28, Boaz Harrosh wrote:
>> On 06/09/2011 09:06 PM, Benny Halevy wrote:
>>> On 2011-06-09 22:53, Boaz Harrosh wrote:
>>>> On 06/09/2011 06:31 PM, Trond Myklebust wrote:
>>>> <snip>
>>>>> +void
>>>>> +pnfs_generic_pg_init_read(struct nfs_pageio_descriptor *pgio, struct nfs_page *req)
>>>>> +{
>>>>> + BUG_ON(pgio->pg_lseg != NULL);
>>>>> +
>>>>> + pgio->pg_lseg = pnfs_update_layout(pgio->pg_inode,
>>>>> + req->wb_context,
>>>>> + req_offset(req),
>>>>> + req->wb_bytes,
>>>>> + IOMODE_READ,
>>>>> + GFP_KERNEL);
>>>>> +}
>>>>> +EXPORT_SYMBOL_GPL(pnfs_generic_pg_init_read);
>>>>> +
>>>>> +void
>>>>> +pnfs_generic_pg_init_write(struct nfs_pageio_descriptor *pgio, struct nfs_page *req)
>>>>> +{
>>>>> + BUG_ON(pgio->pg_lseg != NULL);
>>>>> +
>>>>> + pgio->pg_lseg = pnfs_update_layout(pgio->pg_inode,
>>>>> + req->wb_context,
>>>>> + req_offset(req),
>>>>> + req->wb_bytes,
>>>>> + IOMODE_RW,
>>>>> + GFP_NOFS);
>>>>> +}
>>>>> +EXPORT_SYMBOL_GPL(pnfs_generic_pg_init_write);
>>>>> +
>>>>
>>>> These two above are identical except the IOMODE_{READ,RW} variable.
>>>
>>> And the respective gfp flags...
>>>
>>
>> So is that "we should" or should-not?
>>
>
> That doesn't make much sense when you've defined separate vectors
> for read and write.
So what the same function is set at both vectors, and the few callers (1)
passes a flag.
If you ask me we could do with one vector and a flag throughout this
stack. Actually don't make the flag as function parameter but as a member
of nfs_pageio_descriptor. For example in osd we want to know if we are
reading or writing in the raid5 case it produces different results.
> And in any case, since pg_init is not pnfs specific the caller shouldn't
> pass IOMODE_* values but rather a generic value that will require translation anyway.
> In this case I'd just consider using a common function to be called
> respectively from both methods:
>
> static void
> pnfs_pg_update_layout(struct nfs_pageio_descriptor *pgio, struct nfs_page *req,
> enum pnfs_iomode iomode, gfp_t gfp_flags)
> {
> BUG_ON(pgio->pg_lseg != NULL);
>
> pgio->pg_lseg = pnfs_update_layout(pgio->pg_inode,
> req->wb_context,
> req_offset(req),
> req->wb_bytes,
> iomode,
> gfp_flags);
> }
>
That makes it even more complicated for a do nothing function. We dont do
a different function for each different parameter. We can just do a
"bool write" and unify the dam thing
Boaz
--
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
