On 2012-05-28 19:53, Boaz Harrosh wrote:
> On 05/28/2012 07:09 PM, Benny Halevy wrote:
>
>> Signed-off-by: Benny Halevy <bhalevy@xxxxxxxxxx>
>
>
> Benny I disagree with this patch.
>
> Not specifically with the print but with the !found print.
>
> pnfsd_roc is always called layouts or not. So these
> !found print are bogus.
>
>> ---
>> fs/nfsd/nfs4pnfsd.c | 6 ++++++
>> 1 files changed, 6 insertions(+), 0 deletions(-)
>>
>> diff --git a/fs/nfsd/nfs4pnfsd.c b/fs/nfsd/nfs4pnfsd.c
>> index cfaac56..0a8d5b5 100644
>> --- a/fs/nfsd/nfs4pnfsd.c
>> +++ b/fs/nfsd/nfs4pnfsd.c
>> @@ -1227,7 +1227,9 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh,
>> void pnfsd_roc(struct nfs4_client *clp, struct nfs4_file *fp)
>> {
>> struct nfs4_layout *lo, *nextlp;
>> + bool found = false;
>>
>> + dprintk("%s: fp=%p clp=%p", __func__, fp, clp);
>> spin_lock(&layout_lock);
>> list_for_each_entry_safe (lo, nextlp, &fp->fi_layouts, lo_perfile) {
>> struct nfsd4_pnfs_layoutreturn lr;
>> @@ -1245,11 +1247,15 @@ void pnfsd_roc(struct nfs4_client *clp, struct nfs4_file *fp)
>> destroy_layout(lo); /* do not access lp after this */
>>
>> empty = list_empty(&fp->fi_layouts);
>> + found = true;
>> + dprintk("%s: fp=%p clp=%p: return on close", __func__, fp, clp);
>> fs_layout_return(fp->fi_inode->i_sb, fp->fi_inode, &lr,
>> LR_FLAG_INTERN,
>> empty ? PNFS_LAST_LAYOUT_NO_RECALLS : NULL);
>
>
> fs_layout_return already prints for each call. So we should only add one
> global print so to know we came from ROC
>
> Please don't fix it. I have this covered in my patches. In fact this
> function is totally changed.
>
OK.
>> }
>> spin_unlock(&layout_lock);
>> + if (!found)
>> + dprintk("%s: no layout found", __func__);
>
>
> Again please no prints here. pnfsd_roc is always called unconditionally
> from last close.
> (And found should be conditional on SUNRPC_DEBUG if is left)
>
>> }
>>
>> void pnfs_expire_client(struct nfs4_client *clp)
>
>
> Thanks
> 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
--
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
