On Jun 2, 2012, at 7:00 PM, Boaz Harrosh wrote:
> On 06/01/2012 08:19 PM, andros@xxxxxxxxxx wrote:
>
>> From: Andy Adamson <andros@xxxxxxxxxx>
>>
>
>
> I wish you would send your patches preceded with a [PATCHSET 0/x]
> That gives us a short summery and a little background on its motivation.
I often do. These patches actually stand alone so I didn't this time.
This patch is totally explained by the patch title, patch comments, and in-line comments. With out this patch, we can end up
sending a LAYOUTRETURN with no layout segments that need to be returned! Why? because as noted in the patch comments, mark_matching_lsegs_invalid does not indicate when the plh_segs list is empty…etc. When? well, as noted in the in-line code comments, when the pnfs_layout_hdr has an empty plh_segs which an occur when a LAYOUTGET fails, or when LAYOUTGET succeeded, but the deviceid is marked invalid
What else is there to say?!
I guess I can add the --cover-letter to the git format-patch and cut/paste to it.
-->Andy
>
> For example here, what are the problems you encounter, how did you test
> it and so on...
>
> Do these actually depend and/or fix your previous "send layout_return ..."
> In files layout?
>
> (You can still send all patches by git send-email command line just the same
> just add a [PATCHSET 0/x]-xxx-yyy.txt and it will get sent first, right?)
>
> Thanks
> Boaz
>
>> mark_matching_lsegs_invalid() does not indicate if the plh_segs list is
>> empty on entrance - and it can be empty on exit.
>>
>> When the plh_segs list is emtpy, mark_matching_lsegs_invalid removes
>> the pnfs_layout_hdr creation reference.
>>
>> Signed-off-by: Andy Adamson <andros@xxxxxxxxxx>
>> ---
>> fs/nfs/pnfs.c | 17 +++++++++++++++--
>> 1 files changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
>> index b8323aa..854df5e 100644
>> --- a/fs/nfs/pnfs.c
>> +++ b/fs/nfs/pnfs.c
>> @@ -646,7 +646,14 @@ out_err_free:
>> return NULL;
>> }
>>
>> -/* Initiates a LAYOUTRETURN(FILE) */
>> +/*
>> + * Initiates a LAYOUTRETURN(FILE), and removes the pnfs_layout_hdr
>> + * when the layout segment list is empty.
>> + *
>> + * Note that a pnfs_layout_hdr can exist with an empty layout segment
>> + * list when LAYOUTGET has failed, or when LAYOUTGET succeeded, but the
>> + * deviceid is marked invalid.
>> + */
>> int
>> _pnfs_return_layout(struct inode *ino)
>> {
>> @@ -655,7 +662,7 @@ _pnfs_return_layout(struct inode *ino)
>> LIST_HEAD(tmp_list);
>> struct nfs4_layoutreturn *lrp;
>> nfs4_stateid stateid;
>> - int status = 0;
>> + int status = 0, empty = 0;
>>
>> dprintk("--> %s\n", __func__);
>>
>> @@ -669,11 +676,17 @@ _pnfs_return_layout(struct inode *ino)
>> stateid = nfsi->layout->plh_stateid;
>> /* Reference matched in nfs4_layoutreturn_release */
>> get_layout_hdr(lo);
>> + empty = list_empty(&lo->plh_segs);
>> mark_matching_lsegs_invalid(lo, &tmp_list, NULL);
>> lo->plh_block_lgets++;
>> spin_unlock(&ino->i_lock);
>> pnfs_free_lseg_list(&tmp_list);
>>
>> + if (empty) {
>> + put_layout_hdr(lo);
>> + dprintk("%s: no layout segments to return\n", __func__);
>> + return status;
>> + }
>> WARN_ON(test_bit(NFS_INO_LAYOUTCOMMIT, &nfsi->flags));
>>
>> lrp = kzalloc(sizeof(*lrp), GFP_KERNEL);
>
>
--
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
