From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id BD9CA1CD32; Tue, 6 Feb 2024 23:19:03 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=90.155.50.34 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1707261549; cv=none; b=Xeu2uCD/BmgX9ZNFcd1AEN4n7KMyAS4aHuPjgUA8VyJnMkOppXHSYYzBNDLOJsXuIgb2v+Pv11fhgb65FX/dgAIMWkk7D8zxncc/aueZGHCTSpc2QIS9iBR2WxXjXvi+xkWl+oi5XLj0at27Cr1VfjHV0bCdW7B2UHqkQ6eG3vs= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1707261549; c=relaxed/simple; bh=gvlx9I94wOd50YscJj/a7/Sa/udNt8J7rgpAq6RBAUI=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Z4U9x/hXzs9S1RaCUCnpl4Da4tgaBKj5pbRBnT6VDTV0lWclV00JHblzggb/NOUBiaDyr7sPif8kzUsfBysVXVQVQDZKqxSuAk3Xoyp9y8WB9zeSvtB7u1P8ttjBWY5uhS6Ld8WqoMSLNvEvF1leuxDqRun6vBugO5kYTkPZYPc= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=infradead.org; spf=none smtp.mailfrom=infradead.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b=V3imUbc9; arc=none smtp.client-ip=90.155.50.34 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=infradead.org Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=infradead.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b="V3imUbc9" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=YnBJkpuBoqzeKQSfi3Ju9Ni6rq1pZ0BC60FbPS8wc5o=; b=V3imUbc9ptSdR1OGzam6nKjdiJ OyV0r349xrtsKkfi/bvCvYLawM9Z/6Tx5up5+foBSS3yzqYPK8fSFBIEVrsfby9mNjJZGEqGXVGIy kD+MjK2IZZgy43TNQctzke2PKOOtlRpvtMGzUhPJc04eL0ox+As4GViiykEPpqjGecHMJontQh+ks IsQBTOFn6k55+4eX4BC/MxdzlH7LTfOE+4N0c9Do1nVPZwf8K089r1Tp4HuigdH4mpOCelgnX1k3B P9txNFiiqSCl9j7HkX6+9fLq7up/8JhqGRNPxQY17uD06kE4EJEmG7jfKER3htGK7PoEJ8TW92KzI 57xCNMrw==; Received: from willy by casper.infradead.org with local (Exim 4.97.1 #2 (Red Hat Linux)) id 1rXUiJ-0000000DXtL-1MP3; Tue, 06 Feb 2024 23:18:51 +0000 Date: Tue, 6 Feb 2024 23:18:51 +0000 From: Matthew Wilcox To: Steve French Cc: David Howells , CIFS , linux-fsdevel , ronnie sahlberg , "R. Diez" Subject: Re: [PATCH] fix netfs/folios regression Message-ID: References: Precedence: bulk X-Mailing-List: linux-fsdevel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: 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?)