diff mbox

NFSv4.1: nfs4_fl_prepare_ds must be careful about reporting success.

Message ID 87lgxiwoxi.fsf@notabene.neil.brown.name
State New, archived
Headers show

Commit Message

NeilBrown Oct. 21, 2016, 12:01 a.m. UTC
Various places assume that if nfs4_fl_prepare_ds() turns a non-NULL
'ds', then ds->ds_clp will also be non-NULL.

This is not necessasrily true in the case when the process received a
fatal signal while nfs4_pnfs_ds_connect is waiting in
nfs4_wait_ds_connect().  In that case ->ds_clp may not be set, and the
devid may not recently have been marked unavailable.

So add a test for ->ds_clp == NULL and return NULL in that case.

Fixes: c23266d532b4 ("NFS4.1 Fix data server connection race")
Signed-off-by: NeilBrown <neilb@suse.com>
---
 fs/nfs/filelayout/filelayoutdev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

NeilBrown Nov. 18, 2016, 5:01 a.m. UTC | #1
Ping?
No sign of this in linux-next, and no replies....

Thanks,
NeilBrown


On Fri, Oct 21 2016, NeilBrown wrote:

>
> Various places assume that if nfs4_fl_prepare_ds() turns a non-NULL
> 'ds', then ds->ds_clp will also be non-NULL.
>
> This is not necessasrily true in the case when the process received a
> fatal signal while nfs4_pnfs_ds_connect is waiting in
> nfs4_wait_ds_connect().  In that case ->ds_clp may not be set, and the
> devid may not recently have been marked unavailable.
>
> So add a test for ->ds_clp == NULL and return NULL in that case.
>
> Fixes: c23266d532b4 ("NFS4.1 Fix data server connection race")
> Signed-off-by: NeilBrown <neilb@suse.com>
> ---
>  fs/nfs/filelayout/filelayoutdev.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/fs/nfs/filelayout/filelayoutdev.c b/fs/nfs/filelayout/filelayoutdev.c
> index 4946ef40ba87..85ef38f9765f 100644
> --- a/fs/nfs/filelayout/filelayoutdev.c
> +++ b/fs/nfs/filelayout/filelayoutdev.c
> @@ -283,7 +283,8 @@ nfs4_fl_prepare_ds(struct pnfs_layout_segment *lseg, u32 ds_idx)
>  			     s->nfs_client->cl_rpcclient->cl_auth->au_flavor);
>  
>  out_test_devid:
> -	if (filelayout_test_devid_unavailable(devid))
> +	if (ret->ds_clp == NULL ||
> +	    filelayout_test_devid_unavailable(devid))
>  		ret = NULL;
>  out:
>  	return ret;
> -- 
> 2.10.1
Trond Myklebust Nov. 18, 2016, 2:32 p.m. UTC | #2
DQo+IE9uIE5vdiAxOCwgMjAxNiwgYXQgMDA6MDEsIE5laWxCcm93biA8bmVpbGJAc3VzZS5jb20+
IHdyb3RlOg0KPiANCj4gDQo+IFBpbmc/DQo+IE5vIHNpZ24gb2YgdGhpcyBpbiBsaW51eC1uZXh0
LCBhbmQgbm8gcmVwbGllc+KApi4NCg0KSeKAmWQgbGlrZSBzb21lIGNvbmZpcm1hdGlvbiBmcm9t
IHRoZSBOZXRBcHAgZm9sa3Mgb24gdGhpcy4gVGhleSBhcmUgdGhlIG1haW4gc3Rha2Vob2xkZXJz
IGZvciB0aGUgcE5GUyBmaWxlcyBpbXBsZW1lbnRhdGlvbi4gSSBkb27igJl0IHRoaW5rIHdlIG5l
ZWQgYW4gZXF1aXZhbGVudCBmb3IgZmxleGZpbGVzLg0KDQo+IA0KPiBUaGFua3MsDQo+IE5laWxC
cm93bg0KPiANCj4gDQo+IE9uIEZyaSwgT2N0IDIxIDIwMTYsIE5laWxCcm93biB3cm90ZToNCj4g
DQo+PiANCj4+IFZhcmlvdXMgcGxhY2VzIGFzc3VtZSB0aGF0IGlmIG5mczRfZmxfcHJlcGFyZV9k
cygpIHR1cm5zIGEgbm9uLU5VTEwNCj4+ICdkcycsIHRoZW4gZHMtPmRzX2NscCB3aWxsIGFsc28g
YmUgbm9uLU5VTEwuDQo+PiANCj4+IFRoaXMgaXMgbm90IG5lY2Vzc2FzcmlseSB0cnVlIGluIHRo
ZSBjYXNlIHdoZW4gdGhlIHByb2Nlc3MgcmVjZWl2ZWQgYQ0KPj4gZmF0YWwgc2lnbmFsIHdoaWxl
IG5mczRfcG5mc19kc19jb25uZWN0IGlzIHdhaXRpbmcgaW4NCj4+IG5mczRfd2FpdF9kc19jb25u
ZWN0KCkuICBJbiB0aGF0IGNhc2UgLT5kc19jbHAgbWF5IG5vdCBiZSBzZXQsIGFuZCB0aGUNCj4+
IGRldmlkIG1heSBub3QgcmVjZW50bHkgaGF2ZSBiZWVuIG1hcmtlZCB1bmF2YWlsYWJsZS4NCj4+
IA0KPj4gU28gYWRkIGEgdGVzdCBmb3IgLT5kc19jbHAgPT0gTlVMTCBhbmQgcmV0dXJuIE5VTEwg
aW4gdGhhdCBjYXNlLg0KPj4gDQo+PiBGaXhlczogYzIzMjY2ZDUzMmI0ICgiTkZTNC4xIEZpeCBk
YXRhIHNlcnZlciBjb25uZWN0aW9uIHJhY2UiKQ0KPj4gU2lnbmVkLW9mZi1ieTogTmVpbEJyb3du
IDxuZWlsYkBzdXNlLmNvbT4NCj4+IC0tLQ0KPj4gZnMvbmZzL2ZpbGVsYXlvdXQvZmlsZWxheW91
dGRldi5jIHwgMyArKy0NCj4+IDEgZmlsZSBjaGFuZ2VkLCAyIGluc2VydGlvbnMoKyksIDEgZGVs
ZXRpb24oLSkNCj4+IA0KPj4gZGlmZiAtLWdpdCBhL2ZzL25mcy9maWxlbGF5b3V0L2ZpbGVsYXlv
dXRkZXYuYyBiL2ZzL25mcy9maWxlbGF5b3V0L2ZpbGVsYXlvdXRkZXYuYw0KPj4gaW5kZXggNDk0
NmVmNDBiYTg3Li44NWVmMzhmOTc2NWYgMTAwNjQ0DQo+PiAtLS0gYS9mcy9uZnMvZmlsZWxheW91
dC9maWxlbGF5b3V0ZGV2LmMNCj4+ICsrKyBiL2ZzL25mcy9maWxlbGF5b3V0L2ZpbGVsYXlvdXRk
ZXYuYw0KPj4gQEAgLTI4Myw3ICsyODMsOCBAQCBuZnM0X2ZsX3ByZXBhcmVfZHMoc3RydWN0IHBu
ZnNfbGF5b3V0X3NlZ21lbnQgKmxzZWcsIHUzMiBkc19pZHgpDQo+PiAJCQkgICAgIHMtPm5mc19j
bGllbnQtPmNsX3JwY2NsaWVudC0+Y2xfYXV0aC0+YXVfZmxhdm9yKTsNCj4+IA0KPj4gb3V0X3Rl
c3RfZGV2aWQ6DQo+PiAtCWlmIChmaWxlbGF5b3V0X3Rlc3RfZGV2aWRfdW5hdmFpbGFibGUoZGV2
aWQpKQ0KPj4gKwlpZiAocmV0LT5kc19jbHAgPT0gTlVMTCB8fA0KPj4gKwkgICAgZmlsZWxheW91
dF90ZXN0X2RldmlkX3VuYXZhaWxhYmxlKGRldmlkKSkNCj4+IAkJcmV0ID0gTlVMTDsNCj4+IG91
dDoNCj4+IAlyZXR1cm4gcmV0Ow0KPj4gLS0gDQo+PiAyLjEwLjENCg0K

