linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@infradead.org>
To: Yu Kuai <yukuai1@huaweicloud.com>
Cc: ming.lei@redhat.com, axboe@kernel.dk,
	linux-block@vger.kernel.org, linux-kernel@vger.kernel.org,
	yukuai3@huawei.com, yi.zhang@huawei.com, yangerkun@huawei.com
Subject: Re: [PATCH v3 2/3] block: introduce new field bd_flags in block_device
Date: Tue, 21 Nov 2023 23:28:56 -0800	[thread overview]
Message-ID: <ZV2tuLCH2cPXxQ30@infradead.org> (raw)
In-Reply-To: <20231122103103.1104589-3-yukuai1@huaweicloud.com>

> +	if (partno && bdev_flagged(disk->part0, BD_FLAG_HAS_SUBMIT_BIO))
> +		bdev_set_flag(bdev, BD_FLAG_HAS_SUBMIT_BIO);
>  	else
> +		bdev_clear_flag(bdev, BD_FLAG_HAS_SUBMIT_BIO);

While the block layer has a bit of history of using wrappers for
testing, setting and clearing flags, I have to say I always find them
rather confusing when reading the code.

> +#define BD_FLAG_READ_ONLY	0 /* read-only-policy */

I know this is copied from the existing field, but can you expand
it a bit?

> +#define BD_FLAG_WRITE_HOLDER	1
> +#define BD_FLAG_HAS_SUBMIT_BIO	2
> +#define BD_FLAG_MAKE_IT_FAIL	3

And also write comments for these. 

> +
>  struct block_device {
>  	sector_t		bd_start_sect;
>  	sector_t		bd_nr_sectors;
> @@ -44,10 +49,8 @@ struct block_device {
>  	struct request_queue *	bd_queue;
>  	struct disk_stats __percpu *bd_stats;
>  	unsigned long		bd_stamp;
> -	bool			bd_read_only;	/* read-only policy */
> +	unsigned short		bd_flags;

I suspect you really need an unsigned long and atomic bit ops here.
Even a lock would probably not work on alpha as it could affect
the other fields in the same 32-bit alignment.


  parent reply	other threads:[~2023-11-22  7:28 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-22 10:31 [PATCH v3 0/3] block: warn once for each partition in bio_check_ro() Yu Kuai
2023-11-22 10:31 ` [PATCH v3 1/3] block: move .bd_inode into 1st cacheline of block_device Yu Kuai
2023-11-22  7:23   ` Christoph Hellwig
2023-11-22 11:17   ` Yu Kuai
2023-11-22 10:31 ` [PATCH v3 2/3] block: introduce new field bd_flags in block_device Yu Kuai
2023-11-22  3:30   ` Ming Lei
2023-11-22  6:15     ` Yu Kuai
2023-11-22  3:52   ` Michael Kelley
2023-11-22  7:06     ` Yu Kuai
2023-11-22  7:28   ` Christoph Hellwig [this message]
2023-11-22  7:45     ` Ming Lei
2023-11-22  7:53       ` Christoph Hellwig
2023-11-22  8:19         ` Ming Lei
2023-11-22 12:47           ` Christoph Hellwig
2023-11-23  2:19             ` Ming Lei
2023-11-22 10:31 ` [PATCH v3 3/3] block: warn once for each partition in bio_check_ro() Yu Kuai

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=ZV2tuLCH2cPXxQ30@infradead.org \
    --to=hch@infradead.org \
    --cc=axboe@kernel.dk \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ming.lei@redhat.com \
    --cc=yangerkun@huawei.com \
    --cc=yi.zhang@huawei.com \
    --cc=yukuai1@huaweicloud.com \
    --cc=yukuai3@huawei.com \
    /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;
as well as URLs for NNTP newsgroup(s).