linux-kernel - Re: [dm-devel] [PATCH stable] block: discard bdi_unregister() in favour of bdi

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, 6 May 2015 09:11:19 -0700
From:	Dan Williams <dan.j.williams@...el.com>
To:	device-mapper development <dm-devel@...hat.com>
Cc:	Jens Axboe <axboe@...com>, Jan Kara <jack@...e.cz>,
	Mike Snitzer <snitzer@...hat.com>,
	Azat Khuzhin <a3at.mail@...il.com>,
	Peter Zijlstra <peterz@...radead.org>,
	lkml <linux-kernel@...r.kernel.org>,
	"Kernel.org-Linux-RAID" <linux-raid@...r.kernel.org>,
	Guoqing Jiang <GQJiang@...e.com>, Tejun Heo <tj@...nel.org>,
	Christoph Hellwig <hch@....de>,
	nicholas.w.moulin@...ux.intel.com
Subject: Re: [dm-devel] [PATCH stable] block: discard bdi_unregister() in
 favour of bdi_destroy()

On Wed, Apr 29, 2015 at 5:32 PM, NeilBrown <neilb@...e.de> wrote:
>
> bdi_unregister() now contains very little functionality.
>
> It contains a "WARN_ON" if bdi->dev is NULL.  This warning is of no
> real consequence as bdi->dev isn't needed by anything else in the function,
> and it triggers if
>    blk_cleanup_queue() -> bdi_destroy()
> is called before bdi_unregister, which a subsequent patch will make happen.
> So this isn't wanted.
>
> It also calls bdi_set_min_ratio().  This needs to be called after
> writes through the bdi have all been flushed, and before the bdi is destroyed.
> Calling it early is better than calling it late as it frees up a global
> resource.
>
> Calling it immediately after bdi_wb_shutdown() in bdi_destroy()
> perfectly fits these requirements.
>
> So bdi_unregister can be discarded with the important content moved to
> bdi_destroy, as can the
>   writeback_bdi_unregister
> event which is already not used.
>
> This is tagged for 'stable' as it is a pre-requisite for a subsequent
> patch which moves calls to blk_cleanup_queue() before calls to
> del_gendisk().  The commit identified as 'Fixes' removed a lot of
> other functionality from bdi_unregister(), and made a change which
> necessitated moving the blk_cleanup_queue() calls.
>
> Reported-by: Mike Snitzer <snitzer@...hat.com>
> Cc: Christoph Hellwig <hch@....de>
> Cc: Peter Zijlstra <peterz@...radead.org>
> Cc: stable@...r.kernel.org (v4.0)
> Fixes: c4db59d31e39ea067c32163ac961e9c80198fd37
> Signed-off-by: NeilBrown <neilb@...e.de>
>
> ---
>
> Hi again Jens,
>  would you be able to queue this patch *before* the other one:
>    block: destroy bdi before blockdev is unregistered.
>
>  If it has to come after I'll need to re-write the text a bit.
>  If you could give me the commit hash to reference I'll do that.

Seems it is after:

http://git.kernel.dk/?p=linux-block.git;a=commit;h=6cd18e71

Also, we gave both patches a try internally after seeing the duplicate
sysfs warning.  You can add:

Acked-by: Dan Williams <dan.j.williams@...el.com>
Tested-by: Nicholas Moulin <nicholas.w.moulin@...ux.intel.com>

...on the re-send.

Thanks Neil!
--
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/