----- Original Message -----
> From: "Dan Carpenter" <dan.carpenter@xxxxxxxxxx>
> To: "Tigran Mkrtchyan" <tigran.mkrtchyan@xxxxxxx>
> Cc: "trondmy" <trondmy@xxxxxxxxxx>, "anna schumaker" <anna.schumaker@xxxxxxxxxx>, "linux-nfs"
> <linux-nfs@xxxxxxxxxxxxxxx>
> Sent: Thursday, 28 August, 2025 16:38:29
> Subject: Re: [PATCH] flexfiles/pNFS: fix NULL checks on result of ff_layout_choose_ds_for_read
> On Thu, Aug 28, 2025 at 03:38:43PM +0200, Tigran Mkrtchyan wrote:
>> Recent commit f06bedfa62d5 ("pNFS/flexfiles: don't attempt pnfs on fatal DS
>> errors") has changed the error return type of ff_layout_choose_ds_for_read()
>> from
>> NULL to an error pointer. However, not all code paths have been updated
>> to match the change. Thus, some non-NULL checks will accept error pointers
>> as a valid return value.
>>
>> Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
>> Fixes: f06bedfa62d5 ("pNFS/flexfiles: don't attempt pnfs on fatal DS errors")
>> Signed-off-by: Tigran Mkrtchyan <tigran.mkrtchyan@xxxxxxx>
>> ---
>
> This is still not complete.
>
> 1073 static void ff_layout_resend_pnfs_read(struct nfs_pgio_header *hdr)
> 1074 {
> 1075 u32 idx = hdr->pgio_mirror_idx + 1;
> 1076 u32 new_idx = 0;
> 1077
> 1078 if (ff_layout_choose_any_ds_for_read(hdr->lseg, idx, &new_idx))
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> This should be:
>
> struct nfs4_pnfs_ds *ds;
>
> ds = ff_layout_choose_any_ds_for_read(hdr->lseg, idx, &new_idx);
> if (IS_ERR(ds))
> pnfs_error_mark_layout_for_return(hdr->inode, hdr->lseg);
> else
> ff_layout_send_layouterror(hdr->lseg);
Yeah, makes sense. Thanks!
Tigran.
>
> 1079 ff_layout_send_layouterror(hdr->lseg);
> 1080 else
> 1081 pnfs_error_mark_layout_for_return(hdr->inode, hdr->lseg);
> 1082 pnfs_read_resend_pnfs(hdr, new_idx);
> 1083 }
>
> Also the continue in ff_layout_choose_ds_for_read() still needs to be
> fixed as I mentioned earlier.
>
> regards,
> dan carpenter
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature
