linux-kernel - Re: [GIT PULL] Queue free fix (was Re: [PATCH] block: Free queue resources at blk_release

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]

Date:	Wed, 28 Sep 2011 18:34:49 -0400
From:	Vivek Goyal <vgoyal@...hat.com>
To:	Christoph Hellwig <hch@...radead.org>
Cc:	James Bottomley <James.Bottomley@...senPartnership.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Jens Axboe <axboe@...nel.dk>, Hannes Reinecke <hare@...e.de>,
	James Bottomley <James.Bottomley@...allels.com>,
	"linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
	Linux Kernel <linux-kernel@...r.kernel.org>
Subject: Re: [GIT PULL] Queue free fix (was Re: [PATCH] block: Free queue
 resources at blk_release_queue())

On Wed, Sep 28, 2011 at 02:16:49PM -0400, Christoph Hellwig wrote:
> On Wed, Sep 28, 2011 at 02:09:05PM -0400, Vivek Goyal wrote:
> > I had thought of implementing a separate lock for throttling. Then I
> > noticed few operations like checking for queue flags where I would
> > be required to hold queue locks.
> 
> Can you do a writeup of these issues?  E.g. if the flags are throtteling
> specific we can move them to a separate flags field, if not we can
> see if we can make them atomic or similar.

Sure. I am going through the code and trying to list down some of the
dependencies.

But before that I would say that tyring to optimize the throttling code
to reduce the queue lock contention should be last on the priority list.
The simple reason being that in common case it does not incur any locking
cost. One needs to have IO cgroup configured and needs to have some
throttling rules put in place only then queue lock cost will come into
picture. And for most of the people, I don't think that's the common
case.

Now coming back to question of dependencies. We take some services from
request queue which needs to happen under queue lock.

- Looks like in current code I am only accessing QUEUE_FLAG_DEAD. I see
  that flag is not synchronized using queue lock. So it probably is not
  a concern.

- I am using tracing functionality. blk_add_trace_msg(). I guess it is
  not required to take queue lock for this. But I am not sure.

- I am accessing q->backing_dev_info to get to associated devices's
  major and minor number.

That's it I guess. That's what a quick look tells me. So looks like
it is not too hard to convert existing blk throttle code to use its
own lock. That will also get rid of dependency on queue lock and
in turn uncertainity associated with queue lock being valid (if
driver provided the lock).

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/