--
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
Olga Kornievskaia Nov. 18, 2016, 5:34 p.m. UTC | #3
On Fri, Nov 18, 2016 at 9:32 AM, Trond Myklebust
<trondmy@primarydata.com> wrote:
>
>> On Nov 18, 2016, at 00:01, NeilBrown <neilb@suse.com> wrote:
>>
>>
>> Ping?
>> No sign of this in linux-next, and no replies….
>
> I’d like some confirmation from the NetApp folks on this. They are the main stakeholders for the pNFS files implementation. I don’t think we need an equivalent for flex files.

Just to see if I understand this patch. If "ds" isn't NULL, then the
client has an RPC client with the ds. But it doesn't have a valid NFS
client? Looking at the code I really don't see how that can happen.
Maybe there is a bug elsewhere? If we return here, is there a chance
we are going to have a zombie RPC client with the ds? If that's not a
problem then I don't think there an issue to assume that if there is
no valid NFS client then we would want to return a NULL from
nfs4_fl_prepare_ds().


>
>>
>> Thanks,
>> NeilBrown
>>
>>
>> On Fri, Oct 21 2016, NeilBrown wrote:
>>
>>>
>>> Various places assume that if nfs4_fl_prepare_ds() turns a non-NULL
>>> 'ds', then ds->ds_clp will also be non-NULL.
>>>
>>> This is not necessasrily true in the case when the process received a
>>> fatal signal while nfs4_pnfs_ds_connect is waiting in
>>> nfs4_wait_ds_connect().  In that case ->ds_clp may not be set, and the
>>> devid may not recently have been marked unavailable.
>>>
>>> So add a test for ->ds_clp == NULL and return NULL in that case.
>>>
>>> Fixes: c23266d532b4 ("NFS4.1 Fix data server connection race")
>>> Signed-off-by: NeilBrown <neilb@suse.com>
>>> ---
>>> fs/nfs/filelayout/filelayoutdev.c | 3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/fs/nfs/filelayout/filelayoutdev.c b/fs/nfs/filelayout/filelayoutdev.c
>>> index 4946ef40ba87..85ef38f9765f 100644
>>> --- a/fs/nfs/filelayout/filelayoutdev.c
>>> +++ b/fs/nfs/filelayout/filelayoutdev.c
>>> @@ -283,7 +283,8 @@ nfs4_fl_prepare_ds(struct pnfs_layout_segment *lseg, u32 ds_idx)
>>>                           s->nfs_client->cl_rpcclient->cl_auth->au_flavor);
>>>
>>> out_test_devid:
>>> -    if (filelayout_test_devid_unavailable(devid))
>>> +    if (ret->ds_clp == NULL ||
>>> +        filelayout_test_devid_unavailable(devid))
>>>              ret = NULL;
>>> out:
>>>      return ret;
>>> --
>>> 2.10.1
>
--
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
NeilBrown Nov. 19, 2016, 6:33 a.m. UTC | #4
On Sat, Nov 19 2016, Olga Kornievskaia wrote:

