Sure. I just wonder if it was just a typo, or, intentional for some
"implicit reason(s)", even though I myself can't see any (that could
possibly make sense).
(To state the obvious, the problem was that for devices that does not
support FUA, 'write back' would become an impossible value to write to
the `write_cache` file (even if it is the current value), because
`blk_queue_write_cache` would clear the QUEUE_FLAG_FUA queue flag in
that case and it is the same bit that `queue_wc_store` would check
(for "hw wc").)
Regards,
Tom
On Tue, 14 Nov 2023 at 02:52, Bart Van Assche <bvanassche@xxxxxxx> wrote:
>
> On 11/13/23 10:27, Tom Yan wrote:
> > So I realized that I mixed up the queue write_cache and the sd
> > cache_type sysfs files. cache_type is working fine.
> >
> > But then I noticed that in commit
> > 43c9835b144c7ce29efe142d662529662a9eb376, you define QUEUE_FLAG_HW_WC
> > as 18, which is also the value of QUEUE_FLAG_FUA. Was it intentional?
> > If so, what's the reason behind it?
>
> Does the untested patch below help?
>
> Thanks,
>
> Bart.
>
> [PATCH] block: Make sure that all queue flags have a different value
>
> Queue flags QUEUE_FLAG_HW_WC has the same numerical value as
> QUEUE_FLAG_FUA. Fix this by changing the QUEUE_FLAG_HW_WC definition.
>
> Cc: Christoph Hellwig <hch@xxxxxx>
> Reported-by: Tom Yan <tom.ty89@xxxxxxxxx>
> Fixes: 43c9835b144c ("block: don't allow enabling a cache on devices that don't support it")
> Signed-off-by: Bart Van Assche <bvanassche@xxxxxxx>
> ---
> include/linux/blkdev.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index eef450f25982..f59fcd5b499a 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -538,7 +538,7 @@ struct request_queue {
> #define QUEUE_FLAG_ADD_RANDOM 10 /* Contributes to random pool */
> #define QUEUE_FLAG_SYNCHRONOUS 11 /* always completes in submit context */
> #define QUEUE_FLAG_SAME_FORCE 12 /* force complete on same CPU */
> -#define QUEUE_FLAG_HW_WC 18 /* Write back caching supported */
> +#define QUEUE_FLAG_HW_WC 13 /* Write back caching supported */
> #define QUEUE_FLAG_INIT_DONE 14 /* queue is initialized */
> #define QUEUE_FLAG_STABLE_WRITES 15 /* don't modify blks until WB is done */
> #define QUEUE_FLAG_POLL 16 /* IO polling enabled if set */
>
