> Also, what if the server says its max-write-size is 2048 bytes?
A good question but realistically the worst case I have seen is 16K
and those servers were ancient (most SMB1 were 64K but for any
reasonably current server the worst case is typically write size of
1MB (Azure) or 4MB (Windows and Samba) while some will increase max
write size (to 8MB) not reduce it.
You are right though: setting "smb2 max write" size in Samba
/etc/samba/smb.conf to 4000 (less than 4096) would cause writes to
hang (since it would now round down to write size of 0) - so we can't
set it less than 4K.
When David's patches for netfs/folios integration rewrite are ready I
do want to see if they fix this bug so we can avoid the game of
rounding down max write size (fortunately extremely rare when it is
needed but without this temporary patch we do risk data corruption in
this strange configurations)
On Tue, Feb 6, 2024 at 5:18 PM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote:
>
> On Tue, Feb 06, 2024 at 05:14:42PM -0600, Steve French wrote:
> > The code in question is a little hard to follow, and may eventually
> > get rewritten by later folio/netfs patches from David Howells but the
> > problem is in
> > cifs_write_back_from_locked_folio() and cifs_writepages_region() where
> > after the write (of maximum write size) completes, the next write
> > skips to the beginning of the next page (leaving the tail end of the
> > previous page unwritten). This is not an issue with typical servers
> > and typical wsize values because those will almost always be a
> > multiple of 4096, but in the bug report the server in question was old
> > and had sent a value for maximum write size that was not a multiple of
> > 4096.
> >
> > This can be a temporary fix, that can be removed as netfs/folios
> > implementation improves here - but in the short term the easiest way
> > to fix this seems to be to round the negotiated maximum_write_size
> > down if not a multiple of 4096, to be a multiple of 4096 (this can be
> > removed in the future when the folios code is found which caused
> > this), and also warn the user if they pick a wsize that is not
> > recommended, not a multiple of 4096.
>
> Seems like a sensible stopgap, but probably the patch should use
> PAGE_SIZE rather than plain 4096 (what about
> Alpha/Sparc/powerpc-64k/arm64-{16,64}k?)
>
> Also, what if the server says its max-write-size is 2048 bytes?
> Also, does the code work well if the max-write-size is, say, 20480
> bytes? (ie an odd multiple of PAGE_SIZE is fine; it doesn't need to be
> a power-of-two?)
>
--
Thanks,
Steve
