Dear David,
I have seen that you have made some recent changes in netfs now
visible in linux-next.
In commit 62c3b7481b9a ("netfs: Provide a writepages implementation"),
you have added some code that is included under #ifdef
CONFIG_NETFS_FSCACHE, but if I read the code correctly, the actual
intended config here is called CONFIG_FSCACHE. As I am not 100% sure
and this code just appeared on linux-next, I thought I would just
write you a mail rather than sending a one-line RFC patch.
Best regards,
Lukas
Lukas Bulwahn <[email protected]> wrote:
> In commit 62c3b7481b9a ("netfs: Provide a writepages implementation"),
> you have added some code that is included under #ifdef
> CONFIG_NETFS_FSCACHE, but if I read the code correctly, the actual
> intended config here is called CONFIG_FSCACHE.
Yeah - it should be the latter. Something like the attached patch should fix
it.
David
---
netfs: Fix wrong #ifdef hiding wait
netfs_writepages_begin() has the wait on the fscache folio conditional on
CONFIG_NETFS_FSCACHE - which doesn't exist.
Fix it to be conditional on CONFIG_FSCACHE instead.
Fixes: 62c3b7481b9a ("netfs: Provide a writepages implementation")
Reported-by: Lukas Bulwahn <[email protected]>
Signed-off-by: David Howells <[email protected]>
cc: Matthew Wilcox <[email protected]>
cc: [email protected]
cc: [email protected]
cc: [email protected]
cc: [email protected]
---
fs/netfs/buffered_write.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/netfs/buffered_write.c b/fs/netfs/buffered_write.c
index 0b2b7a60dabc..de517ca70d91 100644
--- a/fs/netfs/buffered_write.c
+++ b/fs/netfs/buffered_write.c
@@ -1076,7 +1076,7 @@ static ssize_t netfs_writepages_begin(struct address_space *mapping,
folio_unlock(folio);
if (wbc->sync_mode != WB_SYNC_NONE) {
folio_wait_writeback(folio);
-#ifdef CONFIG_NETFS_FSCACHE
+#ifdef CONFIG_FSCACHE
folio_wait_fscache(folio);
#endif
goto lock_again;
On Mon, Jan 08, 2024 at 09:41:35PM +0000, David Howells wrote:
> netfs_writepages_begin() has the wait on the fscache folio conditional on
> CONFIG_NETFS_FSCACHE - which doesn't exist.
>
> Fix it to be conditional on CONFIG_FSCACHE instead.
Why is it conditional at all? Why don't we have a stub function if
CONFIG_FSCACHE is not defined?
Matthew Wilcox <[email protected]> wrote:
> > netfs_writepages_begin() has the wait on the fscache folio conditional on
> > CONFIG_NETFS_FSCACHE - which doesn't exist.
> >
> > Fix it to be conditional on CONFIG_FSCACHE instead.
>
> Why is it conditional at all? Why don't we have a stub function if
> CONFIG_FSCACHE is not defined?
At this point, I'd rather just change the #ifdef and then (hopefully) next
cycle get rid of PG_fscache entirely, rendering this unnecessary.
David