> On Fri, Nov 18, 2016 at 9:32 AM, Trond Myklebust
> <trondmy@primarydata.com> wrote:
>>
>>> On Nov 18, 2016, at 00:01, NeilBrown <neilb@suse.com> wrote:
>>>
>>>
>>> Ping?
>>> No sign of this in linux-next, and no replies….
>>
>> I’d like some confirmation from the NetApp folks on this. They are the main stakeholders for the pNFS files implementation. I don’t think we need an equivalent for flex files.
>
> Just to see if I understand this patch. If "ds" isn't NULL, then the
> client has an RPC client with the ds. But it doesn't have a valid NFS
> client? Looking at the code I really don't see how that can happen.

I thought I explained that, but I was rather brief.

nfs4_fl_prepare_ds() calls nfs4_pnfs_ds_connect() in order to create the
NFS client.  The first time it is called it will set NFS4DS_CONNECTING
and then call _nfs4_pnfs_v?_ds_connect() which will establish the
connection and set ds->ds_clp.  If the server is unresponsive, this an
block indefinitely.

If a second thread then tries the same time, it will enter
nfs4_pnfs_ds_connect() and will discovery that NFS4DS_CONNECTING is
already set, so it will call nfs4_wait_ds_connect().

As nfs4_wait_ds_connect() waits with TASK_KILLABLE, it will abort if the
process is kill by an uncaught signal (such as SIGKILL).
In this case it will return even though NFS4DS_CONNECTING is still set,
and ds->ds_clp is still NULL.  i.e. the first thread hasn't established
the connection yet.

