* Question about LAYOUTRETURN stateid
@ 2010-10-12 11:09 P.B.Shelley
2010-10-12 13:11 ` Fred Isaman
0 siblings, 1 reply; 6+ messages in thread
From: P.B.Shelley @ 2010-10-12 11:09 UTC (permalink / raw)
To: linux-nfs
Hi, all
While reading Linux pnfs code, I have a question in layoutreturn code path.
nfs4_layoutreturn_release() only invalidate layout stateid when
res.lrs_present is FALSE. If it is TRUE, client is supposed to set it
to res.stateid, is it? But I do not see somewhere the layout stateid
is updated. Am I missing something?
5683 static void nfs4_layoutreturn_release(void *calldata)
5684 {
5685 struct nfs4_layoutreturn *lrp = calldata;
5686 struct pnfs_layout_hdr *lo = NFS_I(lrp->args.inode)->layout;
5687
5688 dprintk("--> %s return_type %d lo %p\n", __func__,
5689 lrp->args.return_type, lo);
5690
5691 if (lrp->args.return_type == RETURN_FILE) {
5692 if (!lrp->res.lrs_present)
5693 pnfs_invalidate_layout_stateid(lo);
5694 pnfs_layoutreturn_release(lo, &lrp->args.range);
5695 }
5696 kfree(calldata);
5697 dprintk("<-- %s\n", __func__);
5698 }
--
Thanks,
Shelley
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Question about LAYOUTRETURN stateid
2010-10-12 11:09 Question about LAYOUTRETURN stateid P.B.Shelley
@ 2010-10-12 13:11 ` Fred Isaman
[not found] ` <AANLkTik6WBUJrSW5GY+i-iERxU-rMz5D0oWFa-n8HhH+-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: Fred Isaman @ 2010-10-12 13:11 UTC (permalink / raw)
To: P.B.Shelley; +Cc: linux-nfs
On Tue, Oct 12, 2010 at 7:09 AM, P.B.Shelley <shelleypt@gmail.com> wrote:
> Hi, all
>
> While reading Linux pnfs code, I have a question in layoutreturn code path.
>
> nfs4_layoutreturn_release() only invalidate layout stateid when
> res.lrs_present is FALSE. If it is TRUE, client is supposed to set it
> to res.stateid, is it? But I do not see somewhere the layout stateid
> is updated. Am I missing something?
>
No, you are not missing anything. that is a bug.
Fred
> 5683 static void nfs4_layoutreturn_release(void *calldata)
> 5684 {
> 5685 struct nfs4_layoutreturn *lrp = calldata;
> 5686 struct pnfs_layout_hdr *lo = NFS_I(lrp->args.inode)->layout;
> 5687
> 5688 dprintk("--> %s return_type %d lo %p\n", __func__,
> 5689 lrp->args.return_type, lo);
> 5690
> 5691 if (lrp->args.return_type == RETURN_FILE) {
> 5692 if (!lrp->res.lrs_present)
> 5693 pnfs_invalidate_layout_stateid(lo);
> 5694 pnfs_layoutreturn_release(lo, &lrp->args.range);
> 5695 }
> 5696 kfree(calldata);
> 5697 dprintk("<-- %s\n", __func__);
> 5698 }
>
> --
> Thanks,
> Shelley
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Question about LAYOUTRETURN stateid
[not found] ` <AANLkTik6WBUJrSW5GY+i-iERxU-rMz5D0oWFa-n8HhH+-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2010-10-12 14:40 ` Benny Halevy
2010-10-12 15:29 ` William A. (Andy) Adamson
0 siblings, 1 reply; 6+ messages in thread
From: Benny Halevy @ 2010-10-12 14:40 UTC (permalink / raw)
To: Fred Isaman; +Cc: P.B.Shelley, linux-nfs
On 2010-10-12 09:11, Fred Isaman wrote:
> On Tue, Oct 12, 2010 at 7:09 AM, P.B.Shelley <shelleypt@gmail.com> wrote:
>> Hi, all
>>
>> While reading Linux pnfs code, I have a question in layoutreturn code path.
>>
>> nfs4_layoutreturn_release() only invalidate layout stateid when
>> res.lrs_present is FALSE. If it is TRUE, client is supposed to set it
>> to res.stateid, is it? But I do not see somewhere the layout stateid
>> is updated. Am I missing something?
>>
>
> No, you are not missing anything. that is a bug.
>
Agreed.
Who's volunteering to send a fix? :)
Benny
> Fred
>
>> 5683 static void nfs4_layoutreturn_release(void *calldata)
>> 5684 {
>> 5685 struct nfs4_layoutreturn *lrp = calldata;
>> 5686 struct pnfs_layout_hdr *lo = NFS_I(lrp->args.inode)->layout;
>> 5687
>> 5688 dprintk("--> %s return_type %d lo %p\n", __func__,
>> 5689 lrp->args.return_type, lo);
>> 5690
>> 5691 if (lrp->args.return_type == RETURN_FILE) {
>> 5692 if (!lrp->res.lrs_present)
>> 5693 pnfs_invalidate_layout_stateid(lo);
>> 5694 pnfs_layoutreturn_release(lo, &lrp->args.range);
>> 5695 }
>> 5696 kfree(calldata);
>> 5697 dprintk("<-- %s\n", __func__);
>> 5698 }
>>
>> --
>> Thanks,
>> Shelley
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to majordomo@vger.kernel.org
>> 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@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Question about LAYOUTRETURN stateid
2010-10-12 14:40 ` Benny Halevy
@ 2010-10-12 15:29 ` William A. (Andy) Adamson
2010-10-12 15:38 ` Fred Isaman
0 siblings, 1 reply; 6+ messages in thread
From: William A. (Andy) Adamson @ 2010-10-12 15:29 UTC (permalink / raw)
To: Benny Halevy; +Cc: Fred Isaman, P.B.Shelley, linux-nfs
I'll send one
-->Andy
On Tue, Oct 12, 2010 at 10:40 AM, Benny Halevy <bhalevy@panasas.com> wr=
ote:
> On 2010-10-12 09:11, Fred Isaman wrote:
>> On Tue, Oct 12, 2010 at 7:09 AM, P.B.Shelley <shelleypt@gmail.com> w=
rote:
>>> Hi, all
>>>
>>> While reading Linux pnfs code, I have a question in layoutreturn co=
de path.
>>>
>>> nfs4_layoutreturn_release() only invalidate layout stateid when
>>> res.lrs_present is FALSE. If it is TRUE, client is supposed to set =
it
>>> to res.stateid, is it? But I do not see somewhere the layout statei=
d
>>> is updated. Am I missing something?
>>>
>>
>> No, you are not missing anything. =A0that is a bug.
>>
>
> Agreed.
> Who's volunteering to send a fix? :)
>
> Benny
>
>> Fred
>>
>>> 5683 static void nfs4_layoutreturn_release(void *calldata)
>>> 5684 {
>>> 5685 =A0 =A0 =A0 =A0 struct nfs4_layoutreturn *lrp =3D calldata;
>>> 5686 =A0 =A0 =A0 =A0 struct pnfs_layout_hdr *lo =3D NFS_I(lrp->args=
=2Einode)->layout;
>>> 5687
>>> 5688 =A0 =A0 =A0 =A0 dprintk("--> %s return_type %d lo %p\n", __fun=
c__,
>>> 5689 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 lrp->args.return_type, lo);
>>> 5690
>>> 5691 =A0 =A0 =A0 =A0 if (lrp->args.return_type =3D=3D RETURN_FILE) =
{
>>> 5692 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (!lrp->res.lrs_present)
>>> 5693 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 pnfs_invalidat=
e_layout_stateid(lo);
>>> 5694 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 pnfs_layoutreturn_release(lo, =
&lrp->args.range);
>>> 5695 =A0 =A0 =A0 =A0 }
>>> 5696 =A0 =A0 =A0 =A0 kfree(calldata);
>>> 5697 =A0 =A0 =A0 =A0 dprintk("<-- %s\n", __func__);
>>> 5698 }
>>>
>>> --
>>> Thanks,
>>> Shelley
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs=
" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at =A0http://vger.kernel.org/majordomo-info.htm=
l
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs"=
in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at =A0http://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@vger.kernel.org
> More majordomo info at =A0http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Question about LAYOUTRETURN stateid
2010-10-12 15:29 ` William A. (Andy) Adamson
@ 2010-10-12 15:38 ` Fred Isaman
[not found] ` <AANLkTinv9OZ1M9ykACYYavT57W69er_uTYXRiQA_aon9-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: Fred Isaman @ 2010-10-12 15:38 UTC (permalink / raw)
To: William A. (Andy) Adamson; +Cc: Benny Halevy, P.B.Shelley, linux-nfs
On Tue, Oct 12, 2010 at 11:29 AM, William A. (Andy) Adamson
<androsadamson@gmail.com> wrote:
> I'll send one
>
> -->Andy
Another issue in the same code that could be fixed at the same time:
the set/invalidate stateid decision should not be under the type==FILE
check, but should always be done.
Fred
>
> On Tue, Oct 12, 2010 at 10:40 AM, Benny Halevy <bhalevy@panasas.com> wrote:
>> On 2010-10-12 09:11, Fred Isaman wrote:
>>> On Tue, Oct 12, 2010 at 7:09 AM, P.B.Shelley <shelleypt@gmail.com> wrote:
>>>> Hi, all
>>>>
>>>> While reading Linux pnfs code, I have a question in layoutreturn code path.
>>>>
>>>> nfs4_layoutreturn_release() only invalidate layout stateid when
>>>> res.lrs_present is FALSE. If it is TRUE, client is supposed to set it
>>>> to res.stateid, is it? But I do not see somewhere the layout stateid
>>>> is updated. Am I missing something?
>>>>
>>>
>>> No, you are not missing anything. that is a bug.
>>>
>>
>> Agreed.
>> Who's volunteering to send a fix? :)
>>
>> Benny
>>
>>> Fred
>>>
>>>> 5683 static void nfs4_layoutreturn_release(void *calldata)
>>>> 5684 {
>>>> 5685 struct nfs4_layoutreturn *lrp = calldata;
>>>> 5686 struct pnfs_layout_hdr *lo = NFS_I(lrp->args.inode)->layout;
>>>> 5687
>>>> 5688 dprintk("--> %s return_type %d lo %p\n", __func__,
>>>> 5689 lrp->args.return_type, lo);
>>>> 5690
>>>> 5691 if (lrp->args.return_type == RETURN_FILE) {
>>>> 5692 if (!lrp->res.lrs_present)
>>>> 5693 pnfs_invalidate_layout_stateid(lo);
>>>> 5694 pnfs_layoutreturn_release(lo, &lrp->args.range);
>>>> 5695 }
>>>> 5696 kfree(calldata);
>>>> 5697 dprintk("<-- %s\n", __func__);
>>>> 5698 }
>>>>
>>>> --
>>>> Thanks,
>>>> Shelley
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> 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@vger.kernel.org
>>> 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@vger.kernel.org
>> 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@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Question about LAYOUTRETURN stateid
[not found] ` <AANLkTinv9OZ1M9ykACYYavT57W69er_uTYXRiQA_aon9-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2010-10-12 15:41 ` William A. (Andy) Adamson
0 siblings, 0 replies; 6+ messages in thread
From: William A. (Andy) Adamson @ 2010-10-12 15:41 UTC (permalink / raw)
To: Fred Isaman; +Cc: Benny Halevy, P.B.Shelley, linux-nfs
On Tue, Oct 12, 2010 at 11:38 AM, Fred Isaman <iisaman@netapp.com> wrot=
e:
> On Tue, Oct 12, 2010 at 11:29 AM, William A. (Andy) Adamson
> <androsadamson@gmail.com> wrote:
>> I'll send one
>>
>> -->Andy
>
> Another issue in the same code that could be fixed at the same time:
> the set/invalidate stateid decision should not be under the type=3D=3D=
=46ILE
> check, but should always be done.
>
> Fred
OK - I'll look that over.
-->Andy
>
>>
>> On Tue, Oct 12, 2010 at 10:40 AM, Benny Halevy <bhalevy@panasas.com>=
wrote:
>>> On 2010-10-12 09:11, Fred Isaman wrote:
>>>> On Tue, Oct 12, 2010 at 7:09 AM, P.B.Shelley <shelleypt@gmail.com>=
wrote:
>>>>> Hi, all
>>>>>
>>>>> While reading Linux pnfs code, I have a question in layoutreturn =
code path.
>>>>>
>>>>> nfs4_layoutreturn_release() only invalidate layout stateid when
>>>>> res.lrs_present is FALSE. If it is TRUE, client is supposed to se=
t it
>>>>> to res.stateid, is it? But I do not see somewhere the layout stat=
eid
>>>>> is updated. Am I missing something?
>>>>>
>>>>
>>>> No, you are not missing anything. =A0that is a bug.
>>>>
>>>
>>> Agreed.
>>> Who's volunteering to send a fix? :)
>>>
>>> Benny
>>>
>>>> Fred
>>>>
>>>>> 5683 static void nfs4_layoutreturn_release(void *calldata)
>>>>> 5684 {
>>>>> 5685 =A0 =A0 =A0 =A0 struct nfs4_layoutreturn *lrp =3D calldata;
>>>>> 5686 =A0 =A0 =A0 =A0 struct pnfs_layout_hdr *lo =3D NFS_I(lrp->ar=
gs.inode)->layout;
>>>>> 5687
>>>>> 5688 =A0 =A0 =A0 =A0 dprintk("--> %s return_type %d lo %p\n", __f=
unc__,
>>>>> 5689 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 lrp->args.return_type, lo);
>>>>> 5690
>>>>> 5691 =A0 =A0 =A0 =A0 if (lrp->args.return_type =3D=3D RETURN_FILE=
) {
>>>>> 5692 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (!lrp->res.lrs_present)
>>>>> 5693 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 pnfs_invalid=
ate_layout_stateid(lo);
>>>>> 5694 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 pnfs_layoutreturn_release(lo=
, &lrp->args.range);
>>>>> 5695 =A0 =A0 =A0 =A0 }
>>>>> 5696 =A0 =A0 =A0 =A0 kfree(calldata);
>>>>> 5697 =A0 =A0 =A0 =A0 dprintk("<-- %s\n", __func__);
>>>>> 5698 }
>>>>>
>>>>> --
>>>>> Thanks,
>>>>> Shelley
>>>>> --
>>>>> To unsubscribe from this list: send the line "unsubscribe linux-n=
fs" in
>>>>> the body of a message to majordomo@vger.kernel.org
>>>>> More majordomo info at =A0http://vger.kernel.org/majordomo-info.h=
tml
>>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-nf=
s" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at =A0http://vger.kernel.org/majordomo-info.ht=
ml
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs=
" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at =A0http://vger.kernel.org/majordomo-info.htm=
l
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs"=
in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at =A0http://vger.kernel.org/majordomo-info.html
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-10-12 15:41 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-10-12 11:09 Question about LAYOUTRETURN stateid P.B.Shelley
2010-10-12 13:11 ` Fred Isaman
[not found] ` <AANLkTik6WBUJrSW5GY+i-iERxU-rMz5D0oWFa-n8HhH+-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-10-12 14:40 ` Benny Halevy
2010-10-12 15:29 ` William A. (Andy) Adamson
2010-10-12 15:38 ` Fred Isaman
[not found] ` <AANLkTinv9OZ1M9ykACYYavT57W69er_uTYXRiQA_aon9-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-10-12 15:41 ` William A. (Andy) Adamson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.