Re: [PATCH] fs: allow inode time modification with IOCB_NOWAIT

[Dave Chinner <david@fromorbit.com>]

On Sat, Jul 02, 2022 at 09:45:23AM -0600, Jens Axboe wrote:
> On 7/2/22 12:05 AM, Christoph Hellwig wrote:
> > On Fri, Jul 01, 2022 at 02:09:32PM -0600, Jens Axboe wrote:
> >> generic/471 complains because it expects any write done with RWF_NOWAIT
> >> to succeed as long as the blocks for the write are already instantiated.
> >> This isn't necessarily a correct assumption, as there are other conditions
> >> that can cause an RWF_NOWAIT write to fail with -EAGAIN even if the range
> >> is already there.
> >>
> >> Since the risk of blocking off this path is minor, just allow inode
> >> time updates with IOCB_NOWAIT set. Then we can later decide if we should
> >> catch this further down the stack.
> > 
> > I think this is broken.  Please just drop the test, the non-blocking
> > behavior here makes a lot of sense.  At least for XFS, the update
> > will end up allocating and commit a transaction which involves memory
> > allocation, a blocking lock and possibly waiting for lock space.
> 
> I'm fine with that, I've made my opinions on that test case clear in
> previous emails. I'll drop the patch for now.
> 
> I will say though that even in low memory testing, I never saw XFS block
> off the inode time update. So at least we have room for future
> improvements here, it's wasteful to return -EAGAIN here when the vast
> majority of time updates don't end up blocking.

It's not low memory testing that you should be concerned about -
it's when the journal runs out of space that you'll get long,
unbound latencies waiting for timestamp updates. Waiting for journal
space to become available could, in the worst case, entail waiting
for tens of thousands of small random metadata IOs to be submitted
and completed....

> One issue there too is that, by default, XFS uses a high granularity
> threshold for when the time should be updated, making the problem worse.

That's not an XFS issue - we're just following the VFS rules for
when mtime needs to be changed. If you want to avoid frequent
transactional (on-disk) mtime updates, then use the lazytime mount
option to limit on-disk mtime updates to once per day.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com