As the process has been killed, the 'ds' that it holds that doesn't have
an NFS client handle will never be used.  But we need to be sure that
the process will exit cleanly without trying to de-reference that NULL
ds_clp.

That is what the patch ensures.


> Maybe there is a bug elsewhere? If we return here, is there a chance
> we are going to have a zombie RPC client with the ds? If that's not a
> problem then I don't think there an issue to assume that if there is
> no valid NFS client then we would want to return a NULL from
> nfs4_fl_prepare_ds().

It isn't a "zombie RPC client", but rather an unborn RPC client, which
still has NFS4DS_CONNECTING set.

Thanks,
NeilBrown


>
>
>>
>>>
>>> Thanks,
>>> NeilBrown
>>>
>>>
>>> On Fri, Oct 21 2016, NeilBrown wrote:
>>>
>>>>
>>>> Various places assume that if nfs4_fl_prepare_ds() turns a non-NULL
>>>> 'ds', then ds->ds_clp will also be non-NULL.
>>>>
>>>> This is not necessasrily true in the case when the process received a
>>>> fatal signal while nfs4_pnfs_ds_connect is waiting in
>>>> nfs4_wait_ds_connect().  In that case ->ds_clp may not be set, and the
>>>> devid may not recently have been marked unavailable.
>>>>
>>>> So add a test for ->ds_clp == NULL and return NULL in that case.
>>>>
>>>> Fixes: c23266d532b4 ("NFS4.1 Fix data server connection race")
>>>> Signed-off-by: NeilBrown <neilb@suse.com>
>>>> ---
>>>> fs/nfs/filelayout/filelayoutdev.c | 3 ++-
>>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/fs/nfs/filelayout/filelayoutdev.c b/fs/nfs/filelayout/filelayoutdev.c
>>>> index 4946ef40ba87..85ef38f9765f 100644
>>>> --- a/fs/nfs/filelayout/filelayoutdev.c
>>>> +++ b/fs/nfs/filelayout/filelayoutdev.c
>>>> @@ -283,7 +283,8 @@ nfs4_fl_prepare_ds(struct pnfs_layout_segment *lseg, u32 ds_idx)
>>>>                           s->nfs_client->cl_rpcclient->cl_auth->au_flavor);
>>>>
>>>> out_test_devid:
>>>> -    if (filelayout_test_devid_unavailable(devid))
>>>> +    if (ret->ds_clp == NULL ||
>>>> +        filelayout_test_devid_unavailable(devid))
>>>>              ret = NULL;
>>>> out:
>>>>      return ret;
>>>> --
>>>> 2.10.1
>>
Adamson, Andy Nov. 22, 2016, 8:32 p.m. UTC | #5
The patch looks good to me. We do need to ensure that ds_clp is set before returning a non-null nfs4_pnfs_ds from nfs4_fl_prepare_ds.
As Trond pointed out, nfs4_ff_layout_prepare_ds already does this.

→Andy

On 11/19/16, 1:33 AM, "NeilBrown" <neilb@suse.com> wrote:

On Sat, Nov 19 2016, Olga Kornievskaia wrote:

> On Fri, Nov 18, 2016 at 9:32 AM, Trond Myklebust

> <trondmy@primarydata.com> wrote:

>>

>>> On Nov 18, 2016, at 00:01, NeilBrown <neilb@suse.com> wrote:

>>>

>>>

>>> Ping?

>>> No sign of this in linux-next, and no replies….

>>

>> I’d like some confirmation from the NetApp folks on this. They are the main stakeholders for the pNFS files implementation. I don’t think we need an equivalent for flex files.

>

> Just to see if I understand this patch. If "ds" isn't NULL, then the

> client has an RPC client with the ds. But it doesn't have a valid NFS

> client? Looking at the code I really don't see how that can happen.


I thought I explained that, but I was rather brief.

nfs4_fl_prepare_ds() calls nfs4_pnfs_ds_connect() in order to create the
NFS client.  The first time it is called it will set NFS4DS_CONNECTING
and then call _nfs4_pnfs_v?_ds_connect() which will establish the
connection and set ds->ds_clp.  If the server is unresponsive, this an
block indefinitely.

If a second thread then tries the same time, it will enter
nfs4_pnfs_ds_connect() and will discovery that NFS4DS_CONNECTING is
already set, so it will call nfs4_wait_ds_connect().

