linux-kernel - Re: [PATCH] btrfs: Split remaining space to discard in chunks

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]

Message-ID: <20240902202323.GC26776@twin.jikos.cz>
Date: Mon, 2 Sep 2024 22:23:23 +0200
From: David Sterba <dsterba@...e.cz>
To: Luca Stefani <luca.stefani.ge1@...il.com>
Cc: dsterba@...e.cz, Qu Wenruo <quwenruo.btrfs@....com>,
	Chris Mason <clm@...com>, Josef Bacik <josef@...icpanda.com>,
	David Sterba <dsterba@...e.com>, linux-btrfs@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] btrfs: Split remaining space to discard in chunks

On Mon, Sep 02, 2024 at 10:17:37PM +0200, Luca Stefani wrote:
> > 		  sector_t nr_sects, gfp_t gfp_mask, struct bio **biop)
> >    {
> > 	  struct bio *bio;
> > 
> > 	  while ((bio = blk_alloc_discard_bio(bdev, &sector, &nr_sects,
> > 			  gfp_mask)))
> > 		  *biop = bio_chain_and_submit(*biop, bio);
> > 	  return 0;
> >    }
> > 
> > This is basically just a loop, chopping the input range as needed. The
> > btrfs code does effectively the same, there's only the superblock,
> > progress accounting and error handling done.
> > 
> > As the maximum size of a single discard request depends on a device we
> > don't need to artificially limit it because this would require more IO
> > requests and can be slower.
> 
> Thanks for taking a look, this change was prompted after I've been 
> seeing issues due to the discard kthread blocking an userspace process 
> causing device not to suspend.
> https://lore.kernel.org/lkml/20240822164908.4957-1-luca.stefani.ge1@gmail.com/ 
> is the proposed solution, but Qu mentioned that there is another place 
> where it could happen that I didn't cover, and I think what I change 
> here (unless it's the wrong place) allows me to add the similar 
> `btrfs_trim_interrupted` checks to stop.
> 
> Please let me know if that makes sense to you, if that's the case I 
> guess it would make sense to send the 2 patches together?

Yeah for inserting the cancellation points it would make sense to do the
chunking. I'd suggest to do the same logic like blk_alloc_discard_bio()
and use the block device discard request limit and not a fixed constant.