Re: [PATCH 1/2] block: also mark disk-owned queues as dying in __blk_mark_disk_dead

[Sergey Senozhatsky <senozhatsky@chromium.org>]

On (24/10/09 14:41), Christoph Hellwig wrote:
> On Wed, Oct 09, 2024 at 09:31:23PM +0900, Sergey Senozhatsky wrote:
> > >  	if (!test_bit(GD_OWNS_QUEUE, &disk->state)) {
> > > +		if (test_bit(QUEUE_FLAG_RESURRECT, &q->queue_flags)) {
> > > +			clear_bit(QUEUE_FLAG_DYING, &q->queue_flags);
> > > +			clear_bit(QUEUE_FLAG_RESURRECT, &q->queue_flags);
> > > +		}
> > 
> > Christoph, shouldn't QUEUE_FLAG_RESURRECT handling be outside of
> > GD_OWNS_QUEUE if-block? Because __blk_mark_disk_dead() sets
> > QUEUE_FLAG_DYING/QUEUE_FLAG_RESURRECT regardless of GD_OWNS_QUEUE.
> 
> For !GD_OWNS_QUEUE the queue is freed right below, so there isn't much
> of a point.

Oh, right.

> > // A silly nit: it seems the code uses blk_queue_flag_set() and
> > // blk_queue_flag_clear() helpers, but there is no queue_flag_test(),
> > // I don't know what if the preference here - stick to queue_flag
> > // helpers, or is it ok to mix them.
> 
> Yeah.  I looked into a test_and_set wrapper, but then saw how pointless
> the existing wrappers are.

Likewise.

> So for now this just open codes it, and once we're done with the fixes
> I plan to just send a patch to remove the wrappers entirely.

Ack.