linux-kernel - Re: [PATCH v2] blk-cgroup: remove entries in blkg

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: <20180411171538.GN793541@devbig577.frc2.facebook.com>
Date:   Wed, 11 Apr 2018 10:15:38 -0700
From:   "tj@...nel.org" <tj@...nel.org>
To:     Bart Van Assche <Bart.VanAssche@....com>
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-block@...r.kernel.org" <linux-block@...r.kernel.org>,
        "nborisov@...e.com" <nborisov@...e.com>,
        "axboe@...nel.dk" <axboe@...nel.dk>, "shli@...com" <shli@...com>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "00moses.alexander00@...il.com" <00moses.alexander00@...il.com>,
        "arnd@...db.de" <arnd@...db.de>,
        "joseph.qi@...ux.alibaba.com" <joseph.qi@...ux.alibaba.com>
Subject: Re: [PATCH v2] blk-cgroup: remove entries in blkg_tree before queue
 release

Hello,

On Wed, Apr 11, 2018 at 05:06:41PM +0000, Bart Van Assche wrote:
> A simple and effective solution is to dissociate a request queue from the
> block cgroup controller before blk_cleanup_queue() returns. This is why commit
> a063057d7c73 ("block: Fix a race between request queue removal and the block
> cgroup controller") moved the blkcg_exit_queue() call from __blk_release_queue()
> into blk_cleanup_queue().

which is broken.  We can try to switch the lifetime model to revoking
all live objects but that likely is a lot more involving than blindly
moving blkg shootdown from release to cleanup.  Implementing sever
semantics is usually a lot more involved / fragile because it requires
explicit participation from all users (exactly the same way revoking
->queue_lock is difficult).

I'm not necessarily against switching to sever model, but what the
patch did isn't that.  It just moved some code without actually
understanding or auditing what the implications are.

Thanks.

-- 
tejun