linux-kernel - Re: [PATCH 07/10] block: reorganize throtl_get_tg() and blk_throtl

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20111019145622.GE1140@redhat.com>
Date:	Wed, 19 Oct 2011 10:56:22 -0400
From:	Vivek Goyal <vgoyal@...hat.com>
To:	Tejun Heo <tj@...nel.org>
Cc:	axboe@...nel.dk, linux-kernel@...r.kernel.org, ctalbott@...gle.com
Subject: Re: [PATCH 07/10] block: reorganize throtl_get_tg() and
 blk_throtl_bio()

On Tue, Oct 18, 2011 at 09:26:21PM -0700, Tejun Heo wrote:
> blk_throtl_bio() and throtl_get_tg() have rather unusual interface.
> 
> * throtl_get_tg() returns pointer to a valid tg or ERR_PTR(-ENODEV),
>   and drops queue_lock in the latter case.  Different locking context
>   depending on return value is error-prone and DEAD state is scheduled
>   to be protected by queue_lock anyway.  Move DEAD check inside
>   queue_lock and return valid tg or NULL.

Tejun,

A driver could call blk_cleanup_queue(), mark the queue DEAD and then
free the driver provided spin lock. So once queue is DEAD one could
not rely on queue lock still being there. That's the reason I did
not try to take queue lock again if queue is marked DEAD.

Now I see the change that blk_cleanup_queue will start poiting to
internal queue lock (Thought it is racy). This will atleast make
sure that some spinlock is around. So now this change should be
fine.

> 
> * blk_throtl_bio() indicates return status both with its return value
>   and in/out param **@....  The former is used to indicate whether
>   queue is found to be dead during throtl processing.  The latter
>   whether the bio is throttled.
> 
>   There's no point in returning DEAD check result from
>   blk_throtl_bio().  The queue can die after blk_throtl_bio() is
>   finished but before make_request_fn() grabs queue lock.

The reason I was returning error in case of queue DEAD is that I wanted
IO to now return with error instead of continuing to call q->make_request_fn(q, bio) which does not do queue dead check and assumes queue is still alive.

With this change, if queue is DEAD, bio will not be throttled and we
will continue to submit bio to queue and I am not sure who will catch
it in __make_request()?

Thanks
Vivek
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/