Re: git: 6eb503116e88 - main - sdio: don't use CAM_PRIORITY_NONE for queued CCB-s

On Tue, 5 Aug 2025, Andriy Gapon wrote:

> The branch main has been updated by avg:
>
> URL: https://cgit.FreeBSD.org/src/commit/?id=6eb503116e88cc430c2c9f01f48aa979fb0a7e1b
>
> commit 6eb503116e88cc430c2c9f01f48aa979fb0a7e1b
> Author:     Andriy Gapon <avg@FreeBSD.org>
> AuthorDate: 2025-08-05 14:40:21 +0000
> Commit:     Andriy Gapon <avg@FreeBSD.org>
> CommitDate: 2025-08-05 16:27:12 +0000
>
>    sdio: don't use CAM_PRIORITY_NONE for queued CCB-s
>
>    This is similar to changes done in other CAM drivers and fixes a panic
>    because of a sanity check added in b4b166b8c46b.
>
>    While here, remove unneeded ccb setup in sdiobdiscover.
>    It's possible that ccb allocation in that function is not needed as
>    well, but I wasn't sure about that.

I have a lot more changes to sdio here;  I would appreciate if they go
through review if you have more.

Are you testing this with any device?

>    MFC after:      1 week
> ---
> sys/dev/sdio/sdiob.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/sys/dev/sdio/sdiob.c b/sys/dev/sdio/sdiob.c
> index 4ec2058fa2e4..cb2cc0da6b77 100644
> --- a/sys/dev/sdio/sdiob.c
> +++ b/sys/dev/sdio/sdiob.c
> @@ -150,7 +150,7 @@ sdiob_rw_direct_sc(struct sdiob_softc *sc, uint8_t fn, uint32_t addr, bool wr,
> 		sc->ccb = xpt_alloc_ccb();
> 	else
> 		memset(sc->ccb, 0, sizeof(*sc->ccb));
> -	xpt_setup_ccb(&sc->ccb->ccb_h, sc->periph->path, CAM_PRIORITY_NONE);
> +	xpt_setup_ccb(&sc->ccb->ccb_h, sc->periph->path, CAM_PRIORITY_NORMAL);
> 	CAM_DEBUG(sc->ccb->ccb_h.path, CAM_DEBUG_TRACE,
> 	    ("%s(fn=%d, addr=%#02x, wr=%d, *val=%#02x)\n", __func__,
> 	    fn, addr, wr, *val));
> @@ -250,7 +250,7 @@ sdiob_rw_extended_cam(struct sdiob_softc *sc, uint8_t fn, uint32_t addr,
> 		sc->ccb = xpt_alloc_ccb();
> 	else
> 		memset(sc->ccb, 0, sizeof(*sc->ccb));
> -	xpt_setup_ccb(&sc->ccb->ccb_h, sc->periph->path, CAM_PRIORITY_NONE);
> +	xpt_setup_ccb(&sc->ccb->ccb_h, sc->periph->path, CAM_PRIORITY_NORMAL);
> 	CAM_DEBUG(sc->ccb->ccb_h.path, CAM_DEBUG_TRACE,
> 	    ("%s(fn=%d addr=%#0x wr=%d b_count=%u blksz=%u buf=%p incr=%d)\n",
> 	    __func__, fn, addr, wr, b_count, blksz, buffer, incaddr));
> @@ -977,9 +977,6 @@ sdiobdiscover(void *context, int pending)
>
> 	if (sc->ccb == NULL)
> 		sc->ccb = xpt_alloc_ccb();
> -	else
> -		memset(sc->ccb, 0, sizeof(*sc->ccb));
> -	xpt_setup_ccb(&sc->ccb->ccb_h, periph->path, CAM_PRIORITY_NONE);

This likely just made a problem worse of ccb re-use.  I have locally
changed them to be allocated/freed per transaction now.


> 	/*
> 	 * Read CCCR and FBR of each function, get manufacturer and device IDs,
>

-- 
Bjoern A. Zeeb                                                     r15:7