> On Oct 28, 2022, at 4:30 PM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>
> On Fri, 2022-10-28 at 19:50 +0000, Chuck Lever III wrote:
>>
>>> On Oct 28, 2022, at 2:57 PM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>>>
>>> When a GC entry gets added to the LRU, kick off SYNC_NONE writeback
>>> so that we can be ready to close it when the time comes. This should
>>> help minimize delays when freeing objects reaped from the LRU.
>>>
>>> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
>>> ---
>>> fs/nfsd/filecache.c | 23 +++++++++++++++++++++--
>>> 1 file changed, 21 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/fs/nfsd/filecache.c b/fs/nfsd/filecache.c
>>> index 47cdc6129a7b..c43b6cff03e2 100644
>>> --- a/fs/nfsd/filecache.c
>>> +++ b/fs/nfsd/filecache.c
>>> @@ -325,6 +325,20 @@ nfsd_file_fsync(struct nfsd_file *nf)
>>> nfsd_reset_write_verifier(net_generic(nf->nf_net, nfsd_net_id));
>>> }
>>>
>>> +static void
>>> +nfsd_file_flush(struct nfsd_file *nf)
>>> +{
>>> + struct file *file = nf->nf_file;
>>> + struct address_space *mapping;
>>> +
>>> + if (!file || !(file->f_mode & FMODE_WRITE))
>>> + return;
>>> +
>>> + mapping = file->f_mapping;
>>> + if (mapping_tagged(mapping, PAGECACHE_TAG_DIRTY))
>>> + filemap_flush(mapping);
>>> +}
>>> +
>>> static int
>>> nfsd_file_check_write_error(struct nfsd_file *nf)
>>> {
>>> @@ -484,9 +498,14 @@ nfsd_file_put(struct nfsd_file *nf)
>>>
>>> /* Try to add it to the LRU. If that fails, decrement. */
>>> if (nfsd_file_lru_add(nf)) {
>>> - /* If it's still hashed, we're done */
>>> - if (test_bit(NFSD_FILE_HASHED, &nf->nf_flags))
>>> + /*
>>> + * If it's still hashed, we can just return now,
>>> + * after kicking off SYNC_NONE writeback.
>>> + */
>>> + if (test_bit(NFSD_FILE_HASHED, &nf->nf_flags)) {
>>> + nfsd_file_flush(nf);
>>> return;
>>> + }
>>
>> nfsd_write() calls nfsd_file_put() after every nfsd_vfs_write(). In some
>> cases, this new logic adds an async flush after every UNSTABLE NFSv3 WRITE.
>>
>> I'll need to see performance measurements demonstrating no negative
>> impact on throughput or latency of NFSv3 WRITEs with large payloads.
>>
>>
>
> In your earlier mail, you mentioned that you wanted the writeback work
> to be done in the context of nfsd threads. nfsd_file_put is how nfsd
> threads put their references so this seems like the right place to do
> it.
>
> If you're concerned about calling filemap_flush too often because we
> have an entry that's cycling onto and off of the LRU, then another
> (maybe better) idea might be to kick off writeback when we clear the
> REFERENCED flag.
I think we are doing just about that today by flushing in nfsd_file_put
right when the REFERENCED bit is set. :-)
But yes: that is essentially it. nfsd is a good place to do the flush,
but we don't want to flush too often, because that will be noticeable.
> That would need to be done in the gc thread context, however.
Apparently it is already doing this via filp_close(), though it's
not clear how often that call needs to wait for I/O. You could
schedule a worker to complete the tear down if the open file has
dirty pages.
To catch errors that might occur when the client is delaying its
COMMITs for a long while, maybe don't evict nfsd_files that have
dirty pages...?
>>> /*
>>> * We're racing with unhashing, so try to remove it from
>>> --
>>> 2.37.3
>>>
>>
>> --
>> Chuck Lever
>>
>>
>>
>
> --
> Jeff Layton <jlayton@xxxxxxxxxx>
--
Chuck Lever
