On Thu, Oct 10, 2019 at 01:11:38PM -0400, Josef Bacik wrote: > On Mon, Oct 07, 2019 at 04:17:46PM -0400, Dennis Zhou wrote: > > Async discard doesn't remember the discard state of a block_group when > > unmounting or when we crash. So, any block_group that is not fully used > > may have undiscarded regions. However, free space caches are read in on > > demand. Let the discard worker read in the free space cache so we can > > proceed with discarding rather than wait for the block_group to be used. > > This prevents us from indefinitely deferring discards until that > > particular block_group is reused. > > > > Signed-off-by: Dennis Zhou <dennis@xxxxxxxxxx> > > What if we did completely discard the last time, now we're going back and > discarding again? I think by default we just assume we discarded everything. > If we didn't then the user can always initiate a fitrim later. Drop this one. > Thanks, > Yeah this is something I wasn't sure about. It makes me a little uncomfortable to make the lack of persistence a user problem. If in some extreme case where someone frees a large amount of space and then unmounts. We can either make them wait on unmount to discard everything or retrim the whole drive which in an ideal world should just be a noop on already free lba space. If others are in favor of just going the fitrim route for users, I'm happy to drop this patch, but I do like the fact that this makes the whole system consistent without user intervention. Does anyone else have an opinion? On a side note, the find_free_extent() allocator tries pretty hard before allocating subsequent block groups. So maybe it's right to just deprioritize these block groups instead of just not loading them. Thanks, Dennis
