Linux NFS development
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@linaro.org>
To: Tigran Mkrtchyan <tigran.mkrtchyan@desy.de>
Cc: trondmy@kernel.org, anna.schumaker@oracle.com, linux-nfs@vger.kernel.org
Subject: Re: [PATCH] flexfiles/pNFS: fix NULL checks on result of ff_layout_choose_ds_for_read
Date: Thu, 28 Aug 2025 17:38:29 +0300	[thread overview]
Message-ID: <aLBp5eajfo00ew7D@stanley.mountain> (raw)
In-Reply-To: <20250828133843.1057488-1-tigran.mkrtchyan@desy.de>

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@linaro.org>
> Fixes: f06bedfa62d5 ("pNFS/flexfiles: don't attempt pnfs on fatal DS errors")
> Signed-off-by: Tigran Mkrtchyan <tigran.mkrtchyan@desy.de>
> ---

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);

  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


  reply	other threads:[~2025-08-28 14:38 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-28 13:38 [PATCH] flexfiles/pNFS: fix NULL checks on result of ff_layout_choose_ds_for_read Tigran Mkrtchyan
2025-08-28 14:38 ` Dan Carpenter [this message]
2025-08-28 14:51   ` Mkrtchyan, Tigran

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aLBp5eajfo00ew7D@stanley.mountain \
    --to=dan.carpenter@linaro.org \
    --cc=anna.schumaker@oracle.com \
    --cc=linux-nfs@vger.kernel.org \
    --cc=tigran.mkrtchyan@desy.de \
    --cc=trondmy@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox