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

[David Sterba]

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@xxxxxxxxx/ 
> 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.