On 08/15/2014 10:48 AM, Weston Andros Adamson wrote:
> For some reason I can�t reproduce this with:
>
> CONFIG_NFS_FS=m
> CONFIG_NFS_V2=m
> # CONFIG_NFS_V3 is not set
> # CONFIG_NFS_V4 is not set
>
> Are you compiling with some special option? It�s not in the output of make W=1, W=3 or W=3.
>
> It looks like there are several places that call nfs_init_cinfo_from_inode without any ifdef logic�
>
> We could just fix all of them and be done with it, but I�m wondering why you get the warning and I don�t.
Whoa, somehow I missed this email (cats probably walked over my keyboard, sorry!). I don't think I'm setting any special config options, but I can try regenerating my .config. This is what I have set:
CONFIG_NFS_FS=m
CONFIG_NFS_V2=m
# CONFIG_NFS_V3 is not set
# CONFIG_NFS_V4 is not set
CONFIG_NFS_SWAP=y
CONFIG_NFS_FSCACHE=y
CONFIG_NFS_ACL_SUPPORT=m
CONFIG_NFS_COMMON=y
CONFIG_SUNRPC_GSS=m
CONFIG_SUNRPC_SWAP=y
# CONFIG_SUNRPC_DEBUG is not set
Anna
>
> -dros
>
>
>
> On Aug 11, 2014, at 10:54 PM, Weston Andros Adamson <dros@xxxxxxxxxxxxxxx> wrote:
>
>> Thanks Anna, I�ll fix it up.
>>
>> -dros
>>
>>
>>
>> On Aug 11, 2014, at 9:30 AM, Anna Schumaker <schumaker.anna@xxxxxxxxx> wrote:
>>
>>> On 08/08/2014 11:13 AM, Weston Andros Adamson wrote:
>>>> nfs_page_find_head_request_locked looks through the regular nfs commit lists
>>>> when the page is swapped out, but doesn't look through the pnfs commit lists.
>>>>
>>>> I'm not sure if anyone has hit any issues caused by this.
>>>>
>>>> Suggested-by: Peng Tao <tao.peng@xxxxxxxxxxxxxxx>
>>>> Signed-off-by: Weston Andros Adamson <dros@xxxxxxxxxxxxxxx>
>>>> Signed-off-by: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>
>>>> ---
>>>> fs/nfs/filelayout/filelayout.c | 31 ++++++++++++++++++++++++++
>>>> fs/nfs/pnfs.h | 20 +++++++++++++++++
>>>> fs/nfs/write.c | 49 +++++++++++++++++++++++++++++++-----------
>>>> 3 files changed, 88 insertions(+), 12 deletions(-)
>>>>
>>>> diff --git a/fs/nfs/filelayout/filelayout.c b/fs/nfs/filelayout/filelayout.c
>>>> index 2576d28b..524e66f 100644
>>>> --- a/fs/nfs/filelayout/filelayout.c
>>>> +++ b/fs/nfs/filelayout/filelayout.c
>>>> @@ -1237,6 +1237,36 @@ restart:
>>>> spin_unlock(cinfo->lock);
>>>> }
>>>>
>>>> +/* filelayout_search_commit_reqs - Search lists in @cinfo for the head reqest
>>>> + * for @page
>>>> + * @cinfo - commit info for current inode
>>>> + * @page - page to search for matching head request
>>>> + *
>>>> + * Returns a the head request if one is found, otherwise returns NULL.
>>>> + */
>>>> +static struct nfs_page *
>>>> +filelayout_search_commit_reqs(struct nfs_commit_info *cinfo, struct page *page)
>>>> +{
>>>> + struct nfs_page *freq, *t;
>>>> + struct pnfs_commit_bucket *b;
>>>> + int i;
>>>> +
>>>> + /* Linearly search the commit lists for each bucket until a matching
>>>> + * request is found */
>>>> + for (i = 0, b = cinfo->ds->buckets; i < cinfo->ds->nbuckets; i++, b++) {
>>>> + list_for_each_entry_safe(freq, t, &b->written, wb_list) {
>>>> + if (freq->wb_page == page)
>>>> + return freq->wb_head;
>>>> + }
>>>> + list_for_each_entry_safe(freq, t, &b->committing, wb_list) {
>>>> + if (freq->wb_page == page)
>>>> + return freq->wb_head;
>>>> + }
>>>> + }
>>>> +
>>>> + return NULL;
>>>> +}
>>>> +
>>>> static void filelayout_retry_commit(struct nfs_commit_info *cinfo, int idx)
>>>> {
>>>> struct pnfs_ds_commit_info *fl_cinfo = cinfo->ds;
>>>> @@ -1386,6 +1416,7 @@ static struct pnfs_layoutdriver_type filelayout_type = {
>>>> .clear_request_commit = filelayout_clear_request_commit,
>>>> .scan_commit_lists = filelayout_scan_commit_lists,
>>>> .recover_commit_reqs = filelayout_recover_commit_reqs,
>>>> + .search_commit_reqs = filelayout_search_commit_reqs,
>>>> .commit_pagelist = filelayout_commit_pagelist,
>>>> .read_pagelist = filelayout_read_pagelist,
>>>> .write_pagelist = filelayout_write_pagelist,
>>>> diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
>>>> index 27ddecd..203b6c9 100644
>>>> --- a/fs/nfs/pnfs.h
>>>> +++ b/fs/nfs/pnfs.h
>>>> @@ -104,6 +104,8 @@ struct pnfs_layoutdriver_type {
>>>> int max);
>>>> void (*recover_commit_reqs) (struct list_head *list,
>>>> struct nfs_commit_info *cinfo);
>>>> + struct nfs_page * (*search_commit_reqs)(struct nfs_commit_info *cinfo,
>>>> + struct page *page);
>>>> int (*commit_pagelist)(struct inode *inode,
>>>> struct list_head *mds_pages,
>>>> int how,
>>>> @@ -341,6 +343,17 @@ pnfs_recover_commit_reqs(struct inode *inode, struct list_head *list,
>>>> NFS_SERVER(inode)->pnfs_curr_ld->recover_commit_reqs(list, cinfo);
>>>> }
>>>>
>>>> +static inline struct nfs_page *
>>>> +pnfs_search_commit_reqs(struct inode *inode, struct nfs_commit_info *cinfo,
>>>> + struct page *page)
>>>> +{
>>>> + struct pnfs_layoutdriver_type *ld = NFS_SERVER(inode)->pnfs_curr_ld;
>>>> +
>>>> + if (ld == NULL || ld->search_commit_reqs == NULL)
>>>> + return NULL;
>>>> + return ld->search_commit_reqs(cinfo, page);
>>>> +}
>>>> +
>>>> /* Should the pNFS client commit and return the layout upon a setattr */
>>>> static inline bool
>>>> pnfs_ld_layoutret_on_setattr(struct inode *inode)
>>>> @@ -492,6 +505,13 @@ pnfs_recover_commit_reqs(struct inode *inode, struct list_head *list,
>>>> {
>>>> }
>>>>
>>>> +static inline struct nfs_page *
>>>> +pnfs_search_commit_reqs(struct inode *inode, struct nfs_commit_info *cinfo,
>>>> + struct page *page)
>>>> +{
>>>> + return NULL;
>>>> +}
>>>> +
>>>> static inline int pnfs_layoutcommit_inode(struct inode *inode, bool sync)
>>>> {
>>>> return 0;
>>>> diff --git a/fs/nfs/write.c b/fs/nfs/write.c
>>>> index e6bc5b5..ba41769 100644
>>>> --- a/fs/nfs/write.c
>>>> +++ b/fs/nfs/write.c
>>>> @@ -47,6 +47,8 @@ static const struct nfs_pgio_completion_ops nfs_async_write_completion_ops;
>>>> static const struct nfs_commit_completion_ops nfs_commit_completion_ops;
>>>> static const struct nfs_rw_ops nfs_rw_write_ops;
>>>> static void nfs_clear_request_commit(struct nfs_page *req);
>>>> +static void nfs_init_cinfo_from_inode(struct nfs_commit_info *cinfo,
>>>> + struct inode *inode);
>>>>
>>>> static struct kmem_cache *nfs_wdata_cachep;
>>>> static mempool_t *nfs_wdata_mempool;
>>>> @@ -93,6 +95,38 @@ static void nfs_context_set_write_error(struct nfs_open_context *ctx, int error)
>>>> }
>>>>
>>>> /*
>>>> + * nfs_page_search_commits_for_head_request_locked
>>>> + *
>>>> + * Search through commit lists on @inode for the head request for @page.
>>>> + * Must be called while holding the inode (which is cinfo) lock.
>>>> + *
>>>> + * Returns the head request if found, or NULL if not found.
>>>> + */
>>>> +static struct nfs_page *
>>>> +nfs_page_search_commits_for_head_request_locked(struct nfs_inode *nfsi,
>>>> + struct page *page)
>>>> +{
>>>> + struct nfs_page *freq, *t;
>>>> + struct nfs_commit_info cinfo;
>>>> + struct inode *inode = &nfsi->vfs_inode;
>>>> +
>>>> + nfs_init_cinfo_from_inode(&cinfo, inode);
>>> This has a compiler warning when CONFIG_NFS_V4=n, CONFIG_NFS_V3=n, CONFIG_NFS_V2=y:
>>>
>>> fs/nfs/write.c: In function �nfs_page_find_head_request_locked.isra.16�:
>>> include/linux/kernel.h:834:39: error: �cinfo.mds� may be used uninitialized in this function [-Werror=maybe-uninitialized]
>>> const typeof( ((type *)0)->member ) *__mptr = (ptr); \
>>> ^
>>> fs/nfs/write.c:110:25: note: �cinfo.mds� was declared here
>>> struct nfs_commit_info cinfo;
>>>
>>> Probably because nfs_init_cinfo_from_inode() is empty without v3 or v4.
>>>
>>> Anna
>>>> +
>>>> + /* search through pnfs commit lists */
>>>> + freq = pnfs_search_commit_reqs(inode, &cinfo, page);
>>>> + if (freq)
>>>> + return freq->wb_head;
>>>> +
>>>> + /* Linearly search the commit list for the correct request */
>>>> + list_for_each_entry_safe(freq, t, &cinfo.mds->list, wb_list) {
>>>> + if (freq->wb_page == page)
>>>> + return freq->wb_head;
>>>> + }
>>>> +
>>>> + return NULL;
>>>> +}
>>>> +
>>>> +/*
>>>> * nfs_page_find_head_request_locked - find head request associated with @page
>>>> *
>>>> * must be called while holding the inode lock.
>>>> @@ -106,21 +140,12 @@ nfs_page_find_head_request_locked(struct nfs_inode *nfsi, struct page *page)
>>>>
>>>> if (PagePrivate(page))
>>>> req = (struct nfs_page *)page_private(page);
>>>> - else if (unlikely(PageSwapCache(page))) {
>>>> - struct nfs_page *freq, *t;
>>>> -
>>>> - /* Linearly search the commit list for the correct req */
>>>> - list_for_each_entry_safe(freq, t, &nfsi->commit_info.list, wb_list) {
>>>> - if (freq->wb_page == page) {
>>>> - req = freq->wb_head;
>>>> - break;
>>>> - }
>>>> - }
>>>> - }
>>>> + else if (unlikely(PageSwapCache(page)))
>>>> + req = nfs_page_search_commits_for_head_request_locked(nfsi,
>>>> + page);
>>>>
>>>> if (req) {
>>>> WARN_ON_ONCE(req->wb_head != req);
>>>> -
>>>> kref_get(&req->wb_kref);
>>>> }
--
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
