linux-kernel - Re: [PATCH v3] fs/buffer.c: update per-CPU bh

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] [day] [month] [year] [list]

Date:   Thu, 2 Feb 2023 23:04:11 -0300
From:   Marcelo Tosatti <mtosatti@...hat.com>
To:     Matthew Wilcox <willy@...radead.org>
Cc:     Dave Chinner <david@...morbit.com>, linux-kernel@...r.kernel.org,
        linux-fsdevel@...r.kernel.org,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Christoph Hellwig <hch@....de>, Jens Axboe <axboe@...nel.dk>,
        Frederic Weisbecker <frederic@...nel.org>,
        Leonardo Bras <leobras@...hat.com>,
        Yair Podemsky <ypodemsk@...hat.com>, P J P <ppandit@...hat.com>
Subject: Re: [PATCH v3] fs/buffer.c: update per-CPU bh_lru cache via RCU

On Thu, Feb 02, 2023 at 10:51:27PM +0000, Matthew Wilcox wrote:
> On Fri, Feb 03, 2023 at 09:36:53AM +1100, Dave Chinner wrote:
> > On Wed, Feb 01, 2023 at 01:01:47PM -0300, Marcelo Tosatti wrote:
> > > 
> > > umount calls invalidate_bh_lrus which IPIs each
> > 
> > via invalidate_bdev(). So this is only triggered on unmount of
> > filesystems that use the block device mapping directly, right?

While executing:
mount -o loop alpine-standard-3.17.1-x86_64.iso /mnt/loop/

           mount-170027  [004] ...1 53852.213367: invalidate_bdev <-__invalidate_device
           mount-170027  [004] ...1 53852.213468: invalidate_bdev <-bdev_disk_changed.part.0
           mount-170027  [000] ...1 53852.222326: invalidate_bh_lrus <-set_blocksize
           mount-170027  [000] ...1 53852.222398: invalidate_bh_lrus <-set_blocksize
   systemd-udevd-170031  [011] ...1 53852.239794: invalidate_bh_lrus <-blkdev_flush_mapping
   systemd-udevd-170029  [004] ...1 53852.240947: invalidate_bh_lrus <-blkdev_flush_mapping

> > 
> > Or is the problem that userspace is polling the block device (e.g.
> > udisks, blkid, etc) whilst the filesystem is mounted and populating
> > the block device mapping with cached pages so invalidate_bdev()
> > always does work even when the filesystem doesn't actually use the
> > bdev mapping?
> > 
> > > CPU that has non empty per-CPU buffer_head cache:
> > > 
> > >        	on_each_cpu_cond(has_bh_in_lru, invalidate_bh_lru, NULL, 1);
> > > 
> > > This interrupts CPUs which might be executing code sensitive
> > > to interferences.
> > > 
> > > To avoid the IPI, free the per-CPU caches remotely via RCU.
> > > Two bh_lrus structures for each CPU are allocated: one is being
> > > used (assigned to per-CPU bh_lru pointer), and the other is
> > > being freed (or idle).
> > 
> > Rather than adding more complexity to the legacy bufferhead code,
> > wouldn't it be better to switch the block device mapping to use
> > iomap+folios and get rid of the use of bufferheads altogether?
> 
> Pretty sure ext4's journalling relies on the blockdev using
> buffer_heads.  At least, I did a conversion of blockdev to use
> mpage_readahead() and ext4 stopped working.

And its actually pretty simple: the new invalidate_bh_lrus should be
straightforward use of RCU:

1. for_each_online(cpu)
	cpu->bh_lrup = bh_lrus[1]	(or 0)

2. synchronize_rcu_expedited()		(wait for all previous users of
					 bh_lrup pointer to stop
					 referencing it).

3. free bh's in bh_lrus[0]		(or 1)