As nfs4_wait_ds_connect() waits with TASK_KILLABLE, it will abort if the
process is kill by an uncaught signal (such as SIGKILL).
In this case it will return even though NFS4DS_CONNECTING is still set,
and ds->ds_clp is still NULL.  i.e. the first thread hasn't established
the connection yet.

As the process has been killed, the 'ds' that it holds that doesn't have
an NFS client handle will never be used.  But we need to be sure that
the process will exit cleanly without trying to de-reference that NULL
ds_clp.

That is what the patch ensures.


> Maybe there is a bug elsewhere? If we return here, is there a chance

> we are going to have a zombie RPC client with the ds? If that's not a

> problem then I don't think there an issue to assume that if there is

> no valid NFS client then we would want to return a NULL from

> nfs4_fl_prepare_ds().


It isn't a "zombie RPC client", but rather an unborn RPC client, which
still has NFS4DS_CONNECTING set.

Thanks,
NeilBrown


>

>

>>

>>>

>>> Thanks,

>>> NeilBrown

>>>

>>>

>>> On Fri, Oct 21 2016, NeilBrown wrote:

>>>

>>>>

>>>> Various places assume that if nfs4_fl_prepare_ds() turns a non-NULL

>>>> 'ds', then ds->ds_clp will also be non-NULL.

>>>>

>>>> This is not necessasrily true in the case when the process received a

>>>> fatal signal while nfs4_pnfs_ds_connect is waiting in

>>>> nfs4_wait_ds_connect().  In that case ->ds_clp may not be set, and the

>>>> devid may not recently have been marked unavailable.

>>>>

>>>> So add a test for ->ds_clp == NULL and return NULL in that case.

>>>>

>>>> Fixes: c23266d532b4 ("NFS4.1 Fix data server connection race")

>>>> Signed-off-by: NeilBrown <neilb@suse.com>

>>>> ---

>>>> fs/nfs/filelayout/filelayoutdev.c | 3 ++-

>>>> 1 file changed, 2 insertions(+), 1 deletion(-)

>>>>

>>>> diff --git a/fs/nfs/filelayout/filelayoutdev.c b/fs/nfs/filelayout/filelayoutdev.c

>>>> index 4946ef40ba87..85ef38f9765f 100644

>>>> --- a/fs/nfs/filelayout/filelayoutdev.c

>>>> +++ b/fs/nfs/filelayout/filelayoutdev.c

>>>> @@ -283,7 +283,8 @@ nfs4_fl_prepare_ds(struct pnfs_layout_segment *lseg, u32 ds_idx)

>>>>                           s->nfs_client->cl_rpcclient->cl_auth->au_flavor);

>>>>

>>>> out_test_devid:

>>>> -    if (filelayout_test_devid_unavailable(devid))

>>>> +    if (ret->ds_clp == NULL ||

>>>> +        filelayout_test_devid_unavailable(devid))

>>>>              ret = NULL;

>>>> out:

>>>>      return ret;

>>>> --

>>>> 2.10.1

>>
Olga Kornievskaia Nov. 23, 2016, 5:37 p.m. UTC | #6
On Sat, Nov 19, 2016 at 1:33 AM, NeilBrown <neilb@suse.com> wrote:
> On Sat, Nov 19 2016, Olga Kornievskaia wrote:
>
>> On Fri, Nov 18, 2016 at 9:32 AM, Trond Myklebust
>> <trondmy@primarydata.com> wrote:
>>>
>>>> On Nov 18, 2016, at 00:01, NeilBrown <neilb@suse.com> wrote:
>>>>
>>>>
>>>> Ping?
>>>> No sign of this in linux-next, and no replies….
>>>
>>> I’d like some confirmation from the NetApp folks on this. They are the main stakeholders for the pNFS files implementation. I don’t think we need an equivalent for flex files.
>>
>> Just to see if I understand this patch. If "ds" isn't NULL, then the
>> client has an RPC client with the ds. But it doesn't have a valid NFS
>> client? Looking at the code I really don't see how that can happen.
>
> I thought I explained that, but I was rather brief.
>
> nfs4_fl_prepare_ds() calls nfs4_pnfs_ds_connect() in order to create the
> NFS client.  The first time it is called it will set NFS4DS_CONNECTING
> and then call _nfs4_pnfs_v?_ds_connect() which will establish the
> connection and set ds->ds_clp.  If the server is unresponsive, this an
> block indefinitely.
>
> If a second thread then tries the same time, it will enter
> nfs4_pnfs_ds_connect() and will discovery that NFS4DS_CONNECTING is
> already set, so it will call nfs4_wait_ds_connect().
>
> As nfs4_wait_ds_connect() waits with TASK_KILLABLE, it will abort if the
> process is kill by an uncaught signal (such as SIGKILL).
> In this case it will return even though NFS4DS_CONNECTING is still set,
> and ds->ds_clp is still NULL.  i.e. the first thread hasn't established
> the connection yet.
>
> As the process has been killed, the 'ds' that it holds that doesn't have
> an NFS client handle will never be used.  But we need to be sure that
> the process will exit cleanly without trying to de-reference that NULL
> ds_clp.
>
> That is what the patch ensures.

