[PATCH] CIFS: use the correct length when pinning memory for direct I/O for write

[PATCH] CIFS: use the correct length when pinning memory for direct I/O for write

[Long Li]

From: Long Li <longli@microsoft.com>

The current code attempts to pin memory using the largest possible wsize
based on the currect SMB credits. This doesn't cause kernel oops but this is
not optimal as we may pin more pages then actually needed.

Fix this by only pinning what are needed for doing this write I/O.

Signed-off-by: Long Li <longli@microsoft.com>
Cc: stable@vger.kernel.org
---
 fs/cifs/file.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 3467351..c23bf9d 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -2617,11 +2617,13 @@ cifs_write_from_iter(loff_t offset, size_t len, struct iov_iter *from,
 		if (rc)
 			break;
 
+		cur_len = min_t(const size_t, len, wsize);
+
 		if (ctx->direct_io) {
 			ssize_t result;
 
 			result = iov_iter_get_pages_alloc(
-				from, &pagevec, wsize, &start);
+				from, &pagevec, cur_len, &start);
 			if (result < 0) {
 				cifs_dbg(VFS,
 					"direct_writev couldn't get user pages "
-- 
2.7.4

Re: [PATCH] CIFS: use the correct length when pinning memory for direct I/O for write

[Joey Pabalinas]

[-- Attachment #1: Type: text/plain, Size: 1290 bytes --]

On Sun, Dec 16, 2018 at 11:17:04PM +0000, Long Li wrote:
> From: Long Li <longli@microsoft.com>
> 
> The current code attempts to pin memory using the largest possible wsize
> based on the currect SMB credits. This doesn't cause kernel oops but this is
> not optimal as we may pin more pages then actually needed.
> 
> Fix this by only pinning what are needed for doing this write I/O.
> 
> Signed-off-by: Long Li <longli@microsoft.com>
> Cc: stable@vger.kernel.org

Looks sane to me.

Reviewed-by: Joey Pabalinas <joeypabalinas@gmail.com>

> ---
>  fs/cifs/file.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index 3467351..c23bf9d 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -2617,11 +2617,13 @@ cifs_write_from_iter(loff_t offset, size_t len, struct iov_iter *from,
>  		if (rc)
>  			break;
>  
> +		cur_len = min_t(const size_t, len, wsize);
> +
>  		if (ctx->direct_io) {
>  			ssize_t result;
>  
>  			result = iov_iter_get_pages_alloc(
> -				from, &pagevec, wsize, &start);
> +				from, &pagevec, cur_len, &start);
>  			if (result < 0) {
>  				cifs_dbg(VFS,
>  					"direct_writev couldn't get user pages "
> -- 
> 2.7.4
> 

-- 
Cheers,
Joey Pabalinas

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] CIFS: use the correct length when pinning memory for direct I/O for write

[Sasha Levin]

Hi,

[This is an automated email]

This commit has been processed because it contains a -stable tag.
The stable tag indicates that it's relevant for the following trees: all

The bot has tested the following trees: v4.19.9, v4.14.88, v4.9.145, v4.4.167, v3.18.129, 

v4.19.9: Failed to apply! Possible dependencies:
    6e6e2b86c29c ("CIFS: Add support for direct I/O read")
    8c5f9c1ab7cb ("CIFS: Add support for direct I/O write")
    b98e26df0754 ("cifs: fix signed/unsigned mismatch on aio_read patch")

v4.14.88: Failed to apply! Possible dependencies:
    1dbe3466b4d2 ("CIFS: Use offset when reading pages")
    6e6e2b86c29c ("CIFS: Add support for direct I/O read")
    8c5f9c1ab7cb ("CIFS: Add support for direct I/O write")
    b98e26df0754 ("cifs: fix signed/unsigned mismatch on aio_read patch")
    bd3dcc6a22a9 ("CIFS: SMBD: Upper layer performs SMB read via RDMA write through memory registration")

v4.9.145: Failed to apply! Possible dependencies:
    31473fc4f965 ("CIFS: Separate SMB2 header structure")
    7fb8986e7449 ("CIFS: Add capability to transform requests before sending")
    8c5f9c1ab7cb ("CIFS: Add support for direct I/O write")
    9bb17e0916a0 ("CIFS: Add transform header handling callbacks")
    c610c4b619e5 ("CIFS: Add asynchronous write support through kernel AIO")
    ccf7f4088af2 ("CIFS: Add asynchronous context to support kernel AIO")
    da502f7df03d ("CIFS: Make SendReceive2() takes resp iov")
    ef65aaede23f ("smb2: Enforce sec= mount option")

v4.4.167: Failed to apply! Possible dependencies:
    141891f4727c ("SMB3: Add mount parameter to allow user to override max credits")
    166cea4dc3a4 ("SMB2: Separate RawNTLMSSP authentication from SMB2_sess_setup")
    16c568efff82 ("cifs: merge the hash calculation helpers")
    275516cdcfa4 ("Print IP address of unresponsive server")
    31473fc4f965 ("CIFS: Separate SMB2 header structure")
    373512ec5c10 ("Prepare for encryption support (first part). Add decryption and encryption key generation. Thanks to Metze for helping with this.")
    3baf1a7b9215 ("SMB2: Separate Kerberos authentication from SMB2_sess_setup")
    7fb8986e7449 ("CIFS: Add capability to transform requests before sending")
    8c5f9c1ab7cb ("CIFS: Add support for direct I/O write")
    9bb17e0916a0 ("CIFS: Add transform header handling callbacks")
    adfeb3e00e8e ("cifs: Make echo interval tunable")
    c610c4b619e5 ("CIFS: Add asynchronous write support through kernel AIO")
    ccf7f4088af2 ("CIFS: Add asynchronous context to support kernel AIO")
    da502f7df03d ("CIFS: Make SendReceive2() takes resp iov")
    ef65aaede23f ("smb2: Enforce sec= mount option")

v3.18.129: Failed to apply! Possible dependencies:
    064bcc0702a8 ("Fix coverity warning")
    141891f4727c ("SMB3: Add mount parameter to allow user to override max credits")
    275516cdcfa4 ("Print IP address of unresponsive server")
    31473fc4f965 ("CIFS: Separate SMB2 header structure")
    373512ec5c10 ("Prepare for encryption support (first part). Add decryption and encryption key generation. Thanks to Metze for helping with this.")
    3baf1a7b9215 ("SMB2: Separate Kerberos authentication from SMB2_sess_setup")
    5f7fbf733c9d ("Allow parsing vers=3.11 on cifs mount")
    7d414f396c91 ("cifs: Limit the overall credit acquired")
    7fb8986e7449 ("CIFS: Add capability to transform requests before sending")
    8c5f9c1ab7cb ("CIFS: Add support for direct I/O write")
    9235d0987331 ("Convert MessageID in smb2_hdr to LE")
    9bb17e0916a0 ("CIFS: Add transform header handling callbacks")
    aab1893d5fbe ("Add SMB3.11 mount option synonym for new dialect")
    adfeb3e00e8e ("cifs: Make echo interval tunable")
    b618f001a20e ("[SMB3] Enable checking for continuous availability and persistent handle support")
    bc09d141ebb2 ("fs/cifs: remove obsolete __constant")
    c2afb8147e69 ("Set previous session id correctly on SMB3 reconnect")
    c610c4b619e5 ("CIFS: Add asynchronous write support through kernel AIO")
    ccf7f4088af2 ("CIFS: Add asynchronous context to support kernel AIO")
    eed0e1753cbe ("Add defines and structs for smb3.1 dialect")
    ef65aaede23f ("smb2: Enforce sec= mount option")


How should we proceed with this patch?

--
Thanks,
Sasha

Re: [PATCH] CIFS: use the correct length when pinning memory for direct I/O for write

[Steve French]

merged into cifs-2.6.git for-next

On Sun, Dec 16, 2018 at 5:18 PM Long Li <longli@linuxonhyperv.com> wrote:
>
> From: Long Li <longli@microsoft.com>
>
> The current code attempts to pin memory using the largest possible wsize
> based on the currect SMB credits. This doesn't cause kernel oops but this is
> not optimal as we may pin more pages then actually needed.
>
> Fix this by only pinning what are needed for doing this write I/O.
>
> Signed-off-by: Long Li <longli@microsoft.com>
> Cc: stable@vger.kernel.org
> ---
>  fs/cifs/file.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index 3467351..c23bf9d 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -2617,11 +2617,13 @@ cifs_write_from_iter(loff_t offset, size_t len, struct iov_iter *from,
>                 if (rc)
>                         break;
>
> +               cur_len = min_t(const size_t, len, wsize);
> +
>                 if (ctx->direct_io) {
>                         ssize_t result;
>
>                         result = iov_iter_get_pages_alloc(
> -                               from, &pagevec, wsize, &start);
> +                               from, &pagevec, cur_len, &start);
>                         if (result < 0) {
>                                 cifs_dbg(VFS,
>                                         "direct_writev couldn't get user pages "
> --
> 2.7.4
>


-- 
Thanks,

Steve