On Mon, Jul 31, 2023 at 06:28:02PM -0600, Jens Axboe wrote: > On 7/31/23 9:26?AM, Darrick J. Wong wrote: > > I've watched quite a bit of NOWAIT whackamole going on over the past few > > years (i_rwsem, the ILOCK, the IO layer, memory allocations...). IIRC > > these filesystem ios all have to run in process context, right? If so, > > why don't we capture the NOWAIT state in a PF flag? We already do that > > for NOFS/NOIO memory allocations to make sure that /all/ reclaim > > attempts cannot recurse into the fs/io stacks. > > I would greatly prefer passing down the context rather than capitulating > and adding a task_struct flag for this. I think it _kind of_ makes sense > for things like allocations, as you cannot easily track that all the way > down, but it's a really ugly solution. It certainly creates more churn > passing it down, but it also reveals the parts that need to check it. > WHen new code is added, it's much more likely you'll spot the fact that > there's passed in context. For allocation, you end up in the allocator > anyway, which can augment the gfp mask with whatever is set in the task. > The same is not true for locking and other bits, as they don't return a > value to begin with. When we know they are sane, we can flag the fs as > supporting it (like we've done for async buffered reads, for example). > > It's also not an absolute thing, like memory allocations are. It's > perfectly fine to grab a mutex under NOWAIT issue. What you should not > do is grab a mutex that someone else can grab while waiting on IO. This > kind of extra context is only available in the code in question, not > generically for eg mutex locking. > > I'm not a huge fan of the "let's add a bool nowait". Most/all use cases > pass down state anyway, putting it in a suitable type struct seems much We're only going to pass a struct if there really is a need for one though. Meaning, we're shouldn't end up passing a struct with a single element around in the hopes that we'll add more members at some point. > cleaner to me than the out-of-band approach of just adding a > current->please_nowait. I'm not convinced that abusing current/task_struct for this is sane. I not just very much doubt this will go down well with reviewers outside of fs/ we'd also rightly be told that we're punting on a design problem because it would be more churn.
