[bug report] block: remove the write_hint field from struct request

[bug report] block: remove the write_hint field from struct request

[Dan Carpenter]

Hello Christoph Hellwig,

This is a semi-automatic email about new static checker warnings.

Commit 61952bb73486 ("block: remove the write_hint field from struct
request") from Nov 12, 2024, leads to the following Smatch complaint:

block/blk-merge.c:1005 blk_rq_merge_ok() warn: variable dereferenced before check 'rq->bio' (see line 994)

block/blk-merge.c
   993		/* don't merge across cgroup boundaries */
   994		if (!blk_cgroup_mergeable(rq, bio))
                                          ^^
This dereferences rq->bio->

   995			return false;
   996	
   997		/* only merge integrity protected bio into ditto rq */
   998		if (blk_integrity_merge_bio(rq->q, rq, bio) == false)
   999			return false;
  1000	
  1001		/* Only merge if the crypt contexts are compatible */
  1002		if (!bio_crypt_rq_ctx_compatible(rq, bio))
  1003			return false;
  1004	
  1005		if (rq->bio) {
                    ^^^^^^^
So this check shouldn't be required.

  1006			/* Don't merge requests with different write hints. */
  1007			if (rq->bio->bi_write_hint != bio->bi_write_hint)

regards,
dan carpenter

Re: [bug report] block: remove the write_hint field from struct request

[Christoph Hellwig]

On Thu, Nov 14, 2024 at 12:04:03PM +0300, Dan Carpenter wrote:
>    994		if (!blk_cgroup_mergeable(rq, bio))
>                                           ^^
> This dereferences rq->bio->
> 
>    995			return false;
>    996	
>    997		/* only merge integrity protected bio into ditto rq */
>    998		if (blk_integrity_merge_bio(rq->q, rq, bio) == false)
>    999			return false;
>   1000	
>   1001		/* Only merge if the crypt contexts are compatible */
>   1002		if (!bio_crypt_rq_ctx_compatible(rq, bio))
>   1003			return false;
>   1004	
>   1005		if (rq->bio) {
>                     ^^^^^^^
> So this check shouldn't be required.

Hmm.  The only requests without bios are flush and passthrough
requests.  I guess we make sure they never end up here even if
it is not entirely obvious from the code.