* [RFC][PATCH] cifs: Improve use of filemap_get_folios_tag() [not found] ` <20230220135225.91b0f28344c01d5306c31230@linux-foundation.org> @ 2023-02-24 15:13 ` David Howells 2023-02-24 16:22 ` Linus Torvalds 2023-02-24 17:22 ` David Howells 0 siblings, 2 replies; 3+ messages in thread From: David Howells @ 2023-02-24 15:13 UTC (permalink / raw) To: Linus Torvalds, Steve French Cc: dhowells, Vishal Moola, Andrew Morton, Jan Kara, Paulo Alcantara, Matthew Wilcox, Huang Ying, Baolin Wang, Xin Hao, linux-mm, mm-commits, linux-cifs, linux-kernel [This additional to the "cifs: Fix cifs_writepages_region()" patch that I posted] The inefficiency derived from filemap_get_folios_tag() get a batch of contiguous folios in Vishal's change to afs that got copied into cifs can be reduced by skipping over those folios that have been passed by the start position rather than going through the process of locking, checking and trying to write them. A similar change would need to be made in afs, in addition to fixing the bugs there. There's also a fix in cifs_write_back_from_locked_folio() where it doesn't return the amount of data dispatched to the server as ->async_writev() just returns 0 on success. Signed-off-by: David Howells <dhowells@redhat.com> --- diff --git a/fs/cifs/file.c b/fs/cifs/file.c index ebfcaae8c437..bae1a9709e32 100644 --- a/fs/cifs/file.c +++ b/fs/cifs/file.c @@ -2839,6 +2839,7 @@ static ssize_t cifs_write_back_from_locked_folio(struct address_space *mapping, free_xid(xid); if (rc == 0) { wbc->nr_to_write = count; + rc = len; } else if (is_retryable_error(rc)) { cifs_pages_write_redirty(inode, start, len); } else { @@ -2873,6 +2874,13 @@ static int cifs_writepages_region(struct address_space *mapping, for (int i = 0; i < nr; i++) { ssize_t ret; struct folio *folio = fbatch.folios[i]; + unsigned long long fstart; + + fstart = folio_pos(folio); /* May go backwards with THPs */ + if (fstart < start && + folio_size(folio) <= start - fstart) + continue; + start = fstart; redo_folio: start = folio_pos(folio); /* May regress with THPs */ ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [RFC][PATCH] cifs: Improve use of filemap_get_folios_tag() 2023-02-24 15:13 ` [RFC][PATCH] cifs: Improve use of filemap_get_folios_tag() David Howells @ 2023-02-24 16:22 ` Linus Torvalds 2023-02-24 17:22 ` David Howells 1 sibling, 0 replies; 3+ messages in thread From: Linus Torvalds @ 2023-02-24 16:22 UTC (permalink / raw) To: David Howells Cc: Steve French, Vishal Moola, Andrew Morton, Jan Kara, Paulo Alcantara, Matthew Wilcox, Huang Ying, Baolin Wang, Xin Hao, linux-mm, mm-commits, linux-cifs, linux-kernel On Fri, Feb 24, 2023 at 7:13 AM David Howells <dhowells@redhat.com> wrote: > > The inefficiency derived from filemap_get_folios_tag() get a batch of > contiguous folios in Vishal's change to afs that got copied into cifs can > be reduced by skipping over those folios that have been passed by the start > position rather than going through the process of locking, checking and > trying to write them. This patch just makes me go "Ugh". There's something wrong with this code for it to need these games. That just makes me convinced that your other patch that just gets rid of the batching entirely is the right one. Of course, I'd be even happier if Willy is right and the code could use the generic write_cache_pages() and avoid all of these things entirely. I'm not clear on why cifs and afs are being so different in the first place, and some of the differences are just odd (like that skip count). Linus ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [RFC][PATCH] cifs: Improve use of filemap_get_folios_tag() 2023-02-24 15:13 ` [RFC][PATCH] cifs: Improve use of filemap_get_folios_tag() David Howells 2023-02-24 16:22 ` Linus Torvalds @ 2023-02-24 17:22 ` David Howells 1 sibling, 0 replies; 3+ messages in thread From: David Howells @ 2023-02-24 17:22 UTC (permalink / raw) To: Linus Torvalds Cc: dhowells, Steve French, Vishal Moola, Andrew Morton, Jan Kara, Paulo Alcantara, Matthew Wilcox, Huang Ying, Baolin Wang, Xin Hao, linux-mm, mm-commits, linux-cifs, linux-kernel Linus Torvalds <torvalds@linux-foundation.org> wrote: > Of course, I'd be even happier if Willy is right and the code could > use the generic write_cache_pages() and avoid all of these things > entirely. I'm not clear on why cifs and afs are being so different in > the first place, and some of the differences are just odd (like that > skip count). The main reason is that write_cache_pages() doesn't (and can't) check PG_fscache (btrfs uses PG_private_2 for other purposes). NFS, 9p and ceph, for the moment, don't cache files that are open for writing, but I'm intending to change that at some point. The intention is to unify the writepages code for at least 9p, afs, ceph and cifs in netfslib in the future. David ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-02-24 17:23 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <2213409.1677249075@warthog.procyon.org.uk> [not found] ` <2134430.1677240738@warthog.procyon.org.uk> [not found] ` <2009825.1677229488@warthog.procyon.org.uk> [not found] ` <CAHk-=whAAOVBrzwb2uMjCmdRrtudGesYj0tuqdUgi8X_gbw1jw@mail.gmail.com> [not found] ` <20230220135225.91b0f28344c01d5306c31230@linux-foundation.org> 2023-02-24 15:13 ` [RFC][PATCH] cifs: Improve use of filemap_get_folios_tag() David Howells 2023-02-24 16:22 ` Linus Torvalds 2023-02-24 17:22 ` David Howells
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).