Thanks for the explanation. Makes sense.

>> Maybe there is a bug elsewhere? If we return here, is there a chance
>> we are going to have a zombie RPC client with the ds? If that's not a
>> problem then I don't think there an issue to assume that if there is
>> no valid NFS client then we would want to return a NULL from
>> nfs4_fl_prepare_ds().
>
> It isn't a "zombie RPC client", but rather an unborn RPC client, which
> still has NFS4DS_CONNECTING set.
>
> Thanks,
> NeilBrown
>
>
>>
>>
>>>
>>>>
>>>> Thanks,
>>>> NeilBrown
>>>>
>>>>
>>>> On Fri, Oct 21 2016, NeilBrown wrote:
>>>>
>>>>>
>>>>> Various places assume that if nfs4_fl_prepare_ds() turns a non-NULL
>>>>> 'ds', then ds->ds_clp will also be non-NULL.
>>>>>
>>>>> This is not necessasrily true in the case when the process received a
>>>>> fatal signal while nfs4_pnfs_ds_connect is waiting in
>>>>> nfs4_wait_ds_connect().  In that case ->ds_clp may not be set, and the
>>>>> devid may not recently have been marked unavailable.
>>>>>
>>>>> So add a test for ->ds_clp == NULL and return NULL in that case.
>>>>>
>>>>> Fixes: c23266d532b4 ("NFS4.1 Fix data server connection race")
>>>>> Signed-off-by: NeilBrown <neilb@suse.com>
>>>>> ---
>>>>> fs/nfs/filelayout/filelayoutdev.c | 3 ++-
>>>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/fs/nfs/filelayout/filelayoutdev.c b/fs/nfs/filelayout/filelayoutdev.c
>>>>> index 4946ef40ba87..85ef38f9765f 100644
>>>>> --- a/fs/nfs/filelayout/filelayoutdev.c
>>>>> +++ b/fs/nfs/filelayout/filelayoutdev.c
>>>>> @@ -283,7 +283,8 @@ nfs4_fl_prepare_ds(struct pnfs_layout_segment *lseg, u32 ds_idx)
>>>>>                           s->nfs_client->cl_rpcclient->cl_auth->au_flavor);
>>>>>
>>>>> out_test_devid:
>>>>> -    if (filelayout_test_devid_unavailable(devid))
>>>>> +    if (ret->ds_clp == NULL ||
>>>>> +        filelayout_test_devid_unavailable(devid))
>>>>>              ret = NULL;
>>>>> out:
>>>>>      return ret;
>>>>> --
>>>>> 2.10.1
>>>
--
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
diff mbox

Patch

diff --git a/fs/nfs/filelayout/filelayoutdev.c b/fs/nfs/filelayout/filelayoutdev.c
index 4946ef40ba87..85ef38f9765f 100644
--- a/fs/nfs/filelayout/filelayoutdev.c
+++ b/fs/nfs/filelayout/filelayoutdev.c
@@ -283,7 +283,8 @@  nfs4_fl_prepare_ds(struct pnfs_layout_segment *lseg, u32 ds_idx)
 			     s->nfs_client->cl_rpcclient->cl_auth->au_flavor);
 
 out_test_devid:
-	if (filelayout_test_devid_unavailable(devid))
+	if (ret->ds_clp == NULL ||
+	    filelayout_test_devid_unavailable(devid))
 		ret = NULL;
 out:
 	return ret;